From def78466db5de1e10879a1a7d29a52fc1c9e4a4d Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 29 May 2015 13:26:33 +0300 Subject: [PATCH] run code smell problems: highlighting gets canceled sometimes --- .../daemon/impl/analysis/RefCountHolder.java | 11 ++++-- .../daemon/impl/DaemonCodeAnalyzerImpl.java | 18 +++++++-- .../daemon/impl/PassExecutorService.java | 4 +- .../openapi/vcs/CodeSmellDetector.java | 14 ++++--- .../vcs/impl/CodeSmellDetectorImpl.java | 38 +++++++++---------- 5 files changed, 51 insertions(+), 34 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/RefCountHolder.java b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/RefCountHolder.java index 71dfe1355436..dc9be261626f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/RefCountHolder.java +++ b/java/java-analysis-impl/src/com/intellij/codeInsight/daemon/impl/analysis/RefCountHolder.java @@ -276,18 +276,22 @@ class RefCountHolder { TextRange dirtyScope, @NotNull ProgressIndicator indicator, @NotNull Runnable analyze) { + ProgressIndicator result; if (myState.compareAndSet(EMPTY, indicator)) { if (!file.getTextRange().equals(dirtyScope)) { // empty holder needs filling before it can be used, so restart daemon to re-analyze the whole file myState.set(EMPTY); return false; } + result = EMPTY; } - else if (!myState.compareAndSet(READY, indicator)) { + else if (myState.compareAndSet(READY, indicator)) { + result = READY; + } + else { log("a: failed to change ", myState, "->", indicator); return false; } - boolean success = false; try { log("a: changed ", myState, "->", indicator); if (dirtyScope != null) { @@ -300,11 +304,10 @@ class RefCountHolder { } analyze.run(); - success = true; + result = READY; return true; } finally { - ProgressIndicator result = success ? READY : EMPTY; boolean set = myState.compareAndSet(indicator, result); assert set : myState.get(); log("a: changed after analyze", indicator, "->", result); diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java index cca33d54749d..c3d09122ec6b 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java @@ -239,6 +239,9 @@ public class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implements Pers public List runMainPasses(@NotNull PsiFile psiFile, @NotNull Document document, @NotNull final ProgressIndicator progress) { + setUpdateByTimerEnabled(true); // by default we disable daemon while in modal dialog, but here we need to re-enable it because otherwise the paused daemon will conflict with our started passes + restart(); // clear status maps to run passes from scratch so that refCountHolder won't conflict and try to restart itself on partially filled maps + final List result = new ArrayList(); final VirtualFile virtualFile = psiFile.getVirtualFile(); if (virtualFile != null && !virtualFile.getFileType().isBinary()) { @@ -255,10 +258,18 @@ public class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implements Pers } }); - for (TextEditorHighlightingPass pass : passes) { - pass.doCollectInformation(progress); - result.addAll(pass.getInfos()); + LOG.debug("All passes for " + psiFile.getName()+ " started (" + passes+"). progress canceled: "+progress.isCanceled()); + try { + for (TextEditorHighlightingPass pass : passes) { + pass.doCollectInformation(progress); + result.addAll(pass.getInfos()); + } } + catch (ProcessCanceledException e) { + LOG.debug("Canceled: " + progress); + throw e; + } + LOG.debug("All passes for " + psiFile.getName()+ " run. progress canceled: "+progress.isCanceled()+"; infos: "+result); } return result; @@ -873,5 +884,4 @@ public class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implements Pers private List getActiveEditors() { return myEditorTracker.getActiveEditors(); } - } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/PassExecutorService.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/PassExecutorService.java index 5970d1197cb9..ec5cc51dc0c6 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/PassExecutorService.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/PassExecutorService.java @@ -132,7 +132,9 @@ class PassExecutorService implements Disposable { VirtualFile virtualFile = ((FileEditorManagerEx)FileEditorManager.getInstance(myProject)).getFile(fileEditor); document = virtualFile == null ? null : FileDocumentManager.getInstance().getDocument(virtualFile); } - vFiles.add(((FileEditorManagerEx)FileEditorManager.getInstance(myProject)).getFile(fileEditor)); + if (document != null) { + vFiles.add(FileDocumentManager.getInstance().getFile(document)); + } int prevId = 0; for (final HighlightingPass pass : passes) { diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/CodeSmellDetector.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/CodeSmellDetector.java index d9c08370263e..163e8271b1cf 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/CodeSmellDetector.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/CodeSmellDetector.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2015 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. @@ -16,10 +16,11 @@ package com.intellij.openapi.vcs; import com.intellij.codeInsight.CodeSmellInfo; -import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.project.Project; -import com.intellij.openapi.components.ServiceManager; +import com.intellij.openapi.vfs.VirtualFile; +import org.jetbrains.annotations.NotNull; import java.util.List; @@ -36,10 +37,11 @@ public abstract class CodeSmellDetector { * * @param files the files to analyze. * @return the list of problems found during the analysis. - * @throws com.intellij.openapi.progress.ProcessCanceledException if the analysis was cancelled by the user. + * @throws ProcessCanceledException if the analysis was cancelled by the user. * @since 5.1 */ - public abstract List findCodeSmells(List files) throws ProcessCanceledException; + @NotNull + public abstract List findCodeSmells(@NotNull List files) throws ProcessCanceledException; /** * Shows the specified list of problems found during pre-checkin code analysis in a Messages pane. @@ -47,6 +49,6 @@ public abstract class CodeSmellDetector { * @param smells the problems to show. * @since 5.1 */ - public abstract void showCodeSmellErrors(final List smells); + public abstract void showCodeSmellErrors(@NotNull List smells); } \ No newline at end of file diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/CodeSmellDetectorImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/CodeSmellDetectorImpl.java index bc8a731a9750..6cb076692379 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/CodeSmellDetectorImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/CodeSmellDetectorImpl.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2015 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. @@ -60,7 +60,7 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { } @Override - public void showCodeSmellErrors(final List smellList) { + public void showCodeSmellErrors(@NotNull final List smellList) { Collections.sort(smellList, new Comparator() { @Override public int compare(final CodeSmellInfo o1, final CodeSmellInfo o2) { @@ -104,9 +104,10 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { } - + @NotNull @Override - public List findCodeSmells(final List filesToCheck) throws ProcessCanceledException { + public List findCodeSmells(@NotNull final List filesToCheck) throws ProcessCanceledException { + ApplicationManager.getApplication().assertIsDispatchThread(); final List result = new ArrayList(); PsiDocumentManager.getInstance(myProject).commitAllDocuments(); if (ApplicationManager.getApplication().isWriteAccessAllowed()) throw new RuntimeException("Must not run under write action"); @@ -137,7 +138,8 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { } }); if (!exception.isNull()) { - Rethrow.reThrowRuntime(exception.get()); + Exception t = exception.get(); + Rethrow.reThrowRuntime(t); } return result; @@ -145,11 +147,11 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { @NotNull private List findCodeSmells(@NotNull final VirtualFile file, @NotNull final ProgressIndicator progress) { - final List result = new ArrayList(); + final List result = Collections.synchronizedList(new ArrayList()); final DaemonCodeAnalyzerImpl codeAnalyzer = (DaemonCodeAnalyzerImpl)DaemonCodeAnalyzer.getInstance(myProject); - final DaemonProgressIndicator daemonIndicator = new DaemonProgressIndicator(); - ((ProgressIndicatorEx)progress).addStateDelegate(new AbstractProgressIndicatorExBase(){ + final ProgressIndicator daemonIndicator = new DaemonProgressIndicator(); + ((ProgressIndicatorEx)progress).addStateDelegate(new AbstractProgressIndicatorExBase() { @Override public void cancel() { super.cancel(); @@ -163,13 +165,12 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { @Override public void run() { final PsiFile psiFile = PsiManager.getInstance(myProject).findFile(file); - if (psiFile != null) { - final Document document = FileDocumentManager.getInstance().getDocument(file); - if (document != null) { - List infos = codeAnalyzer.runMainPasses(psiFile, document, daemonIndicator); - collectErrorsAndWarnings(infos, result, document); - } + final Document document = FileDocumentManager.getInstance().getDocument(file); + if (psiFile == null || document == null) { + return; } + List infos = codeAnalyzer.runMainPasses(psiFile, document, daemonIndicator); + convertErrorsAndWarnings(infos, result, document); } }); } @@ -178,10 +179,9 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { return result; } - private void collectErrorsAndWarnings(final Collection highlights, - final List result, - final Document document) { - if (highlights == null) return; + private void convertErrorsAndWarnings(@NotNull Collection highlights, + @NotNull List result, + @NotNull Document document) { for (HighlightInfo highlightInfo : highlights) { final HighlightSeverity severity = highlightInfo.getSeverity(); if (SeverityRegistrar.getSeverityRegistrar(myProject).compare(severity, HighlightSeverity.WARNING) >= 0) { @@ -191,7 +191,7 @@ public class CodeSmellDetectorImpl extends CodeSmellDetector { } } - private static String getDescription(final HighlightInfo highlightInfo) { + private static String getDescription(@NotNull HighlightInfo highlightInfo) { final String description = highlightInfo.getDescription(); final HighlightInfoType type = highlightInfo.type; if (type instanceof HighlightInfoType.HighlightInfoTypeSeverityByKey) {