Fix quality issues (#1808)

python-checks/src/test/java/org/sonar/python/checks/NonCallableCalledCheckTest.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.python.checks;
 
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.sonar.python.checks.utils.PythonCheckVerifier;
 
@@ -29,4 +30,9 @@ void test() {
  PythonCheckVerifier.verify("src/test/resources/checks/nonCallableCalled.py", new NonCallableCalledCheck());
  }
 
+ @Test
+ void test_multiple_files() {
+ PythonCheckVerifier.verify(List.of("src/test/resources/checks/nonCallableCalledImporter.py", "src/test/resources/checks/nonCallableCalledImported.py"), new NonCallableCalledCheck());
+ }
+
 }

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

@@ -0,0 +1,7 @@
+class MyNonCallableClass:
+ ...
+
+
+class MyCallableClass:
+ def __call__(self):
+ ...

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

@@ -0,0 +1,15 @@
+from nonCallableCalledImported import MyNonCallableClass, MyCallableClass
+
+
+imported_non_callable = MyNonCallableClass()
+imported_callable = MyCallableClass()
+
+imported_non_callable() # Noncompliant
+imported_callable()
+
+
+class LocallyDefinedNonCallable:
+ ...
+
+local_non_callable = LocallyDefinedNonCallable()
+local_non_callable() # Noncompliant

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

@@ -54,7 +54,7 @@ public Tree root() {
  return rootTree;
  }
 
