Fix quality flaws (SonarSource#860)

guillaume-dequenne · web-flow · commit 3bbbd8ea0693 · 2020-08-28T11:39:54.000+02:00
diff --git a/python-checks/src/main/java/org/sonar/python/checks/CheckUtils.java b/python-checks/src/main/java/org/sonar/python/checks/CheckUtils.java
@@ -92,25 +92,4 @@ public static boolean classHasInheritance(ClassDef classDef) {
     }
     return arguments.size() != 1 || !"object".equals(arguments.get(0).firstToken().value());
   }
-
-  public static boolean isCalledInClassBody(Symbol symbol, ClassDef classDef) {
-    return symbol.usages().stream().anyMatch(usage -> {
-      Tree tree = usage.tree();
-      Tree parentTree = tree.parent();
-
-      if (!parentTree.is(Tree.Kind.CALL_EXPR)) {
-        return false;
-      }
-
-      // We want a call expression which
-      //  1) is a call to our function
-      //  2) is not in a call within another function
-      //  3) it has a class body ancestor AND it is classDef
-      CallExpression call = (CallExpression) parentTree;
-      return call.callee().equals(tree)
-        && TreeUtils.firstAncestorOfKind(call, Tree.Kind.FUNCDEF) == null
-        && classDef.equals(TreeUtils.firstAncestorOfKind(call, Tree.Kind.CLASSDEF));
-    });
-  }
-
 }
