SONARPY-557: Fix FP on S3827 when modules manipulate __all__ property (#562)

guillaume-dequenne · web-flow · commit 02ead7a2859e · 2020-02-12T11:02:09.000+01:00
diff --git a/python-checks-testkit/src/main/java/org/sonar/python/checks/utils/PythonCheckVerifier.java b/python-checks-testkit/src/main/java/org/sonar/python/checks/utils/PythonCheckVerifier.java
@@ -75,6 +75,13 @@ public static void verify(List<String> paths, PythonCheck check) {
     createVerifier(files, check, globalSymbolsPerModule, baseDirFile).assertOneOrMoreIssues();
   }
 
+  public static void verifyNoIssue(List<String> paths, PythonCheck check) {
+    List<File> files = paths.stream().map(File::new).collect(Collectors.toList());
+    File baseDirFile = new File(files.get(0).getParent());
+    Map<String, Set<Symbol>> globalSymbolsPerModule = TestPythonVisitorRunner.globalSymbols(files, baseDirFile);
+    createVerifier(files, check, globalSymbolsPerModule, baseDirFile).assertNoIssues();
+  }
+
   private static MultiFileVerifier createVerifier(List<File> files, PythonCheck check, Map<String, Set<Symbol>> globalSymbolsPerModule, @Nullable File baseDir) {
     MultiFileVerifier multiFileVerifier = MultiFileVerifier.create(files.get(0).toPath(), UTF_8);
     for (File file : files) {
diff --git a/python-checks-testkit/src/test/java/org/sonar/python/checks/utils/PythonCheckVerifierAdditionalTest.java b/python-checks-testkit/src/test/java/org/sonar/python/checks/utils/PythonCheckVerifierAdditionalTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.python.checks.utils;
 
+import java.util.Collections;
 import org.assertj.core.api.Assertions;
 import org.junit.Test;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
@@ -57,6 +58,7 @@ public void initialize(Context context) {
       }
     };
     PythonCheckVerifier.verifyNoIssue(BASE_DIR + "no_issue.py", check);
+    PythonCheckVerifier.verifyNoIssue(Collections.singletonList(BASE_DIR + "no_issue.py"), check);
 
     try {
       PythonCheckVerifier.verifyNoIssue(BASE_DIR + "file_issue.py", check);
diff --git a/python-checks/src/main/java/org/sonar/python/checks/UndeclaredNameUsageCheck.java b/python-checks/src/main/java/org/sonar/python/checks/UndeclaredNameUsageCheck.java
@@ -50,6 +50,9 @@ public class UndeclaredNameUsageCheck extends PythonSubscriptionCheck {
   public void initialize(Context context) {
     context.registerSyntaxNodeConsumer(Tree.Kind.FILE_INPUT, ctx -> {
       FileInput fileInput = (FileInput) ctx.syntaxNode();
+      if (importsManipulatedAllProperty(fileInput)) {
+        return;
+      }
       UnresolvedSymbolsVisitor unresolvedSymbolsVisitor = new UnresolvedSymbolsVisitor();
       fileInput.accept(unresolvedSymbolsVisitor);
       if (!unresolvedSymbolsVisitor.callGlobalsOrLocals && !unresolvedSymbolsVisitor.hasUnresolvedWildcardImport) {
@@ -73,6 +76,10 @@ public void initialize(Context context) {
     });
   }
 
+  private static boolean importsManipulatedAllProperty(FileInput fileInput) {
+    return fileInput.globalVariables().stream().anyMatch(s -> s.name().equals("__all__") && s.fullyQualifiedName() != null);
+  }
+
   private static void checkCfgBlock(CfgBlock cfgBlock, SubscriptionContext ctx, DefinedVariables definedVariables,
                                     Set<CfgBlock> unreachableBlocks, DefinedVariablesAnalysis analysis, List<Symbol> ignoredSymbols) {
     Map<Symbol, DefinedVariablesAnalysis.VariableDefinition> currentState = new HashMap<>(definedVariables.getIn());
diff --git a/python-checks/src/test/java/org/sonar/python/checks/UndeclaredNameUsageCheckTest.java b/python-checks/src/test/java/org/sonar/python/checks/UndeclaredNameUsageCheckTest.java
@@ -37,6 +37,13 @@ public void test_wildcard_import() {
       new UndeclaredNameUsageCheck());
   }
 
+  @Test
+  public void test_wildcard_import_all_property() {
+    PythonCheckVerifier.verifyNoIssue(
+      Arrays.asList("src/test/resources/checks/undeclaredNameUsageImportWithAll.py", "src/test/resources/checks/undeclaredNameUsageAll/__init__.py"),
+      new UndeclaredNameUsageCheck());
+  }
+
   @Test
   public void test_unresolved_wildcard_import() {
     PythonCheckVerifier.verifyNoIssue("src/test/resources/checks/undeclaredNameUsageWithUnresolvedWildcardImport.py", new UndeclaredNameUsageCheck());
diff --git a/python-checks/src/test/resources/checks/undeclaredNameUsageAll/__init__.py b/python-checks/src/test/resources/checks/undeclaredNameUsageAll/__init__.py
@@ -0,0 +1,5 @@
+# mod.py
+def get_exported(): 
+  def f(): pass
+  return f
+__all__ = get_exported()
diff --git a/python-checks/src/test/resources/checks/undeclaredNameUsageImportWithAll.py b/python-checks/src/test/resources/checks/undeclaredNameUsageImportWithAll.py
@@ -0,0 +1,2 @@
+from undeclaredNameUsageAll import *
+f() # OK

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+from undeclaredNameUsageAll import *`
	`2`	`+f() # OK`