From c07d29023130dd0893a30572967dc4638e7a857b Mon Sep 17 00:00:00 2001 From: Andrea Guarino Date: Mon, 30 Mar 2020 13:25:25 +0200 Subject: [PATCH 1/2] Add Symbol#is() API --- .../plugins/python/api/symbols/Symbol.java | 2 + .../org/sonar/python/semantic/SymbolImpl.java | 11 ++++ .../sonar/python/semantic/SymbolImplTest.java | 51 +++++++++++++++++++ 3 files changed, 64 insertions(+) create mode 100644 python-frontend/src/test/java/org/sonar/python/semantic/SymbolImplTest.java diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java index 7e3eb56aca..d63b301202 100644 --- a/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java +++ b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java @@ -31,6 +31,8 @@ public interface Symbol { @CheckForNull String fullyQualifiedName(); + boolean is(Kind... kinds); + Kind kind(); enum Kind { diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/SymbolImpl.java b/python-frontend/src/main/java/org/sonar/python/semantic/SymbolImpl.java index 57e03ec718..ff963573b9 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/SymbolImpl.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/SymbolImpl.java @@ -66,6 +66,17 @@ public String fullyQualifiedName() { return fullyQualifiedName; } + @Override + public boolean is(Kind... kinds) { + Kind symbolKind = kind(); + for (Kind kindIter : kinds) { + if (symbolKind == kindIter) { + return true; + } + } + return false; + } + @Override public Kind kind() { return this.kind; diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/SymbolImplTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/SymbolImplTest.java new file mode 100644 index 0000000000..cc262632a1 --- /dev/null +++ b/python-frontend/src/test/java/org/sonar/python/semantic/SymbolImplTest.java @@ -0,0 +1,51 @@ +/* + * SonarQube Python Plugin + * Copyright (C) 2011-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.python.semantic; + +import java.util.Map; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.junit.Test; +import org.sonar.plugins.python.api.symbols.Symbol; +import org.sonar.plugins.python.api.tree.FileInput; +import org.sonar.python.PythonTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.python.PythonTestUtils.parse; + + +public class SymbolImplTest { + + @Test + public void assert_is() { + Symbol x = symbols("x = 42").get("x"); + assertThat(x.is(Symbol.Kind.OTHER)).isTrue(); + + Symbol foo = symbols("def foo(): ...").get("foo"); + assertThat(foo.is(Symbol.Kind.FUNCTION)).isTrue(); + assertThat(foo.is(Symbol.Kind.OTHER)).isFalse(); + assertThat(foo.is(Symbol.Kind.OTHER, Symbol.Kind.FUNCTION)).isTrue(); + } + + private Map symbols(String... code) { + FileInput fileInput = parse(new SymbolTableBuilder("", PythonTestUtils.pythonFile("foo")), code); + return fileInput.globalVariables().stream().collect(Collectors.toMap(Symbol::name, Function.identity())); + } +} From abb416fee6c5c9ab15ac75fecbbd703f0367eda4 Mon Sep 17 00:00:00 2001 From: Andrea Guarino Date: Mon, 30 Mar 2020 18:12:59 +0200 Subject: [PATCH 2/2] SONARPY-618 Introduce Ambiguous Symbol --- .../test/resources/expected/python-S2638.json | 7 + .../AbstractUnreadPrivateMembersCheck.java | 10 +- .../python/api/symbols/AmbiguousSymbol.java | 26 +++ .../plugins/python/api/symbols/Symbol.java | 1 + .../python/semantic/AmbiguousSymbolImpl.java | 67 ++++++++ .../java/org/sonar/python/semantic/Scope.java | 25 ++- .../python/semantic/SymbolTableBuilder.java | 59 +++++++ .../sonar/python/tree/FunctionDefImpl.java | 11 ++ .../java/org/sonar/python/types/TypeShed.java | 24 ++- .../python/semantic/AmbiguousSymbolTest.java | 155 ++++++++++++++++++ .../python/semantic/ClassSymbolTest.java | 4 +- .../python/semantic/FunctionSymbolTest.java | 6 +- .../semantic/ProjectLevelSymbolTableTest.java | 22 +++ 13 files changed, 394 insertions(+), 23 deletions(-) create mode 100644 python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/AmbiguousSymbol.java create mode 100644 python-frontend/src/main/java/org/sonar/python/semantic/AmbiguousSymbolImpl.java create mode 100644 python-frontend/src/test/java/org/sonar/python/semantic/AmbiguousSymbolTest.java diff --git a/its/ruling/src/test/resources/expected/python-S2638.json b/its/ruling/src/test/resources/expected/python-S2638.json index a239ac8681..7a3e098e5b 100644 --- a/its/ruling/src/test/resources/expected/python-S2638.json +++ b/its/ruling/src/test/resources/expected/python-S2638.json @@ -36,6 +36,9 @@ 'project:django-cms-3.7.1/cms/tests/test_permmod.py':[ 665, ], +'project:numpy-1.16.4/numpy/distutils/fcompiler/pg.py':[ +131, +], 'project:numpy-1.16.4/numpy/ma/core.py':[ 6170, ], @@ -161,6 +164,10 @@ 'project:twisted-12.1.0/twisted/web/_newclient.py':[ 515, ], +'project:twisted-12.1.0/twisted/web/client.py':[ +669, +669, +], 'project:twisted-12.1.0/twisted/web/sux.py':[ 193, ], diff --git a/python-checks/src/main/java/org/sonar/python/checks/AbstractUnreadPrivateMembersCheck.java b/python-checks/src/main/java/org/sonar/python/checks/AbstractUnreadPrivateMembersCheck.java index b3ea59eb38..c8022c1d57 100644 --- a/python-checks/src/main/java/org/sonar/python/checks/AbstractUnreadPrivateMembersCheck.java +++ b/python-checks/src/main/java/org/sonar/python/checks/AbstractUnreadPrivateMembersCheck.java @@ -22,6 +22,7 @@ import java.util.Optional; import org.sonar.plugins.python.api.PythonSubscriptionCheck; import org.sonar.plugins.python.api.SubscriptionContext; +import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; import org.sonar.plugins.python.api.symbols.Symbol; import org.sonar.plugins.python.api.symbols.Usage; import org.sonar.plugins.python.api.tree.ClassDef; @@ -37,11 +38,18 @@ public void initialize(Context context) { context.registerSyntaxNodeConsumer(CLASSDEF, ctx -> { ClassDef classDef = (ClassDef) ctx.syntaxNode(); Optional.ofNullable(getClassSymbolFromDef(classDef)).ifPresent(classSymbol -> classSymbol.declaredMembers().stream() - .filter(s -> s.name().startsWith(memberPrefix) && !s.name().endsWith("__") && s.kind() == kind() && isNeverRead(s)) + .filter(s -> s.name().startsWith(memberPrefix) && !s.name().endsWith("__") && equalsToKind(s) && isNeverRead(s)) .forEach(symbol -> reportIssue(ctx, symbol))); }); } + private boolean equalsToKind(Symbol symbol) { + if (symbol.kind().equals(Symbol.Kind.AMBIGUOUS)) { + return ((AmbiguousSymbol) symbol).alternatives().stream().allMatch(s -> s.kind() == kind()); + } + return symbol.kind() == kind(); + } + private void reportIssue(SubscriptionContext ctx, Symbol symbol) { PreciseIssue preciseIssue = null; for (int i = 0; i < symbol.usages().size(); i++) { diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/AmbiguousSymbol.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/AmbiguousSymbol.java new file mode 100644 index 0000000000..3c0070ec9b --- /dev/null +++ b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/AmbiguousSymbol.java @@ -0,0 +1,26 @@ +/* + * SonarQube Python Plugin + * Copyright (C) 2011-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.plugins.python.api.symbols; + +import java.util.Set; + +public interface AmbiguousSymbol extends Symbol { + Set alternatives(); +} diff --git a/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java index d63b301202..4dc9a1f289 100644 --- a/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java +++ b/python-frontend/src/main/java/org/sonar/plugins/python/api/symbols/Symbol.java @@ -38,6 +38,7 @@ public interface Symbol { enum Kind { FUNCTION, CLASS, + AMBIGUOUS, OTHER } } diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/AmbiguousSymbolImpl.java b/python-frontend/src/main/java/org/sonar/python/semantic/AmbiguousSymbolImpl.java new file mode 100644 index 0000000000..bca4976dab --- /dev/null +++ b/python-frontend/src/main/java/org/sonar/python/semantic/AmbiguousSymbolImpl.java @@ -0,0 +1,67 @@ +/* + * SonarQube Python Plugin + * Copyright (C) 2011-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.python.semantic; + +import java.util.Collections; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import javax.annotation.Nullable; +import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; +import org.sonar.plugins.python.api.symbols.Symbol; + +public class AmbiguousSymbolImpl extends SymbolImpl implements AmbiguousSymbol { + + private final Set symbols; + + private AmbiguousSymbolImpl(String name, @Nullable String fullyQualifiedName, Set symbols) { + super(name, fullyQualifiedName); + setKind(Kind.AMBIGUOUS); + this.symbols = symbols; + } + + public static AmbiguousSymbol create(Set symbols) { + if (symbols.size() < 2) { + throw new IllegalArgumentException("Ambiguous symbol should contain at least two symbols"); + } + Symbol firstSymbol = symbols.iterator().next(); + if (!symbols.stream().map(Symbol::name).allMatch(symbolName -> symbolName.equals(firstSymbol.name()))) { + throw new IllegalArgumentException("Ambiguous symbol should contain symbols with the same name"); + } + if (!symbols.stream().map(Symbol::fullyQualifiedName).allMatch(fqn -> Objects.equals(firstSymbol.fullyQualifiedName(), fqn))) { + return new AmbiguousSymbolImpl(firstSymbol.name(), null, symbols); + } + return new AmbiguousSymbolImpl(firstSymbol.name(), firstSymbol.fullyQualifiedName(), symbols); + } + + @Override + public Set alternatives() { + return symbols; + } + + @Override + AmbiguousSymbolImpl copyWithoutUsages() { + Set copiedAlternativeSymbols = symbols.stream() + .map(SymbolImpl.class::cast) + .map(SymbolImpl::copyWithoutUsages) + .collect(Collectors.toSet()); + return ((AmbiguousSymbolImpl) create(Collections.unmodifiableSet(copiedAlternativeSymbols))); + } +} diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java b/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java index 3aa7ba1c07..dabafcf09a 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/Scope.java @@ -28,6 +28,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.plugins.python.api.PythonFile; +import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; import org.sonar.plugins.python.api.symbols.ClassSymbol; import org.sonar.plugins.python.api.symbols.FunctionSymbol; import org.sonar.plugins.python.api.symbols.Symbol; @@ -65,13 +66,7 @@ void createBuiltinSymbol(String name, Map typeShedSymbols) { SymbolImpl symbol; Symbol typeShedSymbol = typeShedSymbols.get(name); if (typeShedSymbol != null) { - if (typeShedSymbol.kind() == Symbol.Kind.CLASS) { - symbol = ((ClassSymbolImpl) typeShedSymbol).copyWithoutUsages(); - } else if (typeShedSymbol.kind() == Symbol.Kind.FUNCTION) { - symbol = ((FunctionSymbolImpl) typeShedSymbol).copyWithoutUsages(); - } else { - symbol = new SymbolImpl(typeShedSymbol.name(), typeShedSymbol.fullyQualifiedName()); - } + symbol = ((SymbolImpl) typeShedSymbol).copyWithoutUsages(); } else { symbol = new SymbolImpl(name, name); } @@ -116,9 +111,9 @@ void addFunctionSymbol(FunctionDef functionDef, @Nullable String fullyQualifiedN } private static Symbol copySymbol(String symbolName, Symbol symbol, Map globalSymbolsByFQN) { - if (symbol.kind() == Symbol.Kind.FUNCTION) { + if (symbol.is(Symbol.Kind.FUNCTION)) { return new FunctionSymbolImpl(symbolName, (FunctionSymbol) symbol); - } else if (symbol.kind() == Symbol.Kind.CLASS) { + } else if (symbol.is(Symbol.Kind.CLASS)) { ClassSymbolImpl classSymbol = new ClassSymbolImpl(symbolName, symbol.fullyQualifiedName()); for (Symbol originalSymbol : ((ClassSymbol) symbol).superClasses()) { Symbol globalSymbol = globalSymbolsByFQN.get(originalSymbol.fullyQualifiedName()); @@ -133,6 +128,11 @@ private static Symbol copySymbol(String symbolName, Symbol symbol, Map ((SymbolImpl) m).copyWithoutUsages()) .collect(Collectors.toList())); return classSymbol; + } else if (symbol.is(Symbol.Kind.AMBIGUOUS)) { + Set alternativeSymbols = ((AmbiguousSymbol) symbol).alternatives().stream() + .map(s -> copySymbol(s.name(), s, globalSymbolsByFQN)) + .collect(Collectors.toSet()); + return AmbiguousSymbolImpl.create(alternativeSymbols); } return new SymbolImpl(symbolName, symbol.fullyQualifiedName()); } @@ -219,4 +219,11 @@ void addClassSymbol(ClassDef classDef, @Nullable String fullyQualifiedName) { classSymbol.addUsage(classDef.name(), Usage.Kind.CLASS_DECLARATION); } } + + void replaceSymbolWithAmbiguousSymbol(Symbol symbol, AmbiguousSymbol ambiguousSymbol) { + symbols.remove(symbol); + symbols.add(ambiguousSymbol); + symbolsByName.remove(symbol.name()); + symbolsByName.put(symbol.name(), ambiguousSymbol); + } } diff --git a/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java b/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java index 53a28409a3..e60e2a3515 100644 --- a/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java +++ b/python-frontend/src/main/java/org/sonar/python/semantic/SymbolTableBuilder.java @@ -37,6 +37,7 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.plugins.python.api.PythonFile; +import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; import org.sonar.plugins.python.api.symbols.ClassSymbol; import org.sonar.plugins.python.api.symbols.Symbol; import org.sonar.plugins.python.api.symbols.Usage; @@ -132,6 +133,7 @@ public void visitFileInput(FileInput fileInput) { scopesByRootTree = new HashMap<>(); fileInput.accept(new FirstPhaseVisitor()); fileInput.accept(new SecondPhaseVisitor()); + createAmbiguousSymbols(); addSymbolsToTree((FileInputImpl) fileInput); fileInput.accept(new ThirdPhaseVisitor()); if (!SymbolUtils.isTypeShedFile(pythonFile)) { @@ -139,6 +141,63 @@ public void visitFileInput(FileInput fileInput) { } } + private static class SymbolToUpdate { + final Symbol symbol; + final AmbiguousSymbol ambiguousSymbol; + + SymbolToUpdate(Symbol symbol, AmbiguousSymbol ambiguousSymbol) { + this.symbol = symbol; + this.ambiguousSymbol = ambiguousSymbol; + } + } + + private void createAmbiguousSymbols() { + for (Scope scope : scopesByRootTree.values()) { + Set symbolsToUpdate = new HashSet<>(); + for (Symbol symbol : scope.symbols()) { + if (symbol.kind() == Symbol.Kind.OTHER) { + List bindingUsages = symbol.usages().stream().filter(Usage::isBindingUsage).collect(Collectors.toList()); + if (bindingUsages.size() > 1 && + bindingUsages.stream().anyMatch(usage -> usage.kind() == Usage.Kind.FUNC_DECLARATION || usage.kind() == Usage.Kind.CLASS_DECLARATION)) { + + Set alternativeDefinitions = getAlternativeDefinitions(symbol, bindingUsages); + AmbiguousSymbol ambiguousSymbol = AmbiguousSymbolImpl.create(alternativeDefinitions); + // update symbol and usage to newly created ambiguous symbol + symbol.usages().forEach(usage -> ((SymbolImpl) ambiguousSymbol).addUsage(usage.tree(), usage.kind())); + symbolsToUpdate.add(new SymbolToUpdate(symbol, ambiguousSymbol)); + } + } + } + symbolsToUpdate.forEach(symbolToUpdate -> scope.replaceSymbolWithAmbiguousSymbol(symbolToUpdate.symbol, symbolToUpdate.ambiguousSymbol)); + } + } + + private Set getAlternativeDefinitions(Symbol symbol, List bindingUsages) { + Set alternativeDefinitions = new HashSet<>(); + for (Usage bindingUsage : bindingUsages) { + switch (bindingUsage.kind()) { + case FUNC_DECLARATION: + FunctionDef functionDef = (FunctionDef) bindingUsage.tree().parent(); + FunctionSymbolImpl functionSymbol = new FunctionSymbolImpl(functionDef, symbol.fullyQualifiedName(), pythonFile); + ((FunctionDefImpl) functionDef).setFunctionSymbol(functionSymbol); + alternativeDefinitions.add(functionSymbol); + break; + case CLASS_DECLARATION: + ClassSymbolImpl classSymbol = new ClassSymbolImpl(symbol.name(), symbol.fullyQualifiedName()); + ClassDef classDef = (ClassDef) bindingUsage.tree().parent(); + resolveTypeHierarchy(classDef, classSymbol); + Scope classScope = scopesByRootTree.get(classDef); + classSymbol.addMembers(getClassMembers(classScope.symbolsByName, classScope.instanceAttributesByName)); + alternativeDefinitions.add(classSymbol); + break; + default: + SymbolImpl alternativeSymbol = new SymbolImpl(symbol.name(), symbol.fullyQualifiedName()); + alternativeDefinitions.add(alternativeSymbol); + } + } + return alternativeDefinitions; + } + private void addSymbolsToTree(FileInputImpl fileInput) { for (Scope scope : scopesByRootTree.values()) { if (scope.rootTree instanceof FunctionLike) { diff --git a/python-frontend/src/main/java/org/sonar/python/tree/FunctionDefImpl.java b/python-frontend/src/main/java/org/sonar/python/tree/FunctionDefImpl.java index 94d00715c0..dcee492205 100644 --- a/python-frontend/src/main/java/org/sonar/python/tree/FunctionDefImpl.java +++ b/python-frontend/src/main/java/org/sonar/python/tree/FunctionDefImpl.java @@ -28,6 +28,7 @@ import java.util.stream.Stream; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import org.sonar.plugins.python.api.symbols.FunctionSymbol; import org.sonar.plugins.python.api.tree.Decorator; import org.sonar.plugins.python.api.tree.FunctionDef; import org.sonar.plugins.python.api.tree.Name; @@ -59,6 +60,7 @@ public class FunctionDefImpl extends PyTree implements FunctionDef { private final boolean isMethodDefinition; private final StringLiteral docstring; private Set symbols = new HashSet<>(); + private FunctionSymbol functionSymbol; public FunctionDefImpl(List decorators, @Nullable Token asyncKeyword, Token defKeyword, Name name, Token leftPar, @Nullable ParameterList parameters, Token rightPar, @Nullable TypeAnnotation returnType, @@ -169,4 +171,13 @@ public List computeChildren() { return Stream.of(decorators, Arrays.asList(asyncKeyword, defKeyword, name, leftPar, parameters, rightPar, returnType, colon, newLine, indent, body, dedent)) .flatMap(List::stream).filter(Objects::nonNull).collect(Collectors.toList()); } + + public void setFunctionSymbol(FunctionSymbol functionSymbol) { + this.functionSymbol = functionSymbol; + } + + @CheckForNull + public FunctionSymbol functionSymbol() { + return functionSymbol; + } } diff --git a/python-frontend/src/main/java/org/sonar/python/types/TypeShed.java b/python-frontend/src/main/java/org/sonar/python/types/TypeShed.java index 5114879161..e1be310ad7 100644 --- a/python-frontend/src/main/java/org/sonar/python/types/TypeShed.java +++ b/python-frontend/src/main/java/org/sonar/python/types/TypeShed.java @@ -41,6 +41,7 @@ import org.sonar.python.semantic.FunctionSymbolImpl; import org.sonar.python.semantic.SymbolImpl; import org.sonar.python.semantic.SymbolTableBuilder; +import org.sonar.python.tree.FunctionDefImpl; import org.sonar.python.tree.PythonTreeMaker; import static org.sonar.plugins.python.api.types.BuiltinTypes.NONE_TYPE; @@ -66,25 +67,32 @@ public static Map builtinSymbols() { for (Symbol globalVariable : fileInput.globalVariables()) { builtins.put(globalVariable.fullyQualifiedName(), globalVariable); } + TypeShed.builtins = Collections.unmodifiableMap(builtins); BaseTreeVisitor visitor = new BaseTreeVisitor() { @Override public void visitFunctionDef(FunctionDef functionDef) { - TypeAnnotation returnTypeAnnotation = functionDef.returnTypeAnnotation(); - Optional.ofNullable(functionDef.name().symbol()).ifPresent(symbol -> { - if (symbol.kind() == Symbol.Kind.FUNCTION && returnTypeAnnotation != null) { - FunctionSymbolImpl functionSymbol = (FunctionSymbolImpl) symbol; - functionSymbol.setDeclaredReturnType(InferredTypes.declaredType(returnTypeAnnotation, builtins)); - } - }); + Optional.ofNullable(functionDef.name().symbol()).ifPresent(symbol -> setDeclaredReturnType(symbol, functionDef)); super.visitFunctionDef(functionDef); } }; fileInput.accept(visitor); - TypeShed.builtins = Collections.unmodifiableMap(builtins); } return builtins; } + private static void setDeclaredReturnType(Symbol symbol, FunctionDef functionDef) { + TypeAnnotation returnTypeAnnotation = functionDef.returnTypeAnnotation(); + if (returnTypeAnnotation == null) { + return; + } + if (symbol.is(Symbol.Kind.FUNCTION)) { + FunctionSymbolImpl functionSymbol = (FunctionSymbolImpl) symbol; + functionSymbol.setDeclaredReturnType(InferredTypes.declaredType(returnTypeAnnotation, builtins)); + } else if (symbol.is(Symbol.Kind.AMBIGUOUS)) { + Optional.ofNullable(((FunctionDefImpl) functionDef).functionSymbol()).ifPresent(functionSymbol -> setDeclaredReturnType(functionSymbol, functionDef)); + } + } + // visible for testing static Set typingModuleSymbols() { Map typingPython3 = getModuleSymbols("3/typing.pyi", TYPING); diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/AmbiguousSymbolTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/AmbiguousSymbolTest.java new file mode 100644 index 0000000000..a665c5ff93 --- /dev/null +++ b/python-frontend/src/test/java/org/sonar/python/semantic/AmbiguousSymbolTest.java @@ -0,0 +1,155 @@ +/* + * SonarQube Python Plugin + * Copyright (C) 2011-2020 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.python.semantic; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Function; +import java.util.stream.Collectors; +import org.junit.Test; +import org.sonar.plugins.python.api.symbols.AmbiguousSymbol; +import org.sonar.plugins.python.api.symbols.ClassSymbol; +import org.sonar.plugins.python.api.symbols.Symbol; +import org.sonar.plugins.python.api.tree.FileInput; +import org.sonar.python.PythonTestUtils; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.tuple; +import static org.sonar.python.PythonTestUtils.parse; + +public class AmbiguousSymbolTest { + + @Test + public void overloaded_functions() { + Symbol fn = symbols( + "from typing import overload", + "@overload", + "def fn(a, b): ...", + "@overload", + "def fn(a): ..." + ).get("fn"); + assertThat(fn.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); + assertThat(fn.name()).isEqualTo("fn"); + assertThat(fn.fullyQualifiedName()).isEqualTo("foo.fn"); + + AmbiguousSymbol overloadedFn = (AmbiguousSymbol) fn; + assertThat(overloadedFn.alternatives()).extracting(Symbol::kind).containsExactly(Symbol.Kind.FUNCTION, Symbol.Kind.FUNCTION); + } + + @Test + public void redefined_class() { + Symbol a = symbols( + "class A:", + " def meth(self): ...", + "A = 42" + ).get("A"); + + assertThat(a.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); + assertThat(a.name()).isEqualTo("A"); + assertThat(a.fullyQualifiedName()).isNull(); + + Set symbols = ((AmbiguousSymbol) a).alternatives(); + assertThat(symbols).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.CLASS, Symbol.Kind.OTHER); + ClassSymbol classSymbol = (ClassSymbol) symbols.stream().filter(symbol -> symbol.kind() == Symbol.Kind.CLASS).findFirst().get(); + assertThat(classSymbol.declaredMembers()).extracting(Symbol::name).containsExactly("meth"); + } + + @Test + public void redefined_class_member() { + ClassSymbol a = ((ClassSymbol) symbols( + "class A:", + " _foo = 42", + " _foo = 43", + " def meth(self, a, b, c): ...", + " def meth(self): ..." + ).get("A")); + + assertThat(a.declaredMembers()).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.AMBIGUOUS, Symbol.Kind.OTHER); + } + + @Test + public void global_ambiguous_symbol() { + Symbol x = symbols( + "def x(): ...", + "def foo():", + " global x", + " x = 42" + ).get("x"); + assertThat(x.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); + assertThat(((AmbiguousSymbol) x).alternatives()).extracting(Symbol::kind).containsExactlyInAnyOrder(Symbol.Kind.FUNCTION, Symbol.Kind.OTHER); + } + + @Test + public void not_ambiguous_symbols() { + Symbol x = symbols( + "x = 42", + "x = 43" + ).get("x"); + assertThat(x.kind()).isEqualTo(Symbol.Kind.OTHER); + } + + @Test(expected = IllegalArgumentException.class) + public void empty_ambiguous_symbol_creation() { + AmbiguousSymbolImpl.create(Collections.emptySet()); + } + + @Test(expected = IllegalArgumentException.class) + public void singleton_ambiguous_symbol_creation() { + AmbiguousSymbolImpl.create(Collections.singleton(new SymbolImpl("foo", null))); + } + + @Test(expected = IllegalArgumentException.class) + public void ambiguous_symbol_creation_different_name() { + SymbolImpl foo = new SymbolImpl("foo", "mod.foo"); + SymbolImpl bar = new SymbolImpl("bar", "mod.bar"); + AmbiguousSymbolImpl.create(new HashSet<>(Arrays.asList(foo, bar))); + } + + @Test + public void ambiguous_symbol_creation_different_fqn() { + SymbolImpl foo = new SymbolImpl("foo", "mod1.foo"); + SymbolImpl otherFoo = new SymbolImpl("foo", "mod2.foo"); + AmbiguousSymbol ambiguousSymbol = AmbiguousSymbolImpl.create(new HashSet<>(Arrays.asList(foo, otherFoo))); + assertThat(ambiguousSymbol.fullyQualifiedName()).isNull(); + assertThat(ambiguousSymbol.name()).isEqualTo("foo"); + assertThat(ambiguousSymbol.alternatives()).containsExactlyInAnyOrder(foo, otherFoo); + } + + @Test + public void copy_without_usages() { + SymbolImpl foo = new SymbolImpl("foo", "mod1.foo"); + SymbolImpl otherFoo = new SymbolImpl("foo", "mod2.foo"); + AmbiguousSymbol ambiguousSymbol = AmbiguousSymbolImpl.create(new HashSet<>(Arrays.asList(foo, otherFoo))); + AmbiguousSymbolImpl copy = ((AmbiguousSymbolImpl) ambiguousSymbol).copyWithoutUsages(); + assertThat(copy.is(Symbol.Kind.AMBIGUOUS)).isTrue(); + assertThat(copy.usages()).isEmpty(); + assertThat(copy).isNotEqualTo(ambiguousSymbol); + assertThat(copy.alternatives()).doesNotContain(foo, otherFoo); + assertThat(copy.alternatives()).extracting(Symbol::name, Symbol::fullyQualifiedName).containsExactlyInAnyOrder(tuple("foo", "mod1.foo"), tuple("foo", "mod2.foo")); + } + + private Map symbols(String... code) { + FileInput fileInput = parse(new SymbolTableBuilder("", PythonTestUtils.pythonFile("foo")), code); + return fileInput.globalVariables().stream().collect(Collectors.toMap(Symbol::name, Function.identity())); + } +} diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolTest.java index 4e9a583d59..4e435e1a57 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolTest.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/ClassSymbolTest.java @@ -150,8 +150,8 @@ public void multiple_bindings() { "C = \"hello\""); ClassDef classDef = (ClassDef) fileInput.statements().statements().get(0); Symbol symbol = classDef.name().symbol(); - assertThat(symbol instanceof ClassSymbol).isTrue(); - assertThat(symbol.kind().equals(Symbol.Kind.OTHER)).isTrue(); + assertThat(symbol instanceof ClassSymbol).isFalse(); + assertThat(symbol.kind().equals(Symbol.Kind.AMBIGUOUS)).isTrue(); } @Test diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/FunctionSymbolTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/FunctionSymbolTest.java index 344fda66fa..e31612835b 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/FunctionSymbolTest.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/FunctionSymbolTest.java @@ -117,7 +117,7 @@ public void reassigned_symbol() { ); FunctionDef functionDef = (FunctionDef) tree.statements().statements().get(0); Symbol symbol = functionDef.name().symbol(); - assertThat(symbol.kind()).isEqualTo(Symbol.Kind.OTHER); + assertThat(symbol.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); tree = parse( "fn = 42", @@ -125,7 +125,7 @@ public void reassigned_symbol() { ); functionDef = (FunctionDef) tree.statements().statements().get(1); symbol = functionDef.name().symbol(); - assertThat(symbol.kind()).isEqualTo(Symbol.Kind.OTHER); + assertThat(symbol.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); tree = parse( "def fn(p1, p2): pass", @@ -133,7 +133,7 @@ public void reassigned_symbol() { ); functionDef = (FunctionDef) tree.statements().statements().get(0); symbol = functionDef.name().symbol(); - assertThat(symbol.kind()).isEqualTo(Symbol.Kind.OTHER); + assertThat(symbol.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); } @Test diff --git a/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java b/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java index ace421f285..cc70326034 100644 --- a/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java +++ b/python-frontend/src/test/java/org/sonar/python/semantic/ProjectLevelSymbolTableTest.java @@ -326,4 +326,26 @@ public void multi_level_type_hierarchy() { assertThat(classB.superClasses()).hasSize(1); assertThat(classB.superClasses().get(0).kind()).isEqualTo(Symbol.Kind.CLASS); } + + @Test + public void ambiguous_imported_symbol() { + Set modSymbols = parse( + new SymbolTableBuilder("", pythonFile("mod")), + "@overload", + "def foo(a, b): ...", + "@overload", + "def foo(a, b, c): ..." + ).globalVariables(); + + Map> globalSymbols = Collections.singletonMap("mod", modSymbols); + FileInput tree = parse( + new SymbolTableBuilder("my_package", pythonFile("my_module.py"), globalSymbols), + "from mod import foo" + ); + Symbol importedFooSymbol = tree.globalVariables().iterator().next(); + assertThat(importedFooSymbol.name()).isEqualTo("foo"); + assertThat(importedFooSymbol.kind()).isEqualTo(Symbol.Kind.AMBIGUOUS); + assertThat(importedFooSymbol.fullyQualifiedName()).isEqualTo("mod.foo"); + assertThat(importedFooSymbol.usages()).hasSize(1); + } }