Adjust the lexical lookup

This CL adjusts the lexical lookup such that an instance member
declaration in the lexically enclosing syntax is reported using null for
the getter and for the setter result. This ensures that the search ends
(we shouldn't search the library scope in this situation), and it
ensures that the given member is handled in the same way as if the
search had exhausted all levels and found nothing, which will then cause
further processing to prepend `this.` and resolve the identifier as an
instance member.

This change aligns the implemented behavior to match the language
specification more precisely, and it is necessary in order to get the
desired and specified behavior with lookups that originate in an
anonymous method.

Change-Id: Ic11c412b91827d57455a1e7febbb031b5aadad70
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459740
Reviewed-by: Paul Berry <paulberry@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>

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

@@ -274,6 +274,37 @@ class InstanceScope extends EnclosedScope {
     element.setters.forEach(_addSetter);
     element.methods.forEach(_addGetter);
   }
+
+  @override
+  ScopeLookupResult lookup(String id) {
+    // When a lexical lookup encounters an instance member with the requested
+    // basename, the returned result should have null as the getter and setter
+    // (and that is the result, so we should not search the enclosing scope).
+    // This corresponds to the following rule from the specification:
+    // "Consider the case where D is an instance member declaration in a class
+    // or mixin A. The lexical lookup then yields nothing."
+    var getter = _getters[id];
+    var setter = _setters[id];
+    if (getter == null && setter == null) {
+      return _parent.lookup(id);
+    }
+    if (_isStatic(getter) || _isStatic(setter)) {
+      return ScopeLookupResultImpl(getter: getter, setter: setter);
+    }
+    // A declaration with the right basename was found, so the search ends.
+    // Return nulls to ensure that it is handled as with a prepended `this.`.
+    return ScopeLookupResultImpl(getter: null, setter: null);
+  }
+
+  static bool _isStatic(Element? element) {
+    switch (element) {
+      case PropertyAccessorElement(:var isStatic) ||
+          MethodElement(:var isStatic):
+        return isStatic;
+      default:
+        return false;
+    }
+  }
 }
 
 /// The top-level declarations of the library.

pkg/analyzer/test/generated/strong_mode_test.dart

