diff --git a/python/resources/inspectionDescriptions/PyShadowingBuiltinsInspection.html b/python/resources/inspectionDescriptions/PyShadowingBuiltinsInspection.html new file mode 100644 index 000000000000..f25c27847c4f --- /dev/null +++ b/python/resources/inspectionDescriptions/PyShadowingBuiltinsInspection.html @@ -0,0 +1,5 @@ + + +This inspection detects shadowing built-in names, such as 'len' or 'list'. + + \ No newline at end of file diff --git a/python/resources/inspectionDescriptions/PyShadowingNamesInspection.html b/python/resources/inspectionDescriptions/PyShadowingNamesInspection.html index 3f32f07ca884..dabfaf0b94a1 100644 --- a/python/resources/inspectionDescriptions/PyShadowingNamesInspection.html +++ b/python/resources/inspectionDescriptions/PyShadowingNamesInspection.html @@ -1,5 +1,5 @@ -This inspection detects shadowing names defined in outer scopes, including built-in names. +This inspection detects shadowing names defined in outer scopes. \ No newline at end of file diff --git a/python/src/META-INF/python-plugin-core.xml b/python/src/META-INF/python-plugin-core.xml index 12f60d8d6362..8858ffbac17e 100644 --- a/python/src/META-INF/python-plugin-core.xml +++ b/python/src/META-INF/python-plugin-core.xml @@ -323,7 +323,8 @@ - + + diff --git a/python/src/com/jetbrains/python/inspections/PyShadowingBuiltinsInspection.java b/python/src/com/jetbrains/python/inspections/PyShadowingBuiltinsInspection.java new file mode 100644 index 000000000000..94b06b54e474 --- /dev/null +++ b/python/src/com/jetbrains/python/inspections/PyShadowingBuiltinsInspection.java @@ -0,0 +1,144 @@ +package com.jetbrains.python.inspections; + +import com.google.common.collect.ImmutableSet; +import com.intellij.codeInsight.intention.LowPriorityAction; +import com.intellij.codeInspection.*; +import com.intellij.codeInspection.ui.ListEditForm; +import com.intellij.openapi.project.Project; +import com.intellij.profile.codeInspection.InspectionProjectProfileManager; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiNameIdentifierOwner; +import com.intellij.util.Consumer; +import com.jetbrains.python.codeInsight.controlflow.ScopeOwner; +import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil; +import com.jetbrains.python.psi.*; +import com.jetbrains.python.psi.impl.PyBuiltinCache; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import javax.swing.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Set; + +/** + * Warns about shadowing built-in names. + * + * @author vlan + */ +public class PyShadowingBuiltinsInspection extends PyInspection { + // Persistent settings + public List ignoredNames = new ArrayList(); + + @NotNull + @Override + public String getDisplayName() { + return "Shadowing built-ins"; + } + + @Override + public JComponent createOptionsPanel() { + final ListEditForm form = new ListEditForm("Ignore built-ins", ignoredNames); + return form.getContentPanel(); + } + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, + boolean isOnTheFly, + @NotNull LocalInspectionToolSession session) { + return new Visitor(holder, session, ignoredNames); + } + + private static class Visitor extends PyInspectionVisitor { + private final Set myIgnoredNames; + + public Visitor(@Nullable ProblemsHolder holder, @NotNull LocalInspectionToolSession session, @NotNull Collection ignoredNames) { + super(holder, session); + myIgnoredNames = ImmutableSet.copyOf(ignoredNames); + } + + @Override + public void visitPyClass(@NotNull PyClass node) { + processElement(node); + } + + @Override + public void visitPyFunction(@NotNull PyFunction node) { + processElement(node); + } + + @Override + public void visitPyNamedParameter(@NotNull PyNamedParameter node) { + processElement(node); + } + + @Override + public void visitPyTargetExpression(@NotNull PyTargetExpression node) { + if (node.getQualifier() == null) { + processElement(node); + } + } + + private void processElement(@NotNull PsiNameIdentifierOwner element) { + final ScopeOwner owner = ScopeUtil.getScopeOwner(element); + if (owner instanceof PyClass) { + return; + } + final String name = element.getName(); + if (name != null && !myIgnoredNames.contains(name)) { + final PyBuiltinCache builtinCache = PyBuiltinCache.getInstance(element); + final PsiElement builtin = builtinCache.getByName(name); + if (builtin != null && !PyUtil.inSameFile(builtin, element)) { + final PsiElement identifier = element.getNameIdentifier(); + final PsiElement problemElement = identifier != null ? identifier : element; + registerProblem(problemElement, String.format("Shadows built-in name '%s'", name), + ProblemHighlightType.WEAK_WARNING, null, new PyRenameElementQuickFix(), new PyIgnoreBuiltinQuickFix(name)); + } + } + } + + private static class PyIgnoreBuiltinQuickFix implements LocalQuickFix, LowPriorityAction { + @NotNull private final String myName; + + private PyIgnoreBuiltinQuickFix(@NotNull String name) { + myName = name; + } + + @NotNull + @Override + public String getName() { + return getFamilyName() + " \"" + myName + "\""; + } + + @NotNull + @Override + public String getFamilyName() { + return "Ignore shadowed built-in name"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement(); + if (element != null) { + final InspectionProfile profile = InspectionProjectProfileManager.getInstance(project).getInspectionProfile(); + profile.modifyProfile(new Consumer() { + @Override + public void consume(ModifiableModel model) { + final String toolName = PyShadowingBuiltinsInspection.class.getSimpleName(); + final PyShadowingBuiltinsInspection inspection = (PyShadowingBuiltinsInspection)model.getUnwrappedTool(toolName, element); + if (inspection != null) { + if (!inspection.ignoredNames.contains(myName)) { + inspection.ignoredNames.add(myName); + } + } + } + }); + } + } + } + } +} + diff --git a/python/src/com/jetbrains/python/inspections/PyShadowingNamesInspection.java b/python/src/com/jetbrains/python/inspections/PyShadowingNamesInspection.java index c19945da06df..aad248db7c95 100644 --- a/python/src/com/jetbrains/python/inspections/PyShadowingNamesInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyShadowingNamesInspection.java @@ -1,52 +1,32 @@ package com.jetbrains.python.inspections; -import com.google.common.collect.ImmutableSet; -import com.intellij.codeInsight.intention.LowPriorityAction; -import com.intellij.codeInspection.*; -import com.intellij.codeInspection.ui.ListEditForm; -import com.intellij.openapi.project.Project; -import com.intellij.profile.codeInspection.InspectionProjectProfileManager; +import com.intellij.codeInspection.LocalInspectionToolSession; +import com.intellij.codeInspection.ProblemHighlightType; +import com.intellij.codeInspection.ProblemsHolder; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.psi.PsiNameIdentifierOwner; import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.util.Consumer; import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache; import com.jetbrains.python.codeInsight.controlflow.ScopeOwner; import com.jetbrains.python.codeInsight.dataflow.scope.Scope; import com.jetbrains.python.codeInsight.dataflow.scope.ScopeUtil; import com.jetbrains.python.psi.*; -import com.jetbrains.python.psi.impl.PyBuiltinCache; import com.jetbrains.python.psi.resolve.PyResolveUtil; import com.jetbrains.python.psi.resolve.ResolveProcessor; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import javax.swing.*; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; -import java.util.Set; - /** * Warns about shadowing names defined in outer scopes. * * @author vlan */ public class PyShadowingNamesInspection extends PyInspection { - // Persistent settings - public List ignoredNames = new ArrayList(); - @NotNull @Override public String getDisplayName() { - return "Shadowing names"; - } - - @Override - public JComponent createOptionsPanel() { - final ListEditForm form = new ListEditForm("Ignore built-ins", ignoredNames); - return form.getContentPanel(); + return "Shadowing names from outer scopes"; } @NotNull @@ -54,15 +34,12 @@ public class PyShadowingNamesInspection extends PyInspection { public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly, @NotNull LocalInspectionToolSession session) { - return new Visitor(holder, session, ignoredNames); + return new Visitor(holder, session); } private static class Visitor extends PyInspectionVisitor { - private final Set myIgnoredNames; - - public Visitor(@Nullable ProblemsHolder holder, @NotNull LocalInspectionToolSession session, @NotNull Collection ignoredNames) { + public Visitor(@Nullable ProblemsHolder holder, @NotNull LocalInspectionToolSession session) { super(holder, session); - myIgnoredNames = ImmutableSet.copyOf(ignoredNames); } @Override @@ -92,99 +69,35 @@ public class PyShadowingNamesInspection extends PyInspection { private void processElement(@NotNull PsiNameIdentifierOwner element) { final ScopeOwner owner = ScopeUtil.getScopeOwner(element); - // Class-level names are rarely accessed inside at the class level, usually they are accessed as attributes if (owner instanceof PyClass) { return; } final String name = element.getName(); - if (name != null && !myIgnoredNames.contains(name)) { + if (name != null) { final PsiElement identifier = element.getNameIdentifier(); final PsiElement problemElement = identifier != null ? identifier : element; - final PyBuiltinCache builtinCache = PyBuiltinCache.getInstance(element); - final PsiElement builtin = builtinCache.getByName(name); - if (builtin != null) { - processBuiltin(element, name, problemElement, builtin); + if ("_".equals(name)) { + return; } - else { - processOuterScope(element, name, problemElement); - } - } - } - - private void processBuiltin(@NotNull PsiNameIdentifierOwner element, - @NotNull String name, - @NotNull PsiElement problemElement, - @NotNull PsiElement builtin) { - if (!PyUtil.inSameFile(builtin, element)) { - registerProblem(problemElement, String.format("Shadows built-in name '%s'", name), - ProblemHighlightType.WEAK_WARNING, null, new PyRenameElementQuickFix(), - new PyIgnoreBuiltinQuickFix(name)); - } - } - - private void processOuterScope(@NotNull PsiNameIdentifierOwner element, @NotNull String name, @NotNull PsiElement problemElement) { - if ("_".equals(name)) { - return; - } - final ScopeOwner owner = ScopeUtil.getScopeOwner(element); - if (owner != null) { - final ScopeOwner nextOwner = ScopeUtil.getScopeOwner(owner); - if (nextOwner != null) { - final ResolveProcessor processor = new ResolveProcessor(name); - PyResolveUtil.scopeCrawlUp(processor, nextOwner, null, name, null); - final PsiElement resolved = processor.getResult(); - if (resolved != null) { - final PyComprehensionElement comprehension = PsiTreeUtil.getParentOfType(resolved, PyComprehensionElement.class); - if (comprehension != null && PyUtil.isOwnScopeComprehension(comprehension)) { - return; - } - final Scope scope = ControlFlowCache.getScope(owner); - if (scope.isGlobal(name) || scope.isNonlocal(name)) { - return; - } - registerProblem(problemElement, String.format("Shadows name '%s' from outer scope", name), - ProblemHighlightType.WEAK_WARNING, null, new PyRenameElementQuickFix()); - } - } - } - } - - private static class PyIgnoreBuiltinQuickFix implements LocalQuickFix, LowPriorityAction { - @NotNull private final String myName; - - private PyIgnoreBuiltinQuickFix(@NotNull String name) { - myName = name; - } - - @NotNull - @Override - public String getName() { - return getFamilyName() + " \"" + myName + "\""; - } - - @NotNull - @Override - public String getFamilyName() { - return "Ignore shadowed built-in name"; - } - - @Override - public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - final PsiElement element = descriptor.getPsiElement(); - if (element != null) { - final InspectionProfile profile = InspectionProjectProfileManager.getInstance(project).getInspectionProfile(); - profile.modifyProfile(new Consumer() { - @Override - public void consume(ModifiableModel model) { - final String toolName = PyShadowingNamesInspection.class.getSimpleName(); - final PyShadowingNamesInspection inspection = (PyShadowingNamesInspection)model.getUnwrappedTool(toolName, element); - if (inspection != null) { - if (!inspection.ignoredNames.contains(myName)) { - inspection.ignoredNames.add(myName); - } + if (owner != null) { + final ScopeOwner nextOwner = ScopeUtil.getScopeOwner(owner); + if (nextOwner != null) { + final ResolveProcessor processor = new ResolveProcessor(name); + PyResolveUtil.scopeCrawlUp(processor, nextOwner, null, name, null); + final PsiElement resolved = processor.getResult(); + if (resolved != null) { + final PyComprehensionElement comprehension = PsiTreeUtil.getParentOfType(resolved, PyComprehensionElement.class); + if (comprehension != null && PyUtil.isOwnScopeComprehension(comprehension)) { + return; } + final Scope scope = ControlFlowCache.getScope(owner); + if (scope.isGlobal(name) || scope.isNonlocal(name)) { + return; + } + registerProblem(problemElement, String.format("Shadows name '%s' from outer scope", name), + ProblemHighlightType.WEAK_WARNING, null, new PyRenameElementQuickFix()); } - }); + } } } } diff --git a/python/testData/inspections/PyShadowingBuiltinsInspection/test.py b/python/testData/inspections/PyShadowingBuiltinsInspection/test.py new file mode 100644 index 000000000000..4951bf1fdf50 --- /dev/null +++ b/python/testData/inspections/PyShadowingBuiltinsInspection/test.py @@ -0,0 +1,50 @@ +def test_import_builtin_names(): + import float + from foo import float + from bar import baz as float + + +def test_builtin_function_parameters(): + def test1(x, _, len, file=None): + pass + + +def test_builtin_function_name(): + def list(): + pass + + +def test_builtin_assignment_targets(): + foo = 2 + list = [] + for int in range(10): + print(int) + range = [] + list, _ = (1, 2) + return [int for int in range(10)] + + +def test_builtin_class_name(): + class list(object): + pass + + +def test_builtin_method_name(): + class C: + def list(self): + pass + + +# PY-8646 +def test_builtin_qualified_name(): + test1.range = float() + + class C: + def foo(self): + self.list = [] + + +# PY-10164 +def test_builtin_class_attribute(): + class C: + id = 1 diff --git a/python/testData/inspections/PyShadowingNamesInspection/test.py b/python/testData/inspections/PyShadowingNamesInspection/test.py index 98eec4846722..b87b8b27bae3 100644 --- a/python/testData/inspections/PyShadowingNamesInspection/test.py +++ b/python/testData/inspections/PyShadowingNamesInspection/test.py @@ -1,58 +1,6 @@ global_foo = 1 -def test_import_builtin_names(): - import float - from foo import float - from bar import baz as float - - -def test_builtin_function_parameters(): - def test1(x, _, len, file=None): - pass - - -def test_builtin_function_name(): - def list(): - pass - - -def test_builtin_assignment_targets(): - foo = 2 - list = [] - for int in range(10): - print(int) - range = [] - list, _ = (1, 2) - return [int for int in range(10)] - - -def test_builtin_class_name(): - class list(object): - pass - - -def test_builtin_method_name(): - class C: - def list(self): - pass - - -# PY-8646 -def test_builtin_qualified_name(): - test1.range = float() - - class C: - def foo(self): - self.list = [] - - -# PY-10164 -def test_builtin_class_attribute(): - class C: - id = 1 - - def test_outer_function(): foo = 1 def bar(): @@ -91,7 +39,7 @@ def test_outer_global(): global_foo = 2 -def test_comprehensions(): +def test_outer_comprehensions(): print(x for x in range(10)) print([y for y in range(10)]) def f(x, y): diff --git a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java index a1b195d9b8df..d75aa2b88956 100644 --- a/python/testSrc/com/jetbrains/python/PyQuickFixTest.java +++ b/python/testSrc/com/jetbrains/python/PyQuickFixTest.java @@ -445,7 +445,7 @@ public class PyQuickFixTest extends PyTestCase { public void testRenameFunctionShadowingBuiltins() { final String fileName = "RenameFunctionShadowingBuiltins.py"; myFixture.configureByFile(fileName); - myFixture.enableInspections(PyShadowingNamesInspection.class); + myFixture.enableInspections(PyShadowingBuiltinsInspection.class); myFixture.checkHighlighting(true, false, true); final IntentionAction intentionAction = myFixture.getAvailableIntention("Rename element"); assertNotNull(intentionAction); diff --git a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java index 183bad8ed613..06ce32a3e896 100644 --- a/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java +++ b/python/testSrc/com/jetbrains/python/PythonInspectionsTest.java @@ -295,6 +295,10 @@ public class PythonInspectionsTest extends PyTestCase { } // PY-5807 + public void testPyShadowingBuiltinsInspection() { + doHighlightingTest(PyShadowingBuiltinsInspection.class); + } + public void testPyShadowingNamesInspection() { doHighlightingTest(PyShadowingNamesInspection.class); }