From e64757ad8d1fe496167f72002991456c4404359f Mon Sep 17 00:00:00 2001 From: Maksim Grebeniuk Date: Wed, 23 Oct 2024 17:05:27 +0200 Subject: [PATCH] SONARPY-2243 Make the V1 symbols populating out of descriptors collected by the V2 type inference --- ...incorrectExceptionTypeWithRegularImport.py | 2 +- .../semantic/ProjectLevelSymbolTable.java | 43 +++---------------- .../PythonTypeToDescriptorConverter.java | 17 ++++++-- .../semantic/ProjectLevelSymbolTableTest.java | 39 +++++++++-------- .../semantic/v2/TypeInferenceV2Test.java | 12 ++++++ 5 files changed, 53 insertions(+), 60 deletions(-) diff --git a/python-checks/src/test/resources/checks/incorrectExceptionType/incorrectExceptionTypeWithRegularImport.py b/python-checks/src/test/resources/checks/incorrectExceptionType/incorrectExceptionTypeWithRegularImport.py index 9731f52b15..5bce7ee229 100644 --- a/python-checks/src/test/resources/checks/incorrectExceptionType/incorrectExceptionTypeWithRegularImport.py +++ b/python-checks/src/test/resources/checks/incorrectExceptionType/incorrectExceptionTypeWithRegularImport.py @@ -38,7 +38,7 @@ def raise_nested_non_exception_class(): raise Enclsoing.Nested() # FN as only top-level imported symbols are considered def raise_RedefinedBaseExceptionChild(): - raise RedefinedBaseExceptionChild() # FN + raise RedefinedBaseExceptionChild() # Noncompliant def raise_ChildOfActualException(): raise ChildOfActualException() # OK diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java b/python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java index b7ff5def40..5fc9f7f73b 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java @@ -31,9 +31,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.plugins.python.api.PythonFile; -import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; import org.sonar.plugins.python.api.symbols.Symbol; -import org.sonar.plugins.python.api.symbols.Usage; import org.sonar.plugins.python.api.tree.BaseTreeVisitor; import org.sonar.plugins.python.api.tree.CallExpression; import org.sonar.plugins.python.api.tree.FileInput; @@ -41,13 +39,13 @@ import org.sonar.python.index.AmbiguousDescriptor; import org.sonar.python.index.Descriptor; import org.sonar.python.index.DescriptorUtils; -import org.sonar.python.index.VariableDescriptor; import org.sonar.python.semantic.v2.BasicTypeTable; import org.sonar.python.semantic.v2.SymbolTableBuilderV2; import org.sonar.python.semantic.v2.TypeInferenceV2; import org.sonar.python.semantic.v2.UsageV2; import org.sonar.python.semantic.v2.converter.PythonTypeToDescriptorConverter; import org.sonar.python.semantic.v2.typeshed.TypeShedDescriptorsProvider; +import org.sonar.python.types.v2.UnknownType; import static org.sonar.python.tree.TreeUtils.getSymbolFromTree; import static org.sonar.python.tree.TreeUtils.nthArgumentOrKeyword; @@ -56,9 +54,7 @@ public class ProjectLevelSymbolTable { private final PythonTypeToDescriptorConverter pythonTypeToDescriptorConverter = new PythonTypeToDescriptorConverter(); private final Map> globalDescriptorsByModuleName; - private final Map> globalDescriptorsByModuleNameV2; private Map globalDescriptorsByFQN; - private Map globalDescriptorsByFQNV2; private final Set djangoViewsFQN = new HashSet<>(); private final Map> importsByModule = new HashMap<>(); private final Set projectBasePackages = new HashSet<>(); @@ -74,19 +70,15 @@ public static ProjectLevelSymbolTable from(Map> globalSymbol public ProjectLevelSymbolTable() { this.globalDescriptorsByModuleName = new HashMap<>(); - this.globalDescriptorsByModuleNameV2 = new HashMap<>(); } private ProjectLevelSymbolTable(Map> globalSymbolsByModuleName) { this.globalDescriptorsByModuleName = new HashMap<>(); - this.globalDescriptorsByModuleNameV2 = new HashMap<>(); globalSymbolsByModuleName.entrySet().forEach(entry -> { String moduleName = entry.getKey(); Set symbols = entry.getValue(); Set globalDescriptors = symbols.stream().map(DescriptorUtils::descriptor).collect(Collectors.toSet()); globalDescriptorsByModuleName.put(moduleName, globalDescriptors); - globalDescriptors = symbols.stream().map(DescriptorUtils::descriptor).collect(Collectors.toSet()); - globalDescriptorsByModuleNameV2.put(moduleName, globalDescriptors); }); } @@ -95,38 +87,13 @@ public void removeModule(String packageName, String fileName) { globalDescriptorsByModuleName.remove(fullyQualifiedModuleName); // ensure globalDescriptorsByFQN is re-computed this.globalDescriptorsByFQN = null; - this.globalDescriptorsByFQNV2 = null; } public void addModule(FileInput fileInput, String packageName, PythonFile pythonFile) { SymbolTableBuilder symbolTableBuilder = new SymbolTableBuilder(packageName, pythonFile); String fullyQualifiedModuleName = SymbolUtils.fullyQualifiedModuleName(packageName, pythonFile.fileName()); fileInput.accept(symbolTableBuilder); - Set globalDescriptors = new HashSet<>(); importsByModule.put(fullyQualifiedModuleName, symbolTableBuilder.importedModulesFQN()); - for (Symbol globalVariable : fileInput.globalVariables()) { - String fullyQualifiedVariableName = globalVariable.fullyQualifiedName(); - if (((fullyQualifiedVariableName != null) && !fullyQualifiedVariableName.startsWith(fullyQualifiedModuleName)) || - globalVariable.usages().stream().anyMatch(u -> u.kind().equals(Usage.Kind.IMPORT))) { - // TODO: We don't put builtin or imported names in global symbol table to avoid duplicate FQNs in project level symbol table (to fix with SONARPY-647) - continue; - } - if (globalVariable.is(Symbol.Kind.CLASS, Symbol.Kind.FUNCTION)) { - globalDescriptors.add(DescriptorUtils.descriptor(globalVariable)); - } else { - String fullyQualifiedName = fullyQualifiedModuleName + "." + globalVariable.name(); - if (globalVariable.is(Symbol.Kind.AMBIGUOUS)) { - globalDescriptors.add(DescriptorUtils.ambiguousDescriptor((AmbiguousSymbol) globalVariable, fullyQualifiedName)); - } else { - globalDescriptors.add(new VariableDescriptor(globalVariable.name(), fullyQualifiedName, globalVariable.annotatedTypeName())); - } - } - } - globalDescriptorsByModuleName.put(fullyQualifiedModuleName, globalDescriptors); - if (globalDescriptorsByFQN != null) { - // TODO: build globalSymbolsByFQN incrementally - addModuleToGlobalSymbolsByFQN(globalDescriptors); - } DjangoViewsVisitor djangoViewsVisitor = new DjangoViewsVisitor(); fileInput.accept(djangoViewsVisitor); addModuleV2(fileInput, packageName, pythonFile); @@ -136,7 +103,7 @@ private void addModuleToGlobalSymbolsByFQN(Set descriptors) { Map moduleDescriptorsByFQN = descriptors.stream() .filter(d -> d.fullyQualifiedName() != null) .collect(Collectors.toMap(Descriptor::fullyQualifiedName, Function.identity(), AmbiguousDescriptor::create)); - globalDescriptorsByFQN.putAll(moduleDescriptorsByFQN); + globalDescriptorsByFQN().putAll(moduleDescriptorsByFQN); } private Map globalDescriptorsByFQN() { @@ -185,7 +152,7 @@ public Set getDescriptorsFromModule(@Nullable String moduleName) { @CheckForNull public Set getDescriptorsFromModuleV2(@Nullable String moduleName) { - return globalDescriptorsByModuleNameV2.get(moduleName); + return globalDescriptorsByModuleName.get(moduleName); } public Map> importsByModule() { @@ -228,6 +195,7 @@ private void addModuleV2(FileInput astRoot, String packageName, PythonFile pytho var typesBySymbol = typeInferenceV2.inferTypes(astRoot); var moduleDescriptors = typesBySymbol.entrySet() .stream() + .filter(entry -> entry.getValue().stream().noneMatch(UnknownType.UnresolvedImportType.class::isInstance)) .map(entry -> { var descriptor = pythonTypeToDescriptorConverter.convert(fullyQualifiedModuleName, entry.getKey(), entry.getValue()); return Map.entry(entry.getKey(), descriptor); @@ -237,7 +205,8 @@ private void addModuleV2(FileInput astRoot, String packageName, PythonFile pytho || entry.getKey().usages().stream().anyMatch(u -> u.kind().equals(UsageV2.Kind.IMPORT)))) .map(Map.Entry::getValue) .collect(Collectors.toSet()); - globalDescriptorsByModuleNameV2.put(fullyQualifiedModuleName, moduleDescriptors); + globalDescriptorsByModuleName.put(fullyQualifiedModuleName, moduleDescriptors); + addModuleToGlobalSymbolsByFQN(moduleDescriptors); } private class DjangoViewsVisitor extends BaseTreeVisitor { diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java b/python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java index d14bb8c2f4..0b54660412 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java @@ -19,6 +19,7 @@ */ package org.sonar.python.semantic.v2.converter; +import java.util.ArrayList; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -34,7 +35,6 @@ import org.sonar.python.types.v2.FunctionType; import org.sonar.python.types.v2.ParameterV2; import org.sonar.python.types.v2.PythonType; -import org.sonar.python.types.v2.TypeWrapper; import org.sonar.python.types.v2.UnionType; import org.sonar.python.types.v2.UnknownType; @@ -100,14 +100,25 @@ private Descriptor convert(String moduleFqn, String parentFqn, String symbolName .stream() .map(m -> convert(moduleFqn, symbolFqn, m.name(), m.type())) .collect(Collectors.toSet()); - List superClasses = type.superClasses().stream().map(TypeWrapper::type).map(t -> typeFqn(moduleFqn, t)).toList(); + + var hasSuperClassWithoutDescriptor = false; + var superClasses = new ArrayList(); + for (var superClassWrapper : type.superClasses()) { + var superClass = superClassWrapper.type(); + if (superClass != PythonType.UNKNOWN) { + var superClassFqn = typeFqn(moduleFqn, superClass); + superClasses.add(superClassFqn); + } else { + hasSuperClassWithoutDescriptor = true; + } + } return new ClassDescriptor(symbolName, symbolFqn, superClasses, memberDescriptors, type.hasDecorators(), type.definitionLocation().orElse(null), - false, + hasSuperClassWithoutDescriptor, type.hasMetaClass(), null, false diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java index 6bcb8c9933..ee87636166 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java @@ -20,7 +20,6 @@ package org.sonar.python.semantic; import com.google.common.base.Functions; -import com.google.protobuf.InvalidProtocolBufferException; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -29,9 +28,8 @@ import java.util.Map; import java.util.Set; import java.util.stream.Collectors; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.sonar.plugins.python.api.caching.PythonReadCache; -import org.sonar.plugins.python.api.caching.PythonWriteCache; import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; import org.sonar.plugins.python.api.symbols.ClassSymbol; import org.sonar.plugins.python.api.symbols.FunctionSymbol; @@ -43,7 +41,6 @@ import org.sonar.plugins.python.api.tree.FunctionDef; import org.sonar.plugins.python.api.tree.ImportFrom; import org.sonar.plugins.python.api.tree.QualifiedExpression; -import org.sonar.plugins.python.api.tree.Statement; import org.sonar.plugins.python.api.tree.Tree; import org.sonar.python.PythonTestUtils; import org.sonar.python.index.AmbiguousDescriptor; @@ -54,6 +51,7 @@ import org.sonar.python.tree.TreeUtils; import org.sonar.python.types.DeclaredType; import org.sonar.python.types.InferredTypes; +import org.sonar.python.types.TypeShed; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; @@ -614,7 +612,7 @@ void function_symbols() { "fn = 42" ); globalSymbols = globalSymbols(tree, "mod"); - assertThat(globalSymbols).extracting(Symbol::kind).containsExactly(Symbol.Kind.AMBIGUOUS); + assertThat(globalSymbols).extracting(Symbol::kind).containsExactly(Symbol.Kind.OTHER); } @Test @@ -625,10 +623,11 @@ void redefined_class_symbol() { " pass"); Set globalSymbols = globalSymbols(fileInput, "mod"); assertThat(globalSymbols).extracting(Symbol::name).containsExactlyInAnyOrder("C"); - assertThat(globalSymbols).extracting(Symbol::kind).allSatisfy(k -> assertThat(Symbol.Kind.CLASS.equals(k)).isFalse()); + assertThat(globalSymbols).extracting(Symbol::kind).allSatisfy(k -> assertThat(k).isEqualTo(Symbol.Kind.CLASS)); } @Test + @Disabled("SONARPY-2248") void classdef_with_missing_symbol() { FileInput fileInput = parseWithoutSymbols( "class C: ", @@ -636,9 +635,7 @@ void classdef_with_missing_symbol() { "global C"); Set globalSymbols = globalSymbols(fileInput, "mod"); - assertThat(globalSymbols).extracting(Symbol::name).containsExactlyInAnyOrder("C"); - // TODO: Global statements should not alter the kind of a symbol - assertThat(globalSymbols).extracting(Symbol::kind).allSatisfy(k -> assertThat(Symbol.Kind.OTHER.equals(k)).isTrue()); + assertThat(globalSymbols).isNotEmpty(); } @Test @@ -663,16 +660,20 @@ void class_symbol() { assertThat(cSymbol.name()).isEqualTo("C"); assertThat(cSymbol.kind()).isEqualTo(Symbol.Kind.CLASS); assertThat(((ClassSymbol) cSymbol).superClasses()).hasSize(1); + } + @Test + @Disabled("SONARPY-2250") + void class_symbol_inheritance_from_nested_class() { // for the time being, we only consider symbols defined in the global scope - fileInput = parseWithoutSymbols( + var fileInput = parseWithoutSymbols( "class A:", " class A1: pass", "class C(A.A1): ", " pass"); - globalSymbols = globalSymbols(fileInput, "mod"); - symbols = globalSymbols.stream().collect(Collectors.toMap(Symbol::name, Functions.identity())); - cSymbol = symbols.get("C"); + var globalSymbols = globalSymbols(fileInput, "mod"); + var symbols = globalSymbols.stream().collect(Collectors.toMap(Symbol::name, Functions.identity())); + var cSymbol = symbols.get("C"); assertThat(cSymbol.name()).isEqualTo("C"); assertThat(cSymbol.kind()).isEqualTo(Symbol.Kind.CLASS); assertThat(((ClassSymbol) cSymbol).superClasses()).hasSize(1); @@ -710,7 +711,7 @@ void symbol_duplicated_by_wildcard_import() { "def nlargest(n, iterable, key=None): ..." ); Set globalSymbols = globalSymbols(tree, ""); - assertThat(globalSymbols).hasOnlyElementsOfType(AmbiguousSymbolImpl.class); + assertThat(globalSymbols).hasOnlyElementsOfType(FunctionSymbol.class); tree = parseWithoutSymbols( "nonlocal nlargest", @@ -727,12 +728,10 @@ void class_having_itself_as_superclass_should_not_trigger_error() { Set globalSymbols = globalSymbols(fileInput, "mod"); ClassSymbol a = (ClassSymbol) globalSymbols.iterator().next(); // SONARPY-1350: The parent "A" is not yet defined at the time it is read, so this is actually not correct - assertThat(a.superClasses()).containsExactly(a); - ClassDef classDef = (ClassDef) fileInput.statements().statements().get(0); - assertThat(TreeUtils.getParentClassesFQN(classDef)).containsExactly("mod.mod.A"); + assertThat(a.superClasses()).isEmpty(); + assertThat(a.hasUnresolvedTypeHierarchy()).isTrue(); } - @Test void class_having_another_class_with_same_name_should_not_trigger_error() { FileInput fileInput = parseWithoutSymbols( @@ -871,10 +870,12 @@ void class_with_method_parameter_of_same_type() { } @Test + @Disabled("SONARPY-2249") void no_stackoverflow_for_ambiguous_descriptor() { + TypeShed.resetBuiltinSymbols(); String[] foo = { "if cond:", - " Ambiguous = ...", + " Ambiguous = 41", "else:", " class Ambiguous(SomeParent):", " local_var = 'i'", diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java b/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java index 75bb7a2531..b8eadd982e 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java @@ -2884,6 +2884,18 @@ class A: ... assertThat(typesBySymbol).isEmpty(); } + @Test + @Disabled("SONARPY-2248") + void typesBySymbol_global_statement() { + var typesBySymbol = inferTypesBySymbol(""" + class C: + pass + global C + """); + Assertions.assertThat(typesBySymbol).isNotEmpty(); + Assertions.assertThat(typesBySymbol.values().iterator().next()).isInstanceOf(ClassType.class); + } + private static Map> inferTypesBySymbol(String lines) { FileInput root = parse(lines); var symbolTable = new SymbolTableBuilderV2(root).build();