From c902b68e86f702f33ae532500557ebe3e64f0203 Mon Sep 17 00:00:00 2001 From: "Anna.Kozlova" Date: Mon, 13 Feb 2017 14:03:55 +0100 Subject: [PATCH] external annotators: provide api for inspection; avoid long annotation under read action in batch (IDEA-165322) --- .../javaDoc/JavadocHtmlLintAnnotator.kt | 10 ++-- .../javaDoc/JavadocHtmlLintInspection.kt | 12 +--- .../lang/annotation/ExternalAnnotator.java | 7 +++ .../ExternalAnnotatorInspectionVisitor.java | 13 +++-- .../ex/ExternalAnnotatorBatchInspection.java | 57 +++++++++++++++++++ .../daemon/impl/ExternalToolPass.java | 12 ++++ .../ex/GlobalInspectionContextImpl.java | 12 +++- 7 files changed, 101 insertions(+), 22 deletions(-) create mode 100644 platform/analysis-impl/src/com/intellij/codeInspection/ex/ExternalAnnotatorBatchInspection.java diff --git a/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintAnnotator.kt b/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintAnnotator.kt index 066db18f3ad7..8c534ddaef10 100644 --- a/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintAnnotator.kt +++ b/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintAnnotator.kt @@ -38,7 +38,6 @@ import com.intellij.openapi.util.io.FileUtil import com.intellij.openapi.util.text.StringUtil import com.intellij.openapi.vfs.VirtualFile import com.intellij.pom.java.LanguageLevel -import com.intellij.profile.codeInspection.InspectionProjectProfileManager import com.intellij.psi.PsiElement import com.intellij.psi.PsiFile import com.intellij.psi.PsiJavaFile @@ -47,15 +46,17 @@ import com.intellij.psi.util.PsiTreeUtil import com.sun.tools.doclint.DocLint import java.io.File -class JavadocHtmlLintAnnotator(private val manual: Boolean = false) : +class JavadocHtmlLintAnnotator() : ExternalAnnotator() { data class Info(val file: PsiFile) data class Anno(val row: Int, val col: Int, val error: Boolean, val message: String) data class Result(val annotations: List) + override fun getPairedBatchInspectionShortName() = JavadocHtmlLintInspection.SHORT_NAME + override fun collectInformation(file: PsiFile): Info? = - if (isJava8SourceFile(file) && "/**" in file.text && isToolEnabled(file)) Info(file) else null + if (isJava8SourceFile(file) && "/**" in file.text) Info(file) else null override fun doAnnotate(collectedInfo: Info): Result? { val file = collectedInfo.file.virtualFile!! @@ -122,9 +123,6 @@ class JavadocHtmlLintAnnotator(private val manual: Boolean = false) : file is PsiJavaFile && file.languageLevel.isAtLeast(LanguageLevel.JDK_1_8) && file.virtualFile != null && ProjectFileIndex.SERVICE.getInstance(file.project).isInSourceContent(file.virtualFile) - private fun isToolEnabled(file: PsiFile) = - manual || InspectionProjectProfileManager.getInstance(file.project).currentProfile.isToolEnabled(key.value, file) - private fun createTempFile(bytes: ByteArray): File { val tempFile = FileUtil.createTempFile(File(PathManager.getTempPath()), "javadocHtmlLint", ".java") tempFile.writeBytes(bytes) diff --git a/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintInspection.kt b/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintInspection.kt index 4663c136a8f1..a174ec94b592 100644 --- a/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintInspection.kt +++ b/java/java-impl/src/com/intellij/codeInspection/javaDoc/JavadocHtmlLintInspection.kt @@ -15,23 +15,15 @@ */ package com.intellij.codeInspection.javaDoc -import com.intellij.codeInspection.ExternalAnnotatorInspectionVisitor import com.intellij.codeInspection.LocalInspectionTool -import com.intellij.codeInspection.ProblemsHolder import com.intellij.codeInspection.SuppressQuickFix -import com.intellij.codeInspection.ex.PairedUnfairLocalInspectionTool +import com.intellij.codeInspection.ex.ExternalAnnotatorBatchInspection import com.intellij.psi.PsiElement -class JavadocHtmlLintInspection : LocalInspectionTool(), PairedUnfairLocalInspectionTool { +class JavadocHtmlLintInspection : LocalInspectionTool(), ExternalAnnotatorBatchInspection { companion object { val SHORT_NAME = "JavadocHtmlLint" } - private val annotator = lazy { JavadocHtmlLintAnnotator(true) } - - override fun buildVisitor(holder: ProblemsHolder, onTheFly: Boolean) = ExternalAnnotatorInspectionVisitor(holder, annotator.value, onTheFly) - override fun getBatchSuppressActions(element: PsiElement?) = SuppressQuickFix.EMPTY_ARRAY - - override fun getInspectionForBatchShortName() = SHORT_NAME } \ No newline at end of file diff --git a/platform/analysis-api/src/com/intellij/lang/annotation/ExternalAnnotator.java b/platform/analysis-api/src/com/intellij/lang/annotation/ExternalAnnotator.java index 1921f9ecdb18..1c4678e8ee56 100644 --- a/platform/analysis-api/src/com/intellij/lang/annotation/ExternalAnnotator.java +++ b/platform/analysis-api/src/com/intellij/lang/annotation/ExternalAnnotator.java @@ -76,4 +76,11 @@ public abstract class ExternalAnnotator { */ public void apply(@NotNull PsiFile file, AnnotationResultType annotationResult, @NotNull AnnotationHolder holder) { } + + /** + * Return inspection which should run in batch mode. Generally todo + */ + public String getPairedBatchInspectionShortName() { + return null; + } } diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/ExternalAnnotatorInspectionVisitor.java b/platform/analysis-impl/src/com/intellij/codeInspection/ExternalAnnotatorInspectionVisitor.java index 67f19f8a3403..5450302a2739 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/ExternalAnnotatorInspectionVisitor.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/ExternalAnnotatorInspectionVisitor.java @@ -21,6 +21,7 @@ import com.intellij.lang.annotation.Annotation; import com.intellij.lang.annotation.AnnotationSession; import com.intellij.lang.annotation.ExternalAnnotator; import com.intellij.lang.annotation.HighlightSeverity; +import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Iconable; @@ -58,21 +59,23 @@ public class ExternalAnnotatorInspectionVisitor extends PsiElementVisitor { public static ProblemDescriptor[] checkFileWithExternalAnnotator(@NotNull PsiFile file, @NotNull InspectionManager manager, boolean isOnTheFly, - @NotNull ExternalAnnotator annotator) { + @NotNull ExternalAnnotator annotator) { if (isOnTheFly) { // ExternalAnnotator does this work return ProblemDescriptor.EMPTY_ARRAY; } - Init info = annotator.collectInformation(file); + Init info = ReadAction.compute(() -> annotator.collectInformation(file)); if (info != null) { Result annotationResult = annotator.doAnnotate(info); if (annotationResult == null) { return ProblemDescriptor.EMPTY_ARRAY; } - AnnotationHolderImpl annotationHolder = new AnnotationHolderImpl(new AnnotationSession(file)); - annotator.apply(file, annotationResult, annotationHolder); - return convertToProblemDescriptors(annotationHolder, manager, file); + return ReadAction.compute(() -> { + AnnotationHolderImpl annotationHolder = new AnnotationHolderImpl(new AnnotationSession(file)); + annotator.apply(file, annotationResult, annotationHolder); + return convertToProblemDescriptors(annotationHolder, manager, file); + }); } return ProblemDescriptor.EMPTY_ARRAY; } diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/ex/ExternalAnnotatorBatchInspection.java b/platform/analysis-impl/src/com/intellij/codeInspection/ex/ExternalAnnotatorBatchInspection.java new file mode 100644 index 000000000000..d5ce17d2f66a --- /dev/null +++ b/platform/analysis-impl/src/com/intellij/codeInspection/ex/ExternalAnnotatorBatchInspection.java @@ -0,0 +1,57 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.codeInspection.ex; + +import com.intellij.codeInspection.ExternalAnnotatorInspectionVisitor; +import com.intellij.codeInspection.InspectionManager; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.lang.ExternalLanguageAnnotators; +import com.intellij.lang.Language; +import com.intellij.lang.annotation.ExternalAnnotator; +import com.intellij.psi.FileViewProvider; +import com.intellij.psi.PsiFile; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.Set; + +public interface ExternalAnnotatorBatchInspection extends PairedUnfairLocalInspectionTool { + @NotNull + String getShortName(); + + @NotNull + @Override + default String getInspectionForBatchShortName() { + return getShortName(); + } + + default ProblemDescriptor[] checkFile(PsiFile file, InspectionManager manager) { + final String shortName = getShortName(); + final FileViewProvider viewProvider = file.getViewProvider(); + final Set relevantLanguages = viewProvider.getLanguages(); + for (Language language : relevantLanguages) { + PsiFile psiRoot = viewProvider.getPsi(language); + final List externalAnnotators = ExternalLanguageAnnotators.allForFile(language, psiRoot); + + for (ExternalAnnotator annotator : externalAnnotators) { + if (shortName.equals(annotator.getPairedBatchInspectionShortName())) { + return ExternalAnnotatorInspectionVisitor.checkFileWithExternalAnnotator(file, manager, false, annotator); + } + } + } + return ProblemDescriptor.EMPTY_ARRAY; + } +} diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java index ca7c807f6661..fb9da4c0dcd7 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java @@ -16,7 +16,9 @@ package com.intellij.codeInsight.daemon.impl; +import com.intellij.codeInsight.daemon.HighlightDisplayKey; import com.intellij.codeInsight.daemon.impl.analysis.HighlightingLevelManager; +import com.intellij.codeInspection.ex.InspectionProfileImpl; import com.intellij.lang.ExternalLanguageAnnotators; import com.intellij.lang.Language; import com.intellij.lang.annotation.Annotation; @@ -25,11 +27,13 @@ import com.intellij.lang.annotation.ExternalAnnotator; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.application.ex.ApplicationManagerEx; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.project.DumbService; import com.intellij.openapi.util.TextRange; +import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.FileViewProvider; import com.intellij.psi.PsiFile; import com.intellij.util.containers.HashMap; @@ -42,6 +46,7 @@ import java.util.*; * @author ven */ public class ExternalToolPass extends ProgressableTextEditorHighlightingPass { + private static final Logger LOG = Logger.getInstance(ExternalToolPass.class); private final AnnotationHolderImpl myAnnotationHolder; private final Map myAnnotator2DataMap = new HashMap<>(); @@ -99,6 +104,7 @@ public class ExternalToolPass extends ProgressableTextEditorHighlightingPass { } setProgressLimit(externalAnnotatorsInRoots); + InspectionProfileImpl profile = InspectionProjectProfileManager.getInstance(myProject).getCurrentProfile(); for (Language language : relevantLanguages) { PsiFile psiRoot = viewProvider.getPsi(language); if (!HighlightingLevelManager.getInstance(myProject).shouldInspect(psiRoot)) continue; @@ -109,6 +115,12 @@ public class ExternalToolPass extends ProgressableTextEditorHighlightingPass { boolean errorFound = daemonCodeAnalyzer.getFileStatusMap().wasErrorFound(myDocument); for(ExternalAnnotator externalAnnotator: externalAnnotators) { + String shortName = externalAnnotator.getPairedBatchInspectionShortName(); + if (shortName != null) { + HighlightDisplayKey key = HighlightDisplayKey.find(shortName); + LOG.assertTrue(key != null, "Paired tool '" + shortName + "' not found for external annotator: " + externalAnnotator); + if (!profile.isToolEnabled(key, myFile)) continue; + } final Object collectedInfo; Editor editor = getEditor(); if (editor != null) { diff --git a/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java b/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java index 7c227b46d689..3d33484f70c5 100644 --- a/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java @@ -94,6 +94,7 @@ import java.io.OutputStreamWriter; import java.lang.reflect.Constructor; import java.util.*; import java.util.concurrent.*; +import java.util.stream.Collectors; public class GlobalInspectionContextImpl extends GlobalInspectionContextBase implements GlobalInspectionContext { private static final Logger LOG = Logger.getInstance("#com.intellij.codeInspection.ex.GlobalInspectionContextImpl"); @@ -424,6 +425,14 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextBase imp throw new ProcessCanceledException(); } + getWrappersFromTools(localTools, file, includeDoNotShow(getCurrentProfile())).stream() + .filter(wrapper -> wrapper.getTool() instanceof ExternalAnnotatorBatchInspection) + .forEach(wrapper -> { + ProblemDescriptor[] descriptors = ((ExternalAnnotatorBatchInspection)wrapper.getTool()).checkFile(file, inspectionManager); + InspectionToolPresentation toolPresentation = getPresentation(wrapper); + ReadAction.run(() -> LocalDescriptorsUtil.addProblemDescriptors(Arrays.asList(descriptors), false, this, null, CONVERT, toolPresentation)); + }); + return true; }; try { @@ -496,7 +505,8 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextBase imp try { boolean includeDoNotShow = includeDoNotShow(getCurrentProfile()); final List lTools = getWrappersFromTools(localTools, file, includeDoNotShow); - pass.doInspectInBatch(this, inspectionManager, lTools); + List nonExternalAnnotators = lTools.stream().filter(wrapper -> !(wrapper.getTool() instanceof ExternalAnnotatorBatchInspection)).collect(Collectors.toList()); + pass.doInspectInBatch(this, inspectionManager, nonExternalAnnotators); final List tools = getWrappersFromTools(globalSimpleTools, file, includeDoNotShow); JobLauncher.getInstance().invokeConcurrentlyUnderProgress(tools, myProgressIndicator, false, toolWrapper -> {