Skip to content

Commit

Permalink
SONARPY-2243 Make the V1 symbols populating out of descriptors collec…
Browse files Browse the repository at this point in the history 
…ted by the V2 type inference
  • Loading branch information
@maksim-grebeniuk-sonarsource
maksim-grebeniuk-sonarsource committed Oct 25, 2024
1 parent c1511fb commit 3ec482b
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 60 deletions.
2 changes: 1 addition & 1 deletion ...c/test/resources/checks/incorrectExceptionType/incorrectExceptionTypeWithRegularImport.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
43 changes: 6 additions & 37 deletions python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,21 @@
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;
import org.sonar.plugins.python.api.tree.RegularArgument;
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;
Expand All @@ -56,9 +54,7 @@ public class ProjectLevelSymbolTable {

private final PythonTypeToDescriptorConverter pythonTypeToDescriptorConverter = new PythonTypeToDescriptorConverter();
private final Map<String, Set<Descriptor>> globalDescriptorsByModuleName;
private final Map<String, Set<Descriptor>> globalDescriptorsByModuleNameV2;
private Map<String, Descriptor> globalDescriptorsByFQN;
private Map<String, Descriptor> globalDescriptorsByFQNV2;
private final Set<String> djangoViewsFQN = new HashSet<>();
private final Map<String, Set<String>> importsByModule = new HashMap<>();
private final Set<String> projectBasePackages = new HashSet<>();
Expand All @@ -74,19 +70,15 @@ public static ProjectLevelSymbolTable from(Map<String, Set<Symbol>> globalSymbol

public ProjectLevelSymbolTable() {
this.globalDescriptorsByModuleName = new HashMap<>();
this.globalDescriptorsByModuleNameV2 = new HashMap<>();
}

private ProjectLevelSymbolTable(Map<String, Set<Symbol>> globalSymbolsByModuleName) {
this.globalDescriptorsByModuleName = new HashMap<>();
this.globalDescriptorsByModuleNameV2 = new HashMap<>();
globalSymbolsByModuleName.entrySet().forEach(entry -> {
String moduleName = entry.getKey();
Set<Symbol> symbols = entry.getValue();
Set<Descriptor> 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);
});
}

Expand All @@ -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<Descriptor> 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);
Expand All @@ -136,7 +103,7 @@ private void addModuleToGlobalSymbolsByFQN(Set<Descriptor> descriptors) {
Map<String, Descriptor> 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<String, Descriptor> globalDescriptorsByFQN() {
Expand Down Expand Up @@ -185,7 +152,7 @@ public Set<Descriptor> getDescriptorsFromModule(@Nullable String moduleName) {

@CheckForNull
public Set<Descriptor> getDescriptorsFromModuleV2(@Nullable String moduleName) {
return globalDescriptorsByModuleNameV2.get(moduleName);
return globalDescriptorsByModuleName.get(moduleName);
}

public Map<String, Set<String>> importsByModule() {
Expand Down Expand Up @@ -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);
Expand All @@ -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 {
Expand Down
17 changes: 14 additions & 3 deletions ...src/main/java/org/sonar/python/semantic/v2/converter/PythonTypeToDescriptorConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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<String> superClasses = type.superClasses().stream().map(TypeWrapper::type).map(t -> typeFqn(moduleFqn, t)).toList();

var hasSuperClassWithoutDescriptor = false;
var superClasses = new ArrayList<String>();
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
Expand Down
39 changes: 20 additions & 19 deletions python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -625,20 +623,19 @@ void redefined_class_symbol() {
" pass");
Set<Symbol> 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: ",
" pass",
"global C");

Set<Symbol> 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
Expand All @@ -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);
Expand Down Expand Up @@ -710,7 +711,7 @@ void symbol_duplicated_by_wildcard_import() {
"def nlargest(n, iterable, key=None): ..."
);
Set<Symbol> globalSymbols = globalSymbols(tree, "");
assertThat(globalSymbols).hasOnlyElementsOfType(AmbiguousSymbolImpl.class);
assertThat(globalSymbols).hasOnlyElementsOfType(FunctionSymbol.class);

tree = parseWithoutSymbols(
"nonlocal nlargest",
Expand All @@ -727,12 +728,10 @@ void class_having_itself_as_superclass_should_not_trigger_error() {
Set<Symbol> 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(
Expand Down Expand Up @@ -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'",
Expand Down
12 changes: 12 additions & 0 deletions python-frontend/src/test/java/org/sonar/python/semantic/v2/TypeInferenceV2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<SymbolV2, Set<PythonType>> inferTypesBySymbol(String lines) {
FileInput root = parse(lines);
var symbolTable = new SymbolTableBuilderV2(root).build();
Expand Down

0 comments on commit 3ec482b

Please sign in to comment.