From 4c34e780cf4ab065cf2ddf358cd1b869db8f79cb Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Mon, 21 Nov 2022 20:19:06 +0000 Subject: [PATCH] [pkg:js] Require users to use @JS when using @staticInterop Change-Id: Iea0a54ff03b32bc910ef2c6eb633bffd09cf0671 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268108 Reviewed-by: Sigmund Cherem Reviewed-by: Riley Porter --- CHANGELOG.md | 3 ++ .../lib/src/messages/codes_generated.dart | 29 +++++++++++++++++++ .../lib/js_interop_checks.dart | 13 +++++++-- pkg/front_end/messages.status | 2 ++ pkg/front_end/messages.yaml | 4 +++ ..._staticinterop_annotation_static_test.dart | 19 ++++++++++++ ..._staticinterop_annotation_static_test.dart | 21 ++++++++++++++ 7 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 tests/lib/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart create mode 100644 tests/lib_2/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 86da9f151ec..6c85784c4a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -148,6 +148,9 @@ from using type parameters e.g. `external void method(T t)`. Use a non-`external` extension method for type parameters instead. See [#49350][] for more details. + - 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. [#48730]: https://github.com/dart-lang/sdk/issues/48730 [#49941]: https://github.com/dart-lang/sdk/issues/49941 diff --git a/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart b/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart index ea6ca1e9364..c7dc157e20c 100644 --- a/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart +++ b/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart @@ -7502,6 +7502,35 @@ Message _withArgumentsJsInteropStaticInteropMockMissingImplements( arguments: {'name': name, 'name2': name2, 'string': string}); } +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +const Template< + Message Function( + String + name)> templateJsInteropStaticInteropNoJSAnnotation = const Template< + Message Function(String name)>( + problemMessageTemplate: + r"""`@staticInterop` classes should also have the `@JS` annotation.""", + correctionMessageTemplate: r"""Add `@JS` to class '#name'.""", + withArguments: _withArgumentsJsInteropStaticInteropNoJSAnnotation); + +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +const Code + codeJsInteropStaticInteropNoJSAnnotation = + const Code( + "JsInteropStaticInteropNoJSAnnotation", +); + +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +Message _withArgumentsJsInteropStaticInteropNoJSAnnotation(String name) { + if (name.isEmpty) throw 'No name provided'; + name = demangleMixinApplicationName(name); + return new Message(codeJsInteropStaticInteropNoJSAnnotation, + problemMessage: + """`@staticInterop` classes should also have the `@JS` annotation.""", + correctionMessage: """Add `@JS` to class '${name}'.""", + arguments: {'name': name}); +} + // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. const Code codeJsInteropStaticInteropSyntheticConstructor = messageJsInteropStaticInteropSyntheticConstructor; diff --git a/pkg/_js_interop_checks/lib/js_interop_checks.dart b/pkg/_js_interop_checks/lib/js_interop_checks.dart index b1ad5904671..a605c55c6d0 100644 --- a/pkg/_js_interop_checks/lib/js_interop_checks.dart +++ b/pkg/_js_interop_checks/lib/js_interop_checks.dart @@ -25,6 +25,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart' messageJsInteropStaticInteropGenerativeConstructor, messageJsInteropStaticInteropSyntheticConstructor, templateJsInteropDartClassExtendsJSClass, + templateJsInteropStaticInteropNoJSAnnotation, templateJsInteropStaticInteropWithInstanceMembers, templateJsInteropStaticInteropWithNonStaticSupertype, templateJsInteropJSClassExtendsDartClass, @@ -177,9 +178,17 @@ class JsInteropChecks extends RecursiveVisitor { } } } - // Validate that superinterfaces are all annotated as static as well. Note - // that mixins are already disallowed and therefore are not checked here. if (_classHasStaticInteropAnnotation) { + if (!_classHasJSAnnotation) { + _diagnosticsReporter.report( + templateJsInteropStaticInteropNoJSAnnotation + .withArguments(cls.name), + cls.fileOffset, + cls.name.length, + cls.fileUri); + } + // Validate that superinterfaces are all annotated as static as well. Note + // that mixins are already disallowed and therefore are not checked here. for (var supertype in cls.implementedTypes) { if (!hasStaticInteropAnnotation(supertype.classNode)) { _diagnosticsReporter.report( diff --git a/pkg/front_end/messages.status b/pkg/front_end/messages.status index d39a33d55ca..6e012bd36f4 100644 --- a/pkg/front_end/messages.status +++ b/pkg/front_end/messages.status @@ -604,6 +604,8 @@ JsInteropStaticInteropMockMissingImplements/analyzerCode: Fail # Web compiler sp JsInteropStaticInteropMockMissingImplements/example: Fail # Web compiler specific JsInteropStaticInteropMockNotStaticInteropType/analyzerCode: Fail # Web compiler specific JsInteropStaticInteropMockNotStaticInteropType/example: Fail # Web compiler specific +JsInteropStaticInteropNoJSAnnotation/analyzerCode: Fail # Web compiler specific +JsInteropStaticInteropNoJSAnnotation/example: Fail # Web compiler specific JsInteropStaticInteropSyntheticConstructor/analyzerCode: Fail # Web compiler specific JsInteropStaticInteropSyntheticConstructor/example: Fail # Web compiler specific JsInteropStaticInteropTrustTypesUsageNotAllowed/analyzerCode: Fail # Web compiler specific diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml index 2f29ae4a536..483a92d9f47 100644 --- a/pkg/front_end/messages.yaml +++ b/pkg/front_end/messages.yaml @@ -5308,6 +5308,10 @@ JsInteropStaticInteropMockNotStaticInteropType: problemMessage: "Type argument '#type' needs to be a `@staticInterop` type." correctionMessage: "Use a `@staticInterop` class instead." +JsInteropStaticInteropNoJSAnnotation: + problemMessage: "`@staticInterop` classes should also have the `@JS` annotation." + correctionMessage: "Add `@JS` to class '#name'." + JsInteropStaticInteropSyntheticConstructor: problemMessage: "Synthetic constructors on `@staticInterop` classes can not be used." correctionMessage: "Declare an external factory constructor for this `@staticInterop` class and use that instead." diff --git a/tests/lib/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart b/tests/lib/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart new file mode 100644 index 00000000000..0d8ef99cb0c --- /dev/null +++ b/tests/lib/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart @@ -0,0 +1,19 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +@JS() +library js_and_staticinterop_annotation_static_test; + +import 'package:js/js.dart'; + +@staticInterop +class NoJSAnnotation {} +// ^ +// [web] `@staticInterop` classes should also have the `@JS` annotation. + +@anonymous +@staticInterop +class AnonymousNoJSAnnotation {} +// ^ +// [web] `@staticInterop` classes should also have the `@JS` annotation. diff --git a/tests/lib_2/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart b/tests/lib_2/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart new file mode 100644 index 00000000000..470c97d5730 --- /dev/null +++ b/tests/lib_2/js/static_interop_test/js_and_staticinterop_annotation_static_test.dart @@ -0,0 +1,21 @@ +// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// @dart = 2.9 + +@JS() +library js_and_staticinterop_annotation_static_test; + +import 'package:js/js.dart'; + +@staticInterop +class NoJSAnnotation {} +// ^ +// [web] `@staticInterop` classes should also have the `@JS` annotation. + +@anonymous +@staticInterop +class AnonymousNoJSAnnotation {} +// ^ +// [web] `@staticInterop` classes should also have the `@JS` annotation.