[dart:js_interop] Require top-level externals to have @js

To refuse confusion between dart:ffi and dart:js_interop,
top-level externals will need to be annotated when using
dart:js_interop.

Change-Id: I1e4887eb32f135df94426e43fc885346f1b9f1b1
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/310485
Reviewed-by: Joshua Litt <[email protected]>
Commit-Queue: Srujan Gaddam <[email protected]>

Author: srujzs

pkg/_js_interop_checks/lib/js_interop_checks.dart

@@ -894,8 +894,8 @@ class JsInteropChecks extends RecursiveVisitor {
   ///   - inside a JS interop class
   ///   - inside an extension on a JS interop or @Native annotatable
   ///   - inside a JS interop inline class
-  ///   - a top level member that is JS interop annotated or in a JS interop
-  ///     library
+  ///   - a top level member that is JS interop annotated or in a package:js JS
+  ///     interop library
   bool _isJSInteropMember(Member member) {
     if (member.isExternal) {
       if (_classHasJSAnnotation) return true;
@@ -906,7 +906,13 @@ class JsInteropChecks extends RecursiveVisitor {
         return _inlineExtensionIndex.getInlineClass(member) != null;
       }
       if (member.enclosingClass == null) {
-        return hasJSInteropAnnotation(member) || _libraryHasJSAnnotation;
+        // dart:js_interop requires top-levels to be @JS-annotated. package:js
+        // historically does not have this restriction. We add this restriction
+        // to refuse confusion on what an external member does now that we can
+        // have dart:ffi and dart:js_interop in the same code via dart2wasm.
+        final libraryHasPkgJSAnnotation =
+            _libraryHasJSAnnotation && !_libraryHasDartJSInteropAnnotation;
+        return hasJSInteropAnnotation(member) || libraryHasPkgJSAnnotation;
       }
     }
 

tests/lib/js/static_interop_test/top_level_member_annotation_static_test.dart

@@ -0,0 +1,46 @@
+// Copyright (c) 2023, 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.
+
+// Test that top-level external members need an @JS annotation even if the
+// library has one when using dart:js_interop.
+
+@JS()
+library top_level_member_annotation_static_test;
+
+import 'dart:js_interop';
+
+external int field;
+//           ^
+// [web] Only JS interop members may be 'external'.
+
+external int finalField;
+//           ^
+// [web] Only JS interop members may be 'external'.
+
+external int get getter;
+//               ^
+// [web] Only JS interop members may be 'external'.
+
+external set setter(_);
+//           ^
+// [web] Only JS interop members may be 'external'.
+
+external int method();
+//           ^
+// [web] Only JS interop members may be 'external'.
+
+@JS()
+external int annotatedField;
+
+@JS()
+external int annotatedFinalField;
+
+@JS()
+external int get annotatedGetter;
+
+@JS()
+external set annotatedSetter(_);
+
+@JS()
+external int annotatedMethod();

tests/lib/lib.status

@@ -17,6 +17,7 @@ js/(?!export|static_interop_test)*: SkipByDesign
 js/static_interop_test/external_static_member_lowerings_trusttypes_test: SkipByDesign # Tests @trustTypes, which is unsupported on dart2wasm.
 js/static_interop_test/inline_class/non_interop_inline_class_static_test: SkipByDesign # Some external checks are not run on dart2wasm.
 js/static_interop_test/static_external_extension_members_static_test: SkipByDesign # dart:html not supported on dart2wasm.
+js/static_interop_test/top_level_member_annotation_static_test: SkipByDesign # Some external checks are not run on dart2wasm.
 mirrors/*: SkipByDesign
 web/*: SkipByDesign