Reland "[pkg:js] Disallow using @staticInterop synthetic constructors"

This reverts commit aa252e907e.

Reason for revert: Flutter engine is migrated to not use synthetic constructors.

Original change's description:
> Revert "[pkg:js] Disallow using @staticInterop synthetic constructors"
>
> This reverts commit aab6ab8b84.
>
> Reason for revert: Broke Flutter roll https://github.com/flutter/engine/pull/37838
>
> Original change's description:
> > [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>
>
> TBR=sigmund@google.com,joshualitt@google.com,rileyporter@google.com,dart-scoped@luci-project-accounts.iam.gserviceaccount.com,srujzs@google.com
>
> Change-Id: I6caf4b776191e8eed9877c43f900ae6300f1f5c2
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271440
> Reviewed-by: Sigmund Cherem <sigmund@google.com>
> Commit-Queue: Srujan Gaddam <srujzs@google.com>
> Reviewed-by: Srujan Gaddam <srujzs@google.com>

# Not skipping CQ checks because this is a reland.

Change-Id: Iaa538bb5f309a46c320bf1781c3d1a7148ec7be7
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/271500
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Riley Porter <rileyporter@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Joshua Litt <joshualitt@google.com>

CHANGELOG.md

@@ -151,7 +151,8 @@
 - **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][]
@@ -163,6 +164,7 @@
   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
 [#49350]: https://github.com/dart-lang/sdk/issues/49350
 
 ### Tools

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

@@ -7563,6 +7563,18 @@ Message _withArgumentsJsInteropStaticInteropNoJSAnnotation(String name) {
       arguments: {'name': name});
 }
 
+// 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,
         templateJsInteropNonStaticWithStaticInteropSupertype,
         templateJsInteropStaticInteropNoJSAnnotation,
@@ -43,6 +46,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
@@ -369,7 +373,9 @@ class JsInteropChecks extends RecursiveVisitor {
             procedure.fileUri);
       }
     }
+    _inTearoff = isTearOffLowering(procedure);
     super.visitProcedure(procedure);
+    _inTearoff = false;
   }
 
   @override
@@ -413,6 +419,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

@@ -608,6 +608,8 @@ JsInteropStaticInteropMockNotStaticInteropType/analyzerCode: Fail # Web compiler
 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
 JsInteropStaticInteropTrustTypesUsageNotAllowed/example: Fail # Web compiler specific
 JsInteropStaticInteropTrustTypesUsedWithoutStaticInterop/analyzerCode: Fail # Web compiler specific

pkg/front_end/messages.yaml

@@ -5316,6 +5316,10 @@ 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."
+
 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.
+}