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).

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<String> 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<LocalQuickFix> quickFixes = new SmartList<LocalQuickFix>(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<PyClass>() {
-        @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<String> 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<String> myBaseClassNames;
+
+    public IgnoreBaseClassQuickFix(@NotNull PyClass baseClass, @NotNull TypeEvalContext context) {
+      myBaseClassNames = new ArrayList<String>();
+      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<ModifiableModel>() {
+            @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<DataContext>() {
+        @Override
+        public void consume(DataContext dataContext) {
+          new PopupChooserBuilder(list)
+            .setTitle("Ignore base class")
+            .setItemChoosenCallback(updateBlackList)
+            .setFilteringEnabled(new Function<Object, String>() {
+              @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;
   }
 }

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() {