languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-10-14 09:31:58 +00:00
Code Issues Packages Projects Releases Wiki Activity

Issue 34437. Restore checking that optional parameters in derived classes should have the same default values as overridden.

Browse source 
R=brianwilkerson@google.com, paulberry@google.com

Bug: https://github.com/dart-lang/sdk/issues/34437
Change-Id: Ic54d2e074bc764376f970c9c29ba260e7a373d93
Reviewed-on: https://dart-review.googlesource.com/c/91170
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Konstantin Shcheglov 2019-01-26 20:12:55 +00:00 committed by commit-bot@chromium.org
parent df34f65d90
commit 6699384ddd
15 changed files with 332 additions and 225 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
pkg/analysis_server/test/services/completion/dart/local_reference_contributor_test.dart
View file

@ -5,7 +5,6 @@
import 'package:analysis_server/src/protocol_server.dart';
import 'package:analysis_server/src/provisional/completion/dart/completion_dart.dart';
import 'package:analysis_server/src/services/completion/dart/local_reference_contributor.dart';
import 'package:analyzer/file_system/memory_file_system.dart';
import 'package:test/test.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';

12
pkg/analysis_server/test/services/refactoring/rename_local_test.dart
View file

@ -488,12 +488,12 @@ main() {
await indexUnit('/home/test/lib/test2.dart', '''
library test2;
class A {
void foo({int test: 1}) {
void foo({int test}) {
print(test);
}
}
class B extends A {
void foo({int test: 2}) {
void foo({int test}) {
print(test);
}
}
@ -506,7 +506,7 @@ main() {
new C().foo(test: 30);
}
class C extends A {
void foo({int test: 3}) {
void foo({int test}) {
print(test);
}
}
@ -524,7 +524,7 @@ main() {
new C().foo(newName: 30);
}
class C extends A {
void foo({int newName: 3}) {
void foo({int newName}) {
print(newName);
}
}
@ -532,12 +532,12 @@ class C extends A {
assertFileChangeResult('/home/test/lib/test2.dart', '''
library test2;
class A {
void foo({int newName: 1}) {
void foo({int newName}) {
print(newName);
}
}
class B extends A {
void foo({int newName: 2}) {
void foo({int newName}) {
print(newName);
}
}

2
pkg/analyzer/lib/src/dart/analysis/context_locator.dart
View file

@ -75,7 +75,7 @@ class ContextLocatorImpl implements ContextLocator {
@override
List<AnalysisContext> locateContexts(
{@required List<String> includedPaths,
List<String> excludedPaths: null,
List<String> excludedPaths: const <String>[],
String optionsFile: null,
String packagesFile: null,
String sdkPath: null}) {

2
pkg/analyzer/lib/src/dart/analysis/driver.dart
View file

@ -94,7 +94,7 @@ class AnalysisDriver implements AnalysisDriverGeneric {
/**
* The version of data format, should be incremented on every format change.
*/
static const int DATA_VERSION = 77;
static const int DATA_VERSION = 78;
/**
* The number of exception contexts allowed to write. Once this field is

93
pkg/analyzer/lib/src/error/inheritance_override.dart
View file

@ -164,7 +164,8 @@ class _ClassVerifier {
_checkDeclaredMember(fieldList, libraryUri, fieldElement.setter);
}
} else if (member is MethodDeclaration) {
_checkDeclaredMember(member, libraryUri, member.declaredElement);
_checkDeclaredMember(member, libraryUri, member.declaredElement,
methodParameterNodes: member.parameters?.parameters);
}
}
@ -248,8 +249,9 @@ class _ClassVerifier {
void _checkDeclaredMember(
AstNode node,
Uri libraryUri,
ExecutableElement member,
) {
ExecutableElement member, {
List<AstNode> methodParameterNodes,
}) {
if (member == null) return;
if (member.isStatic) return;
@ -276,6 +278,13 @@ class _ClassVerifier {
],
);
}
if (methodParameterNodes != null) {
_checkForOptionalParametersDifferentDefaultValues(
superMemberType.element,
member,
methodParameterNodes,
);
}
}
}
}
@ -406,6 +415,84 @@ class _ClassVerifier {
}
}
void _checkForOptionalParametersDifferentDefaultValues(
ExecutableElement baseExecutable,
ExecutableElement derivedExecutable,
List<AstNode> derivedParameterNodes,
) {
var derivedOptionalNodes = <AstNode>[];
var derivedOptionalElements = <ParameterElementImpl>[];
var derivedParameterElements = derivedExecutable.parameters;
for (var i = 0; i < derivedParameterElements.length; i++) {
var parameterElement = derivedParameterElements[i];
if (parameterElement.isOptional) {
derivedOptionalNodes.add(derivedParameterNodes[i]);
derivedOptionalElements.add(parameterElement);
}
}
var baseOptionalElements = <ParameterElementImpl>[];
var baseParameterElements = baseExecutable.parameters;
for (var i = 0; i < baseParameterElements.length; ++i) {
var baseParameter = baseParameterElements[i];
if (baseParameter.isOptional) {
baseOptionalElements.add(baseParameter);
}
}
// Stop if no optional parameters.
if (baseOptionalElements.isEmpty || derivedOptionalElements.isEmpty) {
return;
}
if (derivedOptionalElements[0].isNamed) {
for (int i = 0; i < derivedOptionalElements.length; i++) {
var derivedElement = derivedOptionalElements[i];
var name = derivedElement.name;
for (var j = 0; j < baseOptionalElements.length; j++) {
var baseParameter = baseOptionalElements[j];
if (name == baseParameter.name && baseParameter.initializer != null) {
var baseValue = baseParameter.computeConstantValue();
var derivedResult = derivedElement.evaluationResult;
if (derivedResult.value != baseValue) {
reporter.reportErrorForNode(
StaticWarningCode
.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_NAMED,
derivedOptionalNodes[i],
[
baseExecutable.enclosingElement.displayName,
baseExecutable.displayName,
name
],
);
}
}
}
}
} else {
for (var i = 0;
i < derivedOptionalElements.length && i < baseOptionalElements.length;
i++) {
var baseElement = baseOptionalElements[i];
if (baseElement.initializer != null) {
var baseValue = baseElement.computeConstantValue();
var derivedResult = derivedOptionalElements[i].evaluationResult;
if (derivedResult.value != baseValue) {
reporter.reportErrorForNode(
StaticWarningCode
.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_POSITIONAL,
derivedOptionalNodes[i],
[
baseExecutable.enclosingElement.displayName,
baseExecutable.displayName
],
);
}
}
}
}
}
/// Check that [classElement] is not a superinterface to itself.
/// The [path] is a list containing the potentially cyclic implements path.
///

67
pkg/analyzer/test/generated/non_error_resolver.dart
View file

@ -2910,73 +2910,6 @@ class B implements A {
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_named() async {
Source source = addSource(r'''
class A {
m({int p : 0}) {}
}
class B extends A {
m({int p : 0}) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_named_function() async {
Source source = addSource(r'''
nothing() => 'nothing';
class A {
thing(String a, {orElse : nothing}) {}
}
class B extends A {
thing(String a, {orElse : nothing}) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_positional() async {
Source source = addSource(r'''
class A {
m([int p = 0]) {}
}
class B extends A {
m([int p = 0]) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_positional_changedOrder() async {
Source source = addSource(r'''
class A {
m([int a = 0, String b = '0']) {}
}
class B extends A {
m([int b = 0, String a = '0']) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_positional_function() async {
Source source = addSource(r'''
nothing() => 'nothing';
class A {
thing(String a, [orElse = nothing]) {}
}
class B extends A {
thing(String a, [orElse = nothing]) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideNamed_unorderedNamedParameter() async {
Source source = addSource(r'''
class A {

130
pkg/analyzer/test/generated/static_warning_code_test.dart
View file

@ -2173,136 +2173,6 @@ class B extends A {
verify([source]);
}
test_invalidOverride_defaultOverridesNonDefault() async {
// If the base class provided an explicit value for a default parameter,
// then it is a static warning for the derived class to provide a different
// value, even if implicitly.
Source source = addSource(r'''
class A {
foo([x = 1]) {}
}
class B extends A {
foo([x]) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverride_defaultOverridesNonDefault_named() async {
// If the base class provided an explicit value for a default parameter,
// then it is a static warning for the derived class to provide a different
// value, even if implicitly.
Source source = addSource(r'''
class A {
foo({x: 1}) {}
}
class B extends A {
foo({x}) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverride_defaultOverridesNonDefaultNull() async {
// If the base class provided an explicit null value for a default
// parameter, then it is ok for the derived class to let the default value
// be implicit, because the implicit default value of null matches the
// explicit default value of null.
Source source = addSource(r'''
class A {
foo([x = null]) {}
}
class B extends A {
foo([x]) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverride_defaultOverridesNonDefaultNull_named() async {
// If the base class provided an explicit null value for a default
// parameter, then it is ok for the derived class to let the default value
// be implicit, because the implicit default value of null matches the
// explicit default value of null.
Source source = addSource(r'''
class A {
foo({x: null}) {}
}
class B extends A {
foo({x}) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverride_nonDefaultOverridesDefault() async {
// If the base class lets the default parameter be implicit, then it is ok
// for the derived class to provide an explicit default value, even if it's
// not null.
Source source = addSource(r'''
class A {
foo([x]) {}
}
class B extends A {
foo([x = 1]) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverride_nonDefaultOverridesDefault_named() async {
// If the base class lets the default parameter be implicit, then it is ok
// for the derived class to provide an explicit default value, even if it's
// not null.
Source source = addSource(r'''
class A {
foo({x}) {}
}
class B extends A {
foo({x: 1}) {}
}
''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_named() async {
Source source = addSource(r'''
class A {
m({int p : 0}) {}
}
class B extends A {
m({int p : 1}) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideDifferentDefaultValues_positional() async {
Source source = addSource(r'''
class A {
m([int p = 0]) {}
}
class B extends A {
m([int p = 1]) {}
}''');
await computeAnalysisResult(source);
assertNoErrors(source);
verify([source]);
}
test_invalidOverrideNamed_fewerNamedParameters() async {
Source source = addSource(r'''
class A {

12
pkg/analyzer/test/src/dart/resolution/resolution.dart
View file

@ -164,6 +164,12 @@ mixin ResolutionTest implements ResourceProviderMixin {
errorListener.assertErrorsWithCodes(expected);
}
Future<void> assertErrorsInCode(String code, List<ErrorCode> errors) async {
addTestFile(code);
await resolveTestFile();
assertTestErrors(errors);
}
void assertHasTestErrors() {
expect(result.errors, isNotEmpty);
}
@ -295,6 +301,12 @@ mixin ResolutionTest implements ResourceProviderMixin {
assertTypeNull(ref);
}
Future<void> assertNoErrorsInCode(String code) async {
addTestFile(code);
await resolveTestFile();
assertNoTestErrors();
}
void assertNoTestErrors() {
assertTestErrors(const <ErrorCode>[]);
}

202
pkg/analyzer/test/src/diagnostics/invalid_override_different_default_values_test.dart Normal file
View file

@ -0,0 +1,202 @@
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
// 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.
import 'package:analyzer/src/error/codes.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import '../dart/resolution/driver_resolution.dart';
main() {
defineReflectiveSuite(() {
defineReflectiveTests(InvalidOverrideDifferentDefaultValuesTest);
});
}
@reflectiveTest
class InvalidOverrideDifferentDefaultValuesTest extends DriverResolutionTest {
test_baseClassInOtherLibrary() async {
newFile('/test/lib/a.dart', content: r'''
class A {
foo([a = 0]) {}
}
''');
await assertNoErrorsInCode(r'''
import 'a.dart';
class C extends A {
foo([a = 0]) {}
}
''');
}
test_differentValues_named() async {
await _assertErrorNamed(r'''
class A {
m({x = 0}) {}
}
class B extends A {
m({x = 1}) {}
}''');
}
test_differentValues_positional() async {
await _assertErrorPositional(r'''
class A {
m([x = 0]) {}
}
class B extends A {
m([x = 1]) {}
}''');
}
test_equalValues_named() async {
await assertNoErrorsInCode(r'''
abstract class A {
foo({x = 1});
}
class C extends A {
foo({x = 3 - 2}) {}
}
''');
}
test_equalValues_named_function() async {
await assertNoErrorsInCode(r'''
nothing() => 'nothing';
class A {
foo(String a, {orElse = nothing}) {}
}
class B extends A {
foo(String a, {orElse = nothing}) {}
}
''');
}
test_equalValues_positional() async {
await assertNoErrorsInCode(r'''
abstract class A {
foo([x = 1]);
}
class C extends A {
foo([x = 3 - 2]) {}
}
''');
}
test_equalValues_positional_function() async {
await assertNoErrorsInCode(r'''
nothing() => 'nothing';
class A {
foo(String a, [orElse = nothing]) {}
}
class B extends A {
foo(String a, [orElse = nothing]) {}
}
''');
}
test_explicitNull_overriddenWith_implicitNull_named() async {
// If the base class provided an explicit null value for a default
// parameter, then it is ok for the derived class to let the default value
// be implicit, because the implicit default value of null matches the
// explicit default value of null.
await assertNoErrorsInCode(r'''
class A {
foo({x: null}) {}
}
class B extends A {
foo({x}) {}
}
''');
}
test_explicitNull_overriddenWith_implicitNull_positional() async {
// If the base class provided an explicit null value for a default
// parameter, then it is ok for the derived class to let the default value
// be implicit, because the implicit default value of null matches the
// explicit default value of null.
await assertNoErrorsInCode(r'''
class A {
foo([x = null]) {}
}
class B extends A {
foo([x]) {}
}
''');
}
test_implicitNull_overriddenWith_value_named() async {
// If the base class lets the default parameter be implicit, then it is ok
// for the derived class to provide an explicit default value, even if it's
// not null.
await assertNoErrorsInCode(r'''
class A {
foo({x}) {}
}
class B extends A {
foo({x = 1}) {}
}
''');
}
test_implicitNull_overriddenWith_value_positional() async {
// If the base class lets the default parameter be implicit, then it is ok
// for the derived class to provide an explicit default value, even if it's
// not null.
await assertNoErrorsInCode(r'''
class A {
foo([x]) {}
}
class B extends A {
foo([x = 1]) {}
}
''');
}
test_value_overriddenWith_implicitNull_named() async {
// If the base class provided an explicit value for a default parameter,
// then it is a static warning for the derived class to provide a different
// value, even if implicitly.
await _assertErrorNamed(r'''
class A {
foo({x: 1}) {}
}
class B extends A {
foo({x}) {}
}
''');
}
test_value_overriddenWith_implicitNull_positional() async {
// If the base class provided an explicit value for a default parameter,
// then it is a static warning for the derived class to provide a different
// value, even if implicitly.
await _assertErrorPositional(r'''
class A {
foo([x = 1]) {}
}
class B extends A {
foo([x]) {}
}
''');
}
Future<void> _assertErrorNamed(String code) async {
await assertErrorsInCode(code, [
StaticWarningCode.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_NAMED,
]);
}
Future<void> _assertErrorPositional(String code) async {
await assertErrorsInCode(code, [
StaticWarningCode.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_POSITIONAL,
]);
}
}

3
pkg/analyzer/test/src/diagnostics/test_all.dart
View file

@ -10,6 +10,8 @@ import 'deprecated_member_use_test.dart' as deprecated_member_use;
import 'division_optimization_test.dart' as division_optimization;
import 'invalid_assignment_test.dart' as invalid_assignment;
import 'invalid_cast_new_expr_test.dart' as invalid_cast_new_expr;
import 'invalid_override_different_default_values_test.dart'
as invalid_override_different_default_values;
import 'invalid_required_param_test.dart' as invalid_required_param;
import 'undefined_getter.dart' as undefined_getter;
import 'unnecessary_cast_test.dart' as unnecessary_cast;
@ -27,6 +29,7 @@ main() {
division_optimization.main();
invalid_assignment.main();
invalid_cast_new_expr.main();
invalid_override_different_default_values.main();
invalid_required_param.main();
undefined_getter.main();
unnecessary_cast.main();

2
pkg/analyzer_plugin/lib/utilities/change_builder/change_builder_dart.dart
View file

@ -214,7 +214,7 @@ abstract class DartEditBuilder implements EditBuilder {
void writeOverride(
FunctionType signature, {
StringBuffer displayTextBuffer,
bool invokeSuper: true,
bool invokeSuper: false,
});
/**

2
pkg/front_end/lib/src/fasta/kernel/kernel_expression_generator.dart
View file

@ -1482,7 +1482,7 @@ class KernelUnlinkedGenerator extends KernelGenerator with UnlinkedGenerator {
super(helper, token);
@override
Expression buildAssignment(Expression value, {bool voidContext}) {
Expression buildAssignment(Expression value, {bool voidContext: false}) {
return new PropertySet(receiver, name, value)
..fileOffset = offsetForToken(token);
}

20
pkg/front_end/lib/src/fasta/kernel/kernel_expression_generator_impl.dart
View file

@ -302,7 +302,7 @@ class SendAccessGenerator extends IncompleteSendGenerator {
}
Expression buildCompoundAssignment(Name binaryOperator, Expression value,
{int offset,
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget,
bool isPreIncDec: false,
@ -312,13 +312,17 @@ class SendAccessGenerator extends IncompleteSendGenerator {
}
Expression buildPrefixIncrement(Name binaryOperator,
{int offset, bool voidContext: false, Procedure interfaceTarget}) {
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget}) {
return unsupported(
"buildPrefixIncrement", offset ?? offsetForToken(token), uri);
}
Expression buildPostfixIncrement(Name binaryOperator,
{int offset, bool voidContext: false, Procedure interfaceTarget}) {
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget}) {
return unsupported(
"buildPostfixIncrement", offset ?? offsetForToken(token), uri);
}
@ -372,7 +376,7 @@ class IncompletePropertyAccessGenerator extends IncompleteSendGenerator {
}
Expression buildCompoundAssignment(Name binaryOperator, Expression value,
{int offset,
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget,
bool isPreIncDec: false,
@ -382,13 +386,17 @@ class IncompletePropertyAccessGenerator extends IncompleteSendGenerator {
}
Expression buildPrefixIncrement(Name binaryOperator,
{int offset, bool voidContext: false, Procedure interfaceTarget}) {
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget}) {
return unsupported(
"buildPrefixIncrement", offset ?? offsetForToken(token), uri);
}
Expression buildPostfixIncrement(Name binaryOperator,
{int offset, bool voidContext: false, Procedure interfaceTarget}) {
{int offset: TreeNode.noOffset,
bool voidContext: false,
Procedure interfaceTarget}) {
return unsupported(
"buildPostfixIncrement", offset ?? offsetForToken(token), uri);
}

7
tests/co19_2/co19_2-analyzer.status
View file

@ -3,15 +3,8 @@
# BSD-style license that can be found in the LICENSE file.
[ $compiler == dart2analyzer ]
Language/Classes/Abstract_Instance_Members/override_default_value_t01: MissingCompileTimeError # Issue 33995
Language/Classes/Abstract_Instance_Members/override_default_value_t02: MissingCompileTimeError # Issue 33995
Language/Classes/Abstract_Instance_Members/override_default_value_t03: MissingCompileTimeError # Issue 33995
Language/Classes/Abstract_Instance_Members/override_default_value_t04: MissingCompileTimeError # Issue 33995
Language/Classes/Abstract_Instance_Members/override_default_value_t05: MissingCompileTimeError # Issue 33995
Language/Classes/Getters/type_object_t01: CompileTimeError # Issue 33995
Language/Classes/Getters/type_object_t02: CompileTimeError # Issue 33995
Language/Classes/Instance_Methods/override_different_default_values_t01: MissingCompileTimeError # Issue 33995
Language/Classes/Instance_Methods/override_different_default_values_t02: MissingCompileTimeError # Issue 33995
Language/Classes/Static_Methods/same_name_method_and_setter_t01: CompileTimeError # Invalid test, see #33237
Language/Classes/method_definition_t06: MissingCompileTimeError # Please triage this failure
Language/Enums/syntax_t08: CompileTimeError # Issue 33995

2
tests/compiler/dart2js/end_to_end/bad_output_io_test.dart
View file

@ -44,7 +44,7 @@ class CollectingFormattingDiagnosticHandler
final messages = [];
void info(var message, [kind]) {
void info(var message, [kind = Diagnostic.VERBOSE_INFO]) {
messages.add([message, kind]);
}