SONARJAVA-5338 Fix variable owner in the case of lambdas

The computation of owners in lambdas was returning the interface method implemented by the lambda (for instance accept for Function) which doesn't allow distinguishing between symbols declared in different lambdas in different methods with the same variable name.

Author: romainbrenguier

java-frontend/src/main/java/org/sonar/java/model/JSymbol.java

@@ -194,6 +194,14 @@ private Symbol variableOwner(IVariableBinding variableBinding) {
     if (!variableBinding.isRecordComponent()) {
       IMethodBinding declaringMethod = variableBinding.getDeclaringMethod();
       if (declaringMethod != null) {
+        IBinding declaringMember = declaringMethod.getDeclaringMember();
+        // declaringMember is non-null in the case of lambdas
+        if (declaringMember instanceof IMethodBinding methodBinding) {
+          return sema.methodSymbol(methodBinding);
+        }
+        if (declaringMember instanceof IVariableBinding varBinding) {
+          return sema.variableSymbol(varBinding);
+        }
         // local variable
         return sema.methodSymbol(declaringMethod);
       }
@@ -203,6 +211,10 @@ private Symbol variableOwner(IVariableBinding variableBinding) {
         return sema.typeSymbol(declaringClass);
       }
     }
+    return ownerOfRecordComponentConstant(variableBinding);
+  }
+
+  private Symbol ownerOfRecordComponentConstant(IVariableBinding variableBinding) {
     Tree node = sema.declarations.get(variableBinding);
     if (node == null) {
       // array.length
@@ -214,8 +226,8 @@ private Symbol variableOwner(IVariableBinding variableBinding) {
       node = node.parent();
       switch (node.kind()) {
         case CLASS,
-          RECORD,
-          ENUM:
+             RECORD,
+             ENUM:
           JTypeSymbol typeSymbol = sema.typeSymbol(((ClassTreeImpl) node).typeBinding);
           if (initializerBlock) {
             return sema.initializerBlockSymbol(typeSymbol);
@@ -232,7 +244,7 @@ private Symbol variableOwner(IVariableBinding variableBinding) {
           staticInitializerBlock = true;
           break;
         case METHOD,
-          CONSTRUCTOR:
+             CONSTRUCTOR:
           // local variable declaration in recovered method
           // and recovered methods do not have bindings
           return Symbols.unknownMethodSymbol;
@@ -252,7 +264,7 @@ public final Type type() {
         ITypeBinding variableType = ((IVariableBinding) binding).getType();
         return variableType != null ? sema.type(variableType) : Symbols.unknownType;
       case IBinding.PACKAGE,
-        IBinding.METHOD:
+           IBinding.METHOD:
         return Symbols.unknownType;
       default:
         throw new IllegalStateException(unexpectedBinding());
@@ -447,8 +459,8 @@ private TypeSymbol variableEnclosingClass(IVariableBinding variableBinding) {
       node = node.parent();
       switch (node.kind()) {
         case CLASS,
-          RECORD,
-          ENUM:
+             RECORD,
+             ENUM:
           // variable declaration in a static or instance initializer
           // or local variable declaration in recovered method
           return sema.typeSymbol(((ClassTreeImpl) node).typeBinding);

java-frontend/src/test/java/org/sonar/java/model/JParserSemanticTest.java

@@ -20,6 +20,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import org.eclipse.jdt.core.JavaCore;
 import org.eclipse.jdt.core.dom.AST;
 import org.eclipse.jdt.core.dom.ASTParser;
@@ -779,6 +780,7 @@ default void forEach(java.util.function.Consumer<? super T> action) {
     assertThat(superKeyword.symbolType().fullyQualifiedName()).isEqualTo("java.lang.Iterable");
   }
 
+  /** Check that the owner of a lambda parameter is the field to which it is assigned. */
   @Test
   void expression_lambda() {
     String source = """
@@ -793,15 +795,47 @@ class A {
     LambdaExpressionTreeImpl lambda = (LambdaExpressionTreeImpl) f.initializer();
     VariableTreeImpl p = (VariableTreeImpl) lambda.parameters().get(0);
 
-    // owner of lambda parameter is the method which defines the functional interface
     Symbol newSymbol = cu.sema.variableSymbol(p.variableBinding);
     assertThat(newSymbol.declaration().firstToken().range().start().line()).isEqualTo(3);
     Symbol newOwner = newSymbol.owner();
     assertThat(newOwner).isNotNull();
-    assertThat(newOwner.isMethodSymbol()).isTrue();
+    assertThat(newOwner.isVariableSymbol()).isTrue();
+    assertThat(newOwner.isTypeSymbol()).isFalse();
+    assertThat(newOwner.name()).isEqualTo("f");
+    assertThat(newOwner.owner().type().fullyQualifiedName()).isEqualTo("org.foo.A");
+  }
+
+  /** Check that the owner of lambda parameter is the method inside which the lambda is declared. */
+  @Test
+  void expression_lambda_variable_owner() {
+    String source = """
+      package org.foo;
+      class A {
+        void foo() {
+          java.util.function.Consumer<String> v = p -> { };
+        }
+      }
+      """;
+    JavaTree.CompilationUnitTreeImpl cu = test(source);
+    ClassTreeImpl c = (ClassTreeImpl) cu.types().get(0);
+    MethodTreeImpl foo = (MethodTreeImpl) c.members().get(0);
+    Optional<BlockTreeImpl> firstBlock = foo.getChildren().stream()
+      .filter(BlockTreeImpl.class::isInstance)
+      .map(BlockTreeImpl.class::cast)
+      .findFirst();
+    VariableTree v = (VariableTree) firstBlock.get().body().get(0);
+    LambdaExpressionTreeImpl lambda = (LambdaExpressionTreeImpl) v.initializer();
+    VariableTreeImpl p = (VariableTreeImpl) lambda.parameters().get(0);
+
+    // owner of lambda parameter is the field or variable to which it is assigned
+    Symbol newSymbol = cu.sema.variableSymbol(p.variableBinding);
+    Symbol newOwner = newSymbol.owner();
+    assertThat(newOwner).isNotNull();
+    assertThat(newOwner.isVariableSymbol()).isFalse();
     assertThat(newOwner.isTypeSymbol()).isFalse();
-    assertThat(newOwner.name()).isEqualTo("accept");
-    assertThat(newOwner.owner().type().fullyQualifiedName()).isEqualTo("java.util.function.Consumer");
+    assertThat(newOwner.isMethodSymbol()).isTrue();
+    assertThat(newOwner.name()).isEqualTo("foo");
+    assertThat(newOwner.owner().type().fullyQualifiedName()).isEqualTo("org.foo.A");
   }
 
   @Test
@@ -1337,7 +1371,7 @@ void declaration_annotation_member() {
     assertThat(cu.sema.declarations.get(m.methodBinding)).isSameAs(m);
   }
 
-  @ParameterizedTest(name="[{index}] Type bindings of variable v should not be null in \"{0}\"")
+  @ParameterizedTest(name = "[{index}] Type bindings of variable v should not be null in \"{0}\"")
   @ValueSource(strings = {
     "interface I { int v; }", // primitive
     "interface I<T> { I<String> v; }", // parameterized
@@ -1634,19 +1668,19 @@ void nested() {
   @Test
   void constructor_with_type_arguments() {
     String source = """
-        class MyClass {
-          <T extends I> MyClass(T t) {}
-          <T extends J & I> MyClass(T t) {}
-          void foo(B b, C c) {
-            new<B>MyClass((I) b);
-            new<C>MyClass(c);
-          }
+      class MyClass {
+        <T extends I> MyClass(T t) {}
+        <T extends J & I> MyClass(T t) {}
+        void foo(B b, C c) {
+          new<B>MyClass((I) b);
+          new<C>MyClass(c);
         }
-        interface I {}
-        interface J {}
-        class B implements I {}
-        class C implements I, J {}
-        """;
+      }
+      interface I {}
+      interface J {}
+      class B implements I {}
+      class C implements I, J {}
+      """;
 
     JavaTree.CompilationUnitTreeImpl cu = test(source);
     ClassTree c = (ClassTree) cu.types().get(0);
@@ -1692,7 +1726,7 @@ void should_skip_implicit_break_statement() {
   private CompilationUnit createAST(String source) {
     JavaVersion version = JParserConfig.MAXIMUM_SUPPORTED_JAVA_VERSION;
     ASTParser astParser = ASTParser.newParser(AST.getJLSLatest());
-    Map<String, String> options =  new HashMap<>(JavaCore.getOptions());
+    Map<String, String> options = new HashMap<>(JavaCore.getOptions());
     JavaCore.setComplianceOptions(version.effectiveJavaVersionAsString(), options);
     options.put(JavaCore.COMPILER_PB_ENABLE_PREVIEW_FEATURES, "enabled");
     astParser.setCompilerOptions(options);
@@ -1737,22 +1771,22 @@ class InnerB extends C.InnerC {
   @Test
   void inner_class_depending_on_outer_class_parametrized_type() {
     final String source = """
-        class X<T> {
-          InnerClass innerClass;
-          class InnerClass {
-            T method() {
-              return null;
-            }
-          }
-          static void test() {
-            new X<Y>().innerClass.method().method1();
+      class X<T> {
+        InnerClass innerClass;
+        class InnerClass {
+          T method() {
+            return null;
           }
         }
-        class Y {
-          void method1() {
-          }
+        static void test() {
+          new X<Y>().innerClass.method().method1();
+        }
+      }
+      class Y {
+        void method1() {
         }
-        """;
+      }
+      """;
     JavaTree.CompilationUnitTreeImpl cu = test(source);
     ClassTree classX = (ClassTree) cu.types().get(0);
     MethodTreeImpl method = (MethodTreeImpl) ((ClassTree) classX.members().get(1)).members().get(0);
@@ -1788,7 +1822,7 @@ void warnings_are_detected() {
     List<JWarning> castWarnings = cu.warnings(JWarning.Type.REDUNDANT_CAST);
     assertThat(castWarnings).hasSize(2);
 
-    TypeCastTree typeCast =  (TypeCastTree)((VariableTree)((MethodTree)(((ClassTree) cu.types().get(0)).members().get(0))).block().body().get(0)).initializer();
+    TypeCastTree typeCast = (TypeCastTree) ((VariableTree) ((MethodTree) (((ClassTree) cu.types().get(0)).members().get(0))).block().body().get(0)).initializer();
     JWarning parentCastWarning = castWarnings.get(0);
     assertThat(parentCastWarning.message()).isEqualTo("Unnecessary cast from String to String");
     assertThat(parentCastWarning.syntaxTree()).isEqualTo(typeCast);
@@ -1798,4 +1832,36 @@ void warnings_are_detected() {
     assertThat(nestedCastWarning.message()).isEqualTo("Unnecessary cast from String to String");
     assertThat(nestedCastWarning.syntaxTree()).isEqualTo(parenthesizedTree);
   }
+
+  @Test
+  void test_variable_equals_in_lambda() {
+    String source = """
+      package org.foo;
+      class A {
+        void f1() {
+          java.util.function.Consumer<String> a = p -> { System.out.println(p); };
+          java.util.function.Consumer<String> c = p -> { System.out.println(p); };
+        }
+        void f2() {
+          java.util.function.Consumer<String> b = p -> { System.out.println(p); };
+        }
+      }
+      """;
+    JavaTree.CompilationUnitTreeImpl cu = test(source);
+    ClassTreeImpl c = (ClassTreeImpl) cu.types().get(0);
+    MethodTree f1 = (MethodTree) c.members().get(0);
+    MethodTree f2 = (MethodTree) c.members().get(1);
+    VariableTree variableTreeA = (VariableTree) f1.block().body().get(0);
+    VariableTree variableTreeC = (VariableTree) f1.block().body().get(1);
+    VariableTree variableTreeB = (VariableTree) f2.block().body().get(0);
+    LambdaExpressionTreeImpl lambdaA = (LambdaExpressionTreeImpl) variableTreeA.initializer();
+    LambdaExpressionTreeImpl lambdaB = (LambdaExpressionTreeImpl) variableTreeB.initializer();
+    LambdaExpressionTreeImpl lambdaC = (LambdaExpressionTreeImpl) variableTreeC.initializer();
+    VariableTreeImpl pOfA = (VariableTreeImpl) lambdaA.parameters().get(0);
+    VariableTreeImpl pOfB = (VariableTreeImpl) lambdaB.parameters().get(0);
+    VariableTreeImpl pOfC = (VariableTreeImpl) lambdaC.parameters().get(0);
+
+    assertThat(pOfA.symbol()).isNotSameAs(pOfB.symbol());
+    assertThat(pOfA.symbol()).isNotSameAs(pOfC.symbol());
+  }
 }

java-frontend/src/test/java/org/sonar/java/model/JSymbolTest.java

@@ -42,6 +42,8 @@
 import org.sonar.plugins.java.api.tree.VariableTree;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.sonar.java.model.assertions.SymbolAssert.assertThat;
@@ -163,6 +165,43 @@ void owner_local_record() {
       .hasOwner(cu.sema.typeSymbol(c1.typeBinding));
   }
 
+  @Test
+  void owner_lambdaVariable_variableBinding() {
+    JavaTree.CompilationUnitTreeImpl cu = test("""
+      class C {
+        java.util.function.Function<Integer, Integer> variableBinding = i -> i;
+      }
+      """
+    );
+    ClassTreeImpl classTree = (ClassTreeImpl) cu.types().get(0);
+    VariableTree variableBinding = (VariableTree) classTree.members().get(0);
+    LambdaExpressionTree lambda = (LambdaExpressionTree) variableBinding.initializer();
+    IdentifierTree i = (IdentifierTree) lambda.body();
+    Symbol owner = i.symbol().owner();
+    assertTrue(owner.isVariableSymbol());
+    assertEquals("variableBinding", owner.name());
+  }
+
+  @Test
+  void owner_lambdaVariable_methodBinding() {
+    JavaTree.CompilationUnitTreeImpl cu = test("""
+      class C {
+         void foo() {
+             java.util.function.Function<Long, Long> methodBinding = l -> l;
+          }
+      }
+      """
+    );
+    ClassTreeImpl classTree = (ClassTreeImpl) cu.types().get(0);
+    MethodTree method = (MethodTree) classTree.members().get(0);
+    VariableTree bindingInsideMethod = (VariableTree) method.block().body().get(0);
+    LambdaExpressionTree lambda = (LambdaExpressionTree) bindingInsideMethod.initializer();
+    VariableTree l = lambda.parameters().get(0);
+    Symbol owner = l.symbol().owner();
+    assertTrue(owner.isMethodSymbol());
+    assertEquals("foo", owner.name());
+  }
+
   @Test
   void primitive_type_hash_code() {
     JavaTree.CompilationUnitTreeImpl cu = test("class C { int u; int v; }");