[package:js] Add error for params in factories of anon classes
Adds a static error to check that factory constructors in anonymous
classes contain no positional parameters.
Change-Id: Iae7c5c1d9e2dc91390c85c58eb5e96718e808f9a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/156145
Reviewed-by: Nate Bosch <nbosch@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
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 61e97af..531cf04 100644
--- a/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
+++ b/pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart
@@ -5410,6 +5410,17 @@
}
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Null> codeJsInteropAnonymousFactoryPositionalParameters =
+ messageJsInteropAnonymousFactoryPositionalParameters;
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const MessageCode messageJsInteropAnonymousFactoryPositionalParameters =
+ const MessageCode("JsInteropAnonymousFactoryPositionalParameters",
+ message:
+ r"""Factory constructors for @anonymous JS interop classes should not contain any positional parameters.""",
+ tip: r"""Try replacing them with named parameters instead.""");
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeJsInteropIndexNotSupported =
messageJsInteropIndexNotSupported;
diff --git a/pkg/_js_interop_checks/lib/js_interop_checks.dart b/pkg/_js_interop_checks/lib/js_interop_checks.dart
index 7005741..08cbb83 100644
--- a/pkg/_js_interop_checks/lib/js_interop_checks.dart
+++ b/pkg/_js_interop_checks/lib/js_interop_checks.dart
@@ -8,6 +8,7 @@
show
Message,
LocatedMessage,
+ messageJsInteropAnonymousFactoryPositionalParameters,
messageJsInteropIndexNotSupported,
messageJsInteropNamedParameters,
messageJsInteropNonExternalConstructor;
@@ -32,7 +33,20 @@
procedure.location.file);
}
- if (!isAnonymousClassMember(procedure) || !procedure.isFactory) {
+ var isAnonymousFactory =
+ isAnonymousClassMember(procedure) && procedure.isFactory;
+
+ if (isAnonymousFactory) {
+ if (procedure.function != null &&
+ !procedure.function.positionalParameters.isEmpty) {
+ var firstPositionalParam = procedure.function.positionalParameters[0];
+ _diagnosticsReporter.report(
+ messageJsInteropAnonymousFactoryPositionalParameters,
+ firstPositionalParam.fileOffset,
+ firstPositionalParam.name.length,
+ firstPositionalParam.location.file);
+ }
+ } else {
// Only factory constructors for anonymous classes are allowed to have
// named parameters.
_checkNoNamedParameters(procedure.function);
@@ -57,12 +71,12 @@
/// Reports an error if [functionNode] has named parameters.
void _checkNoNamedParameters(FunctionNode functionNode) {
if (functionNode != null && !functionNode.namedParameters.isEmpty) {
- var firstNameParam = functionNode.namedParameters[0];
+ var firstNamedParam = functionNode.namedParameters[0];
_diagnosticsReporter.report(
messageJsInteropNamedParameters,
- firstNameParam.fileOffset,
- firstNameParam.name.length,
- firstNameParam.location.file);
+ firstNamedParam.fileOffset,
+ firstNamedParam.name.length,
+ firstNamedParam.location.file);
}
}
}
diff --git a/pkg/compiler/lib/src/diagnostics/messages.dart b/pkg/compiler/lib/src/diagnostics/messages.dart
index db613ff..151df1e 100644
--- a/pkg/compiler/lib/src/diagnostics/messages.dart
+++ b/pkg/compiler/lib/src/diagnostics/messages.dart
@@ -197,27 +197,6 @@
"""
]),
- MessageKind.JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS:
- const MessageTemplate(
- MessageKind
- .JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS,
- "Some parameters in anonymous js-interop class '#{cls}' "
- "object literal constructor are positional instead of named."
- ".",
- howToFix: "Make all arguments in external factory object literal "
- "constructors named.",
- examples: const [
- """
-class Super {
- factory Super.foo() => null;
-}
-class Class extends Super {
- Class() : super.foo();
-}
-main() => new Class();
-"""
- ]),
-
MessageKind.JS_INTEROP_NON_EXTERNAL_MEMBER: const MessageTemplate(
MessageKind.JS_INTEROP_NON_EXTERNAL_MEMBER,
"Js-interop members must be 'external'."),
diff --git a/pkg/compiler/lib/src/kernel/native_basic_data.dart b/pkg/compiler/lib/src/kernel/native_basic_data.dart
index f7a7e97..3d43255 100644
--- a/pkg/compiler/lib/src/kernel/native_basic_data.dart
+++ b/pkg/compiler/lib/src/kernel/native_basic_data.dart
@@ -151,16 +151,6 @@
_nativeBasicDataBuilder.markAsJsInteropMember(
constructor, memberName);
}
-
- if (constructor.isFactoryConstructor && isAnonymous) {
- if (constructor.parameterStructure.positionalParameters > 0) {
- reporter.reportErrorMessage(
- constructor,
- MessageKind
- .JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS,
- {'cls': cls.name});
- }
- }
});
} else {
elementEnvironment.forEachLocalClassMember(cls, (MemberEntity member) {
diff --git a/pkg/compiler/test/jsinterop/declaration_test.dart b/pkg/compiler/test/jsinterop/declaration_test.dart
index 878d20b..0c3a2ce 100644
--- a/pkg/compiler/test/jsinterop/declaration_test.dart
+++ b/pkg/compiler/test/jsinterop/declaration_test.dart
@@ -357,38 +357,6 @@
main() => new A(a: 1);
'''),
- const Test('External factory constructor with required parameters.', '''
-@JS()
-library test;
-
-import 'package:js/js.dart';
-
-@JS()
-@anonymous
-class A {
- external factory A(a, b);
-}
-
-main() => new A(1, 2);
-''', errors: const [
- MessageKind.JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS
- ]),
- const Test('External factory constructor with optional parameters.', '''
-@JS()
-library test;
-
-import 'package:js/js.dart';
-
-@JS()
-@anonymous
-class A {
- external factory A([a, b]);
-}
-
-main() => new A(1);
-''', errors: const [
- MessageKind.JS_OBJECT_LITERAL_CONSTRUCTOR_WITH_POSITIONAL_ARGUMENTS
- ]),
const Test('Function-typed return type', '''
@JS()
library lib;
diff --git a/pkg/front_end/messages.status b/pkg/front_end/messages.status
index e8b5454..6459cc1 100644
--- a/pkg/front_end/messages.status
+++ b/pkg/front_end/messages.status
@@ -439,6 +439,8 @@
InvalidVoid/part_wrapped_script2: Fail
InvalidVoid/script1: Fail
InvalidVoid/script2: Fail
+JsInteropAnonymousFactoryPositionalParameters/analyzerCode: Fail # Web compiler specific
+JsInteropAnonymousFactoryPositionalParameters/example: Fail # Web compiler specific
JsInteropIndexNotSupported/analyzerCode: Fail # Web compiler specific
JsInteropIndexNotSupported/example: Fail # Web compiler specific
JsInteropNamedParameters/analyzerCode: Fail # Web compiler specific
diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
index ecf30d2..62ae4a3 100644
--- a/pkg/front_end/messages.yaml
+++ b/pkg/front_end/messages.yaml
@@ -4086,6 +4086,10 @@
NullableMixinError:
template: "Can't mix '#name' in because it's marked with '?'."
+JsInteropAnonymousFactoryPositionalParameters:
+ template: "Factory constructors for @anonymous JS interop classes should not contain any positional parameters."
+ tip: "Try replacing them with named parameters instead."
+
JsInteropIndexNotSupported:
template: "JS interop classes do not support [] and []= operator methods."
tip: "Try replacing with a normal method."
diff --git a/tests/lib/js/constructor_parameters_static_test.dart b/tests/lib/js/constructor_parameters_static_test.dart
index 597ce1e..3efa2a2 100644
--- a/tests/lib/js/constructor_parameters_static_test.dart
+++ b/tests/lib/js/constructor_parameters_static_test.dart
@@ -30,8 +30,22 @@
// ^
// [web] TODO(srujzs): Add error once supported.
+ // Factories of an anonymous class can only contain named parameters.
+ external factory Bar.barFactoryPositional(int? a);
+ // ^
+ // [web] TODO(srujzs): Add error once supported.
+ external factory Bar.barFactoryOptional([int? a]);
+ // ^
+ // [web] TODO(srujzs): Add error once supported.
+ external factory Bar.barFactoryMixedOptional(int? a, [int? b]);
+ // ^
+ // [web] TODO(srujzs): Add error once supported.
+ external factory Bar.barFactoryMixedNamed(int? a, {int? b});
+ // ^
+ // [web] TODO(srujzs): Add error once supported.
+
// Named parameters are okay only for factories of an anonymous class.
- external factory Bar.barFactory({int? a});
+ external factory Bar.barFactoryNamed({int? a});
}
@JS()
