From b89ea3103c97339e0d1bfa211868f54a3c9232d8 Mon Sep 17 00:00:00 2001 From: Mikhail Golubev Date: Mon, 10 Nov 2014 19:48:39 +0300 Subject: [PATCH] PY-13537 Use configurable list of ignored classes instead of checkbox We've decided that possibility to suppress warnings about bad method names for *all* standard classes is too general and may be non-obvious for user. After all if inspections is too bothering it's simpler to disable it at all. Much better way is too allow to configure black-list of concrete classes for descendants of which such warnings will be suppressed (as already done for some other inspections in PyCharm). --- .../inspections/PyPep8NamingInspection.java | 140 +++++++++++++----- .../PyPep8NamingInspectionTest.java | 6 +- 2 files changed, 110 insertions(+), 36 deletions(-) diff --git a/python/src/com/jetbrains/python/inspections/PyPep8NamingInspection.java b/python/src/com/jetbrains/python/inspections/PyPep8NamingInspection.java index 7dc0148899ec..b7323c87e6e5 100644 --- a/python/src/com/jetbrains/python/inspections/PyPep8NamingInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyPep8NamingInspection.java @@ -15,31 +15,43 @@ */ package com.jetbrains.python.inspections; -import com.intellij.codeInspection.LocalInspectionToolSession; -import com.intellij.codeInspection.ProblemsHolder; -import com.intellij.codeInspection.ui.MultipleCheckboxOptionsPanel; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.intellij.codeInspection.*; +import com.intellij.codeInspection.ui.ListEditForm; +import com.intellij.ide.DataManager; import com.intellij.lang.ASTNode; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.projectRoots.Sdk; -import com.intellij.openapi.util.Condition; +import com.intellij.openapi.actionSystem.DataContext; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.popup.PopupChooserBuilder; +import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.PsiElementVisitor; -import com.intellij.psi.PsiFile; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.QualifiedName; +import com.intellij.ui.components.JBList; +import com.intellij.util.Consumer; +import com.intellij.util.Function; +import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.ui.CheckBox; import com.jetbrains.python.codeInsight.controlflow.ControlFlowCache; import com.jetbrains.python.codeInsight.dataflow.scope.Scope; import com.jetbrains.python.inspections.quickfix.PyRenameElementQuickFix; import com.jetbrains.python.psi.*; -import com.jetbrains.python.psi.impl.PyBuiltinCache; import com.jetbrains.python.psi.search.PySuperMethodsSearch; -import com.jetbrains.python.sdk.PythonSdkType; +import com.jetbrains.python.psi.types.TypeEvalContext; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; +import java.awt.*; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; import java.util.regex.Pattern; +import static com.intellij.util.containers.ContainerUtilRt.addIfNotNull; + /** * User : ktisha */ @@ -49,16 +61,13 @@ public class PyPep8NamingInspection extends PyInspection { private static final Pattern MIXEDCASE_REGEX = Pattern.compile("_?[\\p{javaUpperCase}][\\p{javaLowerCase}\\p{javaUpperCase}0-9]*"); public boolean ignoreOverriddenFunctions = true; - public boolean ignoreDescendantsOfStandardClasses = false; + public List ignoredBaseClasses = Lists.newArrayList("unittest.TestCase", "unittest.case.TestCase"); @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly, @NotNull LocalInspectionToolSession session) { - if (ApplicationManager.getApplication().isUnitTestMode()) { - ignoreDescendantsOfStandardClasses = true; - } return new Visitor(holder, session); } @@ -102,16 +111,18 @@ public class PyPep8NamingInspection extends PyInspection { if (ignoreOverriddenFunctions && isOverriddenMethod(function)) return; final String name = function.getName(); if (name == null) return; - if (containingClass != null && PyUtil.isSpecialName(name)) { - return; - } - if (containingClass != null && ignoreDescendantsOfStandardClasses && isStandardClassDescendant(containingClass)) { + if (containingClass != null && (PyUtil.isSpecialName(name) || isIgnoredOrHasIgnoredAncestor(containingClass))) { return; } if (!LOWERCASE_REGEX.matcher(name).matches()) { final ASTNode nameNode = function.getNameNode(); - if (nameNode != null) - registerProblem(nameNode.getPsi(), "Function name should be lowercase", new PyRenameElementQuickFix()); + if (nameNode != null) { + final List quickFixes = new SmartList(new PyRenameElementQuickFix()); + if (containingClass != null) { + quickFixes.add(new IgnoreBaseClassQuickFix(containingClass, myTypeEvalContext)); + } + registerProblem(nameNode.getPsi(), "Function name should be lowercase", quickFixes.toArray(new LocalQuickFix[quickFixes.size()])); + } } } @@ -119,18 +130,17 @@ public class PyPep8NamingInspection extends PyInspection { return PySuperMethodsSearch.search(function).findFirst() != null; } - private boolean isStandardClassDescendant(@NotNull final PyClass cls) { - return ContainerUtil.exists(cls.getAncestorClasses(myTypeEvalContext), new Condition() { - @Override - public boolean value(PyClass ancestor) { - final PsiFile ancestorsModule = ancestor.getContainingFile(); - final Sdk sdk = PyBuiltinCache.findSdkForFile(ancestorsModule); - if (PythonSdkType.isStdLib(ancestorsModule.getVirtualFile(), sdk) && !PyUtil.isObjectClass(ancestor)) { - return true; - } - return false; + private boolean isIgnoredOrHasIgnoredAncestor(@NotNull PyClass pyClass) { + final Set blackList = Sets.newHashSet(ignoredBaseClasses); + if (blackList.contains(pyClass.getQualifiedName())) { + return true; + } + for (PyClass ancestor : pyClass.getAncestorClasses(myTypeEvalContext)) { + if (blackList.contains(ancestor.getQualifiedName())) { + return true; } - }); + } + return false; } @Override @@ -139,8 +149,9 @@ public class PyPep8NamingInspection extends PyInspection { if (name == null) return; if (!MIXEDCASE_REGEX.matcher(name).matches()) { final ASTNode nameNode = node.getNameNode(); - if (nameNode != null) + if (nameNode != null) { registerProblem(nameNode.getPsi(), "Class names should use CamelCase convention", new PyRenameElementQuickFix()); + } } } @@ -171,12 +182,71 @@ public class PyPep8NamingInspection extends PyInspection { } } + private static class IgnoreBaseClassQuickFix implements LocalQuickFix { + final List myBaseClassNames; + + public IgnoreBaseClassQuickFix(@NotNull PyClass baseClass, @NotNull TypeEvalContext context) { + myBaseClassNames = new ArrayList(); + ContainerUtil.addIfNotNull(myBaseClassNames, baseClass.getQualifiedName()); + for (PyClass ancestor : baseClass.getAncestorClasses(context)) { + ContainerUtil.addIfNotNull(myBaseClassNames, ancestor.getQualifiedName()); + } + } + + @NotNull + @Override + public String getName() { + return "Ignore method names for descendants of class"; + } + + @NotNull + @Override + public String getFamilyName() { + return getName(); + } + + @Override + public void applyFix(@NotNull final Project project, @NotNull final ProblemDescriptor descriptor) { + final JBList list = new JBList(myBaseClassNames); + final Runnable updateBlackList = new Runnable() { + @Override + public void run() { + final InspectionProfile profile = InspectionProjectProfileManager.getInstance(project).getInspectionProfile(); + profile.modifyProfile(new Consumer() { + @Override + public void consume(ModifiableModel model) { + final PyPep8NamingInspection inspection = (PyPep8NamingInspection)model + .getUnwrappedTool(PyPep8NamingInspection.class.getSimpleName(), descriptor.getPsiElement()); + addIfNotNull(inspection.ignoredBaseClasses, (String)list.getSelectedValue()); + } + }); + } + }; + DataManager.getInstance().getDataContextFromFocus().doWhenDone(new Consumer() { + @Override + public void consume(DataContext dataContext) { + new PopupChooserBuilder(list) + .setTitle("Ignore base class") + .setItemChoosenCallback(updateBlackList) + .setFilteringEnabled(new Function() { + @Override + public String fun(Object o) { + return (String)o; + } + }) + .createPopup() + .showInBestPositionFor(dataContext); + } + }); + } + } + @Nullable @Override public JComponent createOptionsPanel() { - MultipleCheckboxOptionsPanel panel = new MultipleCheckboxOptionsPanel(this); - panel.addCheckbox("Ignore overridden functions", "ignoreOverriddenFunctions"); - panel.addCheckbox("Ignore descendants of standard classes", "ignoreDescendantsOfStandardClasses"); - return panel; + final JPanel rootPanel = new JPanel(new BorderLayout()); + rootPanel.add(new CheckBox("Ignore overridden functions", this, "ignoreOverriddenFunctions"), BorderLayout.NORTH); + rootPanel.add(new ListEditForm("Excluded base classes", ignoredBaseClasses).getContentPanel(), BorderLayout.CENTER); + return rootPanel; } } diff --git a/python/testSrc/com/jetbrains/python/inspections/PyPep8NamingInspectionTest.java b/python/testSrc/com/jetbrains/python/inspections/PyPep8NamingInspectionTest.java index 8e2a6bfa2010..89b8b7a877fa 100644 --- a/python/testSrc/com/jetbrains/python/inspections/PyPep8NamingInspectionTest.java +++ b/python/testSrc/com/jetbrains/python/inspections/PyPep8NamingInspectionTest.java @@ -67,7 +67,11 @@ public class PyPep8NamingInspectionTest extends PyTestCase { // PY-13537 public void testDescendantOfStandardClass() { - doTest(); + myFixture.configureByFile("inspections/PyPep8NamingInspection/" + getTestName(true) + ".py"); + final PyPep8NamingInspection inspection = new PyPep8NamingInspection(); + inspection.ignoredBaseClasses.add("collections.OrderedDict"); + myFixture.enableInspections(inspection); + myFixture.checkHighlighting(false, false, true); } public void testTest() {