languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-01 17:12:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

[pkg:js] Disallow non-@staticInterop classes from inheriting @staticInterop

Browse source 
Adds checks that if the derived class does not have a `@staticInterop`
annotation, none of the classes it implements or immediately extends can
have it either. Also cleans up some redundant abstract class tests for
readability.

Change-Id: I2e8528b0fb02d9ce39003d00ee0b3da88ce75d44
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268109
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Riley Porter <rileyporter@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
This commit is contained in:
Srujan Gaddam 2022-11-21 20:19:06 +00:00 committed by Commit Queue
parent 4c34e780cf
commit 7d8e63b0b5
9 changed files with 170 additions and 134 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -151,6 +151,8 @@
- Classes with this annotation should also have the `@JS` annotation. You can
also have the `@anonymous` annotation with these two annotations for an object
literal constructor, but it isn't required.
- Classes with this annotation can not be implemented by classes without this
annotation. This is to avoid confusing type behavior.
[#48730]: https://github.com/dart-lang/sdk/issues/48730
[#49941]: https://github.com/dart-lang/sdk/issues/49941

36
pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
View file

@ -7380,6 +7380,38 @@ const MessageCode messageJsInteropNonExternalMember = const MessageCode(
r"""This JS interop member must be annotated with `external`. Only factories and static methods can be non-external.""",
correctionMessage: r"""Try annotating the member with `external`.""");
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Template<Message Function(String name, String name2)>
templateJsInteropNonStaticWithStaticInteropSupertype =
const Template<Message Function(String name, String name2)>(
problemMessageTemplate:
r"""Class '#name' does not have an `@staticInterop` annotation, but has supertype '#name2', which does.""",
correctionMessageTemplate:
r"""Try marking '#name' as a `@staticInterop` class, or don't inherit '#name2'.""",
withArguments:
_withArgumentsJsInteropNonStaticWithStaticInteropSupertype);
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Message Function(String name, String name2)>
codeJsInteropNonStaticWithStaticInteropSupertype =
const Code<Message Function(String name, String name2)>(
"JsInteropNonStaticWithStaticInteropSupertype",
);
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
Message _withArgumentsJsInteropNonStaticWithStaticInteropSupertype(
String name, String name2) {
if (name.isEmpty) throw 'No name provided';
name = demangleMixinApplicationName(name);
if (name2.isEmpty) throw 'No name provided';
name2 = demangleMixinApplicationName(name2);
return new Message(codeJsInteropNonStaticWithStaticInteropSupertype,
problemMessage:
"""Class '${name}' does not have an `@staticInterop` annotation, but has supertype '${name2}', which does.""",
correctionMessage: """Try marking '${name}' as a `@staticInterop` class, or don't inherit '${name2}'.""",
arguments: {'name': name, 'name2': name2});
}
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeJsInteropOperatorsNotSupported =
messageJsInteropOperatorsNotSupported;
@ -7637,7 +7669,7 @@ const Template<Message Function(String name, String name2)>
templateJsInteropStaticInteropWithNonStaticSupertype =
const Template<Message Function(String name, String name2)>(
problemMessageTemplate:
r"""JS interop class '#name' has an `@staticInterop` annotation, but has supertype '#name2', which is non-static.""",
r"""JS interop class '#name' has an `@staticInterop` annotation, but has supertype '#name2', which does not.""",
correctionMessageTemplate:
r"""Try marking the supertype as a static interop class using `@staticInterop`.""",
withArguments:
@ -7659,7 +7691,7 @@ Message _withArgumentsJsInteropStaticInteropWithNonStaticSupertype(
name2 = demangleMixinApplicationName(name2);
return new Message(codeJsInteropStaticInteropWithNonStaticSupertype,
problemMessage:
"""JS interop class '${name}' has an `@staticInterop` annotation, but has supertype '${name2}', which is non-static.""",
"""JS interop class '${name}' has an `@staticInterop` annotation, but has supertype '${name2}', which does not.""",
correctionMessage: """Try marking the supertype as a static interop class using `@staticInterop`.""",
arguments: {'name': name, 'name2': name2});
}

40
pkg/_js_interop_checks/lib/js_interop_checks.dart
View file

@ -25,6 +25,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
messageJsInteropStaticInteropGenerativeConstructor,
messageJsInteropStaticInteropSyntheticConstructor,
templateJsInteropDartClassExtendsJSClass,
templateJsInteropNonStaticWithStaticInteropSupertype,
templateJsInteropStaticInteropNoJSAnnotation,
templateJsInteropStaticInteropWithInstanceMembers,
templateJsInteropStaticInteropWithNonStaticSupertype,
@ -167,15 +168,22 @@ class JsInteropChecks extends RecursiveVisitor {
cls.fileOffset,
cls.name.length,
cls.fileUri);
} else if (_classHasStaticInteropAnnotation) {
if (!hasStaticInteropAnnotation(superclass)) {
_diagnosticsReporter.report(
templateJsInteropStaticInteropWithNonStaticSupertype
.withArguments(cls.name, superclass.name),
cls.fileOffset,
cls.name.length,
cls.fileUri);
}
} else if (_classHasStaticInteropAnnotation &&
!hasStaticInteropAnnotation(superclass)) {
_diagnosticsReporter.report(
templateJsInteropStaticInteropWithNonStaticSupertype.withArguments(
cls.name, superclass.name),
cls.fileOffset,
cls.name.length,
cls.fileUri);
} else if (!_classHasStaticInteropAnnotation &&
hasStaticInteropAnnotation(superclass)) {
_diagnosticsReporter.report(
templateJsInteropNonStaticWithStaticInteropSupertype.withArguments(
cls.name, superclass.name),
cls.fileOffset,
cls.name.length,
cls.fileUri);
}
}
if (_classHasStaticInteropAnnotation) {
@ -200,6 +208,20 @@ class JsInteropChecks extends RecursiveVisitor {
}
}
}
// The converse of the above. If the class is not marked as static, it
// should not implement a class that is.
if (!_classHasStaticInteropAnnotation) {
for (var supertype in cls.implementedTypes) {
if (hasStaticInteropAnnotation(supertype.classNode)) {
_diagnosticsReporter.report(
templateJsInteropNonStaticWithStaticInteropSupertype
.withArguments(cls.name, supertype.classNode.name),
cls.fileOffset,
cls.name.length,
cls.fileUri);
}
}
}
// Since this is a breaking check, it is language-versioned.
if (cls.enclosingLibrary.languageVersion >= Version(2, 13) &&
_classHasJSAnnotation &&

2
pkg/front_end/messages.status
View file

@ -594,6 +594,8 @@ JsInteropNonExternalMember/analyzerCode: Fail # Web compiler specific
JsInteropNonExternalMember/example: Fail # Web compiler specific
JsInteropOperatorsNotSupported/analyzerCode: Fail # Web compiler specific
JsInteropOperatorsNotSupported/example: Fail # Web compiler specific
JsInteropNonStaticWithStaticInteropSupertype/analyzerCode: Fail # Web compiler specific
JsInteropNonStaticWithStaticInteropSupertype/example: Fail # Web compiler specific
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters/analyzerCode: Fail # Web compiler specific
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters/example: Fail # Web compiler specific
JsInteropStaticInteropGenerativeConstructor/analyzerCode: Fail # Web compiler specific

6
pkg/front_end/messages.yaml
View file

@ -5288,6 +5288,10 @@ JsInteropOperatorsNotSupported:
JsInteropInvalidStaticClassMemberName:
problemMessage: "JS interop static class members cannot have '.' in their JS name."
JsInteropNonStaticWithStaticInteropSupertype:
problemMessage: "Class '#name' does not have an `@staticInterop` annotation, but has supertype '#name2', which does."
correctionMessage: "Try marking '#name' as a `@staticInterop` class, or don't inherit '#name2'."
JsInteropStaticInteropExternalExtensionMembersWithTypeParameters:
problemMessage: "`@staticInterop` classes cannot have external extension members with type parameters."
correctionMessage: "Try using a Dart extension member if you need type parameters instead."
@ -5321,7 +5325,7 @@ JsInteropStaticInteropWithInstanceMembers:
correctionMessage: "Try moving the instance member to a static extension."
JsInteropStaticInteropWithNonStaticSupertype:
problemMessage: "JS interop class '#name' has an `@staticInterop` annotation, but has supertype '#name2', which is non-static."
problemMessage: "JS interop class '#name' has an `@staticInterop` annotation, but has supertype '#name2', which does not."
correctionMessage: "Try marking the supertype as a static interop class using `@staticInterop`."
JsInteropStaticInteropTrustTypesUsedWithoutStaticInterop:

29
tests/lib/js/static_interop_test/member_test.dart
View file

@ -51,3 +51,32 @@ extension StaticJSClassExtension on StaticJSClass {
set getSet(String val) => this.externalGetSet = val;
String method() => 'method';
}
// Abstract classes should behave the same way as concrete classes.
@JS()
@staticInterop
abstract class StaticAbstract {}
// Abstract classes with instance members should be non-static interop. The
// following have abstract or concrete members, so they're considered non-static
// interop.
@JS()
abstract class NonStaticAbstract {
int abstractMethod();
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithAbstractMembers {
int abstractMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithAbstractMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithConcreteMembers {
external int instanceMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithConcreteMembers' with `@staticInterop` annotation cannot declare instance members.
}

80
tests/lib/js/static_interop_test/supertype_test.dart
View file

@ -10,12 +10,12 @@ library supertype_test;
import 'package:js/js.dart';
// Static base interop class.
// Base static interop class.
@JS()
@staticInterop
class Static {}
// Non-static base interop class.
// Base non-static interop class.
@JS()
class NonStatic {
external int instanceMethod();
@ -41,74 +41,32 @@ class StaticExtendsStatic extends Static {}
@staticInterop
class StaticExtendsNonStatic extends NonStatic {}
// ^
// [web] JS interop class 'StaticExtendsNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which is non-static.
// [web] JS interop class 'StaticExtendsNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which does not.
// Non-static interop classes are disallowed from extending static interop
// classes.
@JS()
class NonStaticExtendsStatic extends Static {}
// ^
// [web] Class 'NonStaticExtendsStatic' does not have an `@staticInterop` annotation, but has supertype 'Static', which does.
// Static interop classes can implement each other in order to inherit extension
// methods. Note that a non-abstract static interop class can not implement a
// non-static class by definition, as it would need to contain an
// implementation.
// methods. They cannot implement or be implemented by non-static interop
// classes.
@JS()
@staticInterop
class StaticImplementsStatic implements Static {}
// Abstract classes should behave the same way as concrete classes.
@JS()
@staticInterop
abstract class StaticAbstract {}
class NonStaticImplementsStatic implements Static {}
// ^
// [web] Class 'NonStaticImplementsStatic' does not have an `@staticInterop` annotation, but has supertype 'Static', which does.
// Abstract classes with instance members should be non-static. The following
// have abstract or concrete members, so they're considered non-static.
@JS()
abstract class NonStaticAbstract {
int abstractMethod();
}
class EmptyNonStatic {}
@JS()
@staticInterop
abstract class NonStaticAbstractWithAbstractMembers {
int abstractMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithAbstractMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithConcreteMembers {
external int instanceMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithConcreteMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class StaticAbstractImplementsStaticAbstract
implements StaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractExtendsStaticAbstract extends StaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractImplementsNonStaticAbstract
// ^
// [web] JS interop class 'StaticAbstractImplementsNonStaticAbstract' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
implements
NonStaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractImplementsMultipleNonStatic
// ^
// [web] JS interop class 'StaticAbstractImplementsMultipleNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which is non-static.
// [web] JS interop class 'StaticAbstractImplementsMultipleNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
implements
NonStaticAbstract,
NonStatic {}
@JS()
@staticInterop
abstract class StaticAbstractExtendsNonStaticAbstract
// ^
// [web] JS interop class 'StaticAbstractExtendsNonStaticAbstract' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
extends NonStaticAbstract {}
class StaticImplementsNonStatic implements EmptyNonStatic {}
// ^
// [web] JS interop class 'StaticImplementsNonStatic' has an `@staticInterop` annotation, but has supertype 'EmptyNonStatic', which does not.

29
tests/lib_2/js/static_interop_test/member_test.dart
View file

@ -51,3 +51,32 @@ extension StaticJSClassExtension on StaticJSClass {
set getSet(String val) => this.externalGetSet = val;
String method() => 'method';
}
// Abstract classes should behave the same way as concrete classes.
@JS()
@staticInterop
abstract class StaticAbstract {}
// Abstract classes with instance members should be non-static interop. The
// following have abstract or concrete members, so they're considered non-static
// interop.
@JS()
abstract class NonStaticAbstract {
int abstractMethod();
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithAbstractMembers {
int abstractMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithAbstractMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithConcreteMembers {
external int instanceMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithConcreteMembers' with `@staticInterop` annotation cannot declare instance members.
}

80
tests/lib_2/js/static_interop_test/supertype_test.dart
View file

@ -10,12 +10,12 @@ library supertype_test;
import 'package:js/js.dart';
// Static base interop class.
// Base static interop class.
@JS()
@staticInterop
class Static {}
// Non-static base interop class.
// Base non-static interop class.
@JS()
class NonStatic {
external int instanceMethod();
@ -41,74 +41,32 @@ class StaticExtendsStatic extends Static {}
@staticInterop
class StaticExtendsNonStatic extends NonStatic {}
// ^
// [web] JS interop class 'StaticExtendsNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which is non-static.
// [web] JS interop class 'StaticExtendsNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which does not.
// Non-static interop classes are disallowed from extending static interop
// classes.
@JS()
class NonStaticExtendsStatic extends Static {}
// ^
// [web] Class 'NonStaticExtendsStatic' does not have an `@staticInterop` annotation, but has supertype 'Static', which does.
// Static interop classes can implement each other in order to inherit extension
// methods. Note that a non-abstract static interop class can not implement a
// non-static class by definition, as it would need to contain an
// implementation.
// methods. They cannot implement or be implemented by non-static interop
// classes.
@JS()
@staticInterop
class StaticImplementsStatic implements Static {}
// Abstract classes should behave the same way as concrete classes.
@JS()
@staticInterop
abstract class StaticAbstract {}
class NonStaticImplementsStatic implements Static {}
// ^
// [web] Class 'NonStaticImplementsStatic' does not have an `@staticInterop` annotation, but has supertype 'Static', which does.
// Abstract classes with instance members should be non-static. The following
// have abstract or concrete members, so they're considered non-static.
@JS()
abstract class NonStaticAbstract {
int abstractMethod();
}
class EmptyNonStatic {}
@JS()
@staticInterop
abstract class NonStaticAbstractWithAbstractMembers {
int abstractMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithAbstractMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class NonStaticAbstractWithConcreteMembers {
external int instanceMethod();
// ^
// [web] JS interop class 'NonStaticAbstractWithConcreteMembers' with `@staticInterop` annotation cannot declare instance members.
}
@JS()
@staticInterop
abstract class StaticAbstractImplementsStaticAbstract
implements StaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractExtendsStaticAbstract extends StaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractImplementsNonStaticAbstract
// ^
// [web] JS interop class 'StaticAbstractImplementsNonStaticAbstract' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
implements
NonStaticAbstract {}
@JS()
@staticInterop
abstract class StaticAbstractImplementsMultipleNonStatic
// ^
// [web] JS interop class 'StaticAbstractImplementsMultipleNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStatic', which is non-static.
// [web] JS interop class 'StaticAbstractImplementsMultipleNonStatic' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
implements
NonStaticAbstract,
NonStatic {}
@JS()
@staticInterop
abstract class StaticAbstractExtendsNonStaticAbstract
// ^
// [web] JS interop class 'StaticAbstractExtendsNonStaticAbstract' has an `@staticInterop` annotation, but has supertype 'NonStaticAbstract', which is non-static.
extends NonStaticAbstract {}
class StaticImplementsNonStatic implements EmptyNonStatic {}
// ^
// [web] JS interop class 'StaticImplementsNonStatic' has an `@staticInterop` annotation, but has supertype 'EmptyNonStatic', which does not.