[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>
This commit is contained in:
Srujan Gaddam 2020-08-03 23:20:42 +00:00 committed by commit-bot@chromium.org
parent 43c68d782d
commit 7bb0d1f39a
8 changed files with 51 additions and 69 deletions

View file

@ -5409,6 +5409,17 @@ Message _withArgumentsInvokeNonFunction(String name) {
arguments: {'name': name});
}
// 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;

View file

@ -8,6 +8,7 @@ import 'package:_fe_analyzer_shared/src/messages/codes.dart'
show
Message,
LocatedMessage,
messageJsInteropAnonymousFactoryPositionalParameters,
messageJsInteropIndexNotSupported,
messageJsInteropNamedParameters,
messageJsInteropNonExternalConstructor;
@ -32,7 +33,20 @@ class JsInteropChecks extends RecursiveVisitor<void> {
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 @@ class JsInteropChecks extends RecursiveVisitor<void> {
/// 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);
}
}
}

View file

@ -197,27 +197,6 @@ class MessageTemplate {
"""
]),
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'."),

View file

@ -151,16 +151,6 @@ class KernelAnnotationProcessor implements AnnotationProcessor {
_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) {

View file

@ -357,38 +357,6 @@ class A {
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;

View file

@ -439,6 +439,8 @@ InvalidVoid/part_wrapped_script1: Fail
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

View file

@ -4086,6 +4086,10 @@ NullableInterfaceError:
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."

View file

@ -30,8 +30,22 @@ class Bar {
// ^
// [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()