SONARPY-1824 Fix incorrect inference for global variables assigned within functions (#1804)

Author: guillaume-dequenne

python-frontend/src/main/java/org/sonar/python/semantic/v2/ScopeV2.java

@@ -37,6 +37,8 @@ public class ScopeV2 {
   private final List<ScopeV2> childrenScopes;
   private final Map<String, SymbolV2> symbolsByName = new HashMap<>();
   private final Set<SymbolV2> symbols = new HashSet<>();
+  private final Set<String> globalNames = new HashSet<>();
+  private final Set<String> nonlocalNames = new HashSet<>();
 
   public ScopeV2(@Nullable ScopeV2 parent, Tree rootTree) {
     this.parent = parent;
@@ -79,8 +81,7 @@ SymbolV2 resolve(String symbolName) {
   }
 
   private boolean isExistingSymbol(String symbolName) {
-    //return symbolsByName.containsKey(symbolName) || globalNames.contains(symbolName) || nonlocalNames.contains(symbolName);
-    return symbolsByName.containsKey(symbolName);
+    return symbolsByName.containsKey(symbolName) || globalNames.contains(symbolName) || nonlocalNames.contains(symbolName);
   }
 
   void createSelfParameter(Parameter parameter) {
@@ -96,6 +97,14 @@ void createSelfParameter(Parameter parameter) {
     symbol.addUsage(nameTree, UsageV2.Kind.PARAMETER);
   }
 
+  public void addGlobalName(Name name) {
+    this.globalNames.add(name.name());
+  }
+
+  public void addNonLocalName(Name name) {
+    this.nonlocalNames.add(name.name());
+  }
+
   public Map<String, SymbolV2> symbols() {
     return symbolsByName;
   }

python-frontend/src/main/java/org/sonar/python/semantic/v2/WriteUsagesVisitor.java

@@ -49,6 +49,7 @@
 import org.sonar.plugins.python.api.tree.ImportName;
 import org.sonar.plugins.python.api.tree.LambdaExpression;
 import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.NonlocalStatement;
 import org.sonar.plugins.python.api.tree.Parameter;
 import org.sonar.plugins.python.api.tree.ParameterList;
 import org.sonar.plugins.python.api.tree.Token;
@@ -281,10 +282,19 @@ public void visitAssignmentExpression(AssignmentExpression assignmentExpression)
   @Override
   public void visitGlobalStatement(GlobalStatement globalStatement) {
     // Global statements are not binding usages, but we consider them as such for symbol creation
-    globalStatement.variables().forEach(name -> moduleScope.addBindingUsage(name, UsageV2.Kind.GLOBAL_DECLARATION, null));
+    globalStatement.variables().forEach(name -> {
+      moduleScope.addBindingUsage(name, UsageV2.Kind.GLOBAL_DECLARATION, null);
+      currentScope().addGlobalName(name);
+    });
     super.visitGlobalStatement(globalStatement);
   }
 
+  @Override
+  public void visitNonlocalStatement(NonlocalStatement pyNonlocalStatementTree) {
+    pyNonlocalStatementTree.variables().forEach(name -> currentScope().addNonLocalName(name));
+    super.visitNonlocalStatement(pyNonlocalStatementTree);
+  }
+
   @Override
   public void visitExceptClause(ExceptClause exceptClause) {
     boundNamesFromExpression(exceptClause.exceptionInstance()).forEach(name -> addBindingUsage(name, UsageV2.Kind.EXCEPTION_INSTANCE));

python-frontend/src/test/java/org/sonar/python/semantic/v2/SymbolTableBuilderV2Test.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.python.semantic.v2;
 
+import java.util.List;
 import java.util.Set;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -27,6 +28,7 @@
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.ImportName;
 import org.sonar.plugins.python.api.tree.Name;
+import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.python.PythonTestUtils;
 import org.sonar.python.tree.TreeUtils;
 
@@ -94,6 +96,7 @@ void testSymbolTableGlobalSymbols() {
         global a
         def script_do_something(param):
             global b
+            b = 42
        
         """
     );
@@ -117,6 +120,34 @@ def script_do_something(param):
       .contains("a", "b", "script_do_something");
   }
 
+  @Test
+  void testNonLocalSymbol() {
+    FileInput fileInput = PythonTestUtils.parse(
+      """
+        def foo():
+          a = 42
+          def inner(param):
+            nonlocal a
+            a = "hello"
+            print(a)
+       
+        """
+    );
+
+    var symbolTable = new SymbolTableBuilderV2(fileInput)
+      .build();
+
+    var moduleSymbols = symbolTable.getSymbolsByRootTree(fileInput);
+    List<FunctionDef> functionDefs = PythonTestUtils.getAllDescendant(fileInput, tree -> tree.is(Tree.Kind.FUNCDEF));
+
+    Set<SymbolV2> fooSymbols = symbolTable.getSymbolsByRootTree(functionDefs.get(0));
+    Set<SymbolV2> innerSymbols = symbolTable.getSymbolsByRootTree(functionDefs.get(1));
+
+    Assertions.assertThat(moduleSymbols).extracting(SymbolV2::name).containsExactlyInAnyOrder("foo");
+    Assertions.assertThat(fooSymbols).extracting(SymbolV2::name).containsExactlyInAnyOrder("a", "inner");
+    Assertions.assertThat(innerSymbols).extracting(SymbolV2::name).containsExactlyInAnyOrder("param");
+  }
+
   @Test
   void testNameSymbols() {
     FileInput fileInput = PythonTestUtils.parse(

python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java

@@ -396,16 +396,15 @@ def foo(param: int):
   void inferTypeForReassignedBuiltinsInsideFunction() {
     FileInput root = inferTypes("""
       def foo():
-        global list
-        list = 42
-        list = "hello"
-        list
+        global x
+        x = 42
+        x = "hello"
+        x
       """);
 
     var functionDef = (FunctionDef) root.statements().statements().get(0);
     var expressionStatement = (ExpressionStatement) functionDef.body().statements().get(3);
-    // TODO: Shouldn't this be UNKNOWN due to glboal?
-    assertThat(expressionStatement.expressions().get(0).typeV2().unwrappedType()).isEqualTo(STR_TYPE);
+    assertThat(expressionStatement.expressions().get(0).typeV2().unwrappedType()).isEqualTo(PythonType.UNKNOWN);
   }
 
   @Test