From 771364b2e45e881b980b703fd8fa8a321945ea67 Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Mon, 19 Aug 2024 09:34:32 +0200 Subject: [PATCH] don't instantiate all inspections in dumb mode IJPL-574, IJPL-160462 GitOrigin-RevId: c47be295416b50cf08aa013e818e1bfae4cc607e --- .../impl/LocalInspectionsInDumbModeTest.kt | 63 ++++++++++++++ .../daemon/impl/HighlightInfoUpdaterImpl.java | 4 +- .../daemon/impl/LocalInspectionsPass.java | 86 ++++++++++++------- 3 files changed, 119 insertions(+), 34 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/LocalInspectionsInDumbModeTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/LocalInspectionsInDumbModeTest.kt index f354dbf7982c..a98fb8d45753 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/LocalInspectionsInDumbModeTest.kt +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/LocalInspectionsInDumbModeTest.kt @@ -4,9 +4,11 @@ package com.intellij.codeInsight.daemon.impl import com.intellij.codeInsight.daemon.DaemonAnalyzerTestCase import com.intellij.codeInsight.daemon.DaemonAnalyzerTestCase.CanChangeDocumentDuringHighlighting import com.intellij.codeInsight.daemon.DaemonCodeAnalyzer +import com.intellij.codeInspection.LocalInspectionEP import com.intellij.codeInspection.LocalInspectionTool import com.intellij.codeInspection.LocalInspectionToolSession import com.intellij.codeInspection.ProblemsHolder +import com.intellij.codeInspection.ex.LocalInspectionToolWrapper import com.intellij.ide.highlighter.JavaFileType import com.intellij.openapi.project.DumbAware import com.intellij.openapi.util.Disposer @@ -16,6 +18,7 @@ import com.intellij.psi.PsiElementVisitor import com.intellij.testFramework.* import org.intellij.lang.annotations.Language import java.util.* +import java.util.concurrent.atomic.AtomicBoolean @CanChangeDocumentDuringHighlighting class LocalInspectionsInDumbModeTest : DaemonAnalyzerTestCase() { @@ -74,6 +77,38 @@ class LocalInspectionsInDumbModeTest : DaemonAnalyzerTestCase() { assertExistsInfo(dumbInfos, "Smart0") } + fun testLocalInspectionInDumbModeDontInitializeUnrelatedTools() { + val unrelatedToolWrapper = createUnrelatedToolWrapper() + enableInspectionTool(project, unrelatedToolWrapper, testRootDisposable) + LocalInspectionsPass.forceNoDuplicateCheckInTests(testRootDisposable) + + @Language("JAVA") + val text = """ + // comment + """ + configureByText(JavaFileType.INSTANCE, text) + + doHighlightingInDumbMode() + + assertFalse(unrelatedToolWrapper.isToolInstantiated()) + } + + fun testLocalInspectionDontInitializeUnrelatedTools() { + val unrelatedToolWrapper = createUnrelatedToolWrapper() + enableInspectionTool(project, unrelatedToolWrapper, testRootDisposable) + LocalInspectionsPass.forceNoDuplicateCheckInTests(testRootDisposable) + + @Language("JAVA") + val text = """ + // comment + """ + configureByText(JavaFileType.INSTANCE, text) + + doHighlighting() + + assertFalse(unrelatedToolWrapper.isToolInstantiated()) + } + private fun assertExistsInfo(infos: List, text: String) { assert(infos.any { it.description == text }) { "List [${infos.joinToString { it.description }}] does not contain `$text`" @@ -119,4 +154,32 @@ class LocalInspectionsInDumbModeTest : DaemonAnalyzerTestCase() { } } + private fun createUnrelatedToolWrapper(): UnrelatedToolWrapper { + val ep = LocalInspectionEP() + ep.dumbAware = true + ep.implementationClass = "foo.bar.Baz" + ep.id = "Baz" + ep.language = "TEXT" + ep.displayName = "Baz" + return UnrelatedToolWrapper(ep) + } + + private class UnrelatedToolWrapper(ep: LocalInspectionEP) : LocalInspectionToolWrapper(ep) { + private val toolIsInstantiated = AtomicBoolean(false) + + override fun getTool(): LocalInspectionTool { + toolIsInstantiated.set(true) + return MyLocalInspection() + } + + fun isToolInstantiated(): Boolean { + return toolIsInstantiated.get() + } + + private class MyLocalInspection : LocalInspectionTool() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean, session: LocalInspectionToolSession): PsiElementVisitor { + return PsiElementVisitor.EMPTY_VISITOR + } + } + } } \ No newline at end of file diff --git a/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java b/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java index e9f324e05c52..8526ae453f6e 100644 --- a/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java +++ b/platform/analysis-impl/src/com/intellij/codeInsight/daemon/impl/HighlightInfoUpdaterImpl.java @@ -491,9 +491,7 @@ final class HighlightInfoUpdaterImpl extends HighlightInfoUpdater implements Dis @NotNull List injectedFragments, @NotNull Set> actualToolsRun, @NotNull HighlightingSession highlightingSession, - @NotNull List disabledSmartOnlyToolWrappers) { - - Set inactiveSmartOnlyToolIds = ContainerUtil.map2Set(disabledSmartOnlyToolWrappers, toolWrapper -> toolWrapper.getShortName()); + @NotNull Set inactiveSmartOnlyToolIds) { for (PsiFile psiFile: ContainerUtil.append(injectedFragments, containingFile)) { getData(psiFile, hostDocument).entrySet().removeIf(entry -> { diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/LocalInspectionsPass.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/LocalInspectionsPass.java index 12fe6755e669..626b757ca1ad 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/LocalInspectionsPass.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/LocalInspectionsPass.java @@ -17,6 +17,7 @@ import com.intellij.lang.Language; import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.lang.annotation.ProblemGroup; import com.intellij.lang.injection.InjectedLanguageManager; +import com.intellij.openapi.Disposable; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Document; @@ -29,10 +30,7 @@ import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.project.PossiblyDumbAware; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ProjectTypeService; -import com.intellij.openapi.util.NlsContexts; -import com.intellij.openapi.util.NlsSafe; -import com.intellij.openapi.util.Pair; -import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.util.*; import com.intellij.openapi.util.registry.Registry; import com.intellij.openapi.util.text.StringUtil; import com.intellij.profile.codeInspection.ProjectInspectionProfileManager; @@ -48,6 +46,7 @@ import com.intellij.util.containers.Interner; import com.intellij.xml.util.XmlStringUtil; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; import java.util.*; import java.util.concurrent.ConcurrentHashMap; @@ -100,26 +99,9 @@ final class LocalInspectionsPass extends ProgressableTextEditorHighlightingPass List injectedFragments = List.of(); }; - // In dumb mode, we need to run dumb-aware inspections only. - // But we need to keep highlights from currently enabled but inactive smart-only inspections. - List activeToolWrappers; - List disabledSmartOnlyToolWrappers; - if (isDumbMode()) { - activeToolWrappers = toolWrappers.stream().parallel().filter(wrapper -> wrapper.isDumbAware()).toList(); + DumbToolWrapperCondition dumbToolWrapperCondition = new DumbToolWrapperCondition(isDumbMode()); - if (activeToolWrappers.isEmpty()) { - disabledSmartOnlyToolWrappers = toolWrappers; - } - else { - disabledSmartOnlyToolWrappers = ContainerUtil.filter(toolWrappers, wrapper -> !wrapper.isDumbAware()); - } - } - else { - activeToolWrappers = toolWrappers; - disabledSmartOnlyToolWrappers = List.of(); - } - - if (!activeToolWrappers.isEmpty()) { + if (!toolWrappers.isEmpty()) { Consumer withRecycler = invalidPsiRecycler -> { InspectionRunner.ApplyIncrementallyCallback applyIncrementallyCallback = (descriptors, holder, visitingPsiElement, shortName) -> { List allInfos = descriptors.isEmpty() ? null : new ArrayList<>(descriptors.size()); @@ -162,12 +144,15 @@ final class LocalInspectionsPass extends ProgressableTextEditorHighlightingPass InspectionRunner runner = new InspectionRunner(getFile(), myRestrictRange, myPriorityRange, myInspectInjectedPsi, true, isDumbMode(), progress, myIgnoreSuppressed, myProfileWrapper, mySuppressedElements); - result.resultContexts = runner.inspect(activeToolWrappers, - ((HighlightingSessionImpl)getHighlightingSession()).getMinimumSeverity(), - true, - applyIncrementallyCallback, - contextFinishedCallback, - wrapper -> !wrapper.getTool().isSuppressedFor(getFile())); + + result.resultContexts = runner.inspect(toolWrappers, + ((HighlightingSessionImpl)getHighlightingSession()).getMinimumSeverity(), + true, + applyIncrementallyCallback, + contextFinishedCallback, + wrapper -> dumbToolWrapperCondition.value(wrapper) && + !wrapper.getTool().isSuppressedFor(getFile()) + ); myInfos = fileInfos; result.injectedFragments = runner.getInjectedFragments(); }; @@ -178,9 +163,10 @@ final class LocalInspectionsPass extends ProgressableTextEditorHighlightingPass ManagedHighlighterRecycler.runWithRecycler(getHighlightingSession(), withRecycler); } } + if (myHighlightInfoUpdater instanceof HighlightInfoUpdaterImpl impl) { Set> pairs = ContainerUtil.map2Set(result.resultContexts, context -> Pair.create(context.tool().getShortName(), context.psiFile())); - impl.removeHighlightsForObsoleteTools(getFile(), getDocument(), result.injectedFragments, pairs, getHighlightingSession(), disabledSmartOnlyToolWrappers); + impl.removeHighlightsForObsoleteTools(getFile(), getDocument(), result.injectedFragments, pairs, getHighlightingSession(), dumbToolWrapperCondition.getInactiveToolWrapperIds()); impl.removeWarningsInsideErrors(result.injectedFragments, getDocument(), getHighlightingSession()); // must be the last } } @@ -445,7 +431,7 @@ final class LocalInspectionsPass extends ProgressableTextEditorHighlightingPass private @NotNull List getInspectionTools(@NotNull InspectionProfileWrapper profile) { List> toolWrappers = profile.getInspectionProfile().getInspectionTools(getFile()); - if (LOG.isDebugEnabled()) { + if (LOG.isDebugEnabled() && runDuplicateCheck) { // this triggers heavy class loading of all inspections, do not run if DEBUG not enabled InspectionProfileWrapper.checkInspectionsDuplicates(toolWrappers); } @@ -507,4 +493,42 @@ final class LocalInspectionsPass extends ProgressableTextEditorHighlightingPass return true; } } + + @TestOnly + static void forceNoDuplicateCheckInTests(@NotNull Disposable parent) { + Disposer.register(parent, () -> runDuplicateCheck = true); + runDuplicateCheck = false; + } + + private static boolean runDuplicateCheck = true; + + private static class DumbToolWrapperCondition implements Condition { + private final boolean myDumbMode; + private final Set myInactiveIds = ConcurrentHashMap.newKeySet(); + + private DumbToolWrapperCondition(boolean isDumbMode) { + myDumbMode = isDumbMode; + } + + @Override + public boolean value(LocalInspectionToolWrapper wrapper) { + if (!myDumbMode) return true; + + LocalInspectionTool tool = wrapper.getTool(); + if (tool.isDumbAware()) { + return true; + } + + myInactiveIds.add(tool.getShortName()); + return false; + } + + @NotNull Set getInactiveToolWrapperIds() { + if (myInactiveIds.isEmpty()) { + return Collections.emptySet(); + } + + return new HashSet<>(myInactiveIds); + } + } }