SONARPY-584 Rule S5719: Updated issue message and removed some FPs (SonarSource#650)

gyula-sallai-sonarsource · web-flow · commit 75023f909619 · 2020-03-20T14:20:49.000+01:00
* SONARPY-584 Rule S5719: Updated issue message and removed some FPs

* Small refactoring on usage search
diff --git a/its/ruling/src/test/resources/expected/python-S5719.json b/its/ruling/src/test/resources/expected/python-S5719.json
@@ -29,7 +29,4 @@
 'project:twisted-12.1.0/twisted/protocols/ftp.py':[
 2063,
 ],
-'project:twisted-12.1.0/twisted/trial/test/test_runner.py':[
-893,
-],
 }
diff --git a/python-checks/src/main/java/org/sonar/python/checks/InstanceAndClassMethodsAtLeastOnePositionalCheck.java b/python-checks/src/main/java/org/sonar/python/checks/InstanceAndClassMethodsAtLeastOnePositionalCheck.java
@@ -27,6 +27,7 @@
 import org.sonar.plugins.python.api.SubscriptionContext;
 import org.sonar.plugins.python.api.symbols.ClassSymbol;
 import org.sonar.plugins.python.api.symbols.FunctionSymbol;
+import org.sonar.plugins.python.api.symbols.Usage;
 import org.sonar.plugins.python.api.tree.ClassDef;
 import org.sonar.plugins.python.api.tree.FunctionDef;
 import org.sonar.plugins.python.api.tree.Name;
@@ -39,6 +40,12 @@ public class InstanceAndClassMethodsAtLeastOnePositionalCheck extends PythonSubs
 
   private static final List<String> KNOWN_CLASS_METHODS = Arrays.asList("__new__", "__init_subclass__");
 
+  private static boolean isUsageInClassBody(Usage usage, ClassDef classDef) {
+    // We want all usages that are not function declarations and their closes parent is the class definition
+    return usage.kind() != Usage.Kind.FUNC_DECLARATION
+      && classDef.equals(TreeUtils.firstAncestorOfKind(usage.tree(), Tree.Kind.CLASSDEF, Tree.Kind.FUNCDEF));
+  }
+
   private static void handleFunctionDef(SubscriptionContext ctx, ClassDef classDef, FunctionDef functionDef) {
     List<Parameter> parameters = TreeUtils.positionalParameters(functionDef);
     if (!parameters.isEmpty()) {
@@ -51,7 +58,7 @@ private static void handleFunctionDef(SubscriptionContext ctx, ClassDef classDef
     }
 
     FunctionSymbol functionSymbol = TreeUtils.getFunctionSymbolFromDef(functionDef);
-    if (functionSymbol == null || CheckUtils.isCalledInClassBody(functionSymbol, classDef)) {
+    if (functionSymbol == null || functionSymbol.usages().stream().anyMatch(usage -> isUsageInClassBody(usage, classDef))) {
       return;
     }
 
@@ -69,7 +76,7 @@ private static void handleFunctionDef(SubscriptionContext ctx, ClassDef classDef
     if (KNOWN_CLASS_METHODS.contains(name) || decoratorNames.contains("classmethod")) {
       ctx.addIssue(functionDef.defKeyword(), functionDef.rightPar(), "Add a class parameter");
     } else {
-      ctx.addIssue(functionDef.defKeyword(), functionDef.rightPar(), "Add a \"self\" parameter");
+      ctx.addIssue(functionDef.defKeyword(), functionDef.rightPar(), "Add a \"self\" or class parameter\"");
     }
   }
 
diff --git a/python-checks/src/test/resources/checks/instanceAndClassMethodAtLeastOnePositional.py b/python-checks/src/test/resources/checks/instanceAndClassMethodAtLeastOnePositional.py
@@ -1,5 +1,5 @@
 class Foo:
-    def instance_method(): # Noncompliant {{Add a "self" parameter}}
+    def instance_method(): # Noncompliant {{Add a "self" or class parameter"}}
 #   ^^^^^^^^^^^^^^^^^^^^^
         pass
 
@@ -31,12 +31,23 @@ def _called_in_cls_body(): # OK, this is called in the class body
 
     x = _called_in_cls_body()
 
+    def referenced_in_cls_body():
+        return 1
+
+    options = [referenced_in_cls_body]
+
+    # FP
+    def referenced_outside(): # Noncompliant
+        return 2
+
     def no_pos_args(*, kw): # Noncompliant
         pass
 
     def varargs(*args, kwarg): # OK, unlimited number of positional args
         pass
 
+outside = [Foo.referenced_outside]
+
 import zope.interface as zi
 class MyInterface(zi.Interface):
     def do_something(): # OK, it is a zope interface

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,4 @@`
`29`	`29`	`'project:twisted-12.1.0/twisted/protocols/ftp.py':[`
`30`	`30`	`2063,`
`31`	`31`	`],`
`32`		`-'project:twisted-12.1.0/twisted/trial/test/test_runner.py':[`
`33`		`-893,`
`34`		`-],`
`35`	`32`	`}`