Skip to content

Commit dff933e

Browse files
andrea-guarino-sonarsourceandrea-guarino-sonarsource
authored
SONARPY-799 Avoid StackOverflow Error in presence of loop in class in… (SonarSource#873)
1 parent 4417864 commit dff933e

5 files changed

Lines changed: 105 additions & 34 deletions

File tree

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

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -123,43 +123,54 @@ void addFunctionSymbol(FunctionDef functionDef, @Nullable String fullyQualifiedN
123123
}
124124

125125
private Symbol copySymbol(String symbolName, Symbol symbol) {
126-
if (symbol.is(Symbol.Kind.FUNCTION)) {
127-
return new FunctionSymbolImpl(symbolName, (FunctionSymbol) symbol);
128-
} else if (symbol.is(Symbol.Kind.CLASS)) {
129-
ClassSymbolImpl originalClassSymbol = (ClassSymbolImpl) symbol;
130-
// Must use symbolName to preserve import aliases
131-
ClassSymbolImpl classSymbol =
132-
new ClassSymbolImpl(symbolName, originalClassSymbol.fullyQualifiedName(),
133-
originalClassSymbol.definitionLocation(), originalClassSymbol.hasDecorators(), originalClassSymbol.hasMetaClass(), originalClassSymbol.metaclassFQN());
134-
for (Symbol originalSymbol : originalClassSymbol.superClasses()) {
135-
Symbol globalSymbol = projectLevelSymbolTable.getSymbol(originalSymbol.fullyQualifiedName());
136-
if (globalSymbol != null && globalSymbol.kind() == Symbol.Kind.CLASS) {
137-
classSymbol.addSuperClass(copySymbol(globalSymbol.name(), globalSymbol));
138-
} else {
139-
classSymbol.addSuperClass(originalSymbol);
126+
return copySymbol(symbolName, symbol, new HashSet<>());
127+
}
128+
129+
private Symbol copySymbol(String symbolName, Symbol symbol, Set<Symbol> alreadyVisitedSymbols) {
130+
alreadyVisitedSymbols.add(symbol);
131+
switch (symbol.kind()) {
132+
case FUNCTION:
133+
return new FunctionSymbolImpl(symbolName, (FunctionSymbol) symbol);
134+
case CLASS:
135+
return copyClassSymbol(symbolName, (ClassSymbolImpl) symbol, alreadyVisitedSymbols);
136+
case AMBIGUOUS:
137+
Set<Symbol> alternativeSymbols = ((AmbiguousSymbol) symbol).alternatives().stream()
138+
.map(s -> copySymbol(symbolName, s, alreadyVisitedSymbols))
139+
.collect(Collectors.toSet());
140+
return new AmbiguousSymbolImpl(symbolName, symbol.fullyQualifiedName(), alternativeSymbols);
141+
default:
142+
SymbolImpl copiedSymbol = new SymbolImpl(symbolName, symbol.fullyQualifiedName(), symbol.annotatedTypeName());
143+
for (Map.Entry<String, Symbol> kv: ((SymbolImpl) symbol).getChildrenSymbolByName().entrySet()) {
144+
copiedSymbol.addChildSymbol(((SymbolImpl) kv.getValue()).copyWithoutUsages());
140145
}
146+
return copiedSymbol;
147+
}
148+
}
149+
150+
private ClassSymbolImpl copyClassSymbol(String symbolName, ClassSymbolImpl originalClassSymbol, Set<Symbol> alreadyVisitedSymbols) {
151+
// Must use symbolName to preserve import aliases
152+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(symbolName, originalClassSymbol.fullyQualifiedName(), originalClassSymbol.definitionLocation(),
153+
originalClassSymbol.hasDecorators(), originalClassSymbol.hasMetaClass(), originalClassSymbol.metaclassFQN());
154+
155+
for (Symbol originalSymbol : originalClassSymbol.superClasses()) {
156+
Symbol globalSymbol = projectLevelSymbolTable.getSymbol(originalSymbol.fullyQualifiedName());
157+
if (globalSymbol != null && globalSymbol.kind() == Symbol.Kind.CLASS) {
158+
Symbol parentClass = alreadyVisitedSymbols.contains(globalSymbol)
159+
? new SymbolImpl(globalSymbol.name(), globalSymbol.fullyQualifiedName())
160+
: copySymbol(globalSymbol.name(), globalSymbol, alreadyVisitedSymbols);
161+
classSymbol.addSuperClass(parentClass);
162+
} else {
163+
classSymbol.addSuperClass(originalSymbol);
141164
}
142-
classSymbol.addMembers(originalClassSymbol
143-
.declaredMembers().stream()
144-
.map(m -> ((SymbolImpl) m).copyWithoutUsages())
145-
.collect(Collectors.toList()));
146-
if (originalClassSymbol.hasSuperClassWithoutSymbol()) {
147-
classSymbol.setHasSuperClassWithoutSymbol();
148-
}
149-
return classSymbol;
150-
} else if (symbol.is(Symbol.Kind.AMBIGUOUS)) {
151-
Set<Symbol> alternativeSymbols = ((AmbiguousSymbol) symbol).alternatives().stream()
152-
.map(s -> copySymbol(symbolName, s))
153-
.collect(Collectors.toSet());
154-
return new AmbiguousSymbolImpl(symbolName, symbol.fullyQualifiedName(), alternativeSymbols);
155-
} else if (symbol.is(Symbol.Kind.OTHER)) {
156-
SymbolImpl res = new SymbolImpl(symbolName, symbol.fullyQualifiedName(), symbol.annotatedTypeName());
157-
for (Map.Entry<String, Symbol> kv: ((SymbolImpl) symbol).getChildrenSymbolByName().entrySet()) {
158-
res.addChildSymbol(((SymbolImpl) kv.getValue()).copyWithoutUsages());
159-
}
160-
return res;
161165
}
162-
return new SymbolImpl(symbolName, symbol.fullyQualifiedName());
166+
classSymbol.addMembers(originalClassSymbol
167+
.declaredMembers().stream()
168+
.map(m -> ((SymbolImpl) m).copyWithoutUsages())
169+
.collect(Collectors.toList()));
170+
if (originalClassSymbol.hasSuperClassWithoutSymbol()) {
171+
classSymbol.setHasSuperClassWithoutSymbol();
172+
}
173+
return classSymbol;
163174
}
164175

165176
void addModuleSymbol(Name nameTree, @CheckForNull String fullyQualifiedName) {

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.sonar.python.PythonTestUtils;
4242

4343
import static org.assertj.core.api.Assertions.assertThat;
44+
import static org.assertj.core.api.Assertions.tuple;
4445
import static org.sonar.python.PythonTestUtils.parse;
4546
import static org.sonar.python.PythonTestUtils.parseWithoutSymbols;
4647
import static org.sonar.python.PythonTestUtils.pythonFile;
@@ -613,4 +614,39 @@ public void imports_wont_trigger_typeshed_lookup() {
613614
ImportFrom importFrom = ((ImportFrom) PythonTestUtils.getAllDescendant(tree, t -> t.is(Tree.Kind.IMPORT_FROM)).get(0));
614615
assertThat(importFrom.hasUnresolvedWildcardImport()).isTrue();
615616
}
617+
618+
@Test
619+
public void loop_in_class_inheritance() {
620+
String[] foo = {
621+
"from bar import B",
622+
"class A(B): ..."
623+
};
624+
String[] bar = {
625+
"from foo import A",
626+
"class B(A): ..."
627+
};
628+
629+
ProjectLevelSymbolTable projectSymbolTable = new ProjectLevelSymbolTable();
630+
projectSymbolTable.addModule(parseWithoutSymbols(foo), "", pythonFile("foo.py"));
631+
projectSymbolTable.addModule(parseWithoutSymbols(bar), "", pythonFile("bar.py"));
632+
633+
Map<String, Symbol> barSymbols = getSymbolByName(parse(new SymbolTableBuilder("", pythonFile("bar.py"), projectSymbolTable), bar));
634+
635+
ClassSymbol classB = (ClassSymbol) barSymbols.get("B");
636+
assertThat(classB.superClasses())
637+
.extracting(Symbol::kind, Symbol::fullyQualifiedName)
638+
.containsExactly(tuple(Symbol.Kind.CLASS, "foo.A"));
639+
640+
ClassSymbolImpl importedA = (ClassSymbolImpl) barSymbols.get("A");
641+
assertThat(importedA.superClasses())
642+
.extracting(Symbol::kind, Symbol::fullyQualifiedName)
643+
.containsExactly(tuple(Symbol.Kind.CLASS, "bar.B"));
644+
645+
// at this level, parent of imported class is not Class Symbol anymore because of the loop in class inheritance
646+
ClassSymbolImpl parentOfImportedA = (ClassSymbolImpl) importedA.superClasses().iterator().next();
647+
assertThat(parentOfImportedA.superClasses())
648+
.extracting(Symbol::kind, Symbol::fullyQualifiedName)
649+
.containsExactly(tuple(Symbol.Kind.OTHER, "foo.A"));
650+
}
651+
616652
}

‎sonar-python-plugin/src/test/java/org/sonar/plugins/python/PythonSensorTest.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,21 @@ public void cross_files_secondary_locations() {
283283
assertThat(flow.locations().get(1).inputComponent()).isEqualTo(modFile);
284284
}
285285

286+
@Test
287+
public void loop_in_class_hierarchy() {
288+
activeRules = new ActiveRulesBuilder()
289+
.addRule(new NewActiveRule.Builder()
290+
.setRuleKey(RuleKey.of(CheckList.REPOSITORY_KEY, "S2710"))
291+
.build())
292+
.build();
293+
294+
inputFile("modA.py");
295+
inputFile("modB.py");
296+
sensor().execute(context);
297+
298+
assertThat(context.allIssues()).hasSize(1);
299+
}
300+
286301
@Test
287302
public void test_test_file_highlighting() throws IOException {
288303
activeRules = new ActiveRulesBuilder().build();

‎sonar-python-plugin/src/test/resources/org/sonar/plugins/python/sensor/modA.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
from modB import B
2+
3+
class A(B):
4+
@classmethod
5+
def self_first_param(self, a, b): ... # Noncompliant

‎sonar-python-plugin/src/test/resources/org/sonar/plugins/python/sensor/modB.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
from modA import A
2+
3+
class B(A):
4+
...

0 commit comments

Comments
 (0)