Skip to content

Commit d7183cf

Browse files
andreaguarinoguillaume-dequenne
andreaguarino
authored and
guillaume-dequenne
committed
Protobuf Typeshed module symbols fqn should always be moduleName + localName
1 parent 0c6def9 commit d7183cf

File tree

9 files changed

+47
-55
lines changed

9 files changed

+47
-55
lines changed

‎python-checks/src/main/java/org/sonar/python/checks/WildcardImportCheck.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public void visitAssignmentStatement(AssignmentStatement pyAssignmentStatementTr
8484
@Override
8585
public void visitCallExpression(CallExpression pyCallExpressionTree) {
8686
Symbol symbol = pyCallExpressionTree.calleeSymbol();
87-
this.raiseIssuesIf(() -> symbol == null || !"_warnings.warn".equals(symbol.fullyQualifiedName()));
87+
this.raiseIssuesIf(() -> symbol == null || !"warnings.warn".equals(symbol.fullyQualifiedName()));
8888
}
8989

9090
private void raiseIssuesIf(BooleanSupplier condition) {

‎python-checks/src/test/resources/checks/tooManyParametersCustom.py

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,22 @@
1-
from io import TextIOWrapper
2-
31
def func(p1, p2, p3, p4): ... # Noncompliant
42
# ^^^^^^^^^^^^^^
53

6-
class MyTextIOWrapper(TextIOWrapper):
7-
def __init__(
8-
self,
9-
buffer: IO[bytes],
10-
encoding: Optional[str] = ...,
11-
errors: Optional[str] = ...,
12-
newline: Optional[str] = ...,
13-
line_buffering: bool = ...,
14-
write_through: bool = ...,
15-
) -> None: ... # OK (parent is already non compliant)
4+
class ParentClass:
5+
def __init__(self, p1, p2, p3, p4): ... # Noncompliant
6+
7+
class Wrapper(ParentClass):
8+
def __init__(self, p1, p2, p3, p4, p5): ... # OK (parent is already non compliant)
169

1710
def readline(self, __size: int = ..., p2, p3, p4) -> str: ... # Noncompliant
1811

1912
def new_method_ok(self, p1, p2, *, p3) -> str: ...
2013

2114
def new_method_nok(self, p1, p2, p3, p4) -> str: ... # Noncompliant
2215

23-
class MyOtherTextIOWrapper(TextIOWrapper): ...
24-
25-
class ChildWithComplexHierarchy(MyOtherTextIOWrapper):
26-
def __init__(
27-
self,
28-
buffer: IO[bytes],
29-
encoding: Optional[str] = ...,
30-
errors: Optional[str] = ...,
31-
newline: Optional[str] = ...,
32-
line_buffering: bool = ...,
33-
write_through: bool = ...,
34-
) -> None: ...
16+
class MyOtherWrapper(Wrapper): ...
17+
18+
class ChildWithComplexHierarchy(MyOtherWrapper):
19+
def __init__(self, p1, p2, p3, p4, p5, p6): ...
3520

3621
class SuperBase:
3722
def method_nok(self, p1, p2, p3, p4): ... # Noncompliant

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@
4848
import static org.sonar.python.semantic.SymbolUtils.pathOf;
4949
import static org.sonar.python.tree.TreeUtils.locationInFile;
5050
import static org.sonar.python.types.TypeShed.isValidForProjectPythonVersion;
51-
import static org.sonar.python.types.TypeShed.normalizedFqn;
5251
import static org.sonar.python.types.TypeShed.symbolsFromProtobufDescriptors;
5352

5453
public class ClassSymbolImpl extends SymbolImpl implements ClassSymbol {
@@ -128,8 +127,8 @@ private ClassSymbolImpl(String name, ClassSymbol classSymbol) {
128127
setKind(Kind.CLASS);
129128
}
130129

131-
public ClassSymbolImpl(SymbolsProtos.ClassSymbol classSymbolProto) {
132-
super(classSymbolProto.getName(), normalizedFqn(classSymbolProto.getFullyQualifiedName()));
130+
public ClassSymbolImpl(SymbolsProtos.ClassSymbol classSymbolProto, String moduleName) {
131+
super(classSymbolProto.getName(), TypeShed.normalizedFqn(classSymbolProto.getFullyQualifiedName(), moduleName, classSymbolProto.getName()));
133132
setKind(Kind.CLASS);
134133
classDefinitionLocation = null;
135134
hasDecorators = classSymbolProto.getHasDecorators();
@@ -145,7 +144,7 @@ public ClassSymbolImpl(SymbolsProtos.ClassSymbol classSymbolProto) {
145144
.filter(d -> isValidForProjectPythonVersion(d.getValidForList()))
146145
.forEach(proto -> descriptorsByFqn.computeIfAbsent(proto.getFullname(), d -> new HashSet<>()).add(proto));
147146
for (Map.Entry<String, Set<Object>> entry : descriptorsByFqn.entrySet()) {
148-
Set<Symbol> symbols = symbolsFromProtobufDescriptors(entry.getValue(), true);
147+
Set<Symbol> symbols = symbolsFromProtobufDescriptors(entry.getValue(), true, moduleName);
149148
methods.add(symbols.size() > 1 ? AmbiguousSymbolImpl.create(symbols) : symbols.iterator().next());
150149
}
151150
addMembers(methods);

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,16 +87,16 @@ public class FunctionSymbolImpl extends SymbolImpl implements FunctionSymbol {
8787
functionDefinitionLocation = locationInFile(functionDef.name(), fileId);
8888
}
8989

90-
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto) {
91-
this(functionSymbolProto, false, functionSymbolProto.getValidForList());
90+
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto, String moduleName) {
91+
this(functionSymbolProto, false, functionSymbolProto.getValidForList(), moduleName);
9292
}
9393

94-
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto, boolean insideClass) {
95-
this(functionSymbolProto, insideClass, functionSymbolProto.getValidForList());
94+
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto, boolean insideClass, String moduleName) {
95+
this(functionSymbolProto, insideClass, functionSymbolProto.getValidForList(), moduleName);
9696
}
9797

98-
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto, boolean insideClass, List<String> validFor) {
99-
super(functionSymbolProto.getName(), TypeShed.normalizedFqn(functionSymbolProto.getFullyQualifiedName()));
98+
public FunctionSymbolImpl(SymbolsProtos.FunctionSymbol functionSymbolProto, boolean insideClass, List<String> validFor, String moduleName) {
99+
super(functionSymbolProto.getName(), TypeShed.normalizedFqn(functionSymbolProto.getFullyQualifiedName(), moduleName, functionSymbolProto.getName()));
100100
setKind(Kind.FUNCTION);
101101
isInstanceMethod = insideClass && !functionSymbolProto.getIsStatic() && !functionSymbolProto.getIsClassMethod();
102102
isAsynchronous = functionSymbolProto.getIsAsynchronous();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ public SymbolImpl(String name, @Nullable String fullyQualifiedName, @Nullable St
6868
this.kind = Kind.OTHER;
6969
}
7070

71-
public SymbolImpl(SymbolsProtos.VarSymbol varSymbol) {
71+
public SymbolImpl(SymbolsProtos.VarSymbol varSymbol, String moduleName) {
7272
this.name = varSymbol.getName();
73-
this.fullyQualifiedName = TypeShed.normalizedFqn(varSymbol.getFullyQualifiedName());
73+
this.fullyQualifiedName = TypeShed.normalizedFqn(varSymbol.getFullyQualifiedName(), moduleName, name);
7474
String fqn = varSymbol.getTypeAnnotation().getFullyQualifiedName();
7575
if (!fqn.isEmpty()) {
7676
this.annotatedTypeName = TypeShed.normalizedFqn(fqn);

‎python-frontend/src/main/java/org/sonar/python/types/TypeShed.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,11 @@ public static String normalizedFqn(String fqn) {
178178
return fqn;
179179
}
180180

181+
public static String normalizedFqn(String fqn, String moduleName, String localName) {
182+
if (fqn.startsWith(moduleName)) return normalizedFqn(fqn);
183+
return moduleName + "." + localName;
184+
}
185+
181186
public static boolean isValidForProjectPythonVersion(List<String> validForPythonVersions) {
182187
if (validForPythonVersions.isEmpty()) {
183188
return true;
@@ -187,24 +192,24 @@ public static boolean isValidForProjectPythonVersion(List<String> validForPython
187192
return !intersection.isEmpty();
188193
}
189194

190-
public static Set<Symbol> symbolsFromProtobufDescriptors(Set<Object> protobufDescriptors, boolean isInsideClass) {
195+
public static Set<Symbol> symbolsFromProtobufDescriptors(Set<Object> protobufDescriptors, boolean isInsideClass, String moduleName) {
191196
Set<Symbol> symbols = new HashSet<>();
192197
for (Object descriptor : protobufDescriptors) {
193198
if (descriptor instanceof SymbolsProtos.ClassSymbol) {
194-
symbols.add(new ClassSymbolImpl(((SymbolsProtos.ClassSymbol) descriptor)));
199+
symbols.add(new ClassSymbolImpl(((SymbolsProtos.ClassSymbol) descriptor), moduleName));
195200
}
196201
if (descriptor instanceof SymbolsProtos.FunctionSymbol) {
197-
symbols.add(new FunctionSymbolImpl(((SymbolsProtos.FunctionSymbol) descriptor), isInsideClass));
202+
symbols.add(new FunctionSymbolImpl(((SymbolsProtos.FunctionSymbol) descriptor), isInsideClass, moduleName));
198203
}
199204
if (descriptor instanceof OverloadedFunctionSymbol) {
200205
if (((OverloadedFunctionSymbol) descriptor).getDefinitionsList().size() < 2) {
201206
throw new IllegalStateException("Overloaded function symbols should have at least two definitions.");
202207
}
203-
symbols.add(fromOverloadedFunction(((OverloadedFunctionSymbol) descriptor), isInsideClass));
208+
symbols.add(fromOverloadedFunction(((OverloadedFunctionSymbol) descriptor), isInsideClass, moduleName));
204209
}
205210
if (descriptor instanceof SymbolsProtos.VarSymbol) {
206211
SymbolsProtos.VarSymbol varSymbol = (SymbolsProtos.VarSymbol) descriptor;
207-
SymbolImpl symbol = new SymbolImpl(varSymbol);
212+
SymbolImpl symbol = new SymbolImpl(varSymbol, moduleName);
208213
if (varSymbol.getIsImportedModule()) {
209214
Map<String, Symbol> moduleExportedSymbols = symbolsForModule(varSymbol.getFullyQualifiedName());
210215
moduleExportedSymbols.values().forEach(symbol::addChildSymbol);
@@ -317,7 +322,7 @@ static Map<String, Symbol> getSymbolsFromProtobufModule(@Nullable ModuleSymbol m
317322

318323
for (Map.Entry<String, Set<Object>> entry : descriptorsByName.entrySet()) {
319324
String name = entry.getKey();
320-
Set<Symbol> symbols = symbolsFromProtobufDescriptors(entry.getValue(), false);
325+
Set<Symbol> symbols = symbolsFromProtobufDescriptors(entry.getValue(), false, moduleSymbol.getFullyQualifiedName());
321326
Symbol disambiguatedSymbol = disambiguateSymbolsWithSameName(name, symbols, moduleSymbol.getFullyQualifiedName());
322327
deserializedSymbols.put(name, disambiguatedSymbol);
323328
}
@@ -351,9 +356,9 @@ private static boolean haveAllTheSameFqn(Set<Symbol> symbols) {
351356
return firstFqn != null && symbols.stream().map(Symbol::fullyQualifiedName).allMatch(firstFqn::equals);
352357
}
353358

354-
private static AmbiguousSymbol fromOverloadedFunction(OverloadedFunctionSymbol overloadedFunctionSymbol, boolean isInsideClass) {
359+
private static AmbiguousSymbol fromOverloadedFunction(OverloadedFunctionSymbol overloadedFunctionSymbol, boolean isInsideClass, String moduleName) {
355360
Set<Symbol> overloadedSymbols = overloadedFunctionSymbol.getDefinitionsList().stream()
356-
.map(def -> new FunctionSymbolImpl(def, isInsideClass, overloadedFunctionSymbol.getValidForList()))
361+
.map(def -> new FunctionSymbolImpl(def, isInsideClass, overloadedFunctionSymbol.getValidForList(), moduleName))
357362
.collect(Collectors.toSet());
358363
return AmbiguousSymbolImpl.create(overloadedSymbols);
359364
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ public void from_protobuf() throws TextFormat.ParseException {
238238
"has_decorators: true\n" +
239239
"has_metaclass: true\n" +
240240
"metaclass_name: \"abc.ABCMeta\"";
241-
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
241+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf), "mod");
242242
assertThat(classSymbol.name()).isEqualTo("A");
243243
assertThat(classSymbol.fullyQualifiedName()).isEqualTo("mod.A");
244244
assertThat(classSymbol.superClasses()).extracting(Symbol::fullyQualifiedName).containsExactly("object");
@@ -262,7 +262,7 @@ public void from_protobuf_instance_method() throws TextFormat.ParseException {
262262
" }\n" +
263263
" has_decorators: true\n" +
264264
"}";
265-
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
265+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf), "mod");
266266
FunctionSymbol foo = (FunctionSymbol) classSymbol.declaredMembers().iterator().next();
267267
assertThat(foo.isInstanceMethod()).isTrue();
268268
}
@@ -283,7 +283,7 @@ public void from_protobuf_class_method() throws TextFormat.ParseException {
283283
" has_decorators: true\n" +
284284
" is_class_method: true\n" +
285285
"}";
286-
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
286+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf), "mod");
287287
FunctionSymbol foo = (FunctionSymbol) classSymbol.declaredMembers().iterator().next();
288288
assertThat(foo.isInstanceMethod()).isFalse();
289289
}
@@ -304,7 +304,7 @@ public void from_protobuf_static_method() throws TextFormat.ParseException {
304304
" has_decorators: true\n" +
305305
" is_static: true\n" +
306306
"}";
307-
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
307+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf), "mod");
308308
FunctionSymbol foo = (FunctionSymbol) classSymbol.declaredMembers().iterator().next();
309309
assertThat(foo.isInstanceMethod()).isFalse();
310310
}
@@ -334,7 +334,7 @@ public void overloaded_methods() throws TextFormat.ParseException {
334334
" fully_qualified_name: \"mod.A.foo\"\n" +
335335
" }\n" +
336336
"}\n";
337-
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf));
337+
ClassSymbolImpl classSymbol = new ClassSymbolImpl(classSymbol(protobuf), "mod");
338338
Symbol foo = classSymbol.resolveMember("foo").get();
339339
assertThat(foo.is(Symbol.Kind.AMBIGUOUS)).isTrue();
340340
assertThat(((SymbolImpl) foo).validForPythonVersions()).containsExactlyInAnyOrder("27", "39");

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ public void from_protobuf() throws TextFormat.ParseException {
285285
" name: \"p4\"\n" +
286286
" kind: VAR_KEYWORD\n" +
287287
"}";
288-
FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionSymbol(protobuf));
288+
FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionSymbol(protobuf), "mod");
289289
assertThat(functionSymbol.name()).isEqualTo("fn");
290290
assertThat(functionSymbol.fullyQualifiedName()).isEqualTo("mod.fn");
291291
assertThat(functionSymbol.declaredReturnType()).isEqualTo(InferredTypes.NONE);
@@ -334,7 +334,7 @@ public void from_protobuf_no_return_type() throws TextFormat.ParseException {
334334
String protobuf =
335335
"name: \"fn\"\n" +
336336
"fully_qualified_name: \"mod.fn\"\n";
337-
FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionSymbol(protobuf));
337+
FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionSymbol(protobuf), "mod");
338338
assertThat(functionSymbol.declaredReturnType()).isEqualTo(InferredTypes.anyType());
339339
assertThat(functionSymbol.annotatedReturnTypeName()).isNull();
340340
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public void package_relative_import() {
202202

203203
Symbol timesResult = osSymbols.get("times_result");
204204
assertThat(timesResult.kind()).isEqualTo(Kind.CLASS);
205-
assertThat(timesResult.fullyQualifiedName()).isEqualTo("posix.times_result");
205+
assertThat(timesResult.fullyQualifiedName()).isEqualTo("os.times_result");
206206

207207
Map<String, Symbol> requestsSymbols = symbolsForModule("requests");
208208
Symbol requestSymbol = requestsSymbols.get("request");
@@ -233,8 +233,10 @@ public void two_exported_symbols_with_same_local_names() {
233233
Map<String, Symbol> posixSymbols = symbolsForModule("posix");
234234
Symbol setupSymbolFromPosix = posixSymbols.get("stat_result");
235235
Symbol setupSymbolFromOs = osSymbols.get("stat_result");
236-
assertThat(setupSymbolFromPosix.kind()).isEqualTo(Kind.CLASS);
237-
assertThat(setupSymbolFromOs.kind()).isEqualTo(Kind.CLASS);
236+
assertThat(setupSymbolFromPosix.kind()).isEqualTo(Kind.AMBIGUOUS);
237+
assertThat(setupSymbolFromPosix.fullyQualifiedName()).isEqualTo("posix.stat_result");
238+
assertThat(setupSymbolFromOs.kind()).isEqualTo(Kind.AMBIGUOUS);
239+
assertThat(setupSymbolFromOs.fullyQualifiedName()).isEqualTo("os.stat_result");
238240
}
239241

240242
@Test
@@ -512,6 +514,7 @@ public void symbol_from_submodule_access() {
512514
SymbolImpl path = (SymbolImpl) os.get("path");
513515
Symbol samefile = path.getChildrenSymbolByName().get("samefile");
514516
assertThat(samefile).isNotNull();
517+
assertThat(samefile.fullyQualifiedName()).isEqualTo("os.path.samefile");
515518

516519
Map<String, Symbol> osPath = symbolsForModule("os.path");
517520
Symbol samefileFromSubModule = osPath.get("samefile");

0 commit comments

Comments
 (0)