- void addBindingUsage(Name nameTree, UsageV2.Kind kind, @Nullable String fullyQualifiedName) {
+ void addBindingUsage(Name nameTree, UsageV2.Kind kind) {
  String symbolName = nameTree.name();
  if (!isExistingSymbol(symbolName)) {
  SymbolV2 symbol = new SymbolV2(symbolName);
@@ -90,7 +90,7 @@ void createSelfParameter(Parameter parameter) {
  return;
  }
  String symbolName = nameTree.name();
- //TODO: Check with SelfSymbolImpl
+ //TODO: SONARPY-1865 Represent "self"
  SymbolV2 symbol = new SymbolV2(symbolName);
  symbols.add(symbol);
  symbolsByName.put(symbolName, symbol);

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

@@ -98,11 +98,6 @@ private ModuleType createModuleFromSymbols(@Nullable String name, @Nullable Modu
  return module;
  }
 
- private static PythonType convertToObjectType(Symbol symbol) {
- // What should we have here?
- return PythonType.UNKNOWN;
- }
-
  private static PythonType convertToFunctionType(FunctionSymbol symbol, Map<Symbol, PythonType> createdTypesBySymbol) {
  if (createdTypesBySymbol.containsKey(symbol)) {
  return createdTypesBySymbol.get(symbol);
@@ -163,7 +158,8 @@ private PythonType convertToType(Symbol symbol, Map<Symbol, PythonType> createdT
  case CLASS -> convertToClassType((ClassSymbol) symbol, createdTypesBySymbol);
  case FUNCTION -> convertToFunctionType((FunctionSymbol) symbol, createdTypesBySymbol);
  case AMBIGUOUS -> convertToUnionType((AmbiguousSymbol) symbol, createdTypesBySymbol);
- case OTHER -> convertToObjectType(symbol);
+ // Symbols that are neither classes or function nor ambiguous symbols whose alternatives are all classes or functions are considered of unknown type
+ case OTHER -> PythonType.UNKNOWN;
  };
  }
 

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

@@ -55,7 +55,7 @@ public TypeInferenceV2(ProjectLevelTypeTable projectLevelTypeTable, PythonFile p
  }
 
  public void inferTypes(FileInput fileInput) {
- TrivialTypeInferenceVisitor trivialTypeInferenceVisitor = new TrivialTypeInferenceVisitor(projectLevelTypeTable, pythonFile, symbolTable);
+ TrivialTypeInferenceVisitor trivialTypeInferenceVisitor = new TrivialTypeInferenceVisitor(projectLevelTypeTable, pythonFile);
  fileInput.accept(trivialTypeInferenceVisitor);
 
  inferTypesAndMemberAccessSymbols(fileInput);
@@ -88,7 +88,7 @@ private void inferTypesAndMemberAccessSymbols(FileInput fileInput) {
 
  private void inferTypesAndMemberAccessSymbols(FunctionDef functionDef) {
  Set<Name> parameterNames = TreeUtils.nonTupleParameters(functionDef).stream()
- // TODO: it probably doesn't make sense to restrict to annotated parameters here
+ // TODO SONARPY-1866: it probably doesn't make sense to restrict to annotated parameters here
  .filter(parameter -> parameter.typeAnnotation() != null)
  .map(Parameter::name)
  .collect(Collectors.toSet());

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

@@ -114,7 +114,7 @@ public void visitPyListOrSetCompExpression(ComprehensionExpression tree) {
 
  @Override
  public void visitFunctionDef(FunctionDef functionDef) {
- currentScope().addBindingUsage(functionDef.name(), UsageV2.Kind.FUNC_DECLARATION, null);
+ currentScope().addBindingUsage(functionDef.name(), UsageV2.Kind.FUNC_DECLARATION);
  createAndEnterScope(functionDef, currentScope());
  createTypeParameters(functionDef.typeParams());
  createParameters(functionDef);
@@ -127,7 +127,7 @@ private void createTypeParameters(@Nullable TypeParams typeParams) {
  .map(TypeParams::typeParamsList)
  .stream()
  .flatMap(Collection::stream)
- .forEach(typeParam -> currentScope().addBindingUsage(typeParam.name(), UsageV2.Kind.TYPE_PARAM_DECLARATION, null));
+ .forEach(typeParam -> currentScope().addBindingUsage(typeParam.name(), UsageV2.Kind.TYPE_PARAM_DECLARATION));
  }
 
 
@@ -151,7 +151,7 @@ private void createParameters(FunctionLike function) {
  .skip(hasSelf ? 1 : 0)
  .map(Parameter::name)
  .filter(Objects::nonNull)
- .forEach(param -> currentScope().addBindingUsage(param, UsageV2.Kind.PARAMETER, null));
+ .forEach(param -> currentScope().addBindingUsage(param, UsageV2.Kind.PARAMETER));
 
  parameterList.all().stream()
  .filter(param -> param.is(Tree.Kind.TUPLE_PARAMETER))
@@ -163,7 +163,7 @@ private void addTupleParamElementsToBindingUsage(TupleParameter param) {
  param.parameters().stream()
  .filter(p -> p.is(Tree.Kind.PARAMETER))
  .map(p -> ((Parameter) p).name())
- .forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.PARAMETER, null));
+ .forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.PARAMETER));
  param.parameters().stream()
  .filter(p -> p.is(Tree.Kind.TUPLE_PARAMETER))
  .map(TupleParameter.class::cast)
@@ -172,13 +172,13 @@ private void addTupleParamElementsToBindingUsage(TupleParameter param) {
 
  @Override
  public void visitTypeAliasStatement(TypeAliasStatement typeAliasStatement) {
- currentScope().addBindingUsage(typeAliasStatement.name(), UsageV2.Kind.TYPE_ALIAS_DECLARATION, null);
+ currentScope().addBindingUsage(typeAliasStatement.name(), UsageV2.Kind.TYPE_ALIAS_DECLARATION);
  super.visitTypeAliasStatement(typeAliasStatement);
  }
 
  @Override
  public void visitClassDef(ClassDef classDef) {
- currentScope().addBindingUsage(classDef.name(), UsageV2.Kind.CLASS_DECLARATION, null);
+ currentScope().addBindingUsage(classDef.name(), UsageV2.Kind.CLASS_DECLARATION);
  createAndEnterScope(classDef, currentScope());
  createTypeParameters(classDef.typeParams());
  super.visitClassDef(classDef);
@@ -198,7 +198,7 @@ public void visitImportFrom(ImportFrom importFrom) {
  ? moduleTree.names().stream().map(Name::name).collect(Collectors.joining("."))
  : null;
  if (importFrom.isWildcardImport()) {
- // FIXME: handle wildcard import
+ // TODO: SONARPY-1781 handle wildcard import
  } else {
  createImportedNames(importFrom.importedNames(), moduleName, importFrom.dottedPrefixForModule());
  }
@@ -212,15 +212,15 @@ private void createImportedNames(List<AliasedName> importedNames, @Nullable Stri
  String targetModuleName = fromModuleName;
  Name alias = module.alias();
  if (targetModuleName != null) {
- addBindingUsage(alias == null ? nameTree : alias, UsageV2.Kind.IMPORT, null);
+ addBindingUsage(alias == null ? nameTree : alias, UsageV2.Kind.IMPORT);
  } else if (alias != null) {
- addBindingUsage(alias, UsageV2.Kind.IMPORT, null);
+ addBindingUsage(alias, UsageV2.Kind.IMPORT);
  } else if (dottedPrefix.isEmpty() && dottedNames.size() > 1) {
  // Submodule import
- addBindingUsage(nameTree, UsageV2.Kind.IMPORT, null);
+ addBindingUsage(nameTree, UsageV2.Kind.IMPORT);
  } else {
  // It's a simple case - no "from" imports or aliasing
- addBindingUsage(nameTree, UsageV2.Kind.IMPORT, null);
+ addBindingUsage(nameTree, UsageV2.Kind.IMPORT);
  }
  });
  }
@@ -238,12 +238,12 @@ public void visitComprehensionFor(ComprehensionFor tree) {
  }
 
  private void addCompDeclarationParam(Tree tree) {
- boundNamesFromExpression(tree).forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.COMP_DECLARATION, null));
+ boundNamesFromExpression(tree).forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.COMP_DECLARATION));
  }
 
  private void createLoopVariables(ForStatement loopTree) {
  loopTree.expressions().forEach(expr ->
- boundNamesFromExpression(expr).forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.LOOP_DECLARATION, null)));
+ boundNamesFromExpression(expr).forEach(name -> currentScope().addBindingUsage(name, UsageV2.Kind.LOOP_DECLARATION)));
  }
 
  @Override
@@ -260,7 +260,7 @@ public void visitAssignmentStatement(AssignmentStatement pyAssignmentStatementTr
  public void visitAnnotatedAssignment(AnnotatedAssignment annotatedAssignment) {
  if (annotatedAssignment.variable().is(Tree.Kind.NAME)) {
  Name variable = (Name) annotatedAssignment.variable();
- addBindingUsage(variable, UsageV2.Kind.ASSIGNMENT_LHS, null);
+ addBindingUsage(variable, UsageV2.Kind.ASSIGNMENT_LHS);
  }
  super.visitAnnotatedAssignment(annotatedAssignment);
  }
@@ -283,7 +283,7 @@ public void visitAssignmentExpression(AssignmentExpression assignmentExpression)
  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);
+ moduleScope.addBindingUsage(name, UsageV2.Kind.GLOBAL_DECLARATION);
  currentScope().addGlobalName(name);
  });
  super.visitGlobalStatement(globalStatement);