diff --git a/python-checks/src/main/java/org/sonar/python/checks/FunctionReturnTypeCheck.java b/python-checks/src/main/java/org/sonar/python/checks/FunctionReturnTypeCheck.java
@@ -68,6 +68,7 @@ private static void raiseIssues(SubscriptionContext ctx, FunctionDef functionDef
     String functionName = functionDef.name().name();
     String returnTypeName = InferredTypes.typeName(declaredReturnType);
     if (!returnTypeVisitor.yieldStatements.isEmpty()) {
+      // Here we should probably use an equivalent of "canBeOrExtend" (accepting uncertainty) instead of "mustBeOrExtend"
       if (ITERABLE_TYPES.stream().anyMatch(declaredReturnType::mustBeOrExtend)) {
         return;
       }
diff --git a/python-checks/src/main/java/org/sonar/python/checks/ItemOperationsType.java b/python-checks/src/main/java/org/sonar/python/checks/ItemOperationsType.java
@@ -29,7 +29,6 @@
 import org.sonar.plugins.python.api.tree.AssignmentStatement;
 import org.sonar.plugins.python.api.tree.DelStatement;
 import org.sonar.plugins.python.api.tree.Expression;
-import org.sonar.plugins.python.api.tree.Name;
 import org.sonar.plugins.python.api.tree.SubscriptionExpression;
 import org.sonar.plugins.python.api.tree.Tree;
 import org.sonar.python.tree.TreeUtils;
diff --git a/python-checks/src/test/resources/checks/confusingTypeChecking/incorrectExceptionType.py b/python-checks/src/test/resources/checks/confusingTypeChecking/incorrectExceptionType.py
@@ -1,3 +1,4 @@
+from typing import Union, Optional
 class A: ...
 class CustomException(BaseException): ...
 class AnotherException(CustomException): ...
@@ -12,9 +13,11 @@ def custom(param1: A, param2: CustomException, param3: AnotherException):
   raise param3
 
 
-def builtin(param1: str, param2: Exception, param3: BaseException, param4: ValueError):
+def builtin(param1: str, param2: Exception, param3: BaseException, param4: ValueError, param5: Union[BaseException, str], param6: Optional[BaseException]):
   raise param1  # Noncompliant {{Fix this "raise" statement; Previous type checks suggest that "param1" has type "str" and is not an exception.}}
   raise param2
   raise param3
   raise param4
   raise my_int()  # Noncompliant {{Fix this "raise" statement; Previous type checks suggest that this expression has type "int" and is not an exception.}}
+  raise param5
+  raise param6
diff --git a/python-frontend/src/main/java/org/sonar/python/types/DeclaredType.java b/python-frontend/src/main/java/org/sonar/python/types/DeclaredType.java
@@ -125,13 +125,12 @@ public boolean isCompatibleWith(InferredType other) {
 
   @Override
   public boolean mustBeOrExtend(String typeName) {
-    // TODO: change its implementation to reflect semantic of 'RuntimeType.canOnlyBe'
     return alternativeTypeSymbols().stream().flatMap(a -> {
       if (a.is(Symbol.Kind.AMBIGUOUS)) {
-        return ((AmbiguousSymbol) a).alternatives().stream().filter(alternative -> alternative.is(Symbol.Kind.CLASS));
+        return ((AmbiguousSymbol) a).alternatives().stream().filter(alternative -> alternative.is(CLASS));
       }
       return Stream.of(a);
-    }).filter(a -> a.is(Symbol.Kind.CLASS)).allMatch(a -> ((ClassSymbol) a).isOrExtends(typeName));
+    }).filter(a -> a.is(CLASS)).allMatch(a -> ((ClassSymbol) a).isOrExtends(typeName));
   }
 
   @Override
diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/SelfSymbolImplTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/SelfSymbolImplTest.java
@@ -47,6 +47,7 @@ public void removeUsages() {
     ((SymbolImpl) selfSymbol).removeUsages();
     assertThat(selfSymbol.usages()).isEmpty();
     ClassDef classDef = PythonTestUtils.getLastDescendant(fileInput, tree -> tree.is(Tree.Kind.CLASSDEF));
+    assertThat(classDef.instanceFields()).isNotEmpty();
     assertThat(classDef.instanceFields()).allMatch(symbol -> symbol.usages().isEmpty());
   }
 

Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ private static void raiseIssues(SubscriptionContext ctx, FunctionDef functionDef`
`68`	`68`	`String functionName = functionDef.name().name();`
`69`	`69`	`String returnTypeName = InferredTypes.typeName(declaredReturnType);`
`70`	`70`	`if (!returnTypeVisitor.yieldStatements.isEmpty()) {`
	`71`	`+ // Here we should probably use an equivalent of "canBeOrExtend" (accepting uncertainty) instead of "mustBeOrExtend"`
`71`	`72`	`if (ITERABLE_TYPES.stream().anyMatch(declaredReturnType::mustBeOrExtend)) {`
`72`	`73`	`return;`
`73`	`74`	`}`
Original file line number	Diff line number	Diff line change
`@@ -125,13 +125,12 @@ public boolean isCompatibleWith(InferredType other) {`
`125`	`125`
`126`	`126`	`@Override`
`127`	`127`	`public boolean mustBeOrExtend(String typeName) {`
`128`		`- // TODO: change its implementation to reflect semantic of 'RuntimeType.canOnlyBe'`
`129`	`128`	`return alternativeTypeSymbols().stream().flatMap(a -> {`
`130`	`129`	`if (a.is(Symbol.Kind.AMBIGUOUS)) {`
`131`		`- return ((AmbiguousSymbol) a).alternatives().stream().filter(alternative -> alternative.is(Symbol.Kind.CLASS));`
	`130`	`+ return ((AmbiguousSymbol) a).alternatives().stream().filter(alternative -> alternative.is(CLASS));`
`132`	`131`	`}`
`133`	`132`	`return Stream.of(a);`
`134`		`- }).filter(a -> a.is(Symbol.Kind.CLASS)).allMatch(a -> ((ClassSymbol) a).isOrExtends(typeName));`
	`133`	`+ }).filter(a -> a.is(CLASS)).allMatch(a -> ((ClassSymbol) a).isOrExtends(typeName));`
`135`	`134`	`}`
`136`	`135`
`137`	`136`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,7 @@ public void removeUsages() {`
`47`	`47`	`((SymbolImpl) selfSymbol).removeUsages();`
`48`	`48`	`assertThat(selfSymbol.usages()).isEmpty();`
`49`	`49`	`ClassDef classDef = PythonTestUtils.getLastDescendant(fileInput, tree -> tree.is(Tree.Kind.CLASSDEF));`
	`50`	`+ assertThat(classDef.instanceFields()).isNotEmpty();`
`50`	`51`	`assertThat(classDef.instanceFields()).allMatch(symbol -> symbol.usages().isEmpty());`
`51`	`52`	`}`
`52`	`53`