Fixed #11: Stopping old instances of PyLint when requesting new ones

leinardi · Dec 4, 2021 · 77c1a97 · 77c1a97
1 parent 76d49ca
commit 77c1a97
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 40 deletions.
diff --git a/...ardi/pycharm/pylint/PylintInspection.java → ...nardi/pycharm/pylint/PylintAnnotator.java b/...ardi/pycharm/pylint/PylintInspection.java → ...nardi/pycharm/pylint/PylintAnnotator.java
@@ -16,12 +16,15 @@
 
 package com.leinardi.pycharm.pylint;
 
-import com.intellij.codeInspection.InspectionManager;
-import com.intellij.codeInspection.LocalInspectionTool;
-import com.intellij.codeInspection.ProblemDescriptor;
+import com.intellij.codeInsight.daemon.HighlightDisplayKey;
+import com.intellij.codeInspection.InspectionProfile;
+import com.intellij.lang.annotation.AnnotationHolder;
+import com.intellij.lang.annotation.ExternalAnnotator;
+import com.intellij.lang.annotation.HighlightSeverity;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.progress.ProcessCanceledException;
 import com.intellij.openapi.project.Project;
+import com.intellij.profile.codeInspection.InspectionProjectProfileManager;
 import com.intellij.psi.PsiFile;
 import com.leinardi.pycharm.pylint.checker.Problem;
 import com.leinardi.pycharm.pylint.checker.ScanFiles;
@@ -38,16 +41,28 @@
 import java.util.Map;
 
 import static com.leinardi.pycharm.pylint.PylintBundle.message;
-import static com.leinardi.pycharm.pylint.util.Async.asyncResultOf;
 import static com.leinardi.pycharm.pylint.util.Notifications.showException;
 import static com.leinardi.pycharm.pylint.util.Notifications.showWarning;
 import static java.util.Collections.singletonList;
-import static java.util.Optional.ofNullable;
 