@@ -314,10 +314,6 @@ public void visitCapturePattern(CapturePattern capturePattern) {
  }
 
  private void addBindingUsage(Name nameTree, UsageV2.Kind usage) {
- addBindingUsage(nameTree, usage, null);
- }
-
- private void addBindingUsage(Name nameTree, UsageV2.Kind usage, @Nullable String fullyQualifiedName) {
- currentScope().addBindingUsage(nameTree, usage, fullyQualifiedName);
+ currentScope().addBindingUsage(nameTree, usage);
  }
 }

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

@@ -89,7 +89,7 @@ public void updateProgramState(Tree element, ProgramState programState) {
  } else if (element instanceof FunctionDef functionDef) {
  handleDefinition(functionDef, state);
  } else {
- // TODO: isinstance visitor
+ // Here we should run "isinstance" visitor when we handle declared types, to avoid FPs when type guard checks are made
  updateTree(element, state);
  }
  }

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

@@ -54,7 +54,6 @@
 import org.sonar.python.semantic.v2.ClassTypeBuilder;
 import org.sonar.python.semantic.v2.FunctionTypeBuilder;
 import org.sonar.python.semantic.v2.ProjectLevelTypeTable;
-import org.sonar.python.semantic.v2.SymbolTable;
 import org.sonar.python.semantic.v2.SymbolV2;
 import org.sonar.python.semantic.v2.UsageV2;
 import org.sonar.python.tree.DictionaryLiteralImpl;
