SONARPY-1257 S1144: Should not raise on methods/classes with unknown decorator (#1362)

Author: maksim-grebeniuk-sonarsource

its/ruling/src/test/resources/expected/python-S1144.json

@@ -5,9 +5,6 @@
 'project:biopython/Bio/Pathway/__init__.py':[
 240,
 ],
-'project:django-2.2.3/django/utils/functional.py':[
-161,
-],
 'project:numpy-1.16.4/numpy/lib/_version.py':[
 155,
 ],

python-checks/src/main/java/org/sonar/python/checks/AbstractUnreadPrivateMembersCheck.java

@@ -19,30 +19,44 @@
  */
 package org.sonar.python.checks;
 
+import java.util.Collection;
 import java.util.Optional;
+import java.util.function.Predicate;
 import org.sonar.plugins.python.api.PythonSubscriptionCheck;
 import org.sonar.plugins.python.api.SubscriptionContext;
 import org.sonar.plugins.python.api.symbols.AmbiguousSymbol;
+import org.sonar.plugins.python.api.symbols.ClassSymbol;
 import org.sonar.plugins.python.api.symbols.Symbol;
 import org.sonar.plugins.python.api.symbols.Usage;
 import org.sonar.plugins.python.api.tree.ClassDef;
+import org.sonar.python.tree.TreeUtils;
 
 import static org.sonar.plugins.python.api.tree.Tree.Kind.CLASSDEF;
-import static org.sonar.python.tree.TreeUtils.getClassSymbolFromDef;
 
 public abstract class AbstractUnreadPrivateMembersCheck extends PythonSubscriptionCheck {
 
   @Override
   public void initialize(Context context) {
     String memberPrefix = memberPrefix();
     context.registerSyntaxNodeConsumer(CLASSDEF, ctx -> {
-      ClassDef classDef = (ClassDef) ctx.syntaxNode();
-      Optional.ofNullable(getClassSymbolFromDef(classDef)).ifPresent(classSymbol -> classSymbol.declaredMembers().stream()
+      Optional.of(ctx.syntaxNode())
+        .map(ClassDef.class::cast)
+        // avoid checking for classes with decorators since it is impossible to analyze its final behavior
+        .filter(classDef -> classDef.decorators().isEmpty())
+        .map(TreeUtils::getClassSymbolFromDef)
+        .map(ClassSymbol::declaredMembers)
+        .stream()
+        .flatMap(Collection::stream)
         .filter(s -> s.name().startsWith(memberPrefix) && !s.name().endsWith("__") && equalsToKind(s) && isNeverRead(s))
-        .forEach(symbol -> reportIssue(ctx, symbol)));
+        .filter(Predicate.not(this::isException))
+        .forEach(symbol -> reportIssue(ctx, symbol));
     });
   }
 
+  protected boolean isException(Symbol symbol) {
+    return false;
+  }
+
   private boolean equalsToKind(Symbol symbol) {
     if (symbol.kind().equals(Symbol.Kind.AMBIGUOUS)) {
       return ((AmbiguousSymbol) symbol).alternatives().stream().allMatch(s -> s.kind() == kind());

python-checks/src/main/java/org/sonar/python/checks/UnreadPrivateInnerClassesCheck.java

@@ -19,7 +19,9 @@
  */
 package org.sonar.python.checks;
 
+import java.util.Optional;
 import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.symbols.ClassSymbol;
 import org.sonar.plugins.python.api.symbols.Symbol;
 
 import static org.sonar.plugins.python.api.symbols.Symbol.Kind.CLASS;
@@ -31,6 +33,15 @@ String memberPrefix() {
     return "_";
   }
 
+  @Override
+  protected boolean isException(Symbol symbol) {
+    return Optional.of(symbol)
+      .filter(ClassSymbol.class::isInstance)
+      .map(ClassSymbol.class::cast)
+      .filter(ClassSymbol::hasDecorators)
+      .isPresent();
+  }
+
   @Override
   Symbol.Kind kind() {
     return CLASS;

python-checks/src/main/java/org/sonar/python/checks/UnreadPrivateMethodsCheck.java

@@ -19,8 +19,13 @@
  */
 package org.sonar.python.checks;
 
+import java.util.Collection;
+import java.util.Optional;
+import java.util.function.Predicate;
 import org.sonar.check.Rule;
+import org.sonar.plugins.python.api.symbols.FunctionSymbol;
 import org.sonar.plugins.python.api.symbols.Symbol;
+import org.sonar.python.semantic.BuiltinSymbols;
 
 import static org.sonar.plugins.python.api.symbols.Symbol.Kind.FUNCTION;
 
@@ -45,4 +50,15 @@ String message(String memberName) {
   String secondaryMessage() {
     return null;
   }
+
+  @Override
+  protected boolean isException(Symbol symbol) {
+    return Optional.of(symbol)
+      .filter(FunctionSymbol.class::isInstance)
+      .map(FunctionSymbol.class::cast)
+      .map(FunctionSymbol::decorators)
+      .stream()
+      .flatMap(Collection::stream)
+      .anyMatch(Predicate.not(BuiltinSymbols.STATIC_AND_CLASS_METHOD_DECORATORS::contains));
+  }
 }

python-checks/src/test/resources/checks/unreadPrivateInnerClasses.py

@@ -8,10 +8,13 @@ class __unused_cls: ... # Noncompliant {{Remove this unused private '__unused_cl
 
   class _unused_cls: ... # Noncompliant
 
+  @unknown
+  class _unused_decorated_cls: ...
+
   class __used_cls: ...
 
   class __other_used_cls: ...
 
   def __init__(self):
     print(self.__used_cls)
-    print(A.__other_used_cls)
+    print(A.__other_used_cls)

python-checks/src/test/resources/checks/unreadPrivateMethods.py

@@ -28,3 +28,30 @@ def __init__(self):
     print(self.__used())
     print(A.__used_cls_method())
     print(A.__used_static_method())
+
+
+def method_decorator(func):
+  def inner(*args, **kwargs):
+    return func(*args, **kwargs)
+  return inner
+
+class ClassWithMethodDecorator:
+  @method_decorator
+  def __getVariable(self):
+    return self._v
+
+  @classmethod
+  def __setVariable(self, v): # Noncompliant
+    self._v = v
+
+  def __printVariable(self): # Noncompliant
+    print(self._v)
+
+def class_decorator(c):
+  return c
+
+@class_decorator
+class ClassWithDecorator:
+
+  def __printVariable(self):
+    print(self.__v)

python-frontend/src/main/java/org/sonar/python/semantic/BuiltinSymbols.java

@@ -338,4 +338,9 @@ public static Set<String> all() {
     all.addAll(BuiltinSymbols.MODULE_ATTRIBUTES);
     return all;
   }
+
+  public static final String CLASS_METHOD_DECORATOR = "classmethod";
+  public static final String STATIC_METHOD_DECORATOR = "staticmethod";
+
+  public static final Set<String> STATIC_AND_CLASS_METHOD_DECORATORS = Set.of(CLASS_METHOD_DECORATOR, STATIC_METHOD_DECORATOR);
 }