diff --git a/src/main/java/com/leinardi/pycharm/pylint/PylintInspection.java b/src/main/java/com/leinardi/pycharm/pylint/PylintAnnotator.java similarity index 52% rename from src/main/java/com/leinardi/pycharm/pylint/PylintInspection.java rename to src/main/java/com/leinardi/pycharm/pylint/PylintAnnotator.java index 1703835..f9dcca3 100644 --- a/src/main/java/com/leinardi/pycharm/pylint/PylintInspection.java +++ b/src/main/java/com/leinardi/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. + *

+ * Modeled after `com.jetbrains.python.validation.Pep8ExternalAnnotator` + *

+ * 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 { - private static final Logger LOG = Logger.getInstance(PylintInspection.class); - private static final List 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 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 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,7 +132,7 @@ public List 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 { @@ -110,6 +140,26 @@ public List inspectFile(@NotNull final PsiFile psiFile, } } + @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 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 issues; + + public Results(List 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 new file mode 100644 index 0000000..f386755 --- /dev/null +++ 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 index ad3f679..7aa7e14 100644 --- 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 index c99e21a..f8cf365 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -46,12 +46,17 @@ - + + + unfair="true" + enabledByDefault="true"/>