@@ -79,14 +78,12 @@
 public class TrivialTypeInferenceVisitor extends BaseTreeVisitor {
 
  private final ProjectLevelTypeTable projectLevelTypeTable;
- private final SymbolTable symbolTable;
  private final String fileId;
 
  private final Deque<PythonType> typeStack = new ArrayDeque<>();
 
- public TrivialTypeInferenceVisitor(ProjectLevelTypeTable projectLevelTypeTable, PythonFile pythonFile, SymbolTable symbolTable) {
+ public TrivialTypeInferenceVisitor(ProjectLevelTypeTable projectLevelTypeTable, PythonFile pythonFile) {
  this.projectLevelTypeTable = projectLevelTypeTable;
- this.symbolTable = symbolTable;
  Path path = pathOf(pythonFile);
  this.fileId = path != null ? path.toString() : pythonFile.toString();
  }
@@ -101,7 +98,7 @@ public void visitFileInput(FileInput fileInput) {
  @Override
  public void visitStringLiteral(StringLiteral stringLiteral) {
  ModuleType builtins = this.projectLevelTypeTable.getModule();
- // TODO: multiple object types to represent str instance?
+ // TODO: SONARPY-1867 multiple object types to represent str instance?
  PythonType strType = builtins.resolveMember("str").orElse(PythonType.UNKNOWN);
  ((StringLiteralImpl) stringLiteral).typeV2(new ObjectType(strType, new ArrayList<>(), new ArrayList<>()));
  }
@@ -149,7 +146,7 @@ public void visitNumericLiteral(NumericLiteral numericLiteral) {
  @Override
  public void visitNone(NoneExpression noneExpression) {
  ModuleType builtins = this.projectLevelTypeTable.getModule();
- // TODO: multiple object types to represent str instance?
+ // TODO: SONARPY-1867 multiple object types to represent str instance?
  PythonType noneType = builtins.resolveMember("NoneType").orElse(PythonType.UNKNOWN);
  ((NoneExpressionImpl) noneExpression).typeV2(new ObjectType(noneType, new ArrayList<>(), new ArrayList<>()));
  }
@@ -159,7 +156,6 @@ public void visitListLiteral(ListLiteral listLiteral) {
  ModuleType builtins = this.projectLevelTypeTable.getModule();
  scan(listLiteral.elements());
  List<PythonType> pythonTypes = listLiteral.elements().expressions().stream().map(Expression::typeV2).distinct().toList();
- // TODO: cleanly reduce attributes
  PythonType listType = builtins.resolveMember("list").orElse(PythonType.UNKNOWN);
  ((ListLiteralImpl) listLiteral).typeV2(new ObjectType(listType, pythonTypes, new ArrayList<>()));
  }
@@ -195,7 +191,7 @@ static void resolveTypeHierarchy(ClassDef classDef, ClassTypeBuilder classTypeBu
 
  private static void addParentClass(ClassTypeBuilder classTypeBuilder, RegularArgument regularArgument) {
  Name keyword = regularArgument.keywordArgument();
- // TODO: store names if not resolved properly
+ // TODO: SONARPY-1871 store names if not resolved properly
  if (keyword != null) {
  if ("metaclass".equals(keyword.name())) {
  PythonType argumentType = getTypeV2FromArgument(regularArgument);
@@ -205,7 +201,7 @@ private static void addParentClass(ClassTypeBuilder classTypeBuilder, RegularArg
  }
  PythonType argumentType = getTypeV2FromArgument(regularArgument);
  classTypeBuilder.superClasses().add(argumentType);
- // TODO: handle generics
+ // TODO: SONARPY-1869 handle generics
  }
 
  private static PythonType getTypeV2FromArgument(RegularArgument regularArgument) {
@@ -364,7 +360,7 @@ public void visitName(Name name) {
  .filter(Expression.class::isInstance)
  .map(Expression.class::cast)
  .map(Expression::typeV2)
- // FIXME: classes and functions should be propagated like other types
+ // TODO: classes (SONARPY-1829) and functions should be propagated like other types
  .filter(t -> (t instanceof ClassType) || (t instanceof FunctionType))
  .ifPresent(type -> setTypeToName(name, type));
  }

python-frontend/src/main/java/org/sonar/python/tree/CallExpressionImpl.java

@@ -47,7 +47,6 @@
 import org.sonar.python.types.HasTypeDependencies;
 import org.sonar.python.types.InferredTypes;
 import org.sonar.python.types.v2.ClassType;
-import org.sonar.python.types.v2.FunctionType;
 import org.sonar.python.types.v2.ObjectType;
 import org.sonar.python.types.v2.PythonType;
 

python-frontend/src/main/java/org/sonar/python/tree/ListLiteralImpl.java

@@ -79,6 +79,7 @@ public InferredType type() {
  return InferredTypes.LIST;
  }
 
+ @Override
  public PythonType typeV2() {
  return this.typeV2;
  }

python-frontend/src/main/java/org/sonar/python/tree/NumericLiteralImpl.java

@@ -102,7 +102,7 @@ private InferredType computeType() {
  return InferredTypes.INT;
  }
 
-
+ @Override
  public PythonType typeV2() {
  return this.typeV2;
  }

python-frontend/src/main/java/org/sonar/python/tree/StringLiteralImpl.java

@@ -80,6 +80,7 @@ public InferredType type() {
  return InferredTypes.STR;
  }
 
+ @Override
  public PythonType typeV2() {
  return this.typeV2;
  }

python-frontend/src/main/java/org/sonar/python/types/v2/ClassType.java

@@ -91,7 +91,7 @@ public boolean isASubClassFrom(ClassType other) {
  }
 
  public boolean areAttributesCompatible(ClassType other) {
- return attributes.stream().allMatch(attr -> other.attributes.stream().anyMatch(otherAttr -> attr.isCompatibleWith(otherAttr)));
+ return attributes.stream().allMatch(attr -> other.attributes.stream().anyMatch(attr::isCompatibleWith));
  }
 
  @Override

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

@@ -24,7 +24,6 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Disabled;
@@ -351,7 +350,7 @@ def foo(param: int):
 
  var functionDef = (FunctionDef) root.statements().statements().get(0);
  var lastExpressionStatement = (ExpressionStatement) functionDef.body().statements().get(functionDef.body().statements().size() -1);
- // TODO: should be declared int
+ // TODO SONARPY-1773: should be declared int
  Assertions.assertThat(lastExpressionStatement.expressions().get(0).typeV2().unwrappedType()).isEqualTo(PythonType.UNKNOWN);
  }
 
@@ -778,8 +777,6 @@ class MyClass:
  MyClass()
  """);
 
- ClassDef classDef = ((ClassDef) fileInput.statements().statements().get(0));
-
  List<CallExpression> calls = PythonTestUtils.getAllDescendant(fileInput, tree -> tree.is(Tree.Kind.CALL_EXPR));
  PythonType calleeType1 = calls.get(0).callee().typeV2();
  PythonType calleeType2 = calls.get(1).callee().typeV2();