Skip to content

Commit

Permalink
Consider both getter and setter when deciding whether an extension re…
Browse files Browse the repository at this point in the history 
…ference is ambiguous.

Change-Id: I8acbabda7f748e7415bd38a957e96f906afd7f6b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/116250
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 9, 2019
1 parent 4310d15 commit 0851b9a
Show file tree
Hide file tree
Showing 7 changed files with 267 additions and 238 deletions.
20 changes: 20 additions & 0 deletions pkg/analyzer/lib/src/dart/element/member.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,16 @@ class FieldMember extends VariableMember implements FieldElement {
Substitution.fromInterfaceType(definingType),
);
}

static FieldElement from2(
FieldElement element,
MapSubstitution substitution,
) {
if (substitution.map.isEmpty) {
return element;
}
return FieldMember(element, substitution);
}
}

/**
Expand Down Expand Up @@ -643,6 +653,16 @@ class MethodMember extends ExecutableMember implements MethodElement {
Substitution.fromInterfaceType(definingType),
);
}

static MethodElement from2(
MethodElement element,
MapSubstitution substitution,
) {
if (substitution.map.isEmpty) {
return element;
}
return MethodMember(element, substitution);
}
}

/**
Expand Down
165 changes: 78 additions & 87 deletions pkg/analyzer/lib/src/dart/resolver/extension_member_resolver.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,56 +31,64 @@ class ExtensionMemberResolver {
TypeSystem get _typeSystem => _resolver.typeSystem;

/// Return the most specific extension in the current scope for this [type],
/// that defines the member with the the [name] and [kind].
/// that defines the member with the given [name].
///
/// If no applicable extensions, return `null`.
/// If no applicable extensions, return [ResolutionResult.none].
///
/// If the match is ambiguous, report an error and return `null`.
/// If the match is ambiguous, report an error and return
/// [ResolutionResult.ambiguous].
ResolutionResult findExtension(
DartType type, String name, Expression target, ElementKind kind) {
var extensions = _getApplicable(type, name, kind);
DartType type,
String name,
Expression target,
) {
var extensions = _getApplicable(type, name);

if (extensions.isEmpty) {
return ResolutionResult.none;
}

if (extensions.length == 1) {
return ResolutionResult(extensions[0].instantiatedMember);
return extensions[0].asResolutionResult;
}

var extension = _chooseMostSpecific(extensions);
if (extension != null) {
return ResolutionResult(extension.instantiatedMember);
return extension.asResolutionResult;
}

_errorReporter.reportErrorForNode(
CompileTimeErrorCode.AMBIGUOUS_EXTENSION_MEMBER_ACCESS,
target,
[
name,
extensions[0].element.name,
extensions[1].element.name,
extensions[0].extension.name,
extensions[1].extension.name,
],
);
return ResolutionResult.ambiguous;
}

/// Return the member with the [name] (without `=`) of the given [kind].
/// Return the member with the [name] (without `=`).
///
/// The [node] is fully resolved, and its type arguments are set.
ExecutableElement getOverrideMember(
ExtensionOverride node, String name, ElementKind kind) {
ExtensionOverride node,
String name, {
bool setter = false,
}) {
ExtensionElement element = node.extensionName.staticElement;

ExecutableElement member;
if (kind == ElementKind.GETTER) {
member = element.getGetter(name);
} else if (kind == ElementKind.METHOD) {
member = element.getMethod(name);
} else if (kind == ElementKind.SETTER) {
if (setter) {
member = element.getSetter(name);
} else {
member = element.getGetter(name) ?? element.getMethod(name);
}

if (member == null) {
return null;
}
if (member == null) return null;

return ExecutableMember.from2(
member,
Expand Down Expand Up @@ -232,9 +240,8 @@ class ExtensionMemberResolver {

/// Return extensions for the [type] that match the given [name] in the
/// current scope.
List<_InstantiatedExtension> _getApplicable(
DartType type, String name, ElementKind kind) {
var candidates = _getExtensionsWithMember(name, kind);
List<_InstantiatedExtension> _getApplicable(DartType type, String name) {
var candidates = _getExtensionsWithMember(name);

var instantiatedExtensions = <_InstantiatedExtension>[];
for (var candidate in candidates) {
Expand Down Expand Up @@ -266,69 +273,35 @@ class ExtensionMemberResolver {
}

instantiatedExtensions.add(
_InstantiatedExtension(
candidate.extension,
extendedType,
ExecutableMember.from2(
candidate.member,
substitution,
),
),
_InstantiatedExtension(candidate, substitution, extendedType),
);
}

return instantiatedExtensions;
}

/// Return extensions from the current scope, that define a member with the
/// given[name].
List<_CandidateExtension> _getExtensionsWithMember(
String name,
ElementKind kind,
) {
/// given [name].
List<_CandidateExtension> _getExtensionsWithMember(String name) {
var candidates = <_CandidateExtension>[];

/// Return `true` if the [elementName] matches the target [name], taking
/// into account the `=` on the end of the names of setters.
bool matchesName(String elementName) {
if (elementName.endsWith('=') && !name.endsWith('=')) {
elementName = elementName.substring(0, elementName.length - 1);
}
return elementName == name;
}

/// Add the given [extension] to the list of [candidates] if it defined a
/// member whose name matches the target [name].
void checkExtension(ExtensionElement extension) {
if (kind == ElementKind.GETTER) {
for (var accessor in extension.accessors) {
if (accessor.isGetter && matchesName(accessor.name)) {
candidates.add(_CandidateExtension(extension, accessor));
return;
}
}
} else if (kind == ElementKind.SETTER) {
for (var accessor in extension.accessors) {
if (accessor.isSetter && matchesName(accessor.name)) {
candidates.add(_CandidateExtension(extension, accessor));
return;
}
for (var field in extension.fields) {
if (field.name == name) {
candidates.add(
_CandidateExtension(extension, field: field),
);
return;
}
} else if (kind == ElementKind.METHOD) {
for (var method in extension.methods) {
if (matchesName(method.name)) {
candidates.add(_CandidateExtension(extension, method));
return;
}
}
// Check for a getter that matches a function type.
for (var accessor in extension.accessors) {
if (accessor.type is FunctionType &&
accessor.isGetter &&
matchesName(accessor.name)) {
candidates.add(_CandidateExtension(extension, accessor));
return;
}
}
for (var method in extension.methods) {
if (method.name == name) {
candidates.add(
_CandidateExtension(extension, method: method),
);
return;
}
}
}
Expand Down Expand Up @@ -400,16 +373,16 @@ class ExtensionMemberResolver {
// former extension is not.
// 2. They are both declared in platform libraries, or both declared in
// non-platform libraries.
var e1_isInSdk = e1.element.library.isInSdk;
var e2_isInSdk = e2.element.library.isInSdk;
var e1_isInSdk = e1.extension.library.isInSdk;
var e2_isInSdk = e2.extension.library.isInSdk;
if (e1_isInSdk && !e2_isInSdk) {
return false;
} else if (!e1_isInSdk && e2_isInSdk) {
return true;
}

var extendedType1 = e1._extendedType;
var extendedType2 = e2._extendedType;
var extendedType1 = e1.extendedType;
var extendedType2 = e2.extendedType;

// 3. The instantiated type (the type after applying type inference from
// the receiver) of T1 is a subtype of the instantiated type of T2,
Expand All @@ -426,8 +399,8 @@ class ExtensionMemberResolver {
// 5. ...the instantiate-to-bounds type of T1 is a subtype of the
// instantiate-to-bounds type of T2 and not vice versa.
// TODO(scheglov) store instantiated types
var extendedTypeBound1 = _instantiateToBounds(e1.element);
var extendedTypeBound2 = _instantiateToBounds(e2.element);
var extendedTypeBound1 = _instantiateToBounds(e1.extension);
var extendedTypeBound2 = _instantiateToBounds(e2.extension);
return _isSubtypeOf(extendedTypeBound1, extendedTypeBound2) &&
!_isSubtypeOf(extendedTypeBound2, extendedTypeBound1);
}
Expand Down Expand Up @@ -455,19 +428,37 @@ class ExtensionMemberResolver {

class _CandidateExtension {
final ExtensionElement extension;
final ExecutableElement member;
final FieldElement field;
final MethodElement method;

_CandidateExtension(this.extension, this.member);
_CandidateExtension(this.extension, {this.field, this.method})
: assert(field != null || method != null);
}

class _InstantiatedExtension {
final ExtensionElement element;
final DartType _extendedType;
final ExecutableElement instantiatedMember;

_InstantiatedExtension(
this.element,
this._extendedType,
this.instantiatedMember,
);
final _CandidateExtension candidate;
final MapSubstitution substitution;
final DartType extendedType;

_InstantiatedExtension(this.candidate, this.substitution, this.extendedType);

ResolutionResult get asResolutionResult {
return ResolutionResult(function: method, property: field);
}

ExtensionElement get extension => candidate.extension;

FieldElement get field {
if (candidate.field == null) {
return null;
}
return FieldMember.from2(candidate.field, substitution);
}

MethodElement get method {
if (candidate.method == null) {
return null;
}
return MethodMember.from2(candidate.method, substitution);
}
}
25 changes: 10 additions & 15 deletions pkg/analyzer/lib/src/dart/resolver/method_invocation_resolver.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,14 @@ class MethodInvocationResolver {
receiverType,
name,
nameNode,
ElementKind.METHOD,
);

if (!result.isSingle) {
_setDynamicResolution(node);
return result;
}

ExecutableElement member = result.element;
ExecutableElement member = result.getter;
nameNode.staticElement = member;

if (member.isStatic) {
Expand Down Expand Up @@ -392,10 +391,7 @@ class MethodInvocationResolver {

void _resolveExtensionOverride(MethodInvocation node,
ExtensionOverride override, SimpleIdentifier nameNode, String name) {
var member = _extensionResolver.getOverrideMember(
override, name, ElementKind.METHOD) ??
_extensionResolver.getOverrideMember(
override, name, ElementKind.GETTER);
var member = _extensionResolver.getOverrideMember(override, name);

if (member == null) {
_setDynamicResolution(node);
Expand Down Expand Up @@ -439,11 +435,11 @@ class MethodInvocationResolver {
return;
}

ResolutionResult result = _extensionResolver.findExtension(
receiverType, name, nameNode, ElementKind.METHOD);
ResolutionResult result =
_extensionResolver.findExtension(receiverType, name, nameNode);
if (result.isSingle) {
nameNode.staticElement = result.element;
var calleeType = _getCalleeType(node, result.element);
nameNode.staticElement = result.getter;
var calleeType = _getCalleeType(node, result.getter);
return _setResolution(node, calleeType);
} else if (result.isAmbiguous) {
return;
Expand Down Expand Up @@ -578,10 +574,9 @@ class MethodInvocationResolver {
return;
}

var result = _extensionResolver.findExtension(
receiverType, name, nameNode, ElementKind.METHOD);
var result = _extensionResolver.findExtension(receiverType, name, nameNode);
if (result.isSingle) {
var target = result.element;
var target = result.getter;
if (target != null) {
nameNode.staticElement = target;
var calleeType = _getCalleeType(node, target);
Expand Down Expand Up @@ -744,9 +739,9 @@ class MethodInvocationResolver {
var call = _inheritance.getMember(type, _nameCall);
if (call == null) {
var result = _extensionResolver.findExtension(
type, _nameCall.name, node.methodName, ElementKind.METHOD);
type, _nameCall.name, node.methodName);
if (result.isSingle) {
call = result.element;
call = result.function;
} else if (result.isAmbiguous) {
return;
}
Expand Down

0 comments on commit 0851b9a

Please sign in to comment.