[pkg:js] Disallow using @staticInterop synthetic constructors

Change-Id: I5c74369ee8ae97fcc21032464fcb8fea987022d9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268107
Reviewed-by: Joshua Litt <joshualitt@google.com>
Reviewed-by: Riley Porter <rileyporter@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

CHANGELOG.md

@@ -142,13 +142,15 @@
 - **Breaking changes to the preview feature `@staticInterop`**:
   - Classes with this annotation are now disallowed from using `external`
   generative constructors. Use `external factory`s for these classes instead,
-  and the behavior should be identical. See [#48730][] for more details.
+  and the behavior should be identical. This includes use of synthetic
+  constructors. See [#48730][] and [#49941][] for more details.
   - Classes with this annotation's external extension members are now disallowed
   from using type parameters e.g. `external void method<T>(T t)`. Use a
   non-`external` extension method for type parameters instead. See [#49350][]
   for more details.
 
 [#48730]: https://github.com/dart-lang/sdk/issues/48730
+[#49941]: https://github.com/dart-lang/sdk/issues/49941
 [#49350]: https://github.com/dart-lang/sdk/issues/49350
 
 ### Tools

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

@@ -7502,6 +7502,18 @@ Message _withArgumentsJsInteropStaticInteropMockMissingImplements(
       arguments: {'name': name, 'name2': name2, 'string': string});
 }
 
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeJsInteropStaticInteropSyntheticConstructor =
+    messageJsInteropStaticInteropSyntheticConstructor;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageJsInteropStaticInteropSyntheticConstructor = const MessageCode(
+    "JsInteropStaticInteropSyntheticConstructor",
+    problemMessage:
+        r"""Synthetic constructors on `@staticInterop` classes can not be used.""",
+    correctionMessage:
+        r"""Declare an external factory constructor for this `@staticInterop` class and use that instead.""");
+
 // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
 const Template<Message Function(String name)>
     templateJsInteropStaticInteropTrustTypesUsageNotAllowed =

pkg/_js_interop_checks/lib/js_interop_checks.dart

@@ -2,6 +2,8 @@
 // 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.
 
+// Used for importing CFE utility functions for constructor tear-offs.
+import 'package:front_end/src/api_prototype/lowering_predicates.dart';
 import 'package:kernel/core_types.dart';
 import 'package:kernel/kernel.dart';
 import 'package:kernel/target/targets.dart';
@@ -21,6 +23,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
         messageJsInteropOperatorsNotSupported,
         messageJsInteropStaticInteropExternalExtensionMembersWithTypeParameters,
         messageJsInteropStaticInteropGenerativeConstructor,
+        messageJsInteropStaticInteropSyntheticConstructor,
         templateJsInteropDartClassExtendsJSClass,
         templateJsInteropStaticInteropWithInstanceMembers,
         templateJsInteropStaticInteropWithNonStaticSupertype,
@@ -41,6 +44,7 @@ class JsInteropChecks extends RecursiveVisitor {
   bool _classHasJSAnnotation = false;
   bool _classHasAnonymousAnnotation = false;
   bool _classHasStaticInteropAnnotation = false;
+  bool _inTearoff = false;
   bool _libraryHasJSAnnotation = false;
   Map<Reference, Extension>? _libraryExtensionsIndex;
   // TODO(joshualitt): These checks add value for our users, but unfortunately
@@ -338,7 +342,9 @@ class JsInteropChecks extends RecursiveVisitor {
             procedure.fileUri);
       }
     }
+    _inTearoff = isTearOffLowering(procedure);
     super.visitProcedure(procedure);
+    _inTearoff = false;
   }
 
   @override
@@ -382,6 +388,30 @@ class JsInteropChecks extends RecursiveVisitor {
     }
   }
 
+  @override
+  void visitConstructorInvocation(ConstructorInvocation node) {
+    var constructor = node.target;
+    if (constructor.isSynthetic &&
+        // Synthetic tear-offs are created for synthetic constructors by
+        // invoking them, so they need to be excluded here.
+        !_inTearoff &&
+        hasStaticInteropAnnotation(constructor.enclosingClass)) {
+      // TODO(srujzs): This is insufficient to disallow use of synthetic
+      // constructors, as tear-offs may be used. However, use of such tear-offs
+      // are lowered as a StaticTearOffConstant. This means that we'll need a
+      // constant visitor in order to handle that correctly. It should be rare
+      // for users to use those tear-offs in favor of just invocation, but it's
+      // plausible. For now, in order to avoid the complexity and the extra
+      // visiting, we don't check tear-off usage.
+      _diagnosticsReporter.report(
+          messageJsInteropStaticInteropSyntheticConstructor,
+          node.fileOffset,
+          node.name.text.length,
+          node.location?.file);
+    }
+    super.visitConstructorInvocation(node);
+  }
+
   /// Reports an error if [functionNode] has named parameters.
   void _checkNoNamedParameters(FunctionNode functionNode) {
     // ignore: unnecessary_null_comparison

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
+JsInteropStaticInteropSyntheticConstructor/analyzerCode: Fail # Web compiler specific
+JsInteropStaticInteropSyntheticConstructor/example: Fail # Web compiler specific
 JsInteropStaticInteropTrustTypesUsageNotAllowed/analyzerCode: Fail # Web compiler specific
 JsInteropStaticInteropTrustTypesUsageNotAllowed/example: Fail # Web compiler specific
 JsInteropStaticInteropTrustTypesUsedWithoutStaticInterop/analyzerCode: Fail # Web compiler specific

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."
 
+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."
+
 JsInteropStaticInteropWithInstanceMembers:
   problemMessage: "JS interop class '#name' with `@staticInterop` annotation cannot declare instance members."
   correctionMessage: "Try moving the instance member to a static extension."

tests/lib/js/static_interop_test/generative_constructor_static_test.dart

@@ -22,4 +22,9 @@ class JSClass {
 @staticInterop
 class SyntheticConstructor {}
 
-void main() {}
+void main() {
+  // Error on use only for synthetic constructors.
+  SyntheticConstructor();
+//^
+// [web] Synthetic constructors on `@staticInterop` classes can not be used.
+}

tests/lib_2/js/static_interop_test/generative_constructor_static_test.dart

@@ -24,4 +24,9 @@ class JSClass {
 @staticInterop
 class SyntheticConstructor {}
 
-void main() {}
+void main() {
+  // Error on use only for synthetic constructors.
+  SyntheticConstructor();
+//^
+// [web] Synthetic constructors on `@staticInterop` classes can not be used.
+}