-public class PylintInspection extends LocalInspectionTool {
+/**
+ * Using the `ExternalAnnotator` API instead of `LocalInspectionTool`, because the former has better behavior with
+ * long-running expensive checkers like Pylint. Following multiple successive changes to a file, `LocalInspectionTool`
+ * can invoke the checker for each modification from multiple threads in parallel, which can bog down the system
+ * (see https://github.com/leinardi/pylint-pycharm/issues/11).
+ * `ExternalAnnotator` cancels the previous running check (if any) before running the next one.
+ * <p>
+ * Modeled after `com.jetbrains.python.validation.Pep8ExternalAnnotator`
+ * <p>
+ * IDE calls methods in three phases:
+ * 1. `State collectInformation(PsiFile)`: preparation.
+ * 2. `Results doAnnotate(State)`: called in the background.
+ * 3. `void apply(PsiFile, Results, AnnotationHolder)`: apply annotations to the editor.
+ */
+public class PylintAnnotator extends ExternalAnnotator<PylintAnnotator.State, PylintAnnotator.Results> {
 
-    private static final Logger LOG = Logger.getInstance(PylintInspection.class);
-    private static final List<Problem> NO_PROBLEMS_FOUND = Collections.emptyList();
+    private static final Logger LOG = Logger.getInstance(PylintAnnotator.class);
+    private static final Results NO_PROBLEMS_FOUND = new Results(Collections.emptyList());
     private static final String ERROR_MESSAGE_ID_SYNTAX_ERROR = "E0001";
 
     private PylintPlugin plugin(final Project project) {
@@ -58,20 +73,32 @@ private PylintPlugin plugin(final Project project) {
         return pylintPlugin;
     }
 
+    /**
+     * Integration with `PylintBatchInspection`
+     */
     @Override
-    public ProblemDescriptor[] checkFile(@NotNull final PsiFile psiFile,
-                                         @NotNull final InspectionManager manager,
-                                         final boolean isOnTheFly) {
-        return asProblemDescriptors(asyncResultOf(() -> inspectFile(psiFile, manager), NO_PROBLEMS_FOUND),
-                manager);
+    public String getPairedBatchInspectionShortName() {
+        return PylintBatchInspection.INSPECTION_SHORT_NAME;
     }
 
     @Nullable
-    public List<Problem> inspectFile(@NotNull final PsiFile psiFile,
-                                     @NotNull final InspectionManager manager) {
-        LOG.debug("Inspection has been invoked.");
+    @Override
+    public State collectInformation(@NotNull PsiFile file) {
+        LOG.debug("Pylint collectInformation " + file.getName()
+                + " modified=" + file.getModificationStamp()
+                + " thread=" + Thread.currentThread().getName()
+        );
+
+        return new State(file);
+    }
 
-        final PylintPlugin plugin = plugin(manager.getProject());
+    @Nullable
+    @Override
+    public Results doAnnotate(State state) {
+        PsiFile psiFile = state.file;
+        Project project = psiFile.getProject();
+        final PylintPlugin plugin = plugin(project);
+        long startTime = System.currentTimeMillis();
 
         if (!PylintRunner.checkPylintAvailable(plugin.getProject())) {
             LOG.debug("Scan failed: Pylint not available.");
@@ -91,7 +118,10 @@ public List<Problem> inspectFile(@NotNull final PsiFile psiFile,
             if (map.isEmpty()) {
                 return NO_PROBLEMS_FOUND;
             }
-            return map.get(psiFile);
+
+            long duration = System.currentTimeMillis() - startTime;
+            LOG.debug("Pylint scan completed: " + psiFile.getName() + " in " + duration + " ms");
+            return new Results(map.get(psiFile));
 
         } catch (ProcessCanceledException | AssertionError e) {
             LOG.debug("Process cancelled when scanning: " + psiFile.getName());
@@ -102,14 +132,34 @@ public List<Problem> inspectFile(@NotNull final PsiFile psiFile,
             return NO_PROBLEMS_FOUND;
 
         } catch (Throwable e) {
-            handlePluginException(e, psiFile, manager.getProject());
+            handlePluginException(e, psiFile, project);
             return NO_PROBLEMS_FOUND;
 
         } finally {
             scannableFiles.forEach(ScannableFile::deleteIfRequired);
         }
     }
 
+    @Override
+    public void apply(@NotNull PsiFile file, Results results, @NotNull AnnotationHolder holder) {
+        if (results == null || !file.isValid()) {
+            return;
+        }
+
+        LOG.debug("Applying " + results.issues.size() + " annotations for " + file.getName());
+
+        // Get severity from inspection profile
+        final InspectionProfile profile =
+                InspectionProjectProfileManager.getInstance(file.getProject()).getCurrentProfile();
+        final HighlightDisplayKey key = HighlightDisplayKey.find(PylintBatchInspection.INSPECTION_SHORT_NAME);
+        HighlightSeverity severity = profile.getErrorLevel(key, file).getSeverity();
+
+        for (Problem problem : results.issues) {
+            LOG.debug("                " + problem.getLine() + ": " + problem.getMessage());
+            problem.createAnnotation(holder, severity);
+        }
+    }
+
     private void handlePluginException(final Throwable e,
                                        final @NotNull PsiFile psiFile,
                                        final @NotNull Project project) {
@@ -125,12 +175,20 @@ private void handlePluginException(final Throwable e,
         }
     }
 
-    @NotNull
-    private ProblemDescriptor[] asProblemDescriptors(final List<Problem> results, final InspectionManager manager) {
-        return ofNullable(results)
-                .map(problems -> problems.stream()
-                        .map(problem -> problem.toProblemDescriptor(manager))
-                        .toArray(ProblemDescriptor[]::new))
-                .orElse(ProblemDescriptor.EMPTY_ARRAY);
+    /* Inner classes storing intermediate results */
+    static class State {
+        PsiFile file;
+
+        public State(PsiFile file) {
+            this.file = file;
+        }
+    }
+
+    static class Results {
+        List<Problem> issues;
+
+        public Results(List<Problem> issues) {
+            this.issues = issues;
+        }
     }
 }
diff --git a/src/main/java/com/leinardi/pycharm/pylint/PylintBatchInspection.java b/src/main/java/com/leinardi/pycharm/pylint/PylintBatchInspection.java
@@ -0,0 +1,21 @@
+package com.leinardi.pycharm.pylint;
+
+import com.intellij.codeInspection.LocalInspectionTool;
+import com.intellij.codeInspection.ex.ExternalAnnotatorBatchInspection;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * By itself, the `PylintAnnotator` class does not provide support for the explicit "Inspect code" feature.
+ *
+ * This class uses `ExternalAnnotatorBatchInspection` middleware to provides that functionality.
+ *
+ * Modeled after `com.jetbrains.python.inspections.PyPep8Inspection`
+ */
+public class PylintBatchInspection extends LocalInspectionTool implements ExternalAnnotatorBatchInspection {
+    public static final String INSPECTION_SHORT_NAME = "Pylint";
+
+    @Override
+    public @NotNull String getShortName() {
+        return INSPECTION_SHORT_NAME;
+    }
+}
diff --git a/src/main/java/com/leinardi/pycharm/pylint/checker/Problem.java b/src/main/java/com/leinardi/pycharm/pylint/checker/Problem.java
@@ -16,9 +16,9 @@
 
 package com.leinardi.pycharm.pylint.checker;
 
-import com.intellij.codeInspection.InspectionManager;
-import com.intellij.codeInspection.ProblemDescriptor;
-import com.intellij.codeInspection.ProblemHighlightType;
+import com.intellij.lang.annotation.AnnotationBuilder;
+import com.intellij.lang.annotation.AnnotationHolder;
+import com.intellij.lang.annotation.HighlightSeverity;
 import com.intellij.psi.PsiElement;
 import com.leinardi.pycharm.pylint.PylintBundle;
 import com.leinardi.pycharm.pylint.plapi.SeverityLevel;
@@ -58,11 +58,15 @@ public Problem(final PsiElement target,
         this.suppressErrors = suppressErrors;
     }
 
-    @NotNull
-    public ProblemDescriptor toProblemDescriptor(final InspectionManager inspectionManager) {
-        return inspectionManager.createProblemDescriptor(target,
-                PylintBundle.message("inspection.message", getMessage()),
-                null, problemHighlightType(), false, afterEndOfLine);
+    public void createAnnotation(@NotNull AnnotationHolder holder, @NotNull HighlightSeverity severity) {
+        String message = PylintBundle.message("inspection.message", getMessage());
+        AnnotationBuilder annotation = holder
+                .newAnnotation(severity, message)
+                .range(target.getTextRange());
+        if (isAfterEndOfLine()) {
+            annotation = annotation.afterEndOfLine();
+        }
+        annotation.create();
     }
 
     public SeverityLevel getSeverityLevel() {
@@ -97,10 +101,6 @@ public boolean isSuppressErrors() {
         return suppressErrors;
     }
 
-    private ProblemHighlightType problemHighlightType() {
-        return ProblemHighlightType.GENERIC_ERROR_OR_WARNING;
-    }
-
     @Override
     public String toString() {
         return new ToStringBuilder(this)

diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -46,12 +46,17 @@
 
         <projectConfigurable instance="com.leinardi.pycharm.pylint.PylintConfigurable"/>
 
-        <localInspection implementationClass="com.leinardi.pycharm.pylint.PylintInspection"
+        <externalAnnotator language="Python" implementationClass="com.leinardi.pycharm.pylint.PylintAnnotator"/>
+
+        <localInspection implementationClass="com.leinardi.pycharm.pylint.PylintBatchInspection"
+                         language="Python"
                          bundle="com.leinardi.pycharm.pylint.PylintBundle"
                          key="inspection.display-name"
                          groupKey="inspection.group"
+                         shortName="Pylint"
                          level="WARNING"
-                         enabledByDefault="false"/>
+                         unfair="true"
+                         enabledByDefault="true"/>
 
         <checkinHandlerFactory id="CheckStyleIDEACheckInHandlerFactory"
                                implementation="com.leinardi.pycharm.pylint.handlers.ScanFilesBeforeCheckinHandlerFactory"/>