Skip to content

Commit fb4e7cf

Browse files
andrea-guarino-sonarsourceguillaume-dequenne
andrea-guarino-sonarsource
authored and
guillaume-dequenne
committed
SONARPY-941 Make sure there are no conflicting symbols having the sam… (#1036)
1 parent 0db132d commit fb4e7cf

File tree

2 files changed

+36
-26
lines changed

2 files changed

+36
-26
lines changed

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -414,12 +414,19 @@ static Map<String, Symbol> getSymbolsFromProtobufModule(@Nullable ModuleSymbol m
414414
return deserializedSymbols;
415415
}
416416

417-
// TODO: to be checked when implementing SONARPY-941
418417
private static Symbol disambiguateSymbolsWithSameName(String name, Set<Symbol> symbols, String moduleFqn) {
419418
if (symbols.size() > 1) {
420419
if (haveAllTheSameFqn(symbols) && !isBuiltinToDisambiguate(moduleFqn, name)) {
421420
return AmbiguousSymbolImpl.create(symbols);
422421
}
422+
if (!moduleFqn.equals(BUILTINS_FQN)) {
423+
String fqns = symbols.stream()
424+
.map(Symbol::fullyQualifiedName)
425+
.map(fqn -> fqn == null ? "N/A" : fqn)
426+
.collect(Collectors.joining(","));
427+
LOG.debug("Symbol " + name + " has conflicting fully qualified names:" + fqns);
428+
LOG.debug("It has been disambiguated with its latest Python version available symbol.");
429+
}
423430
return disambiguateWithLatestPythonSymbol(symbols);
424431
}
425432
return symbols.iterator().next();

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

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.function.Function;
2929
import java.util.stream.Collectors;
3030
import org.junit.Before;
31-
import org.junit.Ignore;
3231
import org.junit.Test;
3332
import org.sonar.api.utils.log.LogTester;
3433
import org.sonar.api.utils.log.LoggerLevel;
@@ -115,7 +114,7 @@ public void none_type() {
115114

116115
@Test
117116
public void typing_module() {
118-
Map<String, Symbol> symbols = TypeShed.symbolsForModule("typing").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
117+
Map<String, Symbol> symbols = symbolsForModule("typing");
119118
assertThat(symbols.values()).allMatch(symbol -> symbol.usages().isEmpty());
120119
// python3 specific
121120
assertThat(symbols.get("Awaitable").kind()).isEqualTo(Kind.CLASS);
@@ -125,25 +124,25 @@ public void typing_module() {
125124

126125
@Test
127126
public void stdlib_symbols() {
128-
Map<String, Symbol> mathSymbols = TypeShed.symbolsForModule("math").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
127+
Map<String, Symbol> mathSymbols = symbolsForModule("math");
129128
Symbol acosSymbol = mathSymbols.get("acos");
130129
assertThat(acosSymbol.kind()).isEqualTo(Kind.FUNCTION);
131130
assertThat(((FunctionSymbolImpl) acosSymbol).declaredReturnType().canOnlyBe("float")).isTrue();
132131
assertThat(TypeShed.symbolWithFQN("math", "math.acos")).isSameAs(acosSymbol);
133132
assertThat(mathSymbols.values()).allMatch(symbol -> symbol.usages().isEmpty());
134133

135-
Map<String, Symbol> threadingSymbols = TypeShed.symbolsForModule("threading").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
134+
Map<String, Symbol> threadingSymbols = symbolsForModule("threading");
136135
assertThat(threadingSymbols.get("Thread").kind()).isEqualTo(Kind.CLASS);
137136
assertThat(threadingSymbols.values()).allMatch(symbol -> symbol.usages().isEmpty());
138137

139-
Map<String, Symbol> imaplibSymbols = TypeShed.symbolsForModule("imaplib").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
138+
Map<String, Symbol> imaplibSymbols = symbolsForModule("imaplib");
140139
assertThat(imaplibSymbols).isNotEmpty();
141140
assertThat(imaplibSymbols.values()).allMatch(symbol -> symbol.usages().isEmpty());
142141
}
143142

144143
@Test
145144
public void third_party_symbols() {
146-
Map<String, Symbol> emojiSymbols = TypeShed.symbolsForModule("emoji").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
145+
Map<String, Symbol> emojiSymbols = symbolsForModule("emoji");
147146
Symbol emojizeSymbol = emojiSymbols.get("emojize");
148147
assertThat(emojizeSymbol.kind()).isEqualTo(Kind.FUNCTION);
149148
assertThat(((FunctionSymbolImpl) emojizeSymbol).declaredReturnType().canOnlyBe("str")).isTrue();
@@ -166,7 +165,7 @@ public void should_resolve_packages() {
166165

167166
@Test
168167
public void package_symbols() {
169-
Map<String, Symbol> cursesSymbols = TypeShed.symbolsForModule("curses").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
168+
Map<String, Symbol> cursesSymbols = symbolsForModule("curses");
170169
Symbol wrapperSymbol = cursesSymbols.get("wrapper");
171170
assertThat(wrapperSymbol.kind()).isEqualTo(Kind.FUNCTION);
172171
assertThat(((FunctionSymbolImpl) wrapperSymbol).declaredReturnType()).isEqualTo(AnyType.ANY);
@@ -175,7 +174,7 @@ public void package_symbols() {
175174

176175
@Test
177176
public void package_submodules_symbols() {
178-
Map<String, Symbol> asciiSymbols = TypeShed.symbolsForModule("curses.ascii").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
177+
Map<String, Symbol> asciiSymbols = symbolsForModule("curses.ascii");
179178
Symbol isalnumSymbol = asciiSymbols.get("isalnum");
180179
assertThat(isalnumSymbol.kind()).isEqualTo(Kind.FUNCTION);
181180
assertThat(((FunctionSymbolImpl) isalnumSymbol).declaredReturnType().canOnlyBe("bool")).isTrue();
@@ -184,7 +183,7 @@ public void package_submodules_symbols() {
184183

185184
@Test
186185
public void package_inner_submodules_symbols() {
187-
Map<String, Symbol> driverSymbols = TypeShed.symbolsForModule("lib2to3.pgen2.driver").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
186+
Map<String, Symbol> driverSymbols = symbolsForModule("lib2to3.pgen2.driver");
188187
Symbol loadGrammarSymbol = driverSymbols.get("load_grammar");
189188
// There is a small difference between Python 2 and Python 3 symbols: Python 2 uses Text instead of str
190189
assertThat(loadGrammarSymbol.kind()).isEqualTo(Kind.AMBIGUOUS);
@@ -193,7 +192,7 @@ public void package_inner_submodules_symbols() {
193192

194193
@Test
195194
public void package_relative_import() {
196-
Map<String, Symbol> osSymbols = TypeShed.symbolsForModule("os").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
195+
Map<String, Symbol> osSymbols = symbolsForModule("os");
197196
// TODO: Add imported symbols SONARPY-938
198197
// Symbol sysSymbol = osSymbols.get("sys");
199198
// assertThat(sysSymbol.kind()).isEqualTo(Kind.AMBIGUOUS);
@@ -202,35 +201,33 @@ public void package_relative_import() {
202201
assertThat(timesResult.kind()).isEqualTo(Kind.CLASS);
203202
assertThat(timesResult.fullyQualifiedName()).isEqualTo("posix.times_result");
204203

205-
Map<String, Symbol> requestsSymbols = TypeShed.symbolsForModule("requests").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
204+
Map<String, Symbol> requestsSymbols = symbolsForModule("requests");
206205
Symbol requestSymbol = requestsSymbols.get("request");
207206
assertThat(requestSymbol.kind()).isEqualTo(Kind.FUNCTION);
208207
assertThat(requestSymbol.fullyQualifiedName()).isEqualTo("requests.api.request");
209208
}
210209

211210
@Test
212211
public void package_member_fqn_points_to_original_fqn() {
213-
Map<String, Symbol> symbols = TypeShed.symbolsForModule("flask").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
212+
Map<String, Symbol> symbols = symbolsForModule("flask");
214213
Symbol targetSymbol = symbols.get("Response");
215214
assertThat(targetSymbol.fullyQualifiedName()).isEqualTo("flask.wrappers.Response");
216215
assertThat(TypeShed.symbolWithFQN("flask", "flask.Response")).isSameAs(targetSymbol);
217216
}
218217

219-
// TODO: there shouldn't be more than two symbols with the same name SONARPY-941
220-
// TODO: FileIO is ambiguous and it has FQN null
221-
@Ignore
218+
219+
@Test
222220
public void package_member_ambigous_symbol_common_fqn() {
223-
Map<String, Symbol> symbols = TypeShed.symbolsForModule("io").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
221+
Map<String, Symbol> symbols = symbolsForModule("io");
224222
Symbol targetSymbol = symbols.get("FileIO");
225223
assertThat(targetSymbol.fullyQualifiedName()).isEqualTo("io.FileIO");
226224
assertThat(TypeShed.symbolWithFQN("io", "io.FileIO")).isSameAs(targetSymbol);
227225
}
228226

229227
@Test
230228
public void two_exported_symbols_with_same_local_names() {
231-
// TODO: there shouldn't be more than two symbols with the same name SONARPY-941
232-
Map<String, Symbol> osSymbols = TypeShed.symbolsForModule("os").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
233-
Map<String, Symbol> posixSymbols = TypeShed.symbolsForModule("posix").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
229+
Map<String, Symbol> osSymbols = symbolsForModule("os");
230+
Map<String, Symbol> posixSymbols = symbolsForModule("posix");
234231
Symbol setupSymbolFromPosix = posixSymbols.get("stat_result");
235232
Symbol setupSymbolFromOs = osSymbols.get("stat_result");
236233
assertThat(setupSymbolFromPosix.kind()).isEqualTo(Kind.CLASS);
@@ -239,32 +236,32 @@ public void two_exported_symbols_with_same_local_names() {
239236

240237
@Test
241238
public void package_django() {
242-
Map<String, Symbol> djangoSymbols = TypeShed.symbolsForModule("django.http").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
239+
Map<String, Symbol> djangoSymbols = symbolsForModule("django.http");
243240
Symbol responseSymbol = djangoSymbols.get("HttpResponse");
244241
assertThat(responseSymbol.kind()).isEqualTo(Kind.CLASS);
245242
assertThat(responseSymbol.fullyQualifiedName()).isEqualTo("django.http.response.HttpResponse");
246243
}
247244

248245
@Test
249246
public void return_type_hints() {
250-
Map<String, Symbol> symbols = TypeShed.symbolsForModule("typing").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
247+
Map<String, Symbol> symbols = symbolsForModule("typing");
251248
assertThat(((FunctionSymbolImpl) symbols.get("get_args")).annotatedReturnTypeName()).isEqualTo("tuple");
252-
symbols = TypeShed.symbolsForModule("flask_mail").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
249+
symbols = symbolsForModule("flask_mail");
253250
ClassSymbol mail = (ClassSymbol) symbols.get("Mail");
254251
assertThat(((FunctionSymbol) mail.declaredMembers().stream().iterator().next()).annotatedReturnTypeName()).isNull();
255252
}
256253

257254
@Test
258255
public void package_django_class_property_type() {
259-
Map<String, Symbol> djangoSymbols = TypeShed.symbolsForModule("django.http.request").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
256+
Map<String, Symbol> djangoSymbols = symbolsForModule("django.http.request");
260257
Symbol requestSymbol = djangoSymbols.get("HttpRequest");
261258
assertThat(requestSymbol.kind()).isEqualTo(Kind.CLASS);
262259
assertThat(((ClassSymbol) requestSymbol).declaredMembers().iterator().next().annotatedTypeName()).isEqualTo("django.http.request.QueryDict");
263260
}
264261

265262
@Test
266263
public void package_sqlite3_connect_type_in_ambiguous_symbol() {
267-
Map<String, Symbol> djangoSymbols = TypeShed.symbolsForModule("sqlite3").stream().collect(Collectors.toMap(Symbol::name, Function.identity(), AmbiguousSymbolImpl::create));
264+
Map<String, Symbol> djangoSymbols = symbolsForModule("sqlite3");
268265
Symbol requestSymbol = djangoSymbols.get("connect");
269266
assertThat(((FunctionSymbolImpl) ((((AmbiguousSymbolImpl) requestSymbol).alternatives()).toArray()[0])).annotatedReturnTypeName()).isEqualTo("sqlite3.dbapi2.Connection");
270267
}
@@ -454,7 +451,7 @@ public void symbolWithFQN_should_be_consistent() {
454451
TypeShed.symbolsForModule("smtplib");
455452
Symbol sequence = TypeShed.symbolWithFQN("typing.Sequence");
456453
assertThat(sequence.kind()).isEqualTo(Kind.AMBIGUOUS);
457-
Map<String, Symbol> typing = TypeShed.symbolsForModule("typing").stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
454+
Map<String, Symbol> typing = symbolsForModule("typing");
458455
assertThat(sequence).isSameAs(typing.get("Sequence"));
459456
}
460457

@@ -463,4 +460,10 @@ private static SymbolsProtos.ModuleSymbol moduleSymbol(String protobuf) throws T
463460
TextFormat.merge(protobuf, builder);
464461
return builder.build();
465462
}
463+
464+
private static Map<String, Symbol> symbolsForModule(String moduleName) {
465+
Set<Symbol> symbols = TypeShed.symbolsForModule(moduleName);
466+
assertThat(symbols.stream().map(Symbol::name)).doesNotHaveDuplicates();
467+
return symbols.stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
468+
}
466469
}

0 commit comments

Comments
 (0)