Skip to content

Commit 0c6def9

Browse files
andreaguarinoguillaume-dequenne
authored andcommitted
Typeshed symbols should always be retreived by name
1 parent 769d786 commit 0c6def9

File tree

6 files changed

+58
-53
lines changed

6 files changed

+58
-53
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
*/
2020
package org.sonar.python.semantic;
2121

22+
import java.util.Collection;
2223
import java.util.Collections;
2324
import java.util.HashMap;
2425
import java.util.HashSet;
@@ -182,7 +183,7 @@ void addModuleSymbol(Name nameTree, @CheckForNull String fullyQualifiedName) {
182183
this.symbols.add(moduleSymbol);
183184
symbolsByName.put(symbolName, moduleSymbol);
184185
} else if (!isExistingSymbol(symbolName) && fullyQualifiedName != null && !fullyQualifiedName.equals(fullyQualifiedModuleName)) {
185-
Set<Symbol> standardLibrarySymbols = TypeShed.symbolsForModule(fullyQualifiedName);
186+
Collection<Symbol> standardLibrarySymbols = TypeShed.symbolsForModule(fullyQualifiedName).values();
186187
if (!standardLibrarySymbols.isEmpty()) {
187188
SymbolImpl moduleSymbol = new SymbolImpl(symbolName, fullyQualifiedName);
188189
standardLibrarySymbols.forEach(symbol -> moduleSymbol.addChildSymbol(copySymbol(symbol.name(), symbol)));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,7 @@ 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).stream()
368+
importedModuleSymbols = TypeShed.symbolsForModule(moduleName).values().stream()
369369
.map(importedSymbol -> currentScope().copySymbol(importedSymbol.name(), importedSymbol)).collect(Collectors.toSet());
370370
}
371371
if (importedModuleSymbols != null && !importedModuleSymbols.isEmpty()) {

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

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -115,20 +115,23 @@ public static ClassSymbol typeShedClass(String fullyQualifiedName) {
115115
return (ClassSymbol) symbol;
116116
}
117117

118-
public static Set<Symbol> symbolsForModule(String moduleName) {
118+
/**
119+
* Returns map of exported symbols by name for a given module
120+
*/
121+
public static Map<String, Symbol> symbolsForModule(String moduleName) {
119122
if (!TypeShed.typeShedSymbols.containsKey(moduleName)) {
120-
Set<Symbol> symbols = searchTypeShedForModule(moduleName);
121-
Map<String, Symbol> symbolsByFqn = symbols.stream().collect(Collectors.toMap(Symbol::fullyQualifiedName, s -> s, (fst, snd) -> fst));
122-
typeShedSymbols.put(moduleName, symbolsByFqn);
123+
Map<String, Symbol> symbols = searchTypeShedForModule(moduleName);
124+
typeShedSymbols.put(moduleName, symbols);
123125
return symbols;
124126
}
125-
return new HashSet<>(TypeShed.typeShedSymbols.get(moduleName).values());
127+
return TypeShed.typeShedSymbols.get(moduleName);
126128
}
127129

128130
@CheckForNull
129131
public static Symbol symbolWithFQN(String stdLibModuleName, String fullyQualifiedName) {
130-
Set<Symbol> symbols = symbolsForModule(stdLibModuleName);
131-
Symbol symbolByFqn = symbols.stream().filter(s -> fullyQualifiedName.equals(s.fullyQualifiedName())).findFirst().orElse(null);
132+
Map<String, Symbol> symbols = symbolsForModule(stdLibModuleName);
133+
// TODO: improve performance - see SONARPY-955
134+
Symbol symbolByFqn = symbols.values().stream().filter(s -> fullyQualifiedName.equals(s.fullyQualifiedName())).findFirst().orElse(null);
132135
if (symbolByFqn != null || !fullyQualifiedName.contains(".")) {
133136
return symbolByFqn;
134137
}
@@ -139,14 +142,7 @@ public static Symbol symbolWithFQN(String stdLibModuleName, String fullyQualifie
139142
// FQN and try to look up by local symbol name, rather than FQN
140143
String[] fqnSplittedByDot = fullyQualifiedName.split("\\.");
141144
String symbolLocalNameFromFqn = fqnSplittedByDot[fqnSplittedByDot.length - 1];
142-
143-
// TODO: improve performance - see SONARPY-955
144-
Set<Symbol> matchByName = symbols.stream().filter(s -> symbolLocalNameFromFqn.equals(s.name())).collect(Collectors.toSet());
145-
if (matchByName.size() == 1) {
146-
return matchByName.iterator().next();
147-
}
148-
149-
return null;
145+
return symbols.get(symbolLocalNameFromFqn);
150146
}
151147

152148
@CheckForNull
@@ -210,8 +206,8 @@ public static Set<Symbol> symbolsFromProtobufDescriptors(Set<Object> protobufDes
210206
SymbolsProtos.VarSymbol varSymbol = (SymbolsProtos.VarSymbol) descriptor;
211207
SymbolImpl symbol = new SymbolImpl(varSymbol);
212208
if (varSymbol.getIsImportedModule()) {
213-
Set<Symbol> moduleExportedSymbols = searchTypeShedForModule(varSymbol.getFullyQualifiedName());
214-
moduleExportedSymbols.forEach(symbol::addChildSymbol);
209+
Map<String, Symbol> moduleExportedSymbols = symbolsForModule(varSymbol.getFullyQualifiedName());
210+
moduleExportedSymbols.values().forEach(symbol::addChildSymbol);
215211
}
216212
symbols.add(symbol);
217213
}
@@ -230,22 +226,22 @@ static void resetBuiltinSymbols() {
230226
builtinSymbols();
231227
}
232228

233-
private static Set<Symbol> searchTypeShedForModule(String moduleName) {
229+
private static Map<String, Symbol> searchTypeShedForModule(String moduleName) {
234230
if (modulesInProgress.contains(moduleName)) {
235-
return new HashSet<>();
231+
return new HashMap<>();
236232
}
237233
modulesInProgress.add(moduleName);
238-
Set<Symbol> customSymbols = new HashSet<>(getModuleSymbols(moduleName, CUSTOM_THIRD_PARTY, builtinGlobalSymbols).values());
234+
Map<String, Symbol> customSymbols = getModuleSymbols(moduleName, CUSTOM_THIRD_PARTY, builtinGlobalSymbols);
239235
if (!customSymbols.isEmpty()) {
240236
modulesInProgress.remove(moduleName);
241237
return customSymbols;
242238
}
243-
Collection<Symbol> symbolsFromProtobuf = getSymbolsFromProtobufModule(moduleName).values();
239+
Map<String, Symbol> symbolsFromProtobuf = getSymbolsFromProtobufModule(moduleName);
244240
if (!symbolsFromProtobuf.isEmpty()) {
245241
modulesInProgress.remove(moduleName);
246-
return new HashSet<>(symbolsFromProtobuf);
242+
return symbolsFromProtobuf;
247243
}
248-
Set<Symbol> thirdPartySymbols = new HashSet<>(getModuleSymbols(moduleName, THIRD_PARTY_2AND3, builtinGlobalSymbols).values());
244+
Map<String, Symbol> thirdPartySymbols = getModuleSymbols(moduleName, THIRD_PARTY_2AND3, builtinGlobalSymbols);
249245
if (thirdPartySymbols.isEmpty()) {
250246
thirdPartySymbols = commonSymbols(getModuleSymbols(moduleName, THIRD_PARTY_2, builtinGlobalSymbols),
251247
getModuleSymbols(moduleName, THIRD_PARTY_3, builtinGlobalSymbols), moduleName);

python-frontend/src/main/java/org/sonar/python/types/TypeShedThirdParties.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.InputStream;
2424
import java.util.Arrays;
2525
import java.util.Collections;
26+
import java.util.HashMap;
2627
import java.util.HashSet;
2728
import java.util.Map;
2829
import java.util.Optional;
@@ -91,23 +92,23 @@ private static ModuleDescription getResourceForModule(String moduleName, String
9192
return new ModuleDescription(resource, moduleFileName, packageName);
9293
}
9394

94-
static Set<Symbol> commonSymbols(Map<String, Symbol> symbolsPython2, Map<String, Symbol> symbolsPython3, String packageName) {
95-
Set<Symbol> commonSymbols = new HashSet<>();
95+
static Map<String, Symbol> commonSymbols(Map<String, Symbol> symbolsPython2, Map<String, Symbol> symbolsPython3, String packageName) {
96+
Map<String, Symbol> commonSymbols = new HashMap<>();
9697
symbolsPython3.forEach((localName, python3Symbol) -> {
9798
Symbol python2Symbol = symbolsPython2.get(localName);
9899
if (python2Symbol == null || python2Symbol == python3Symbol) {
99-
commonSymbols.add(python3Symbol);
100+
commonSymbols.put(localName, python3Symbol);
100101
} else {
101102
Set<Symbol> symbols = new HashSet<>();
102103
symbols.add(python2Symbol);
103104
symbols.add(python3Symbol);
104-
commonSymbols.add(new AmbiguousSymbolImpl(localName, packageName + "." + localName, symbols));
105+
commonSymbols.put(localName, new AmbiguousSymbolImpl(localName, packageName + "." + localName, symbols));
105106
}
106107
});
107108

108109
symbolsPython2.forEach((localName, python2Symbol) -> {
109110
if (symbolsPython3.get(localName) == null) {
110-
commonSymbols.add(python2Symbol);
111+
commonSymbols.put(localName, python2Symbol);
111112
}
112113
});
113114

python-frontend/src/test/java/org/sonar/python/types/CustomStubSanityTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.nio.file.Files;
2828
import java.nio.file.Path;
2929
import java.nio.file.Paths;
30+
import java.util.Collection;
3031
import java.util.Set;
3132
import java.util.stream.Collectors;
3233
import org.junit.Test;
@@ -51,7 +52,7 @@ public void test() throws IOException, URISyntaxException {
5152
for (String module : moduleNames) {
5253
// Make sure that each module we describe with a custom stub declares something and has a valid syntax.
5354
try {
54-
Set<Symbol> symbols = TypeShed.symbolsForModule(module);
55+
Collection<Symbol> symbols = TypeShed.symbolsForModule(module).values();
5556
assertThat(symbols).isNotEmpty();
5657
} catch (RecognitionException ex) {
5758
fail(String.format("Syntax error in stub file while resolving module '%s' (error may be in a referenced module).", module), ex);

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

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.HashMap;
2828
import java.util.Map;
2929
import java.util.Set;
30-
import java.util.function.Function;
3130
import java.util.stream.Collectors;
3231
import org.junit.Before;
3332
import org.junit.Test;
@@ -48,6 +47,7 @@
4847

4948
import static org.assertj.core.api.Assertions.assertThat;
5049
import static org.assertj.core.api.Assertions.tuple;
50+
import static org.sonar.python.types.TypeShed.symbolsForModule;
5151

5252
public class TypeShedTest {
5353

@@ -153,16 +153,16 @@ public void third_party_symbols() {
153153

154154
@Test
155155
public void should_resolve_packages() {
156-
assertThat(TypeShed.symbolsForModule("urllib")).isNotEmpty();
157-
assertThat(TypeShed.symbolsForModule("ctypes")).isNotEmpty();
158-
assertThat(TypeShed.symbolsForModule("email")).isNotEmpty();
159-
assertThat(TypeShed.symbolsForModule("json")).isNotEmpty();
160-
assertThat(TypeShed.symbolsForModule("docutils")).isNotEmpty();
161-
assertThat(TypeShed.symbolsForModule("ctypes.util")).isNotEmpty();
162-
assertThat(TypeShed.symbolsForModule("lib2to3.pgen2.grammar")).isNotEmpty();
156+
assertThat(symbolsForModule("urllib")).isNotEmpty();
157+
assertThat(symbolsForModule("ctypes")).isNotEmpty();
158+
assertThat(symbolsForModule("email")).isNotEmpty();
159+
assertThat(symbolsForModule("json")).isNotEmpty();
160+
assertThat(symbolsForModule("docutils")).isNotEmpty();
161+
assertThat(symbolsForModule("ctypes.util")).isNotEmpty();
162+
assertThat(symbolsForModule("lib2to3.pgen2.grammar")).isNotEmpty();
163163
// resolved but still empty
164-
assertThat(TypeShed.symbolsForModule("cryptography")).isEmpty();
165-
assertThat(TypeShed.symbolsForModule("kazoo")).isEmpty();
164+
assertThat(symbolsForModule("cryptography")).isEmpty();
165+
assertThat(symbolsForModule("kazoo")).isEmpty();
166166
}
167167

168168
@Test
@@ -271,8 +271,8 @@ public void package_sqlite3_connect_type_in_ambiguous_symbol() {
271271

272272
@Test
273273
public void stub_files_symbols() {
274-
Set<Symbol> mathSymbols = TypeShed.symbolsForModule("math");
275-
Set<Symbol> djangoHttpSymbols = TypeShed.symbolsForModule("django.http");
274+
Collection<Symbol> mathSymbols = symbolsForModule("math").values();
275+
Collection<Symbol> djangoHttpSymbols = symbolsForModule("django.http").values();
276276

277277
Collection<Symbol> symbols = TypeShed.stubFilesSymbols();
278278
assertThat(symbols)
@@ -282,7 +282,7 @@ public void stub_files_symbols() {
282282

283283
@Test
284284
public void deserialize_annoy_protobuf() {
285-
Map<String, Symbol> deserializedAnnoySymbols = TypeShed.symbolsForModule("annoy").stream()
285+
Map<String, Symbol> deserializedAnnoySymbols = symbolsForModule("annoy").values().stream()
286286
.collect(Collectors.toMap(Symbol::fullyQualifiedName, s -> s));
287287
assertThat(deserializedAnnoySymbols.values()).extracting(Symbol::kind, Symbol::fullyQualifiedName)
288288
.containsExactlyInAnyOrder(tuple(Kind.CLASS, "annoy._Vector"), tuple(Kind.CLASS, "annoy.AnnoyIndex"));
@@ -321,7 +321,7 @@ public void deserialize_annoy_protobuf() {
321321

322322
@Test
323323
public void deserialize_nonexistent_or_incorrect_protobuf() {
324-
assertThat(TypeShed.symbolsForModule("NOT_EXISTENT")).isEmpty();
324+
assertThat(symbolsForModule("NOT_EXISTENT")).isEmpty();
325325
assertThat(TypeShed.getSymbolsFromProtobufModule(null)).isEmpty();
326326
InputStream targetStream = new ByteArrayInputStream("foo".getBytes());
327327
assertThat(TypeShed.deserializedModule("mod", targetStream)).isNull();
@@ -451,7 +451,7 @@ public void pythonVersions() {
451451
@Test
452452
public void symbolWithFQN_should_be_consistent() {
453453
// smtplib imports typing.Sequence only in Python3, hence typing.Sequence has kind CLASS
454-
TypeShed.symbolsForModule("smtplib");
454+
symbolsForModule("smtplib");
455455
Symbol sequence = TypeShed.symbolWithFQN("typing.Sequence");
456456
assertThat(sequence.kind()).isEqualTo(Kind.AMBIGUOUS);
457457
Map<String, Symbol> typing = symbolsForModule("typing");
@@ -464,12 +464,6 @@ private static SymbolsProtos.ModuleSymbol moduleSymbol(String protobuf) throws T
464464
return builder.build();
465465
}
466466

467-
private static Map<String, Symbol> symbolsForModule(String moduleName) {
468-
Set<Symbol> symbols = TypeShed.symbolsForModule(moduleName);
469-
assertThat(symbols.stream().map(Symbol::name)).doesNotHaveDuplicates();
470-
return symbols.stream().collect(Collectors.toMap(Symbol::name, Function.identity()));
471-
}
472-
473467
@Test
474468
public void variables_from_protobuf() throws TextFormat.ParseException {
475469
SymbolsProtos.ModuleSymbol moduleSymbol = moduleSymbol(
@@ -508,7 +502,19 @@ public void typeshed_common_symbols() {
508502
python2Symbols.put("a", a1);
509503
python2Symbols.put("b", b);
510504
Map<String, Symbol> python3Symbols = Collections.singletonMap("a", a2);
511-
Set<Symbol> symbols = TypeShedThirdParties.commonSymbols(python2Symbols, python3Symbols, "mod");
505+
Collection<Symbol> symbols = TypeShedThirdParties.commonSymbols(python2Symbols, python3Symbols, "mod").values();
512506
assertThat(symbols).extracting(Symbol::kind, Symbol::name).containsExactlyInAnyOrder(tuple(Kind.AMBIGUOUS, "a"), tuple(Kind.OTHER, "b"));
513507
}
508+
509+
@Test
510+
public void symbol_from_submodule_access() {
511+
Map<String, Symbol> os = symbolsForModule("os");
512+
SymbolImpl path = (SymbolImpl) os.get("path");
513+
Symbol samefile = path.getChildrenSymbolByName().get("samefile");
514+
assertThat(samefile).isNotNull();
515+
516+
Map<String, Symbol> osPath = symbolsForModule("os.path");
517+
Symbol samefileFromSubModule = osPath.get("samefile");
518+
assertThat(samefileFromSubModule).isSameAs(samefile);
519+
}
514520
}

0 commit comments

Comments
 (0)