Skip to content

Commit 5f7804b

Browse files
guillaume-dequenneguillaume-dequenne
authored
Avoid copying symbols that come from the project table and fix bug for ambiguous descriptors with null FQN (SonarSource#1040)
1 parent efeeede commit 5f7804b

8 files changed

Lines changed: 97 additions & 46 deletions

File tree

‎python-frontend/src/main/java/org/sonar/python/index/DescriptorUtils.java‎

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.sonar.python.index;
2121

2222
import java.util.Collections;
23+
import java.util.HashSet;
2324
import java.util.List;
2425
import java.util.Map;
2526
import java.util.Objects;
@@ -116,43 +117,52 @@ private static List<FunctionDescriptor.Parameter> parameters(List<FunctionSymbol
116117
)).collect(Collectors.toList());
117118
}
118119

120+
// TODO SONARPY-958: Cleanup the symbol construction from descriptors by extracting this logic in a builder class
119121
public static Symbol symbolFromDescriptor(Descriptor descriptor, ProjectLevelSymbolTable projectLevelSymbolTable,
120-
@Nullable String localSymbolName, Map<String, Symbol> createdSymbols) {
121-
// The symbol generated from the descriptor will not have the descriptor name if an alias (localSymbolName) is defined
122-
if (createdSymbols.containsKey(descriptor.fullyQualifiedName())) {
123-
return createdSymbols.get(descriptor.fullyQualifiedName());
122+
@Nullable String localSymbolName, Map<Descriptor, Symbol> createdSymbolsByDescriptor, Map<String, Symbol> createdSymbolsByFqn) {
123+
if (createdSymbolsByDescriptor.containsKey(descriptor)) {
124+
return createdSymbolsByDescriptor.get(descriptor);
125+
} else if (descriptor.fullyQualifiedName() != null && createdSymbolsByFqn.containsKey(descriptor.fullyQualifiedName())) {
126+
return createdSymbolsByFqn.get(descriptor.fullyQualifiedName());
124127
}
128+
// The symbol generated from the descriptor will not have the descriptor name if an alias (localSymbolName) is defined
125129
String symbolName = localSymbolName != null ? localSymbolName : descriptor.name();
126130
switch (descriptor.kind()) {
127131
case CLASS:
128-
return createClassSymbol(descriptor, projectLevelSymbolTable, createdSymbols, symbolName);
132+
return createClassSymbol(descriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolsByFqn, symbolName);
129133
case FUNCTION:
130-
return createFunctionSymbol((FunctionDescriptor) descriptor, projectLevelSymbolTable, createdSymbols, symbolName);
134+
return createFunctionSymbol((FunctionDescriptor) descriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolsByFqn, symbolName);
131135
case VARIABLE:
132136
return new SymbolImpl(symbolName, descriptor.fullyQualifiedName());
133137
case AMBIGUOUS:
134-
Set<Symbol> alternatives = ((AmbiguousDescriptor) descriptor).alternatives().stream()
135-
.map(a -> DescriptorUtils.symbolFromDescriptor(a, projectLevelSymbolTable, symbolName, createdSymbols))
136-
.collect(Collectors.toSet());
137-
return new AmbiguousSymbolImpl(symbolName, descriptor.fullyQualifiedName(), alternatives);
138+
Set<Symbol> alternatives = new HashSet<>();
139+
AmbiguousSymbolImpl ambiguousSymbol = new AmbiguousSymbolImpl(symbolName, descriptor.fullyQualifiedName(), alternatives);
140+
createdSymbolsByDescriptor.put(descriptor, ambiguousSymbol);
141+
alternatives.addAll(((AmbiguousDescriptor) descriptor).alternatives().stream()
142+
.map(a -> DescriptorUtils.symbolFromDescriptor(a, projectLevelSymbolTable, symbolName, createdSymbolsByDescriptor, createdSymbolsByFqn))
143+
.collect(Collectors.toSet()));
144+
return ambiguousSymbol;
138145
default:
139146
throw new IllegalStateException(String.format("Error while creating a Symbol from a Descriptor: Unexpected descriptor kind: %s", descriptor.kind()));
140147
}
141148
}
142149

143-
private static ClassSymbolImpl createClassSymbol(Descriptor descriptor, ProjectLevelSymbolTable projectLevelSymbolTable, Map<String, Symbol> createdSymbols, String symbolName) {
150+
private static ClassSymbolImpl createClassSymbol(Descriptor descriptor, ProjectLevelSymbolTable projectLevelSymbolTable, Map<Descriptor, Symbol> createdSymbolsByDescriptor,
151+
Map<String, Symbol> createdSymbolByFqn, String symbolName) {
144152
ClassDescriptor classDescriptor = (ClassDescriptor) descriptor;
145153
ClassSymbolImpl classSymbol = new ClassSymbolImpl((ClassDescriptor) descriptor, symbolName);
146-
createdSymbols.put(descriptor.fullyQualifiedName(), classSymbol);
147-
addSuperClasses(classSymbol, classDescriptor, projectLevelSymbolTable, createdSymbols);
148-
addMembers(classSymbol, classDescriptor, projectLevelSymbolTable, createdSymbols);
154+
createdSymbolsByDescriptor.put(descriptor, classSymbol);
155+
createdSymbolByFqn.put(descriptor.fullyQualifiedName(), classSymbol);
156+
addSuperClasses(classSymbol, classDescriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolByFqn);
157+
addMembers(classSymbol, classDescriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolByFqn);
149158
return classSymbol;
150159
}
151160

152161
private static void addMembers(ClassSymbolImpl classSymbol, ClassDescriptor classDescriptor,
153-
ProjectLevelSymbolTable projectLevelSymbolTable, Map<String, Symbol> createdSymbols) {
162+
ProjectLevelSymbolTable projectLevelSymbolTable, Map<Descriptor, Symbol> createdSymbolsByDescriptor,
163+
Map<String , Symbol> createdSymbolsByFqn) {
154164
classSymbol.addMembers(classDescriptor.members().stream()
155-
.map(memberFqn -> DescriptorUtils.symbolFromDescriptor(memberFqn, projectLevelSymbolTable, null, createdSymbols))
165+
.map(memberFqn -> DescriptorUtils.symbolFromDescriptor(memberFqn, projectLevelSymbolTable, null, createdSymbolsByDescriptor, createdSymbolsByFqn))
156166
.map(member -> {
157167
if (member instanceof FunctionSymbolImpl) {
158168
((FunctionSymbolImpl) member).setOwner(classSymbol);
@@ -163,47 +173,49 @@ private static void addMembers(ClassSymbolImpl classSymbol, ClassDescriptor clas
163173
}
164174

165175
private static void addSuperClasses(ClassSymbolImpl classSymbol, ClassDescriptor classDescriptor,
166-
ProjectLevelSymbolTable projectLevelSymbolTable, Map<String, Symbol> createdSymbols) {
176+
ProjectLevelSymbolTable projectLevelSymbolTable, Map<Descriptor, Symbol> createdSymbolsByDescriptor,
177+
Map<String, Symbol> createdSymbolsByFqn) {
167178
classDescriptor.superClasses().stream()
168179
.map(superClassFqn -> {
169-
if (createdSymbols.containsKey(superClassFqn)) {
170-
return createdSymbols.get(superClassFqn);
180+
if (createdSymbolsByFqn.containsKey(superClassFqn)) {
181+
return createdSymbolsByFqn.get(superClassFqn);
171182
}
172-
Symbol symbol = projectLevelSymbolTable.getSymbol(superClassFqn, null, createdSymbols);
183+
Symbol symbol = projectLevelSymbolTable.getSymbol(superClassFqn, null, createdSymbolsByDescriptor, createdSymbolsByFqn);
173184
symbol = symbol != null ? symbol : typeshedSymbolWithFQN(superClassFqn);
174-
createdSymbols.put(superClassFqn, symbol);
185+
createdSymbolsByFqn.put(superClassFqn, symbol);
175186
return symbol;
176187
}
177188
)
178189
.forEach(classSymbol::addSuperClass);
179190
}
180191

181192
private static FunctionSymbolImpl createFunctionSymbol(FunctionDescriptor functionDescriptor, ProjectLevelSymbolTable projectLevelSymbolTable,
182-
Map<String, Symbol> createdSymbols, String symbolName) {
193+
Map<Descriptor, Symbol> createdSymbolsByDescriptor, Map<String, Symbol> createdSymbolsByFqn,
194+
String symbolName) {
183195
FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionDescriptor, symbolName);
184-
addParameters(functionSymbol, functionDescriptor, projectLevelSymbolTable, createdSymbols);
196+
addParameters(functionSymbol, functionDescriptor, projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolsByFqn);
185197
return functionSymbol;
186198
}
187199

188200
private static void addParameters(FunctionSymbolImpl functionSymbol, FunctionDescriptor functionDescriptor,
189-
ProjectLevelSymbolTable projectLevelSymbolTable, Map<String, Symbol> createdSymbols) {
201+
ProjectLevelSymbolTable projectLevelSymbolTable, Map<Descriptor, Symbol> createdSymbolsByDescriptor, Map<String, Symbol> createdSymbolsByFqn) {
190202
functionDescriptor.parameters().stream().map(parameterDescriptor -> {
191203
FunctionSymbolImpl.ParameterImpl parameter = new FunctionSymbolImpl.ParameterImpl(parameterDescriptor);
192-
setParameterType(parameter, parameterDescriptor.annotatedType(), projectLevelSymbolTable, createdSymbols);
204+
setParameterType(parameter, parameterDescriptor.annotatedType(), projectLevelSymbolTable, createdSymbolsByDescriptor, createdSymbolsByFqn);
193205
return parameter;
194206
}).forEach(functionSymbol::addParameter);
195207
}
196208

197-
private static void setParameterType(FunctionSymbolImpl.ParameterImpl parameter, String annotatedType,
198-
ProjectLevelSymbolTable projectLevelSymbolTable, Map<String, Symbol> createdSymbols) {
209+
private static void setParameterType(FunctionSymbolImpl.ParameterImpl parameter, String annotatedType, ProjectLevelSymbolTable projectLevelSymbolTable,
210+
Map<Descriptor, Symbol> createdSymbolsByDescriptor, Map<String, Symbol> createdSymbolsByFqn) {
199211
InferredType declaredType;
200212
if (parameter.isKeywordVariadic()) {
201213
declaredType = InferredTypes.DICT;
202214
} else if (parameter.isPositionalVariadic()) {
203215
declaredType = InferredTypes.TUPLE;
204216
} else {
205-
Symbol existingSymbol = createdSymbols.get(annotatedType);
206-
Symbol typeSymbol = existingSymbol != null ? existingSymbol : projectLevelSymbolTable.getSymbol(annotatedType, null, createdSymbols);
217+
Symbol existingSymbol = createdSymbolsByFqn.get(annotatedType);
218+
Symbol typeSymbol = existingSymbol != null ? existingSymbol : projectLevelSymbolTable.getSymbol(annotatedType, null, createdSymbolsByDescriptor, createdSymbolsByFqn);
207219
String annotatedTypeName = parameter.annotatedTypeName();
208220
if (typeSymbol == null && annotatedTypeName != null) {
209221
typeSymbol = typeshedSymbolWithFQN(annotatedTypeName);

‎python-frontend/src/main/java/org/sonar/python/semantic/ProjectLevelSymbolTable.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,24 +137,26 @@ public Symbol getSymbol(@Nullable String fullyQualifiedName) {
137137
}
138138

139139
public Symbol getSymbol(@Nullable String fullyQualifiedName, @Nullable String localSymbolName) {
140-
return getSymbol(fullyQualifiedName, localSymbolName, new HashMap<>());
140+
return getSymbol(fullyQualifiedName, localSymbolName, new HashMap<>(), new HashMap<>());
141141
}
142142

143-
public Symbol getSymbol(@Nullable String fullyQualifiedName, @Nullable String localSymbolName, Map<String, Symbol> createdSymbols) {
143+
public Symbol getSymbol(@Nullable String fullyQualifiedName, @Nullable String localSymbolName,
144+
Map<Descriptor, Symbol> createdSymbolsByDescriptor, Map<String, Symbol> createdSymbolsByFqn) {
144145
if (fullyQualifiedName == null) return null;
145146
Descriptor descriptor = globalDescriptorsByFQN().get(fullyQualifiedName);
146-
return descriptor == null ? null : DescriptorUtils.symbolFromDescriptor(descriptor, this, localSymbolName, createdSymbols);
147+
return descriptor == null ? null : DescriptorUtils.symbolFromDescriptor(descriptor, this, localSymbolName, createdSymbolsByDescriptor, createdSymbolsByFqn);
147148
}
148149

149150
@CheckForNull
150151
public Set<Symbol> getSymbolsFromModule(@Nullable String moduleName) {
151152
Set<Descriptor> descriptors = globalDescriptorsByModuleName.get(moduleName);
152-
Map<String, Symbol> createdSymbols = new HashMap<>();
153153
if (descriptors == null) {
154154
return null;
155155
}
156+
Map<Descriptor, Symbol> createdSymbolsByDescriptor = new HashMap<>();
157+
Map<String, Symbol> createdSymbolsByFqn = new HashMap<>();
156158
return descriptors.stream()
157-
.map(desc -> DescriptorUtils.symbolFromDescriptor(desc, this, null, createdSymbols)).collect(Collectors.toSet());
159+
.map(desc -> DescriptorUtils.symbolFromDescriptor(desc, this, null, createdSymbolsByDescriptor, createdSymbolsByFqn)).collect(Collectors.toSet());
158160
}
159161

160162
public boolean isDjangoView(@Nullable String fqn) {

‎python-frontend/src/main/java/org/sonar/python/semantic/Scope.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,14 @@ void createBuiltinSymbol(String name, Map<String, Symbol> typeShedSymbols) {
8282

8383
void createSymbolsFromWildcardImport(Set<Symbol> importedSymbols, ImportFrom importFrom) {
8484
importedSymbols.forEach(symbol -> {
85-
Symbol importedSymbol = copySymbol(symbol.name(), symbol);
86-
if (!isExistingSymbol(importedSymbol.name())) {
87-
symbols.add(importedSymbol);
88-
symbolsByName.put(symbol.name(), importedSymbol);
89-
((SymbolImpl) importedSymbol).addUsage(importFrom, Usage.Kind.IMPORT);
85+
if (!isExistingSymbol(symbol.name())) {
86+
symbols.add(symbol);
87+
symbolsByName.put(symbol.name(), symbol);
88+
((SymbolImpl) symbol).addUsage(importFrom, Usage.Kind.IMPORT);
9089
} else {
9190
SymbolImpl originalSymbol = resolve(symbol.name());
9291
if (originalSymbol != null) {
93-
resetSymbolInfo(importedSymbol.fullyQualifiedName(), originalSymbol);
92+
resetSymbolInfo(symbol.fullyQualifiedName(), originalSymbol);
9493
originalSymbol.addUsage(importFrom, Usage.Kind.IMPORT);
9594
}
9695
}
@@ -123,7 +122,7 @@ void addFunctionSymbol(FunctionDef functionDef, @Nullable String fullyQualifiedN
123122
}
124123
}
125124

126-
private Symbol copySymbol(String symbolName, Symbol symbol) {
125+
public Symbol copySymbol(String symbolName, Symbol symbol) {
127126
return copySymbol(symbolName, symbol, new HashSet<>());
128127
}
129128

‎python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,8 @@ public void visitImportFrom(ImportFrom importFrom) {
365365
if (importFrom.isWildcardImport()) {
366366
Set<Symbol> importedModuleSymbols = projectLevelSymbolTable.getSymbolsFromModule(moduleName);
367367
if (importedModuleSymbols == null && moduleName != null && !moduleName.equals(fullyQualifiedModuleName)) {
368-
importedModuleSymbols = TypeShed.symbolsForModule(moduleName);
368+
importedModuleSymbols = TypeShed.symbolsForModule(moduleName).stream()
369+
.map(importedSymbol -> currentScope().copySymbol(importedSymbol.name(), importedSymbol)).collect(Collectors.toSet());
369370
}
370371
if (importedModuleSymbols != null && !importedModuleSymbols.isEmpty()) {
371372
currentScope().createSymbolsFromWildcardImport(importedModuleSymbols, importFrom);

‎python-frontend/src/main/java/org/sonar/python/semantic/SymbolUtils.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,6 @@ public static Symbol typeshedSymbolWithFQN(String fullyQualifiedName) {
286286
String[] fqnSplitByDot = fullyQualifiedName.split("\\.");
287287
String localName = fqnSplitByDot[fqnSplitByDot.length - 1];
288288
Symbol symbol = TypeShed.symbolWithFQN(fullyQualifiedName);
289-
return symbol == null ? new SymbolImpl(localName, fullyQualifiedName) : symbol;
289+
return symbol == null ? new SymbolImpl(localName, fullyQualifiedName) : ((SymbolImpl) symbol).copyWithoutUsages();
290290
}
291291
}

‎python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolImplTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public void from_protobuf() throws TextFormat.ParseException {
241241
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
242242
assertThat(classSymbol.name()).isEqualTo("A");
243243
assertThat(classSymbol.fullyQualifiedName()).isEqualTo("mod.A");
244-
assertThat(classSymbol.superClasses()).containsExactly(TypeShed.typeShedClass("object"));
244+
assertThat(classSymbol.superClasses()).extracting(Symbol::fullyQualifiedName).containsExactly("object");
245245
assertThat(classSymbol.hasDecorators()).isTrue();
246246
assertThat(classSymbol.hasMetaClass()).isTrue();
247247
assertThat(classSymbol.metaclassFQN()).isEqualTo("abc.ABCMeta");

‎python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.Set;
3030
import java.util.stream.Collectors;
3131
import org.junit.Test;
32+
import org.sonar.plugins.python.api.symbols.AmbiguousSymbol;
3233
import org.sonar.plugins.python.api.symbols.ClassSymbol;
3334
import org.sonar.plugins.python.api.symbols.FunctionSymbol;
3435
import org.sonar.plugins.python.api.symbols.Symbol;
@@ -740,6 +741,42 @@ public void class_with_method_parameter_of_same_type() {
740741
assertThat(declaredType.getTypeClass()).isSameAs(classSymbol);
741742
}
742743

744+
@Test
745+
public void no_stackoverflow_for_ambiguous_descriptor() {
746+
String[] foo = {
747+
"if cond:",
748+
" Ambiguous = ...",
749+
"else:",
750+
" class Ambiguous(SomeParent):",
751+
" local_var = 'i'",
752+
" def func(param: Ambiguous):",
753+
" ..."
754+
};
755+
String[] bar = {
756+
"from foo import *\n",
757+
};
758+
ProjectLevelSymbolTable projectSymbolTable = new ProjectLevelSymbolTable();
759+
projectSymbolTable.addModule(parseWithoutSymbols(foo), "", pythonFile("foo.py"));
760+
projectSymbolTable.addModule(parseWithoutSymbols(bar), "", pythonFile("bar.py"));
761+
762+
Set<Symbol> fooSymbols = projectSymbolTable.getSymbolsFromModule("foo");
763+
assertThat(fooSymbols).hasSize(1);
764+
AmbiguousSymbol symbolFromProjectTable = ((AmbiguousSymbol) fooSymbols.stream().findFirst().get());
765+
assertThat(symbolFromProjectTable.fullyQualifiedName()).isEqualTo("foo.Ambiguous");
766+
assertThat(symbolFromProjectTable.alternatives()).hasSize(2);
767+
assertThat(symbolFromProjectTable.alternatives()).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.CLASS, Symbol.Kind.OTHER);
768+
ClassSymbol classSymbol = (ClassSymbol) symbolFromProjectTable.alternatives().stream().filter(s -> s.kind().equals(Symbol.Kind.CLASS)).findFirst().get();
769+
assertThat(classSymbol.declaredMembers()).hasSize(2);
770+
assertThat(classSymbol.declaredMembers()).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.FUNCTION, Symbol.Kind.OTHER);
771+
772+
FileInput tree = parse(new SymbolTableBuilder("", pythonFile("bar.py"), projectSymbolTable), bar);
773+
assertThat(tree.globalVariables()).hasSize(1);
774+
AmbiguousSymbol localSymbol = (AmbiguousSymbol) tree.globalVariables().stream().findFirst().get();
775+
assertThat(localSymbol.fullyQualifiedName()).isEqualTo("foo.Ambiguous");
776+
assertThat(localSymbol.alternatives()).hasSize(2);
777+
assertThat(localSymbol.alternatives()).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.CLASS, Symbol.Kind.OTHER);
778+
}
779+
743780
@Test
744781
public void loop_in_inheritance_with_method_paraneters_of_same_type() {
745782
String[] foo = {

‎python-frontend/src/test/java/org/sonar/python/types/TypeShedTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public void classes() {
6666
assertThat(intClass.hasUnresolvedTypeHierarchy()).isFalse();
6767
assertThat(intClass.usages()).isEmpty();
6868
assertThat(intClass.declaredMembers()).allMatch(member -> member.usages().isEmpty());
69-
assertThat(TypeShed.typeShedClass("bool").superClasses()).containsExactly(intClass);
69+
assertThat(TypeShed.typeShedClass("bool").superClasses()).extracting(Symbol::fullyQualifiedName).containsExactly("int");
7070
}
7171

7272
@Test
@@ -342,7 +342,7 @@ public void class_symbols_from_protobuf() throws TextFormat.ParseException {
342342
.containsExactlyInAnyOrder(tuple(Kind.CLASS, "mod.Base"), tuple(Kind.CLASS, "mod.C"), tuple(Kind.CLASS, "mod.D"));
343343

344344
ClassSymbol C = (ClassSymbol) symbols.get("mod.C");
345-
assertThat(C.superClasses()).containsExactly(TypeShed.typeShedClass("str"));
345+
assertThat(C.superClasses()).extracting(Symbol::fullyQualifiedName).containsExactly("str");
346346
ClassSymbol D = (ClassSymbol) symbols.get("mod.D");
347347
assertThat(D.superClasses()).extracting(Symbol::kind, Symbol::fullyQualifiedName)
348348
.containsExactly(tuple(Kind.OTHER, "NOT_EXISTENT"));

0 commit comments

Comments
 (0)