Skip to content

Commit 84f32fe

Browse files
committed
Validation of @protected method invocations.
* Defines `isProtected` accessors. * Adds a simple mechanism to our test `AnalysisContextFactory` allowing us to contribute package resolution to test cases. * Adds `meta` awareness to `ResolverTest`. NB: this *only* adds method invocation support. I'll generalize to all members in a follow-up. There's enough here though that I wanted to get some early feedback. BUG= [email protected] Review URL: https://codereview.chromium.org/1723243002 .
1 parent b39b2c8 commit 84f32fe

File tree

7 files changed

+306
-52
lines changed

7 files changed

+306
-52
lines changed

pkg/analyzer/lib/dart/element/element.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -634,6 +634,11 @@ abstract class Element implements AnalysisTarget {
634634
*/
635635
bool get isPrivate;
636636

637+
/**
638+
* Return `true` if this element has an annotation of the form '@protected'.
639+
*/
640+
bool get isProtected;
641+
637642
/**
638643
* Return `true` if this element is public. Public elements are visible within
639644
* any library that imports the library in which they are declared.
@@ -808,6 +813,13 @@ abstract class ElementAnnotation implements ConstantEvaluationTarget {
808813
*/
809814
bool get isOverride;
810815

816+
817+
/**
818+
* Return `true` if this annotation marks the associated member as being
819+
* protected.
820+
*/
821+
bool get isProtected;
822+
811823
/**
812824
* Return `true` if this annotation marks the associated class as implementing
813825
* a proxy object.

pkg/analyzer/lib/src/dart/element/element.dart

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1561,12 +1561,23 @@ class ElementAnnotationImpl implements ElementAnnotation {
15611561
*/
15621562
static String _DEPRECATED_VARIABLE_NAME = "deprecated";
15631563

1564+
/**
1565+
* The name of `meta` library, used to define analysis annotations.
1566+
*/
1567+
static String _META_LIB_NAME = "meta";
1568+
15641569
/**
15651570
* The name of the top-level variable used to mark a method as being expected
15661571
* to override an inherited method.
15671572
*/
15681573
static String _OVERRIDE_VARIABLE_NAME = "override";
15691574

1575+
/**
1576+
* The name of the top-level variable used to mark a method as being
1577+
* protected.
1578+
*/
1579+
static String _PROTECTED_VARIABLE_NAME = "protected";
1580+
15701581
/**
15711582
* The name of the top-level variable used to mark a class as implementing a
15721583
* proxy object.
@@ -1643,6 +1654,20 @@ class ElementAnnotationImpl implements ElementAnnotation {
16431654
return false;
16441655
}
16451656

1657+
@override
1658+
bool get isProtected {
1659+
if (element != null) {
1660+
LibraryElement library = element.library;
1661+
if (library != null && library.name == _META_LIB_NAME) {
1662+
if (element is PropertyAccessorElement &&
1663+
element.name == _PROTECTED_VARIABLE_NAME) {
1664+
return true;
1665+
}
1666+
}
1667+
}
1668+
return false;
1669+
}
1670+
16461671
@override
16471672
bool get isProxy {
16481673
if (element != null) {
@@ -1836,6 +1861,16 @@ abstract class ElementImpl implements Element {
18361861
return Identifier.isPrivateName(name);
18371862
}
18381863

1864+
@override
1865+
bool get isProtected {
1866+
for (ElementAnnotation annotation in metadata) {
1867+
if (annotation.isProtected) {
1868+
return true;
1869+
}
1870+
}
1871+
return false;
1872+
}
1873+
18391874
@override
18401875
bool get isPublic => !isPrivate;
18411876

@@ -3920,6 +3955,9 @@ class MultiplyDefinedElementImpl implements MultiplyDefinedElement {
39203955
return Identifier.isPrivateName(name);
39213956
}
39223957

3958+
@override
3959+
bool get isProtected => false;
3960+
39233961
@override
39243962
bool get isPublic => !isPrivate;
39253963

pkg/analyzer/lib/src/dart/element/member.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,9 @@ abstract class Member implements Element {
466466
@override
467467
bool get isPrivate => _baseElement.isPrivate;
468468

469+
@override
470+
bool get isProtected => _baseElement.isProtected;
471+
469472
@override
470473
bool get isPublic => _baseElement.isPublic;
471474

pkg/analyzer/lib/src/generated/element_handle.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,9 @@ abstract class ElementHandle implements Element {
357357
@override
358358
bool get isPrivate => actualElement.isPrivate;
359359

360+
@override
361+
bool get isProtected => actualElement.isProtected;
362+
360363
@override
361364
bool get isPublic => actualElement.isPublic;
362365

pkg/analyzer/lib/src/generated/error.dart

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3532,6 +3532,18 @@ class HintCode extends ErrorCode {
35323532
'INVALID_ASSIGNMENT',
35333533
"A value of type '{0}' cannot be assigned to a variable of type '{1}'");
35343534

3535+
/**
3536+
* This hint is generated anywhere where a member annotated with `@protected`
3537+
* is used outside an instance member of a subclass.
3538+
*
3539+
* Parameters:
3540+
* 0: the name of the member
3541+
* 1: the name of the defining class
3542+
*/
3543+
static const HintCode INVALID_USE_OF_PROTECTED_MEMBER = const HintCode(
3544+
'INVALID_USE_OF_PROTECTED_MEMBER',
3545+
"The member '{0}' can only be used within instance members of subclasses of '{1}'");
3546+
35353547
/**
35363548
* Generate a hint for methods or functions that have a return type, but do
35373549
* not have a non-void return statement on all branches. At the end of methods

pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 100 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
206206
@override
207207
Object visitMethodInvocation(MethodInvocation node) {
208208
_checkForCanBeNullAfterNullAware(node.realTarget, node.operator);
209+
_checkForInvalidProtectedMethodCalls(node);
209210
return super.visitMethodInvocation(node);
210211
}
211212

@@ -607,6 +608,39 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
607608
return false;
608609
}
609610

611+
/**
612+
* Produces a hint if the given invocation is of a protected method outside
613+
* a subclass instance method.
614+
*/
615+
void _checkForInvalidProtectedMethodCalls(MethodInvocation node) {
616+
Element element = node.methodName.bestElement;
617+
if (element == null || !element.isProtected) {
618+
return;
619+
}
620+
621+
ClassElement definingClass = element.enclosingElement;
622+
623+
MethodDeclaration decl =
624+
node.getAncestor((AstNode node) => node is MethodDeclaration);
625+
if (decl == null) {
626+
_errorReporter.reportErrorForNode(
627+
HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
628+
node,
629+
[node.methodName.toString(), definingClass.name]);
630+
return;
631+
}
632+
633+
ClassElement invokingClass = decl.element?.enclosingElement;
634+
if (invokingClass != null) {
635+
if (!_hasSuperClassOrMixin(invokingClass, definingClass.type)) {
636+
_errorReporter.reportErrorForNode(
637+
HintCode.INVALID_USE_OF_PROTECTED_MEMBER,
638+
node,
639+
[node.methodName.toString(), definingClass.name]);
640+
}
641+
}
642+
}
643+
610644
/**
611645
* Check that the imported library does not define a loadLibrary function. The import has already
612646
* been determined to be deferred when this is called.
@@ -789,35 +823,6 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
789823
return false;
790824
}
791825

792-
/**
793-
* Check for the passed class declaration for the
794-
* [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE] hint code.
795-
*
796-
* @param node the class declaration to check
797-
* @return `true` if and only if a hint code is generated on the passed node
798-
* See [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE].
799-
*/
800-
// bool _checkForOverrideEqualsButNotHashCode(ClassDeclaration node) {
801-
// ClassElement classElement = node.element;
802-
// if (classElement == null) {
803-
// return false;
804-
// }
805-
// MethodElement equalsOperatorMethodElement =
806-
// classElement.getMethod(sc.TokenType.EQ_EQ.lexeme);
807-
// if (equalsOperatorMethodElement != null) {
808-
// PropertyAccessorElement hashCodeElement =
809-
// classElement.getGetter(_HASHCODE_GETTER_NAME);
810-
// if (hashCodeElement == null) {
811-
// _errorReporter.reportErrorForNode(
812-
// HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE,
813-
// node.name,
814-
// [classElement.displayName]);
815-
// return true;
816-
// }
817-
// }
818-
// return false;
819-
// }
820-
821826
/**
822827
* Generate a hint for `noSuchMethod` methods that do nothing except of
823828
* calling another `noSuchMethod` that is not defined by `Object`.
@@ -866,6 +871,35 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
866871
return false;
867872
}
868873

874+
/**
875+
* Check for the passed class declaration for the
876+
* [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE] hint code.
877+
*
878+
* @param node the class declaration to check
879+
* @return `true` if and only if a hint code is generated on the passed node
880+
* See [HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE].
881+
*/
882+
// bool _checkForOverrideEqualsButNotHashCode(ClassDeclaration node) {
883+
// ClassElement classElement = node.element;
884+
// if (classElement == null) {
885+
// return false;
886+
// }
887+
// MethodElement equalsOperatorMethodElement =
888+
// classElement.getMethod(sc.TokenType.EQ_EQ.lexeme);
889+
// if (equalsOperatorMethodElement != null) {
890+
// PropertyAccessorElement hashCodeElement =
891+
// classElement.getGetter(_HASHCODE_GETTER_NAME);
892+
// if (hashCodeElement == null) {
893+
// _errorReporter.reportErrorForNode(
894+
// HintCode.OVERRIDE_EQUALS_BUT_NOT_HASH_CODE,
895+
// node.name,
896+
// [classElement.displayName]);
897+
// return true;
898+
// }
899+
// }
900+
// return false;
901+
// }
902+
869903
/**
870904
* Check for situations where the result of a method or function is used, when it returns 'void'.
871905
*
@@ -891,6 +925,24 @@ class BestPracticesVerifier extends RecursiveAstVisitor<Object> {
891925
return false;
892926
}
893927

928+
bool _hasSuperClassOrMixin(ClassElement element, InterfaceType type) {
929+
List<ClassElement> seenClasses = <ClassElement>[];
930+
while (element != null && !seenClasses.contains(element)) {
931+
if (element.type == type) {
932+
return true;
933+
}
934+
935+
if (element.mixins.any((InterfaceType t) => t == type)) {
936+
return true;
937+
}
938+
939+
seenClasses.add(element);
940+
element = element.supertype?.element;
941+
}
942+
943+
return false;
944+
}
945+
894946
/**
895947
* Given a parenthesized expression, this returns the parent (or recursively grand-parent) of the
896948
* expression that is a parenthesized expression, but whose parent is not a parenthesized
@@ -8383,26 +8435,6 @@ class ResolverVisitor extends ScopedVisitor {
83838435
return null;
83848436
}
83858437

8386-
void _inferArgumentTypesFromContext(InvocationExpression node) {
8387-
DartType contextType = node.staticInvokeType;
8388-
if (contextType is FunctionType) {
8389-
DartType originalType = node.function.staticType;
8390-
DartType returnContextType = InferenceContext.getType(node);
8391-
TypeSystem ts = typeSystem;
8392-
if (returnContextType != null &&
8393-
node.typeArguments == null &&
8394-
originalType is FunctionType &&
8395-
originalType.typeFormals.isNotEmpty &&
8396-
ts is StrongTypeSystemImpl) {
8397-
8398-
contextType = ts.inferGenericFunctionCall(typeProvider, originalType,
8399-
DartType.EMPTY_LIST, DartType.EMPTY_LIST, returnContextType);
8400-
}
8401-
8402-
InferenceContext.setType(node.argumentList, contextType);
8403-
}
8404-
}
8405-
84068438
@override
84078439
Object visitNamedExpression(NamedExpression node) {
84088440
InferenceContext.setType(node.expression, InferenceContext.getType(node));
@@ -8730,6 +8762,25 @@ class ResolverVisitor extends ScopedVisitor {
87308762
return null;
87318763
}
87328764

8765+
void _inferArgumentTypesFromContext(InvocationExpression node) {
8766+
DartType contextType = node.staticInvokeType;
8767+
if (contextType is FunctionType) {
8768+
DartType originalType = node.function.staticType;
8769+
DartType returnContextType = InferenceContext.getType(node);
8770+
TypeSystem ts = typeSystem;
8771+
if (returnContextType != null &&
8772+
node.typeArguments == null &&
8773+
originalType is FunctionType &&
8774+
originalType.typeFormals.isNotEmpty &&
8775+
ts is StrongTypeSystemImpl) {
8776+
contextType = ts.inferGenericFunctionCall(typeProvider, originalType,
8777+
DartType.EMPTY_LIST, DartType.EMPTY_LIST, returnContextType);
8778+
}
8779+
8780+
InferenceContext.setType(node.argumentList, contextType);
8781+
}
8782+
}
8783+
87338784
void _inferFormalParameterList(FormalParameterList node, DartType type) {
87348785
if (typeAnalyzer.inferFormalParameterList(node, type)) {
87358786
// TODO(leafp): This gets dropped on the floor if we're in the field

0 commit comments

Comments
 (0)