Disallow default values in redirecting factory constructors.
Closes https://github.com/dart-lang/sdk/issues/34160
Change-Id: Idf6ee172b5fe81aa17920d12b68525a37a3c2231
Reviewed-on: https://dart-review.googlesource.com/72660
Commit-Queue: Daniel Hillerström <hillerstrom@google.com>
Reviewed-by: Peter von der Ahé <ahe@google.com>
diff --git a/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart b/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart
index 705d94e..b04f0f2 100644
--- a/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart
+++ b/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart
@@ -1709,6 +1709,33 @@
message: r"""This is the inherited member.""");
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Template<Message Function(String name)>
+ templateDefaultValueInRedirectingFactoryConstructor =
+ const Template<Message Function(String name)>(
+ messageTemplate:
+ r"""Can't have a default value here because any default values of '#name' would be used instead.""",
+ tipTemplate: r"""Try removing the default value.""",
+ withArguments:
+ _withArgumentsDefaultValueInRedirectingFactoryConstructor);
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+const Code<Message Function(String name)>
+ codeDefaultValueInRedirectingFactoryConstructor =
+ const Code<Message Function(String name)>(
+ "DefaultValueInRedirectingFactoryConstructor",
+ templateDefaultValueInRedirectingFactoryConstructor,
+ analyzerCode: "DEFAULT_VALUE_IN_REDIRECTING_FACTORY_CONSTRUCTOR");
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
+Message _withArgumentsDefaultValueInRedirectingFactoryConstructor(String name) {
+ return new Message(codeDefaultValueInRedirectingFactoryConstructor,
+ message:
+ """Can't have a default value here because any default values of '${name}' would be used instead.""",
+ tip: """Try removing the default value.""",
+ arguments: {'name': name});
+}
+
+// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
const Code<Null> codeDeferredAfterPrefix = messageDeferredAfterPrefix;
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
diff --git a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
index b6bd338..60fb6cc 100644
--- a/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
+++ b/pkg/front_end/lib/src/fasta/kernel/body_builder.dart
@@ -2480,7 +2480,17 @@
member.fileUri);
} else {
variable = formal.build(library);
- variable.initializer = name.initializer;
+ if (member is KernelRedirectingFactoryBuilder &&
+ name.initializer != null) {
+ KernelRedirectingFactoryBuilder factory = member;
+ addCompileTimeError(
+ fasta.templateDefaultValueInRedirectingFactoryConstructor
+ .withArguments(factory.redirectionTarget.fullNameForErrors),
+ name.initializer.fileOffset,
+ noLength);
+ } else {
+ variable.initializer = name.initializer;
+ }
}
} else {
variable = new VariableDeclarationJudgment(
diff --git a/pkg/front_end/lib/src/fasta/source/diet_listener.dart b/pkg/front_end/lib/src/fasta/source/diet_listener.dart
index fcab286..1a218f2 100644
--- a/pkg/front_end/lib/src/fasta/source/diet_listener.dart
+++ b/pkg/front_end/lib/src/fasta/source/diet_listener.dart
@@ -42,9 +42,6 @@
import '../kernel/kernel_function_type_alias_builder.dart'
show KernelFunctionTypeAliasBuilder;
-import '../kernel/kernel_procedure_builder.dart'
- show KernelRedirectingFactoryBuilder;
-
import '../parser.dart' show Assert, MemberKind, Parser, optional;
import '../problems.dart' show DebugAbort, internalProblem, unexpected;
@@ -282,8 +279,9 @@
String name = pop();
Token metadata = pop();
checkEmpty(beginToken.charOffset);
- buildFunctionBody(bodyToken, lookupBuilder(beginToken, getOrSet, name),
- MemberKind.TopLevelMethod, metadata);
+ final StackListener listener =
+ createFunctionListener(lookupBuilder(beginToken, getOrSet, name));
+ buildFunctionBody(listener, bodyToken, metadata, MemberKind.TopLevelMethod);
}
@override
@@ -492,37 +490,16 @@
Object name = pop();
Token metadata = pop();
checkEmpty(beginToken.charOffset);
- if (bodyToken == null || optional("=", bodyToken.endGroup.next)) {
- // TODO(dmitryas): Consider building redirecting factory bodies here.
- KernelRedirectingFactoryBuilder factory =
- lookupConstructor(beginToken, name);
- parseMetadata(factory, metadata, factory.target);
- if (factory.formals != null) {
- List<int> metadataOffsets = new List<int>(factory.formals.length);
- for (int i = 0; i < factory.formals.length; ++i) {
- List<MetadataBuilder> metadata = factory.formals[i].metadata;
- if (metadata != null && metadata.length > 0) {
- // [parseMetadata] is using [Parser.parseMetadataStar] under the
- // hood, so we only need the offset of the first annotation.
- metadataOffsets[i] = metadata[0].charOffset;
- } else {
- metadataOffsets[i] = -1;
- }
- }
- List<Token> metadataTokens =
- tokensForOffsets(beginToken, endToken, metadataOffsets);
- for (int i = 0; i < factory.formals.length; ++i) {
- Token metadata = metadataTokens[i];
- if (metadata == null) continue;
- parseMetadata(
- factory.formals[i], metadata, factory.formals[i].target);
- }
- }
- return;
+ ProcedureBuilder builder = lookupConstructor(beginToken, name);
+ if (bodyToken == null || optional("=", bodyToken.endGroup.next)) {
+ parseMetadata(builder, metadata, builder.target);
+ buildRedirectingFactoryMethod(
+ bodyToken, builder, MemberKind.Factory, metadata);
+ } else {
+ buildFunctionBody(createFunctionListener(builder), bodyToken, metadata,
+ MemberKind.Factory);
}
- buildFunctionBody(bodyToken, lookupConstructor(beginToken, name),
- MemberKind.Factory, metadata);
}
@override
@@ -569,10 +546,12 @@
builder = lookupBuilder(beginToken, getOrSet, name);
}
buildFunctionBody(
+ createFunctionListener(builder),
beginParam,
- builder,
- builder.isStatic ? MemberKind.StaticMethod : MemberKind.NonStaticMethod,
- metadata);
+ metadata,
+ builder.isStatic
+ ? MemberKind.StaticMethod
+ : MemberKind.NonStaticMethod);
}
StackListener createListener(
@@ -604,19 +583,40 @@
..constantContext = constantContext;
}
- void buildFunctionBody(
- Token token, ProcedureBuilder builder, MemberKind kind, Token metadata) {
- Scope typeParameterScope = builder.computeTypeParameterScope(memberScope);
- Scope formalParameterScope =
+ StackListener createFunctionListener(ProcedureBuilder builder) {
+ final Scope typeParameterScope =
+ builder.computeTypeParameterScope(memberScope);
+ final Scope formalParameterScope =
builder.computeFormalParameterScope(typeParameterScope);
assert(typeParameterScope != null);
assert(formalParameterScope != null);
- parseFunctionBody(
- createListener(builder, typeParameterScope, builder.isInstanceMember,
- formalParameterScope),
- token,
- metadata,
- kind);
+ return createListener(builder, typeParameterScope, builder.isInstanceMember,
+ formalParameterScope);
+ }
+
+ void buildRedirectingFactoryMethod(
+ Token token, ProcedureBuilder builder, MemberKind kind, Token metadata) {
+ final StackListener listener = createFunctionListener(builder);
+ try {
+ Parser parser = new Parser(listener);
+
+ if (metadata != null) {
+ parser.parseMetadataStar(parser.syntheticPreviousToken(metadata));
+ listener.pop();
+ }
+
+ token = parser.parseFormalParametersOpt(
+ parser.syntheticPreviousToken(token), MemberKind.Factory);
+
+ listener.pop();
+ listener.checkEmpty(token.next.charOffset);
+ } on DebugAbort {
+ rethrow;
+ } on deprecated_InputError {
+ rethrow;
+ } catch (e, s) {
+ throw new Crash(uri, token.charOffset, e, s);
+ }
}
void buildFields(int count, Token token, bool isTopLevel) {
@@ -745,7 +745,7 @@
listener.finishFields();
}
- void parseFunctionBody(StackListener listener, Token startToken,
+ void buildFunctionBody(StackListener listener, Token startToken,
Token metadata, MemberKind kind) {
Token token = startToken;
try {
@@ -817,14 +817,11 @@
Declaration lookupConstructor(Token token, Object nameOrQualified) {
assert(currentClass != null);
Declaration declaration;
- String name;
String suffix;
if (nameOrQualified is QualifiedName) {
- name = nameOrQualified.prefix;
suffix = nameOrQualified.suffix;
} else {
- name = nameOrQualified;
- suffix = name == currentClass.name ? "" : name;
+ suffix = nameOrQualified == currentClass.name ? "" : nameOrQualified;
}
declaration = currentClass.constructors.local[suffix];
checkBuilder(token, declaration, nameOrQualified);
diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml
index 013e5f6..a61d019 100644
--- a/pkg/front_end/messages.yaml
+++ b/pkg/front_end/messages.yaml
@@ -2740,3 +2740,14 @@
analyzerCode: CONST_INSTANCE_FIELD
script:
- "class C { const field = 0; }"
+
+DefaultValueInRedirectingFactoryConstructor:
+ template: "Can't have a default value here because any default values of '#name' would be used instead."
+ tip: "Try removing the default value."
+ analyzerCode: DEFAULT_VALUE_IN_REDIRECTING_FACTORY_CONSTRUCTOR
+ script:
+ - >-
+ class A {
+ factory A.f({int x = 42}) = A.g;
+ A.g() {}
+ }
diff --git a/tests/co19/co19-kernel.status b/tests/co19/co19-kernel.status
index e53e8f7..70c8436 100644
--- a/tests/co19/co19-kernel.status
+++ b/tests/co19/co19-kernel.status
@@ -159,8 +159,6 @@
Language/Classes/Constructors/Factories/accessible_class_t03/01: CompileTimeError
Language/Classes/Constructors/Factories/accessible_class_t04/01: CompileTimeError
Language/Classes/Constructors/Factories/accessible_class_t04/02: CompileTimeError
-Language/Classes/Constructors/Factories/default_value_t01: MissingCompileTimeError
-Language/Classes/Constructors/Factories/default_value_t02: MissingCompileTimeError
Language/Classes/Constructors/Factories/function_type_t02: CompileTimeError
Language/Classes/Constructors/Factories/redirecting_constructor_call_t01: CompileTimeError
Language/Classes/Constructors/Factories/redirecting_constructor_call_t02: CompileTimeError
diff --git a/tests/co19_2/co19_2-kernel.status b/tests/co19_2/co19_2-kernel.status
index 1fdcfee1..82afac9 100644
--- a/tests/co19_2/co19_2-kernel.status
+++ b/tests/co19_2/co19_2-kernel.status
@@ -19,8 +19,6 @@
Language/Classes/Constructors/Constant_Constructors/invalid_constant_initializer_t02: MissingCompileTimeError # Issue 34192
Language/Classes/Constructors/Constant_Constructors/potentially_constant_expression_t01: MissingCompileTimeError # Issue 34192
Language/Classes/Constructors/Factories/arguments_type_t01: MissingCompileTimeError # Issue 33308
-Language/Classes/Constructors/Factories/default_value_t01: MissingCompileTimeError # Issue 34160
-Language/Classes/Constructors/Factories/default_value_t02: MissingCompileTimeError # Issue 34160
Language/Classes/Constructors/Factories/function_type_t01: MissingCompileTimeError # Issue 31590
Language/Classes/Constructors/Factories/function_type_t02: MissingCompileTimeError # Issue 31590
Language/Classes/Constructors/Generative_Constructors/final_variables_t01: CompileTimeError # Expects a warning, but this is an error in Dart 2
diff --git a/tests/language_2/language_2_kernel.status b/tests/language_2/language_2_kernel.status
index b65a22f..c6a6d0e 100644
--- a/tests/language_2/language_2_kernel.status
+++ b/tests/language_2/language_2_kernel.status
@@ -269,9 +269,6 @@
mixin_invalid_bound_test/10: MissingCompileTimeError # Issue 33308
mixin_super_bound_test/01: MissingCompileTimeError # Issue 33308
mixin_super_bound_test/02: MissingCompileTimeError # Issue 33308
-named_parameters_default_eq_test/02: MissingCompileTimeError # Issue 34160
-redirecting_factory_default_values_test/01: MissingCompileTimeError # Issue 34160
-redirecting_factory_default_values_test/02: MissingCompileTimeError # Issue 34160
regress_22976_test/*: CompileTimeError # Issue 31935
syntax_test/28: MissingCompileTimeError # Issue 29763 - low priority
syntax_test/29: MissingCompileTimeError # Issue 29763 - low priority
