Skip to content

Commit

Permalink
Revert "Implement inheritance/override checks from the spec."
Browse files Browse the repository at this point in the history
This reverts commit 836a1d7.

Revert "Don't use ClassElementImpl for now in override checking."

This reverts commit 58e44c1.

Revert "large_class_declaration_test is slow now."

This reverts commit 56f6c52.

Revert "Add regression test for issue 34392."

This reverts commit ef7d144.

Revert "Mixin declarations don't have supertype, fix isMoreSpecificThan()."

This reverts commit 95b8a19.

Change-Id: Icda9cf9091ef35acc8fd61ac5dc135b3717eba0a
Reviewed-on: https://dart-review.googlesource.com/76301
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Zach Anderson <zra@google.com>
  • Loading branch information
zanderso authored and commit-bot@chromium.org committed Sep 24, 2018
1 parent 355c73d commit 95d37b0
Show file tree
Hide file tree
Showing 25 changed files with 1,034 additions and 845 deletions.
15 changes: 12 additions & 3 deletions pkg/analyzer/lib/error/error.dart
Expand Up @@ -145,8 +145,6 @@ 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 @@ -166,7 +164,6 @@ 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 @@ -535,6 +532,7 @@ 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 @@ -593,6 +591,14 @@ 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 @@ -668,6 +674,9 @@ 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: 8 additions & 15 deletions pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
Expand Up @@ -19,7 +19,6 @@ 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 @@ -295,13 +294,14 @@ class LibraryAnalyzer {
RecordingErrorListener errorListener = _getErrorListener(file);

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

CodeChecker checker =
new CodeChecker(_typeProvider, typeSystem, errorListener, options);
CodeChecker checker = new CodeChecker(
_typeProvider,
new StrongTypeSystemImpl(_typeProvider,
implicitCasts: options.implicitCasts,
declarationCasts: options.declarationCasts,
nonnullableTypes: options.nonnullableTypes),
errorListener,
options);
checker.visitCompilationUnit(unit);

ErrorReporter errorReporter = _getErrorReporter(file);
Expand All @@ -316,13 +316,6 @@ 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
25 changes: 11 additions & 14 deletions pkg/analyzer/lib/src/dart/element/type.dart
Expand Up @@ -1508,11 +1508,9 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
//
List<DartType> jArgs = j.typeArguments;
List<DartType> jVars = jElement.type.typeArguments;
if (supertype != null) {
supertype = supertype.substitute2(jArgs, jVars);
if (supertype == i) {
return true;
}
supertype = supertype.substitute2(jArgs, jVars);
if (supertype == i) {
return true;
}
//
// I is listed in the on clause of J.
Expand Down Expand Up @@ -1766,16 +1764,13 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
}

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

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

var mixins = type.mixins;
startMixinIndex ??= mixins.length;
for (var i = startMixinIndex - 1; i >= 0; i--) {
for (var i = 0; i < mixins.length; i++) {
if (stopMixinIndex != null && i >= stopMixinIndex) {
break;
}
var result = lookUpImpl(mixins[i], acceptAbstract: acceptAbstract);
if (result != null) {
return result;
Expand Down Expand Up @@ -1834,8 +1831,8 @@ class InterfaceTypeImpl extends TypeImpl implements InterfaceType {
} else {
return lookUpImpl(
this,
includeType: thisType,
startMixinIndex: startMixinIndex,
includeType: false,
stopMixinIndex: stopMixinIndex,
);
}
}
Expand Down
66 changes: 64 additions & 2 deletions pkg/analyzer/lib/src/dart/resolver/inheritance_manager.dart
Expand Up @@ -15,6 +15,7 @@ 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 @@ -715,10 +716,34 @@ 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].
* by [classElt]. Also report static warnings
* [StaticTypeWarningCode.INCONSISTENT_METHOD_INHERITANCE] and
* [StaticWarningCode.INCONSISTENT_METHOD_INHERITANCE_GETTER_AND_METHOD] if appropriate.
*
* @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 @@ -813,7 +838,38 @@ class InheritanceManager {
//
resultMap[key] = elements[subtypesOfAllOtherTypesIndexes[0]];
} else {
if (!subtypesOfAllOtherTypesIndexes.isEmpty) {
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 {
//
// Example: class A inherits 2 methods named 'm'.
// One has the function type '(int) -> dynamic' and one has the
Expand All @@ -840,6 +896,12 @@ class InheritanceManager {
resultMap[key] = mergedExecutableElement;
}
}
} else {
_reportError(
classElt,
StaticWarningCode
.INCONSISTENT_METHOD_INHERITANCE_GETTER_AND_METHOD,
[key]);
}
}
});
Expand Down

0 comments on commit 95d37b0

Please sign in to comment.