@@ -5017,7 +5017,9 @@ class C<T> {
 MethodInvocation
   methodName: SimpleIdentifier
     token: m
-    element: <testLibrary>::@class::C::@method::m
+    element: MethodMember
+      baseElement: <testLibrary>::@class::C::@method::m
+      substitution: {T: T, S0: S0, S1: S1}
     staticType: void Function<S0 extends T, S1 extends List<S0>>(S0, S1)
   argumentList: ArgumentList
     leftParenthesis: (
@@ -5025,13 +5027,13 @@ MethodInvocation
       NullLiteral
         literal: null
         correspondingParameter: ParameterMember
-          baseElement: <testLibrary>::@class::C::@method::m::@formalParameter::p0
+          baseElement: p0@null
           substitution: {S0: T, S1: List<T>}
         staticType: Null
       NullLiteral
         literal: null
         correspondingParameter: ParameterMember
-          baseElement: <testLibrary>::@class::C::@method::m::@formalParameter::p1
+          baseElement: p1@null
           substitution: {S0: T, S1: List<T>}
         staticType: Null
     rightParenthesis: )
@@ -5062,7 +5064,9 @@ class C<T> {
 MethodInvocation
   methodName: SimpleIdentifier
     token: m
-    element: <testLibrary>::@class::C::@method::m
+    element: MethodMember
+      baseElement: <testLibrary>::@class::C::@method::m
+      substitution: {T: T, S0: S0, S1: S1}
     staticType: Map<S0, S1> Function<S0 extends T, S1 extends List<S0>>()
   argumentList: ArgumentList
     leftParenthesis: (
@@ -5094,7 +5098,9 @@ class C<T> {
 MethodInvocation
   methodName: SimpleIdentifier
     token: m
-    element: <testLibrary>::@class::C::@method::m
+    element: MethodMember
+      baseElement: <testLibrary>::@class::C::@method::m
+      substitution: {T: T, S: S}
     staticType: void Function<S extends T>(S)
   argumentList: ArgumentList
     leftParenthesis: (
@@ -5102,7 +5108,7 @@ MethodInvocation
       NullLiteral
         literal: null
         correspondingParameter: ParameterMember
-          baseElement: <testLibrary>::@class::C::@method::m::@formalParameter::p0
+          baseElement: p0@null
           substitution: {S: T}
         staticType: Null
     rightParenthesis: )
@@ -5132,7 +5138,9 @@ class C<T> {
 MethodInvocation
   methodName: SimpleIdentifier
     token: m
-    element: <testLibrary>::@class::C::@method::m
+    element: MethodMember
+      baseElement: <testLibrary>::@class::C::@method::m
+      substitution: {T: T, S: S}
     staticType: S Function<S extends T>()
   argumentList: ArgumentList
     leftParenthesis: (

pkg/analyzer/test/src/dart/resolution/field_promotion_test.dart

@@ -1043,7 +1043,9 @@ Block
     ExpressionStatement
       expression: SimpleIdentifier
         token: _t
-        element: <testLibrary>::@class::C::@getter::_t
+        element: GetterMember
+          baseElement: <testLibrary>::@class::C::@getter::_t
+          substitution: {T: T}
         staticType: T
       semicolon: ;
     ExpressionStatement
@@ -1070,7 +1072,9 @@ Block
     ExpressionStatement
       expression: SimpleIdentifier
         token: _t
-        element: <testLibrary>::@class::C::@getter::_t
+        element: GetterMember
+          baseElement: <testLibrary>::@class::C::@getter::_t
+          substitution: {T: T}
         staticType: T?
       semicolon: ;
     ExpressionStatement
@@ -1224,7 +1228,9 @@ Block
       expression: FunctionExpressionInvocation
         function: SimpleIdentifier
           token: _f
-          element: <testLibrary>::@class::C::@getter::_f
+          element: GetterMember
+            baseElement: <testLibrary>::@class::C::@getter::_f
+            substitution: {T: T}
           staticType: T Function()
         argumentList: ArgumentList
           leftParenthesis: (
@@ -1265,7 +1271,9 @@ Block
       expression: FunctionExpressionInvocation
         function: SimpleIdentifier
           token: _f
-          element: <testLibrary>::@class::C::@getter::_f
+          element: GetterMember
+            baseElement: <testLibrary>::@class::C::@getter::_f
+            substitution: {T: T}
           staticType: T? Function()
         argumentList: ArgumentList
           leftParenthesis: (

pkg/analyzer/test/src/dart/resolution/method_invocation_test.dart

@@ -1928,7 +1928,9 @@ class C<T extends MyFunction> {
 FunctionExpressionInvocation
   function: SimpleIdentifier
     token: foo
-    element: <testLibrary>::@class::C::@getter::foo
+    element: GetterMember
+      baseElement: <testLibrary>::@class::C::@getter::foo
+      substitution: {T: T}
     staticType: T
   argumentList: ArgumentList
     leftParenthesis: (
@@ -4217,7 +4219,9 @@ class C<T extends A> {
 MethodInvocation
   target: SimpleIdentifier
     token: a
-    element: <testLibrary>::@class::C::@getter::a
+    element: GetterMember
+      baseElement: <testLibrary>::@class::C::@getter::a
+      substitution: {T: T}
     staticType: T
   operator: .
   methodName: SimpleIdentifier

pkg/linter/lib/src/util/leak_detector_visitor.dart

@@ -82,9 +82,10 @@ bool _isPostfixExpressionOperandEqualToVariable(
   VariableElement variableElement,
 ) {
   if (n is PostfixExpression) {
-    var operand = n.operand;
-    return operand is SimpleIdentifier &&
-        _isElementEqualToVariable(operand.element, variableElement);
+    return _isSimpleIdentifierElementEqualToVariable(
+      n.operand,
+      variableElement,
+    );
   }
   return false;
 }
@@ -198,9 +199,11 @@ class _ValidUseVisitor extends RecursiveAstVisitor<void> {
   @override
   void visitAssignmentExpression(AssignmentExpression node) {
     // Being assigned another reference.
-    if (node.rightHandSide is SimpleIdentifier) {
+    var rightHandSide = node.rightHandSide;
+    if (rightHandSide is SimpleIdentifier) {
+      var assignedElement = node.writeElement;
       if (_isElementEqualToVariable(
-        node.writeElement,
+        assignedElement,
         variable.declaredFragment?.element,
       )) {
         containsValidUse = true;
@@ -213,6 +216,15 @@ class _ValidUseVisitor extends RecursiveAstVisitor<void> {
         containsValidUse = true;
         return;
       }
+      // Assigned element is instance member.
+      if (assignedElement == null &&
+          _isSimpleIdentifierElementEqualToVariable(
+            rightHandSide,
+            variableElement,
+          )) {
+        containsValidUse = true;
+        return;
+      }
     }
     super.visitAssignmentExpression(node);
   }
@@ -310,8 +322,13 @@ class _ValidUseVisitor extends RecursiveAstVisitor<void> {
 enum _VariableType { field, local }
 
 extension on Element? {
-  bool matches(VariableElement? requested) => switch (this) {
-    PropertyAccessorElement(:var variable) => variable == requested,
-    _ => false,
-  };
+  bool matches(VariableElement? requested) {
+    var baseElement = this?.baseElement;
+    if (baseElement is PropertyAccessorElement) {
+      return baseElement.variable == requested;
+    } else if (baseElement is FieldElement) {
+      return baseElement == requested;
+    }
+    return false;
+  }
 }

pkg/linter/test/rules/cancel_subscriptions_test.dart

@@ -112,6 +112,21 @@ class B {
 ''');
   }
 
+  test_privateField_canceled_withNullCheck() async {
+    await assertNoDiagnostics(r'''
+import 'dart:async';
+class C<T> {
+  StreamSubscription<T>? subscription;
+  void unsubscribe() {
+    if (subscription != null) {
+      subscription!.cancel();
+      subscription = null;
+    }
+  }
+}
+''');
+  }
+
   test_privateField_notCanceled() async {
     await assertDiagnostics(
       r'''

pkg/linter/test/scope_util_test.dart

@@ -39,23 +39,6 @@ class ResolveNameInScopeTest extends PubPackageResolutionTest {
     return result;
   }
 
-  test_class_getter_different_fromExtends_thisClassSetter() async {
-    await assertNoDiagnostics('''
-class A {
-  int get foo => 0;
-}
-
-class B extends A {
-  set foo(int _) {}
-
-  void bar() {
-    this.foo;
-  }
-}
-''');
-    _checkGetterDifferent(findElement.setter('foo'));
-  }
-
   test_class_getter_different_importScope() async {
     newFile('$testPackageLibPath/a.dart', r'''
 set foo(int _) {}
@@ -123,6 +106,36 @@ class B extends A {
     _checkGetterNone();
   }
 
+  test_class_getter_none_fromThisClass() async {
+    await assertNoDiagnostics('''
+class A {
+  int get foo => 0;
+}
+
+class B extends A {
+  set foo(int _) {}
+
+  void bar() {
+    this.foo;
+  }
+}
+''');
+    _checkGetterNone();
+  }
+
+  test_class_getter_none_thisClass() async {
+    await assertNoDiagnostics('''
+class A {
+  int get foo => 0;
+
+  void bar() {
+    this.foo;
+  }
+}
+''');
+    _checkGetterNone();
+  }
+
   test_class_getter_requested_importScope() async {
     newFile('$testPackageLibPath/a.dart', r'''
 int get foo => 0;
@@ -147,19 +160,6 @@ class B extends A {
     _checkGetterRequested(import.topGet('foo'));
   }
 
-  test_class_getter_requested_thisClass() async {
-    await assertNoDiagnostics('''
-class A {
-  int get foo => 0;
-
-  void bar() {
-    this.foo;
-  }
-}
-''');
-    _checkGetterRequested(findElement.getter('foo'));
-  }
-
   test_class_method_different_fromExtends_topSetter() async {
     await assertNoDiagnostics('''
 class A {
@@ -207,6 +207,19 @@ class A {
     _checkMethodNone();
   }
 
+  test_class_method_none_thisClass() async {
+    await assertNoDiagnostics('''
+class A {
+  void foo() {}
+
+  void bar() {
+    this.foo();
+  }
+}
+''');
+    _checkMethodNone();
+  }
+
   test_class_method_requested_formalParameter_constructor() async {
     await assertNoDiagnostics('''
 class A {
@@ -477,19 +490,6 @@ class A {
     _checkMethodRequestedLocalVariable();
   }
 
-  test_class_method_requested_thisClass() async {
-    await assertNoDiagnostics('''
-class A {
-  void foo() {}
-
-  void bar() {
-    this.foo();
-  }
-}
-''');
-    _checkMethodRequested(findElement.method('foo'));
-  }
-
   test_class_method_requested_typeParameter_method() async {
     await assertNoDiagnostics('''
 class A {
@@ -575,6 +575,34 @@ class B extends A {
     _checkSetterNone();
   }
 
+  test_class_setter_none_thisClass() async {
+    await assertNoDiagnostics('''
+class A {
+  set foo(int _) {}
+
+  void bar() {
+    this.foo = 0;
+  }
+}
+''');
+    _checkSetterNone();
+  }
+
+  test_class_setter_none_thisClass_topLevelFunction() async {
+    await assertNoDiagnostics('''
+class A {
+  set foo(int _) {}
+
+  void bar() {
+    this.foo = 0;
+  }
+}
+
+void foo() {}
+''');
+    _checkSetterNone();
+  }
+
   test_class_setter_requested_fromExtends_topLevelVariable() async {
     await assertNoDiagnostics('''
 class A {
@@ -616,34 +644,6 @@ class B extends A {
     _checkSetterRequested(import.topSet('foo'));
   }
 
-  test_class_setter_requested_thisClass() async {
-    await assertNoDiagnostics('''
-class A {
-  set foo(int _) {}
-
-  void bar() {
-    this.foo = 0;
-  }
-}
-''');
-    _checkSetterRequested(findElement.setter('foo'));
-  }
-
-  test_class_setter_requested_thisClass_topLevelFunction() async {
-    await assertNoDiagnostics('''
-class A {
-  set foo(int _) {}
-
-  void bar() {
-    this.foo = 0;
-  }
-}
-
-void foo() {}
-''');
-    _checkSetterRequested(findElement.setter('foo'));
-  }
-
   test_class_typeParameter_inConstructor() async {
     await assertNoDiagnostics('''
 class A<T> {
@@ -781,6 +781,19 @@ typedef A<T> = List<T>;
     _resultRequested(node, 'T', false, findElement.typeParameter('T'));
   }
 
+  test_mixin_method_none_thisClass() async {
+    await assertNoDiagnostics('''
+mixin M {
+  void foo() {}
+
+  void bar() {
+    this.foo();
+  }
+}
+''');
+    _checkMethodNone();
+  }
+
   test_mixin_method_requested_formalParameter_method() async {
     await assertNoDiagnostics('''
 mixin M {
@@ -794,19 +807,6 @@ mixin M {
     _checkMethodRequested(findElement.parameter('foo'));
   }
 
-  test_mixin_method_requested_thisClass() async {
-    await assertNoDiagnostics('''
-mixin M {
-  void foo() {}
-
-  void bar() {
-    this.foo();
-  }
-}
-''');
-    _checkMethodRequested(findElement.method('foo'));
-  }
-
   test_mixin_typeParameter_inField() async {
     await assertNoDiagnostics('''
 mixin A<T> {