Skip to content

Commit

Permalink
Implement inheritance/override checks from the spec.
Browse files Browse the repository at this point in the history 
This CL starts moving checks from strong-mode specific checker,
and old InheritanceManager into an implementation that is based
on the current spec, and avoids old baggage. It also fixes the issue
we were asked to fix for Dart 2.1.

Bug: #34392
Change-Id: Id5a23c5db7704b2b530bb894ae92628a08eaa70f
Reviewed-on: https://dart-review.googlesource.com/76061
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
  • Loading branch information
@scheglov
scheglov authored and commit-bot@chromium.org committed Sep 24, 2018
1 parent d070397 commit 836a1d7
Show file tree
Hide file tree
Showing 24 changed files with 832 additions and 1,056 deletions.
15 changes: 3 additions & 12 deletions pkg/analyzer/lib/error/error.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ const List<ErrorCode> errorCodeValues = const [
CompileTimeErrorCode.IMPORT_INTERNAL_LIBRARY,
CompileTimeErrorCode.IMPORT_OF_NON_LIBRARY,
CompileTimeErrorCode.INCONSISTENT_CASE_EXPRESSION_TYPES,
CompileTimeErrorCode.INCONSISTENT_INHERITANCE,
CompileTimeErrorCode.INCONSISTENT_INHERITANCE_GETTER_AND_METHOD,
CompileTimeErrorCode.INITIALIZER_FOR_NON_EXISTENT_FIELD,
CompileTimeErrorCode.INITIALIZER_FOR_STATIC_FIELD,
CompileTimeErrorCode.INITIALIZING_FORMAL_FOR_NON_EXISTENT_FIELD,
Expand All @@ -164,6 +166,7 @@ const List<ErrorCode> errorCodeValues = const [
CompileTimeErrorCode.INVALID_MODIFIER_ON_CONSTRUCTOR,
CompileTimeErrorCode.INVALID_MODIFIER_ON_SETTER,
CompileTimeErrorCode.INVALID_INLINE_FUNCTION_TYPE,
CompileTimeErrorCode.INVALID_OVERRIDE,
CompileTimeErrorCode.INVALID_REFERENCE_TO_THIS,
CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_LIST,
CompileTimeErrorCode.INVALID_TYPE_ARGUMENT_IN_CONST_MAP,
Expand Down Expand Up @@ -532,7 +535,6 @@ const List<ErrorCode> errorCodeValues = const [
StaticTypeWarningCode.ILLEGAL_ASYNC_GENERATOR_RETURN_TYPE,
StaticTypeWarningCode.ILLEGAL_ASYNC_RETURN_TYPE,
StaticTypeWarningCode.ILLEGAL_SYNC_GENERATOR_RETURN_TYPE,
StaticTypeWarningCode.INCONSISTENT_METHOD_INHERITANCE,
StaticTypeWarningCode.INSTANCE_ACCESS_TO_STATIC_MEMBER,
StaticTypeWarningCode.INVALID_ASSIGNMENT,
StaticTypeWarningCode.INVOCATION_OF_NON_FUNCTION,
Expand Down Expand Up @@ -591,14 +593,6 @@ const List<ErrorCode> errorCodeValues = const [
StaticWarningCode.FUNCTION_WITHOUT_CALL,
StaticWarningCode.IMPORT_DUPLICATED_LIBRARY_NAMED,
StaticWarningCode.IMPORT_OF_NON_LIBRARY,
StaticWarningCode.INCONSISTENT_METHOD_INHERITANCE_GETTER_AND_METHOD,
StaticWarningCode.INVALID_GETTER_OVERRIDE_RETURN_TYPE,
StaticWarningCode.INVALID_METHOD_OVERRIDE_NAMED_PARAM_TYPE,
StaticWarningCode.INVALID_METHOD_OVERRIDE_NORMAL_PARAM_TYPE,
StaticWarningCode.INVALID_METHOD_OVERRIDE_OPTIONAL_PARAM_TYPE,
StaticWarningCode.INVALID_METHOD_OVERRIDE_RETURN_TYPE,
StaticWarningCode.INVALID_METHOD_OVERRIDE_TYPE_PARAMETERS,
StaticWarningCode.INVALID_METHOD_OVERRIDE_TYPE_PARAMETER_BOUND,
StaticWarningCode.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_NAMED,
StaticWarningCode.INVALID_OVERRIDE_DIFFERENT_DEFAULT_VALUES_POSITIONAL,
StaticWarningCode.INVALID_OVERRIDE_NAMED,
Expand Down Expand Up @@ -674,9 +668,6 @@ const List<ErrorCode> errorCodeValues = const [
StrongModeCode.INVALID_CAST_METHOD,
StrongModeCode.INVALID_CAST_FUNCTION,
StrongModeCode.INVALID_FIELD_OVERRIDE,
StrongModeCode.INVALID_METHOD_OVERRIDE,
StrongModeCode.INVALID_METHOD_OVERRIDE_FROM_BASE,
StrongModeCode.INVALID_METHOD_OVERRIDE_FROM_MIXIN,
StrongModeCode.INVALID_PARAMETER_DECLARATION,
StrongModeCode.INVALID_SUPER_INVOCATION,
StrongModeCode.NO_DEFAULT_BOUNDS,
Expand Down
23 changes: 15 additions & 8 deletions pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import 'package:analyzer/src/dart/constant/utilities.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/dart/element/handle.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/error/inheritance_override.dart';
import 'package:analyzer/src/error/pending_error.dart';
import 'package:analyzer/src/generated/declaration_resolver.dart';
import 'package:analyzer/src/generated/engine.dart';
Expand Down Expand Up @@ -294,14 +295,13 @@ class LibraryAnalyzer {
RecordingErrorListener errorListener = _getErrorListener(file);

AnalysisOptionsImpl options = _analysisOptions as AnalysisOptionsImpl;
CodeChecker checker = new CodeChecker(
_typeProvider,
new StrongTypeSystemImpl(_typeProvider,
implicitCasts: options.implicitCasts,
declarationCasts: options.declarationCasts,
nonnullableTypes: options.nonnullableTypes),
errorListener,
options);
var typeSystem = new StrongTypeSystemImpl(_typeProvider,
implicitCasts: options.implicitCasts,
declarationCasts: options.declarationCasts,
nonnullableTypes: options.nonnullableTypes);

CodeChecker checker =
new CodeChecker(_typeProvider, typeSystem, errorListener, options);
checker.visitCompilationUnit(unit);

ErrorReporter errorReporter = _getErrorReporter(file);
Expand All @@ -316,6 +316,13 @@ class LibraryAnalyzer {
//
_computeConstantErrors(errorReporter, unit);

//
// Compute inheritance and override errors.
//
var inheritanceOverrideVerifier =
new InheritanceOverrideVerifier(typeSystem, errorReporter);
inheritanceOverrideVerifier.verifyUnit(unit);

//
// Use the ErrorVerifier to compute errors.
//
Expand Down
17 changes: 9 additions & 8 deletions pkg/analyzer/lib/src/dart/element/type.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1761,13 +1761,16 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
}

ExecutableElement lookUpInheritedMember(String name, LibraryElement library,
{bool concrete: false, int stopMixinIndex, bool setter: false}) {
{bool concrete: false,
int startMixinIndex,
bool setter: false,
bool thisType: false}) {
HashSet<ClassElement> visitedClasses = new HashSet<ClassElement>();

ExecutableElement lookUpImpl(InterfaceTypeImpl type,
{bool acceptAbstract: false,
bool includeType: true,
int stopMixinIndex}) {
int startMixinIndex}) {
if (type == null || !visitedClasses.add(type.element)) {
return null;
}
Expand All @@ -1793,10 +1796,8 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
}

var mixins = type.mixins;
for (var i = 0; i < mixins.length; i++) {
if (stopMixinIndex != null && i >= stopMixinIndex) {
break;
}
startMixinIndex ??= mixins.length;
for (var i = startMixinIndex - 1; i >= 0; i--) {
var result = lookUpImpl(mixins[i], acceptAbstract: acceptAbstract);
if (result != null) {
return result;
Expand Down Expand Up @@ -1828,8 +1829,8 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
} else {
return lookUpImpl(
this,
includeType: false,
stopMixinIndex: stopMixinIndex,
includeType: thisType,
startMixinIndex: startMixinIndex,
);
}
}
Expand Down
66 changes: 2 additions & 64 deletions pkg/analyzer/lib/src/dart/resolver/inheritance_manager.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import 'package:analyzer/src/dart/ast/token.dart';
import 'package:analyzer/src/dart/element/element.dart';
import 'package:analyzer/src/dart/element/member.dart';
import 'package:analyzer/src/dart/element/type.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:analyzer/src/generated/type_system.dart';
import 'package:analyzer/src/generated/utilities_dart.dart';

Expand Down Expand Up @@ -716,34 +715,10 @@ class InheritanceManager {
}
}

/**
* This method is used to report errors on when they are found computing inheritance information.
* See [ErrorVerifier.checkForInconsistentMethodInheritance] to see where these generated
* error codes are reported back into the analysis engine.
*
* @param classElt the location of the source for which the exception occurred
* @param offset the offset of the location of the error
* @param length the length of the location of the error
* @param errorCode the error code to be associated with this error
* @param arguments the arguments used to build the error message
*/
void _reportError(
ClassElement classElt, ErrorCode errorCode, List<Object> arguments) {
if (ignoreErrors) {
return;
}
HashSet<AnalysisError> errorSet = _errorsInClassElement.putIfAbsent(
classElt, () => new HashSet<AnalysisError>());
errorSet.add(new AnalysisError(classElt.source, classElt.nameOffset,
classElt.nameLength, errorCode, arguments));
}

/**
* Given the set of methods defined by classes above [classElt] in the class hierarchy,
* apply the appropriate inheritance rules to determine those methods inherited by or overridden
* by [classElt]. Also report static warnings
* [StaticTypeWarningCode.INCONSISTENT_METHOD_INHERITANCE] and
* [StaticWarningCode.INCONSISTENT_METHOD_INHERITANCE_GETTER_AND_METHOD] if appropriate.
* by [classElt].
*
* @param classElt the class element to query.
* @param unionMap a mapping from method name to the set of unique (in terms of signature) methods
Expand Down Expand Up @@ -838,38 +813,7 @@ class InheritanceManager {
//
resultMap[key] = elements[subtypesOfAllOtherTypesIndexes[0]];
} else {
if (subtypesOfAllOtherTypesIndexes.isEmpty) {
//
// Determine if the current class has a method or accessor with
// the member name, if it does then then this class does not
// "inherit" from any of the supertypes. See issue 16134.
//
bool classHasMember = false;
if (allMethods) {
classHasMember = classElt.getMethod(key) != null;
} else {
List<PropertyAccessorElement> accessors = classElt.accessors;
for (int i = 0; i < accessors.length; i++) {
if (accessors[i].name == key) {
classHasMember = true;
}
}
}
//
// Example: class A inherited only 2 method named 'm'.
// One has the function type '() -> int' and one has the function
// type '() -> String'. Since neither is a subtype of the other,
// we create a warning, and have this class inherit nothing.
//
if (!classHasMember) {
String firstTwoFunctionTypesStr =
"${executableElementTypes[0]}, ${executableElementTypes[1]}";
_reportError(
classElt,
StaticTypeWarningCode.INCONSISTENT_METHOD_INHERITANCE,
[key, firstTwoFunctionTypesStr]);
}
} else {
if (!subtypesOfAllOtherTypesIndexes.isEmpty) {
//
// Example: class A inherits 2 methods named 'm'.
// One has the function type '(int) -> dynamic' and one has the
Expand All @@ -896,12 +840,6 @@ class InheritanceManager {
resultMap[key] = mergedExecutableElement;
}
}
} else {
_reportError(
classElt,
StaticWarningCode
.INCONSISTENT_METHOD_INHERITANCE_GETTER_AND_METHOD,
[key]);
}
}
});
Expand Down
Loading

0 comments on commit 836a1d7

Please sign in to comment.