From 3f1955a97bb6fdd996f68fa0065c384ffac9105d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Peter=20von=20der=20Ahe=CC=81?= Date: Fri, 28 Sep 2018 12:36:55 +0000 Subject: [PATCH] Improve diagnostics about duplication in enums MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I20f8ab79a8dbe6f2604f64cc26c2a91f46fcd2c2 Reviewed-on: https://dart-review.googlesource.com/76600 Commit-Queue: Aske Simon Christensen Reviewed-by: Aske Simon Christensen Auto-Submit: Peter von der Ahé --- .../lib/src/fasta/fasta_codes_generated.dart | 26 +++++++++++++++++++ .../src/fasta/kernel/kernel_enum_builder.dart | 20 +++++++++----- pkg/front_end/messages.yaml | 4 +++ ...duplicated_declarations.dart.direct.expect | 16 ++++++------ ...uplicated_declarations.dart.outline.expect | 16 ++++++------ ...duplicated_declarations.dart.strong.expect | 16 ++++++------ 6 files changed, 68 insertions(+), 30 deletions(-) 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 09102c21728..c7b7ab02209 100644 --- a/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart +++ b/pkg/front_end/lib/src/fasta/fasta_codes_generated.dart @@ -2087,6 +2087,32 @@ Message _withArgumentsDuplicatedDeclarationCause(String name) { arguments: {'name': name}); } +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +const Template< + Message Function( + String + name)> templateDuplicatedDeclarationSyntheticCause = const Template< + Message Function(String name)>( + messageTemplate: + r"""Previous declaration of '#name' is implied by this definition.""", + withArguments: _withArgumentsDuplicatedDeclarationSyntheticCause); + +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +const Code + codeDuplicatedDeclarationSyntheticCause = + const Code( + "DuplicatedDeclarationSyntheticCause", + templateDuplicatedDeclarationSyntheticCause, + severity: Severity.context); + +// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. +Message _withArgumentsDuplicatedDeclarationSyntheticCause(String name) { + return new Message(codeDuplicatedDeclarationSyntheticCause, + message: + """Previous declaration of '${name}' is implied by this definition.""", + arguments: {'name': name}); +} + // DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE. const Template templateDuplicatedDeclarationUse = const Template( diff --git a/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart b/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart index 908aa05e9c0..ba576ed9cfb 100644 --- a/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart +++ b/pkg/front_end/lib/src/fasta/kernel/kernel_enum_builder.dart @@ -34,6 +34,7 @@ import '../fasta_codes.dart' messageNoUnnamedConstructorInObject, templateDuplicatedDeclaration, templateDuplicatedDeclarationCause, + templateDuplicatedDeclarationSyntheticCause, templateEnumConstantSameNameAsEnclosing; import '../modifier.dart' show constMask, finalMask, staticMask; @@ -170,12 +171,19 @@ class KernelEnumBuilder extends SourceClassBuilder EnumConstantInfo enumConstantInfo = enumConstantInfos[i]; List metadata = enumConstantInfo.metadata; String name = enumConstantInfo.name; - int charOffset = enumConstantInfo.charOffset; String documentationComment = enumConstantInfo.documentationComment; MemberBuilder existing = members[name]; if (existing != null) { - List context = existing.isSynthetic - ? null + // The existing declaration is synthetic if it has the same + // charOffset as the enclosing enum. + bool isSynthetic = existing.charOffset == charOffset; + List context = isSynthetic + ? [ + templateDuplicatedDeclarationSyntheticCause + .withArguments(name) + .withLocation( + parent.fileUri, charOffset, className.length) + ] : [ templateDuplicatedDeclarationCause .withArguments(name) @@ -183,13 +191,13 @@ class KernelEnumBuilder extends SourceClassBuilder parent.fileUri, existing.charOffset, name.length) ]; parent.addProblem(templateDuplicatedDeclaration.withArguments(name), - charOffset, name.length, parent.fileUri, + enumConstantInfo.charOffset, name.length, parent.fileUri, context: context); enumConstantInfos[i] = null; } else if (name == className) { parent.addProblem( templateEnumConstantSameNameAsEnclosing.withArguments(name), - charOffset, + enumConstantInfo.charOffset, name.length, parent.fileUri); } @@ -199,7 +207,7 @@ class KernelEnumBuilder extends SourceClassBuilder name, constMask | staticMask, parent, - charOffset, + enumConstantInfo.charOffset, null, true); metadataCollector?.setDocumentationComment( diff --git a/pkg/front_end/messages.yaml b/pkg/front_end/messages.yaml index dd205181c4c..eab3c7f6a65 100644 --- a/pkg/front_end/messages.yaml +++ b/pkg/front_end/messages.yaml @@ -2203,6 +2203,10 @@ DuplicatedDeclarationCause: template: "Previous declaration of '#name'." severity: CONTEXT +DuplicatedDeclarationSyntheticCause: + template: "Previous declaration of '#name' is implied by this definition." + severity: CONTEXT + # Use this message when a duplicated declaration is used. DuplicatedDeclarationUse: template: "Can't use '#name' because it is declared more than once." diff --git a/pkg/front_end/testcases/duplicated_declarations.dart.direct.expect b/pkg/front_end/testcases/duplicated_declarations.dart.direct.expect index 68761c9b582..2ffc2c5856a 100644 --- a/pkg/front_end/testcases/duplicated_declarations.dart.direct.expect +++ b/pkg/front_end/testcases/duplicated_declarations.dart.direct.expect @@ -84,30 +84,30 @@ // pkg/front_end/testcases/duplicated_declarations.dart:83:3: Error: '_name' is already declared in this scope. // _name, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:84:3: Error: 'index' is already declared in this scope. // index, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:85:3: Error: 'toString' is already declared in this scope. // toString, // ^^^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString' is implied by this definition. // enum AnotherEnum { -// ^^^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:86:3: Error: 'values' is already declared in this scope. // values, // ^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values' is implied by this definition. // enum AnotherEnum { -// ^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:57:19: Error: Type 'C' not found. // class Sub extends C { diff --git a/pkg/front_end/testcases/duplicated_declarations.dart.outline.expect b/pkg/front_end/testcases/duplicated_declarations.dart.outline.expect index 4059cc38583..53a54689f71 100644 --- a/pkg/front_end/testcases/duplicated_declarations.dart.outline.expect +++ b/pkg/front_end/testcases/duplicated_declarations.dart.outline.expect @@ -84,30 +84,30 @@ // pkg/front_end/testcases/duplicated_declarations.dart:83:3: Error: '_name' is already declared in this scope. // _name, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:84:3: Error: 'index' is already declared in this scope. // index, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:85:3: Error: 'toString' is already declared in this scope. // toString, // ^^^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString' is implied by this definition. // enum AnotherEnum { -// ^^^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:86:3: Error: 'values' is already declared in this scope. // values, // ^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values' is implied by this definition. // enum AnotherEnum { -// ^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:57:19: Error: Type 'C' not found. // class Sub extends C { diff --git a/pkg/front_end/testcases/duplicated_declarations.dart.strong.expect b/pkg/front_end/testcases/duplicated_declarations.dart.strong.expect index 082eaf5e5a4..22e527ecad8 100644 --- a/pkg/front_end/testcases/duplicated_declarations.dart.strong.expect +++ b/pkg/front_end/testcases/duplicated_declarations.dart.strong.expect @@ -84,30 +84,30 @@ // pkg/front_end/testcases/duplicated_declarations.dart:83:3: Error: '_name' is already declared in this scope. // _name, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of '_name' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:84:3: Error: 'index' is already declared in this scope. // index, // ^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'index' is implied by this definition. // enum AnotherEnum { -// ^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:85:3: Error: 'toString' is already declared in this scope. // toString, // ^^^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'toString' is implied by this definition. // enum AnotherEnum { -// ^^^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:86:3: Error: 'values' is already declared in this scope. // values, // ^^^^^^ -// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values'. +// pkg/front_end/testcases/duplicated_declarations.dart:79:6: Error: Previous declaration of 'values' is implied by this definition. // enum AnotherEnum { -// ^^^^^^ +// ^^^^^^^^^^^ // // pkg/front_end/testcases/duplicated_declarations.dart:57:19: Error: Type 'C' not found. // class Sub extends C {