From 8b1c502b64ca6ecafee1639cb106d1969c41f48d Mon Sep 17 00:00:00 2001 From: Johannes Koenen Date: Thu, 26 Oct 2023 09:36:30 +0200 Subject: [PATCH] [QD-7418] Refactor GlobalInspectionContextImpl & InspectionEngine - Formatting - Revise creation of inspect listener - Extract large lambdas - Drop unused class GitOrigin-RevId: 64d57f2d1a325ab86ba9a830a4d5da00732780a7 --- .../codeInspection/InspectionEngine.java | 28 +-- .../ex/GlobalInspectionContextImpl.java | 179 ++++++++---------- 2 files changed, 97 insertions(+), 110 deletions(-) diff --git a/platform/analysis-impl/src/com/intellij/codeInspection/InspectionEngine.java b/platform/analysis-impl/src/com/intellij/codeInspection/InspectionEngine.java index e18f08cc8622..8bd57ccfd96e 100644 --- a/platform/analysis-impl/src/com/intellij/codeInspection/InspectionEngine.java +++ b/platform/analysis-impl/src/com/intellij/codeInspection/InspectionEngine.java @@ -288,6 +288,9 @@ public final class InspectionEngine { Map, Collection>> targetPsiClasses = InspectionVisitorsOptimizer.getTargetPsiClasses(elements); + final @Nullable var inspectionListener = + isOnTheFly ? null : psiFile.getProject().getMessageBus().syncPublisher(GlobalInspectionContextEx.INSPECT_TOPIC); + Processor processor = toolWrapper -> { ProblemsHolder holder = new ProblemsHolder(InspectionManager.getInstance(psiFile.getProject()), psiFile, isOnTheFly){ @Override @@ -313,7 +316,17 @@ public final class InspectionEngine { boolean inspectionWasRun = createVisitorAndAcceptElements(tool, holder, isOnTheFly, session, elements, targetPsiClasses); long inspectionDuration = TimeoutUtil.getDurationMillis(inspectionStartTime); - reportToQodana(psiFile, isOnTheFly, toolWrapper, inspectionWasRun, inspectionDuration, holder.getResultCount()); + if (inspectionListener != null && inspectionWasRun) { + inspectionListener.inspectionFinished( + inspectionDuration, + Thread.currentThread().getId(), + holder.getResultCount(), + toolWrapper, + InspectListener.InspectionKind.LOCAL, + psiFile, + psiFile.getProject() + ); + } if (holder.hasResults()) { List descriptors = ContainerUtil.filter(holder.getResults(), descriptor -> { @@ -331,19 +344,6 @@ public final class InspectionEngine { return resultDescriptors; } - private static void reportToQodana(@NotNull PsiFile psiFile, - boolean isOnTheFly, - @NotNull LocalInspectionToolWrapper toolWrapper, - boolean inspectionWasRun, - long inspectionDuration, int resultCount) { - boolean needToReportStatsToQodana = inspectionWasRun && !isOnTheFly; - if (needToReportStatsToQodana) { - InspectListener publisher = psiFile.getProject().getMessageBus().syncPublisher(GlobalInspectionContextEx.INSPECT_TOPIC); - publisher.inspectionFinished(inspectionDuration, Thread.currentThread().getId(), resultCount, toolWrapper, - InspectListener.InspectionKind.LOCAL, psiFile, psiFile.getProject()); - } - } - public static @NotNull @Unmodifiable List runInspectionOnFile(@NotNull PsiFile psiFile, @NotNull InspectionToolWrapper toolWrapper, @NotNull GlobalInspectionContext inspectionContext) { 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 b6d2aa8d431d..f5f8fd80b074 100644 --- a/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInspection/ex/GlobalInspectionContextImpl.java @@ -127,7 +127,7 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { myContentManager = contentManager; } - protected @NotNull InspectListener getEventPublisher() { + protected @NotNull InspectListener getInspectionEventPublisher() { return getProject().getMessageBus().syncPublisher(GlobalInspectionContextEx.INSPECT_TOPIC); } @@ -325,7 +325,6 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { } boolean headlessEnvironment = ApplicationManager.getApplication().isHeadlessEnvironment(); - boolean inspectInjectedPsi = Registry.is("idea.batch.inspections.inspect.injected.psi", true); Map> map = getInspectionWrappersMap(localTools); @@ -335,58 +334,8 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { Future future = startIterateScopeInBackground(scope, fileScanningIndicator, headlessEnvironment, localScopeFiles, filesToInspect); var enabledInspectionsProvider = createEnabledInspectionsProvider(localTools, globalSimpleTools, getProject()); - PsiManager psiManager = PsiManager.getInstance(getProject()); - Processor processor = virtualFile -> { - ProgressManager.checkCanceled(); - - final var cachedWrappers = new AtomicReference(null); - - Boolean readActionSuccess = DumbService.getInstance(getProject()).tryRunReadActionInSmartMode(() -> { - long start = getPathProfile() == null ? 0 : System.currentTimeMillis(); - PsiFile file = virtualFile.isValid() ? psiManager.findFile(virtualFile) : null; - if (file == null) { - return true; - } - if (!scope.contains(virtualFile)) { - LOG.info(file.getName() + "; scope: " + scope + "; " + virtualFile); - return true; - } - boolean includeDoNotShow = includeDoNotShow(getCurrentProfile()); - var wrappers = getWrappersFromTools(enabledInspectionsProvider, file, includeDoNotShow); - cachedWrappers.set(wrappers); - - inspectFile(file, getEffectiveRange(searchScope, file), inspectionManager, map, - wrappers.getGlobalSimpleRegularWrappers(), - wrappers.getLocalRegularWrappers(), - inspectInjectedPsi && scope.isAnalyzeInjectedCode()); - if (start != 0) { - updateProfile(virtualFile, System.currentTimeMillis() - start); - } - return true; - }, LangBundle.message("popup.content.inspect.code.not.available.until.indices.are.ready"), DumbModeBlockedFunctionality.GlobalInspectionContext); - if (readActionSuccess == null || !readActionSuccess) { - throw new ProcessCanceledException(); - } - - PsiFile file = ReadAction.compute(() -> psiManager.findFile(virtualFile)); - if (file == null) { - return true; - } - getEventPublisher().fileAnalyzed(file, getProject()); - - boolean includeDoNotShow = includeDoNotShow(getCurrentProfile()); - - var cachedWrappersValue = cachedWrappers.get(); - var wrappers = (cachedWrappersValue != null) ? cachedWrappersValue : getWrappersFromTools(enabledInspectionsProvider, file, includeDoNotShow); - wrappers.getExternalAnnotatorWrappers().forEach(wrapper -> { - ProblemDescriptor[] descriptors = ((ExternalAnnotatorBatchInspection)wrapper.getTool()).checkFile(file, this, inspectionManager); - InspectionToolResultExporter toolPresentation = getPresentation(wrapper); - ReadAction.run(() -> BatchModeDescriptorsUtil - .addProblemDescriptors(Arrays.asList(descriptors), false, this, null, toolPresentation, CONVERT)); - }); - - return true; - }; + Processor processor = + buildProcessor(scope, enabledInspectionsProvider, searchScope, inspectionManager, map); var localInspectionsSpan = tracer.spanBuilder("localInspectionsAnalysis").startSpan(); try { Queue filesFailedToInspect = new LinkedBlockingQueue<>(); @@ -442,6 +391,80 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { addProblemsToView(globalSimpleTools); } + @NotNull + private Processor buildProcessor(@NotNull AnalysisScope scope, + EnabledInspectionsProvider enabledInspectionsProvider, + SearchScope searchScope, + InspectionManager inspectionManager, + Map> map) { + + PsiManager psiManager = PsiManager.getInstance(getProject()); + boolean inspectInjectedPsi = Registry.is("idea.batch.inspections.inspect.injected.psi", true); + + return virtualFile -> { + ProgressManager.checkCanceled(); + + final var wrappersForThisFile = new AtomicReference(null); + + Computable inspection = () -> { + long start = getPathProfile() == null ? 0 : System.currentTimeMillis(); + PsiFile file = virtualFile.isValid() ? psiManager.findFile(virtualFile) : null; + if (file == null) { + return true; + } + if (!scope.contains(virtualFile)) { + LOG.info(file.getName() + "; scope: " + scope + "; " + virtualFile); + return true; + } + boolean includeDoNotShow = includeDoNotShow(getCurrentProfile()); + var wrappers = getWrappersFromTools(enabledInspectionsProvider, file, includeDoNotShow); + wrappersForThisFile.set(wrappers); + + inspectFile(file, getEffectiveRange(searchScope, file), inspectionManager, map, + wrappers.getGlobalSimpleRegularWrappers(), + wrappers.getLocalRegularWrappers(), + inspectInjectedPsi && scope.isAnalyzeInjectedCode()); + if (start != 0) { + updateProfile(virtualFile, System.currentTimeMillis() - start); + } + return true; + }; + Boolean readActionSuccess = DumbService.getInstance(getProject()) + .tryRunReadActionInSmartMode( + inspection, + LangBundle.message("popup.content.inspect.code.not.available.until.indices.are.ready"), + DumbModeBlockedFunctionality.GlobalInspectionContext + ); + + if (readActionSuccess == null || !readActionSuccess) { + throw new ProcessCanceledException(); + } + + PsiFile file = ReadAction.compute(() -> psiManager.findFile(virtualFile)); + if (file == null) { + return true; + } + getInspectionEventPublisher().fileAnalyzed(file, getProject()); + + boolean includeDoNotShow = includeDoNotShow(getCurrentProfile()); + + var cachedWrappersValue = wrappersForThisFile.get(); + var wrappers = + (cachedWrappersValue != null) ? cachedWrappersValue : getWrappersFromTools(enabledInspectionsProvider, file, includeDoNotShow); + + wrappers.getExternalAnnotatorWrappers() + .forEach(wrapper -> { + var tool = ((ExternalAnnotatorBatchInspection)wrapper.getTool()); + var descriptors = tool.checkFile(file, this, inspectionManager); + InspectionToolResultExporter toolPresentation = getPresentation(wrapper); + ReadAction.run(() -> BatchModeDescriptorsUtil + .addProblemDescriptors(Arrays.asList(descriptors), false, this, null, toolPresentation, CONVERT)); + }); + + return true; + }; + } + public static void setupCancelOnWriteProgress(@NotNull Disposable disposable, @NotNull ProgressIndicator progressIndicator) { // avoid "attach listener"/"write action" race ReadAction.run(() -> { @@ -467,19 +490,19 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { localTools.stream() .map(tool -> tool.getEnabledTool(psiFile, includeDoNotShow)) .filter(wrapper -> wrapper instanceof LocalInspectionToolWrapper) - .map(wrapper -> (LocalInspectionToolWrapper) wrapper) + .map(wrapper -> (LocalInspectionToolWrapper)wrapper) .toList(), globalSimpleTools.stream() .map(tool -> tool.getEnabledTool(psiFile, includeDoNotShow)) .filter(wrapper -> wrapper instanceof GlobalInspectionToolWrapper) - .map(wrapper -> (GlobalInspectionToolWrapper) wrapper) + .map(wrapper -> (GlobalInspectionToolWrapper)wrapper) .toList() ); } }; } - protected void runExternalTools() {} + protected void runExternalTools() { } private static @NotNull TextRange getEffectiveRange(@NotNull SearchScope searchScope, @NotNull PsiFile file) { if (searchScope instanceof LocalSearchScope) { @@ -546,7 +569,7 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { ProblemsHolder holder = new ProblemsHolder(inspectionManager, file, false); ProblemDescriptionsProcessor problemDescriptionProcessor = getProblemDescriptionProcessor(toolWrapper, wrappersMap); reportWhenInspectionFinished( - getEventPublisher(), + getInspectionEventPublisher(), toolWrapper, GLOBAL_SIMPLE, file, @@ -684,7 +707,7 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { List> needRepeatSearchRequest = new ArrayList<>(); SearchScope initialSearchScope = ReadAction.compute(scope::toSearchScope); boolean canBeExternalUsages = !scope.isTotalScope(); - InspectListener eventPublisher = getEventPublisher(); + InspectListener eventPublisher = getInspectionEventPublisher(); TraceUtil.runWithSpanThrows(tracer, "globalInspectionsAnalysis", (__) -> { for (Tools tools : globalTools) { @@ -756,7 +779,7 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { if (tool.isGraphNeeded()) { try { reportWhenActivityFinished( - getEventPublisher(), + getInspectionEventPublisher(), InspectListener.ActivityKind.REFERENCE_SEARCH, getProject(), () -> ((RefManagerImpl)getRefManager()).findAllDeclarations()); @@ -1345,42 +1368,6 @@ public class GlobalInspectionContextImpl extends GlobalInspectionContextEx { }); } - public static final class Wrappers { - @NotNull - private final List localWrappers; - @NotNull - private final List globalSimpleWrappers; - @NotNull - private final List> externalAnnotatorWrappers; - @NotNull - private final List> allWrappers; - - public Wrappers(@NotNull List localWrappers, - @NotNull List globalSimpleWrappers, - @NotNull List> externalAnnotatorWrappers) { - this.localWrappers = localWrappers; - this.globalSimpleWrappers = globalSimpleWrappers; - this.externalAnnotatorWrappers = externalAnnotatorWrappers; - this.allWrappers = ContainerUtil.concat(localWrappers, globalSimpleWrappers, externalAnnotatorWrappers); - } - - public List getLocalWrappers() { - return localWrappers; - } - - public List getGlobalSimpleWrappers() { - return globalSimpleWrappers; - } - - public List> getExternalAnnotatorWrappers() { - return externalAnnotatorWrappers; - } - - public List> getAllWrappers() { - return allWrappers; - } - } - static final class InspectionPerformanceCollector extends CounterUsagesCollector { private static final EventLogGroup GROUP = new EventLogGroup("inspection.performance", 3);