From d55c683edbb96957d6c6aa5517b1f315a75a7626 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 28 Oct 2016 18:48:10 +0200 Subject: [PATCH 01/39] a test proving that root changes increase psi mod counts --- .../intellij/psi/PsiModificationTrackerTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/java/java-tests/testSrc/com/intellij/psi/PsiModificationTrackerTest.java b/java/java-tests/testSrc/com/intellij/psi/PsiModificationTrackerTest.java index 0249fd186577..b3d0f10a079c 100644 --- a/java/java-tests/testSrc/com/intellij/psi/PsiModificationTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/PsiModificationTrackerTest.java @@ -18,11 +18,14 @@ package com.intellij.psi; import com.intellij.codeInsight.CodeInsightTestCase; import com.intellij.ide.highlighter.JavaFileType; import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.WriteAction; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Document; import com.intellij.openapi.editor.SelectionModel; import com.intellij.openapi.fileEditor.FileDocumentManager; import com.intellij.openapi.fileEditor.FileEditorManager; +import com.intellij.openapi.roots.ex.ProjectRootManagerEx; +import com.intellij.openapi.util.EmptyRunnable; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.vfs.CharsetToolkit; import com.intellij.openapi.vfs.LocalFileSystem; @@ -384,4 +387,17 @@ public class PsiModificationTrackerTest extends CodeInsightTestCase { VirtualFile virtualFile = LocalFileSystem.getInstance().refreshAndFindFileByIoFile(file); return PsiManager.getInstance(getProject()).findFile(virtualFile); } + + public void testRootsChangeIncreasesCounts() { + PsiModificationTracker tracker = PsiManager.getInstance(getProject()).getModificationTracker(); + long mc = tracker.getModificationCount(); + long js = tracker.getJavaStructureModificationCount(); + long ocb = tracker.getOutOfCodeBlockModificationCount(); + + WriteAction.run(() -> ProjectRootManagerEx.getInstanceEx(getProject()).makeRootsChange(EmptyRunnable.INSTANCE, false, true)); + + assertTrue(mc != tracker.getModificationCount()); + assertTrue(js != tracker.getJavaStructureModificationCount()); + assertTrue(ocb != tracker.getOutOfCodeBlockModificationCount()); + } } From f6a57edb2dfb82beba8a51d409c918228977b6bf Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 28 Oct 2016 18:58:49 +0200 Subject: [PATCH 02/39] teach more tests that find in path is multithreaded (IDEA-CR-15039) --- .../testSrc/com/intellij/psi/search/SearchInLibsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/java-tests/testSrc/com/intellij/psi/search/SearchInLibsTest.java b/java/java-tests/testSrc/com/intellij/psi/search/SearchInLibsTest.java index 1f07b8ed863e..61dffabf8aed 100644 --- a/java/java-tests/testSrc/com/intellij/psi/search/SearchInLibsTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/search/SearchInLibsTest.java @@ -131,7 +131,7 @@ public class SearchInLibsTest extends PsiTestCase { model.setStringToFind("xxx"); model.setProjectScope(false); - List usages = new ArrayList<>(); + List usages = Collections.synchronizedList(new ArrayList<>()); CommonProcessors.CollectProcessor consumer = new CommonProcessors.CollectProcessor<>(usages); FindUsagesProcessPresentation presentation = FindInProjectUtil.setupProcessPresentation(getProject(), false, FindInProjectUtil.setupViewPresentation(false, model)); FindInProjectUtil.findUsages(model, getProject(), consumer, presentation); From 10385e6e5636a5732cd62d3430af16d021b07de1 Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 29 Oct 2016 10:51:24 +0200 Subject: [PATCH 03/39] attempt as many tearDown activities as possible despite exceptions --- .../testFramework/LightPlatformTestCase.java | 227 ++++++++---------- .../testFramework/PlatformTestCase.java | 171 +++++-------- .../testFramework/UsefulTestCase.java | 98 +++----- .../impl/HeavyIdeaTestFixtureImpl.java | 74 +++--- .../impl/LightIdeaTestFixtureImpl.java | 24 +- .../util/lang/CompoundRuntimeException.java | 4 + 6 files changed, 232 insertions(+), 366 deletions(-) diff --git a/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java b/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java index acd884a43280..812d1eda30d3 100644 --- a/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/LightPlatformTestCase.java @@ -85,12 +85,15 @@ import com.intellij.psi.impl.PsiDocumentManagerImpl; import com.intellij.psi.impl.PsiManagerImpl; import com.intellij.psi.impl.source.tree.injected.InjectedLanguageManagerImpl; import com.intellij.psi.templateLanguages.TemplateDataLanguageMappings; -import com.intellij.util.*; +import com.intellij.util.GCUtil; +import com.intellij.util.IncorrectOperationException; +import com.intellij.util.LocalTimeCounter; +import com.intellij.util.ReflectionUtil; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.indexing.UnindexedFilesUpdater; -import com.intellij.util.lang.CompoundRuntimeException; import com.intellij.util.messages.MessageBusConnection; import com.intellij.util.ui.UIUtil; +import junit.framework.AssertionFailedError; import junit.framework.TestCase; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; @@ -104,9 +107,11 @@ import java.io.IOException; import java.io.PrintStream; import java.lang.management.GarbageCollectorMXBean; import java.lang.management.ManagementFactory; +import java.lang.reflect.Method; import java.util.Arrays; -import java.util.List; -import java.util.function.Consumer; +import java.util.concurrent.DelayQueue; +import java.util.concurrent.Delayed; +import java.util.concurrent.TimeUnit; /** * @author yole @@ -355,121 +360,104 @@ public abstract class LightPlatformTestCase extends UsefulTestCase implements Da @Override protected void tearDown() throws Exception { Project project = getProject(); - List errors = new SmartList<>(); - Consumer> runSafe = c -> { - try { - c.run(); - } - catch (Throwable e) { - errors.add(e); - } - }; - try { - runSafe.accept(() -> CodeStyleSettingsManager.getInstance(project).dropTemporarySettings()); - runSafe.accept(() -> checkForSettingsDamage(errors)); - runSafe.accept(() -> doTearDown(project, ourApplication, true, errors)); - runSafe.accept(super::tearDown); - runSafe.accept(() -> myThreadTracker.checkLeak()); - runSafe.accept(() -> InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project)); - runSafe.accept(() -> ((VirtualFilePointerManagerImpl)VirtualFilePointerManager.getInstance()).assertPointersAreDisposed()); - } - catch (Throwable e) { - errors.add(e); - } - finally { - CompoundRuntimeException.throwIfNotEmpty(errors); - } + + new RunAll( + () -> CodeStyleSettingsManager.getInstance(project).dropTemporarySettings(), + () -> checkForSettingsDamage(), + () -> doTearDown(project, ourApplication, true), + super::tearDown, + () -> myThreadTracker.checkLeak(), + () -> InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project), + () -> ((VirtualFilePointerManagerImpl)VirtualFilePointerManager.getInstance()).assertPointersAreDisposed() + ).run(); } - public static void doTearDown(@NotNull final Project project, @NotNull IdeaTestApplication application, boolean checkForEditors, @NotNull List exceptions) throws Exception { - PsiDocumentManagerImpl documentManager; - try { - ((FileTypeManagerImpl)FileTypeManager.getInstance()).drainReDetectQueue(); - DocumentCommitThread.getInstance().clearQueue(); - CodeStyleSettingsManager.getInstance(project).dropTemporarySettings(); - - checkJavaSwingTimersAreDisposed(exceptions); - - UsefulTestCase.doPostponedFormatting(project); - - LookupManager lookupManager = LookupManager.getInstance(project); - if (lookupManager != null) { - lookupManager.hideActiveLookup(); - } - ((StartupManagerImpl)StartupManager.getInstance(project)).prepareForNextTest(); - if (ProjectManager.getInstance() == null) { - exceptions.add(new AssertionError("Application components damaged")); - } - - ContainerUtil.addIfNotNull(exceptions, new WriteCommandAction.Simple(project) { - @Override - protected void run() throws Throwable { - if (ourSourceRoot != null) { - try { - final VirtualFile[] children = ourSourceRoot.getChildren(); - for (VirtualFile child : children) { - child.delete(this); - } - } - catch (IOException e) { - //noinspection CallToPrintStackTrace - e.printStackTrace(); + public static void doTearDown(@NotNull final Project project, @NotNull IdeaTestApplication application, boolean checkForEditors) throws Exception { + new RunAll(). + append(() -> ((FileTypeManagerImpl)FileTypeManager.getInstance()).drainReDetectQueue()). + append(() -> CodeStyleSettingsManager.getInstance(project).dropTemporarySettings()). + append(() -> checkJavaSwingTimersAreDisposed()). + append(() -> UsefulTestCase.doPostponedFormatting(project)). + append(() -> LookupManager.getInstance(project).hideActiveLookup()). + append(() -> ((StartupManagerImpl)StartupManager.getInstance(project)).prepareForNextTest()). + append(() -> { if (ProjectManager.getInstance() == null) throw new AssertionError("Application components damaged"); }). + append(() -> WriteCommandAction.runWriteCommandAction(project, () -> { + if (ourSourceRoot != null) { + try { + for (VirtualFile child : ourSourceRoot.getChildren()) { + child.delete(LightPlatformTestCase.class); } } - EncodingManager encodingManager = EncodingManager.getInstance(); - if (encodingManager instanceof EncodingManagerImpl) ((EncodingManagerImpl)encodingManager).clearDocumentQueue(); - - FileDocumentManager manager = FileDocumentManager.getInstance(); - - ApplicationManager.getApplication().runWriteAction(EmptyRunnable.getInstance()); // Flush postponed formatting if any. - manager.saveAllDocuments(); - if (manager instanceof FileDocumentManagerImpl) { - ((FileDocumentManagerImpl)manager).dropAllUnsavedDocuments(); + catch (IOException e) { + //noinspection CallToPrintStackTrace + e.printStackTrace(); } } - }.execute().getThrowable()); + EncodingManager encodingManager = EncodingManager.getInstance(); + if (encodingManager instanceof EncodingManagerImpl) ((EncodingManagerImpl)encodingManager).clearDocumentQueue(); - assertFalse(PsiManager.getInstance(project).isDisposed()); - if (!ourAssertionsInTestDetected) { - if (IdeaLogger.ourErrorsOccurred != null) { - throw IdeaLogger.ourErrorsOccurred; + FileDocumentManager manager = FileDocumentManager.getInstance(); + + ApplicationManager.getApplication().runWriteAction(EmptyRunnable.getInstance()); // Flush postponed formatting if any. + manager.saveAllDocuments(); + if (manager instanceof FileDocumentManagerImpl) { + ((FileDocumentManagerImpl)manager).dropAllUnsavedDocuments(); } - } - documentManager = clearUncommittedDocuments(project); - ((HintManagerImpl)HintManager.getInstance()).cleanup(); - DocumentCommitThread.getInstance().clearQueue(); - - EdtTestUtil.runInEdtAndWait(() -> { - ((UndoManagerImpl)UndoManager.getGlobalInstance()).dropHistoryInTests(); - ((UndoManagerImpl)UndoManager.getInstance(project)).dropHistoryInTests(); - - UIUtil.dispatchAllInvocationEvents(); - }); - - TemplateDataLanguageMappings.getInstance(project).cleanupForNextTest(); - } - finally { - ProjectManagerEx.getInstanceEx().closeTestProject(project); - application.setDataProvider(null); - ourTestCase = null; - ((PsiManagerImpl)PsiManager.getInstance(project)).cleanupForNextTest(); - CompletionProgressIndicator.cleanupForNextTest(); - } - - if (checkForEditors) { - checkEditorsReleased(exceptions); - } - documentManager.clearUncommittedDocuments(); - - if (ourTestCount++ % 100 == 0) { - // some tests are written in Groovy, and running all of them may result in some 40M of memory wasted on bean infos - // so let's clear the cache every now and then to ensure it doesn't grow too large - GCUtil.clearBeanInfoCache(); - } + })). + append(() -> assertFalse(PsiManager.getInstance(project).isDisposed())). + append(() -> { + if (!ourAssertionsInTestDetected) { + if (IdeaLogger.ourErrorsOccurred != null) { + throw IdeaLogger.ourErrorsOccurred; + } + } + }). + append(() -> clearUncommittedDocuments(project)). + append(() -> ((HintManagerImpl)HintManager.getInstance()).cleanup()). + append(() -> DocumentCommitThread.getInstance().clearQueue()). + append(() -> ((UndoManagerImpl)UndoManager.getGlobalInstance()).dropHistoryInTests()). + append(() -> ((UndoManagerImpl)UndoManager.getInstance(project)).dropHistoryInTests()). + append(() -> UIUtil.dispatchAllInvocationEvents()). + append(() -> TemplateDataLanguageMappings.getInstance(project).cleanupForNextTest()). + append(() -> ProjectManagerEx.getInstanceEx().closeTestProject(project)). + append(() -> application.setDataProvider(null)). + append(() -> ourTestCase = null). + append(() -> ((PsiManagerImpl)PsiManager.getInstance(project)).cleanupForNextTest()). + append(() -> CompletionProgressIndicator.cleanupForNextTest()). + append(() -> { + if (checkForEditors) { + checkEditorsReleased(); + } + }). + append(() -> { + if (ourTestCount++ % 100 == 0) { + // some tests are written in Groovy, and running all of them may result in some 40M of memory wasted on bean infos + // so let's clear the cache every now and then to ensure it doesn't grow too large + GCUtil.clearBeanInfoCache(); + } + }). + run(); } private static int ourTestCount; + private static void checkJavaSwingTimersAreDisposed() throws Exception { + Class TimerQueueClass = Class.forName("javax.swing.TimerQueue"); + Method sharedInstance = ReflectionUtil.getMethod(TimerQueueClass, "sharedInstance"); + + Object timerQueue = sharedInstance.invoke(null); + DelayQueue delayQueue = ReflectionUtil.getField(TimerQueueClass, timerQueue, DelayQueue.class, "queue"); + Delayed timer = delayQueue.peek(); + if (timer != null) { + long delay = timer.getDelay(TimeUnit.MILLISECONDS); + String text = "(delayed for " + delay + "ms)"; + Method getTimer = ReflectionUtil.getDeclaredMethod(timer.getClass(), "getTimer"); + Timer swingTimer = (Timer)getTimer.invoke(timer); + text = "Timer (listeners: "+Arrays.asList(swingTimer.getActionListeners()) + ") "+text; + throw new AssertionFailedError("Not disposed java.swing.Timer: " + text + "; queue:" + timerQueue); + } + } + public static PsiDocumentManagerImpl clearUncommittedDocuments(@NotNull Project project) { PsiDocumentManagerImpl documentManager = (PsiDocumentManagerImpl)PsiDocumentManager.getInstance(project); documentManager.clearUncommittedDocuments(); @@ -482,30 +470,21 @@ public abstract class LightPlatformTestCase extends UsefulTestCase implements Da return documentManager; } - public static void checkEditorsReleased(@NotNull List exceptions) { + public static void checkEditorsReleased() { Editor[] allEditors = EditorFactory.getInstance().getAllEditors(); if (allEditors.length == 0) { return; } - + RunAll runAll = new RunAll(); for (Editor editor : allEditors) { - try { - EditorFactoryImpl.throwNotReleasedError(editor); - } - catch (Throwable e) { - exceptions.add(e); - } - finally { - EditorFactory.getInstance().releaseEditor(editor); - } + runAll = runAll + .append(() -> EditorFactoryImpl.throwNotReleasedError(editor)) + .append(() -> EditorFactory.getInstance().releaseEditor(editor)); } - try { - ((EditorImpl)allEditors[0]).throwDisposalError("Unreleased editors: " + allEditors.length); - } - catch (Throwable e) { - exceptions.add(e); - } + runAll + .append(() -> ((EditorImpl)allEditors[0]).throwDisposalError("Unreleased editors: " + allEditors.length)) + .run(); } @SuppressWarnings("AssignmentToStaticFieldFromInstanceMethod") diff --git a/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java b/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java index 328653faaf83..d59d83fef025 100644 --- a/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/PlatformTestCase.java @@ -66,11 +66,13 @@ import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.impl.DocumentCommitThread; import com.intellij.psi.impl.PsiManagerImpl; import com.intellij.psi.impl.source.tree.injected.InjectedLanguageManagerImpl; -import com.intellij.util.*; +import com.intellij.util.MemoryDumpHelper; +import com.intellij.util.PathUtilRt; +import com.intellij.util.PlatformUtils; +import com.intellij.util.ThrowableRunnable; import com.intellij.util.indexing.FileBasedIndex; import com.intellij.util.indexing.FileBasedIndexImpl; import com.intellij.util.indexing.IndexableSetContributor; -import com.intellij.util.lang.CompoundRuntimeException; import com.intellij.util.ui.UIUtil; import junit.framework.TestCase; import org.jetbrains.annotations.NonNls; @@ -91,7 +93,6 @@ import java.net.URL; import java.nio.charset.Charset; import java.util.Collection; import java.util.HashSet; -import java.util.List; import java.util.Set; /** @@ -427,144 +428,88 @@ public abstract class PlatformTestCase extends UsefulTestCase implements DataPro } } + @SuppressWarnings("MethodDoesntCallSuperMethod") @Override protected void tearDown() throws Exception { - List exceptions = new SmartList<>(); Project project = myProject; - if (project != null) { - try { - LightPlatformTestCase.doTearDown(project, ourApplication, false, exceptions); - } - catch (Throwable e) { - exceptions.add(e); - } - disposeProject(exceptions); - } - - try { - checkForSettingsDamage(exceptions); - } - catch (Throwable e) { - exceptions.add(e); - } - try { - if (project != null) { - try { + new RunAll() + .append(() -> { + if (project != null) { + LightPlatformTestCase.doTearDown(project, ourApplication, false); + } + }) + .append(() -> disposeProject()) + .append(() -> checkForSettingsDamage()) + .append(() -> { + if (project != null) { InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project); } - catch (AssertionError e) { - exceptions.add(e); - } - } - try { + }) + .append(() -> { for (final File fileToDelete : myFilesToDelete) { delete(fileToDelete); } LocalFileSystem.getInstance().refreshIoFiles(myFilesToDelete); - } - catch (Throwable e) { - exceptions.add(e); - } - - if (!myAssertionsInTestDetected) { - if (IdeaLogger.ourErrorsOccurred != null) { - exceptions.add(IdeaLogger.ourErrorsOccurred); + }) + .append(() -> { + if (!myAssertionsInTestDetected) { + if (IdeaLogger.ourErrorsOccurred != null) { + throw IdeaLogger.ourErrorsOccurred; + } } - } - - try { - super.tearDown(); - } - catch (Throwable e) { - exceptions.add(e); - } - - try { + }) + .append(super::tearDown) + .append(() -> { if (myEditorListenerTracker != null) { myEditorListenerTracker.checkListenersLeak(); } - } - catch (AssertionError error) { - exceptions.add(error); - } - try { + }) + .append(() -> { if (myThreadTracker != null) { myThreadTracker.checkLeak(); } - } - catch (AssertionError error) { - exceptions.add(error); - } - try { - LightPlatformTestCase.checkEditorsReleased(exceptions); - } - catch (Throwable error) { - exceptions.add(error); - } - } - finally { - myProjectManager = null; - myProject = null; - myModule = null; - myFilesToDelete.clear(); - myEditorListenerTracker = null; - myThreadTracker = null; - ourTestCase = null; - - CompoundRuntimeException.throwIfNotEmpty(exceptions); - } + }) + .append(() -> LightPlatformTestCase.checkEditorsReleased()) + .append(() -> { + myProjectManager = null; + myProject = null; + myModule = null; + myFilesToDelete.clear(); + myEditorListenerTracker = null; + myThreadTracker = null; + //noinspection AssignmentToStaticFieldFromInstanceMethod + ourTestCase = null; + }) + .run(); } - private void disposeProject(@NotNull List exceptions) { - try { + private void disposeProject() { + new RunAll(() -> { DocumentCommitThread.getInstance().clearQueue(); // sometimes SwingUtilities maybe confused about EDT at this point if (SwingUtilities.isEventDispatchThread()) { UIUtil.dispatchAllInvocationEvents(); } - } - catch (Throwable e) { - exceptions.add(e); - } - - Project project = myProject; - if (project == null) { - return; - } - - closeAndDisposeProjectAndCheckThatNoOpenProjects(project, exceptions); - myProject = null; + }, () -> { + if (myProject != null) { + closeAndDisposeProjectAndCheckThatNoOpenProjects(myProject); + myProject = null; + } + }).run(); } - public static void closeAndDisposeProjectAndCheckThatNoOpenProjects(@NotNull final Project projectToClose, @NotNull final List exceptions) { - try { - ProjectManagerEx projectManager = ProjectManagerEx.getInstanceEx(); - if (projectManager instanceof ProjectManagerImpl) { - for (Project project : projectManager.closeTestProject(projectToClose)) { - exceptions.add(new IllegalStateException("Test project is not disposed: " + project + ";\n created in: " + getCreationPlace(project))); - try { - ((ProjectManagerImpl)projectManager).closeProject(project, false, true, false); - } - catch (Throwable e) { - exceptions.add(e); - } - } + public static void closeAndDisposeProjectAndCheckThatNoOpenProjects(@NotNull final Project projectToClose) { + RunAll runAll = new RunAll(); + ProjectManagerEx projectManager = ProjectManagerEx.getInstanceEx(); + if (projectManager instanceof ProjectManagerImpl) { + for (Project project : projectManager.closeTestProject(projectToClose)) { + runAll = runAll + .append(() -> { throw new IllegalStateException("Test project is not disposed: " + project + ";\n created in: " + getCreationPlace(project)); }) + .append(() -> ((ProjectManagerImpl)projectManager).closeProject(project, false, true, false)); } } - catch (Throwable e) { - exceptions.add(e); - } - finally { - ApplicationManager.getApplication().runWriteAction(() -> { - try { - Disposer.dispose(projectToClose); - } - catch (Throwable e) { - exceptions.add(e); - } - }); - } + runAll.append(() -> WriteAction.run(() -> Disposer.dispose(projectToClose))).run(); } protected void resetAllFields() { diff --git a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java index f86a15f2d6fe..4b733a9646b7 100644 --- a/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java +++ b/platform/testFramework/src/com/intellij/testFramework/UsefulTestCase.java @@ -58,7 +58,6 @@ import org.jetbrains.annotations.Nullable; import org.junit.Assert; import javax.swing.*; -import javax.swing.Timer; import java.awt.*; import java.io.File; import java.io.FileNotFoundException; @@ -69,9 +68,6 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.*; import java.util.List; -import java.util.concurrent.DelayQueue; -import java.util.concurrent.Delayed; -import java.util.concurrent.TimeUnit; import java.util.regex.Pattern; /** @@ -242,7 +238,7 @@ public abstract class UsefulTestCase extends TestCase { containerMap.clear(); } - protected void checkForSettingsDamage(@NotNull List exceptions) { + protected void checkForSettingsDamage() { Application app = ApplicationManager.getApplication(); if (isPerformanceTest() || app == null || app instanceof MockApplication) { return; @@ -255,53 +251,43 @@ public abstract class UsefulTestCase extends TestCase { myOldCodeStyleSettings = null; - doCheckForSettingsDamage(oldCodeStyleSettings, getCurrentCodeStyleSettings(), exceptions); + doCheckForSettingsDamage(oldCodeStyleSettings, getCurrentCodeStyleSettings()); } public static void doCheckForSettingsDamage(@NotNull CodeStyleSettings oldCodeStyleSettings, - @NotNull CodeStyleSettings currentCodeStyleSettings, - @NotNull List exceptions) { + @NotNull CodeStyleSettings currentCodeStyleSettings) { final CodeInsightSettings settings = CodeInsightSettings.getInstance(); - try { - Element newS = new Element("temp"); - settings.writeExternal(newS); - Assert.assertEquals("Code insight settings damaged", DEFAULT_SETTINGS_EXTERNALIZED, JDOMUtil.writeElement(newS, "\n")); - } - catch (AssertionError error) { - CodeInsightSettings clean = new CodeInsightSettings(); - for (Field field : clean.getClass().getFields()) { + new RunAll() + .append(() -> { try { - ReflectionUtil.copyFieldValue(clean, settings, field); + Element newS = new Element("temp"); + settings.writeExternal(newS); + Assert.assertEquals("Code insight settings damaged", DEFAULT_SETTINGS_EXTERNALIZED, JDOMUtil.writeElement(newS, "\n")); } - catch (Exception ignored) { + catch (AssertionError error) { + CodeInsightSettings clean = new CodeInsightSettings(); + for (Field field : clean.getClass().getFields()) { + try { + ReflectionUtil.copyFieldValue(clean, settings, field); + } + catch (Exception ignored) { + } + } + throw error; } - } - exceptions.add(error); - } - - currentCodeStyleSettings.getIndentOptions(StdFileTypes.JAVA); - try { - checkSettingsEqual(oldCodeStyleSettings, currentCodeStyleSettings, "Code style settings damaged"); - } - catch (Throwable e) { - exceptions.add(e); - } - finally { - currentCodeStyleSettings.clearCodeStyleSettings(); - } - - try { - InplaceRefactoring.checkCleared(); - } - catch (AssertionError e) { - exceptions.add(e); - } - try { - StartMarkAction.checkCleared(); - } - catch (AssertionError e) { - exceptions.add(e); - } + }) + .append(() -> { + currentCodeStyleSettings.getIndentOptions(StdFileTypes.JAVA); + try { + checkSettingsEqual(oldCodeStyleSettings, currentCodeStyleSettings, "Code style settings damaged"); + } + finally { + currentCodeStyleSettings.clearCodeStyleSettings(); + } + }) + .append(() -> InplaceRefactoring.checkCleared()) + .append(() -> StartMarkAction.checkCleared()) + .run(); } void storeSettings() { @@ -845,28 +831,6 @@ public abstract class UsefulTestCase extends TestCase { }); } - static void checkJavaSwingTimersAreDisposed(@NotNull List exceptions) { - try { - Class TimerQueueClass = Class.forName("javax.swing.TimerQueue"); - Method sharedInstance = ReflectionUtil.getMethod(TimerQueueClass, "sharedInstance"); - - Object timerQueue = sharedInstance.invoke(null); - DelayQueue delayQueue = ReflectionUtil.getField(TimerQueueClass, timerQueue, DelayQueue.class, "queue"); - Delayed timer = delayQueue.peek(); - if (timer != null) { - long delay = timer.getDelay(TimeUnit.MILLISECONDS); - String text = "(delayed for " + delay + "ms)"; - Method getTimer = ReflectionUtil.getDeclaredMethod(timer.getClass(), "getTimer"); - Timer swingTimer = (Timer)getTimer.invoke(timer); - text = "Timer (listeners: "+Arrays.asList(swingTimer.getActionListeners()) + ") "+text; - exceptions.add(new AssertionFailedError("Not disposed java.swing.Timer: " + text + "; queue:" + timerQueue)); - } - } - catch (Throwable e) { - exceptions.add(e); - } - } - /** * Checks that code block throw corresponding exception. * diff --git a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/HeavyIdeaTestFixtureImpl.java b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/HeavyIdeaTestFixtureImpl.java index 9ac857d7bb76..54a56552c142 100644 --- a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/HeavyIdeaTestFixtureImpl.java +++ b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/HeavyIdeaTestFixtureImpl.java @@ -22,7 +22,7 @@ import com.intellij.idea.IdeaTestApplication; import com.intellij.openapi.actionSystem.CommonDataKeys; import com.intellij.openapi.actionSystem.DataProvider; import com.intellij.openapi.actionSystem.LangDataKeys; -import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.fileEditor.FileEditorManager; @@ -35,7 +35,6 @@ import com.intellij.openapi.module.ModuleManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.project.ex.ProjectManagerEx; import com.intellij.openapi.roots.ProjectRootManager; -import com.intellij.openapi.util.Computable; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.LocalFileSystem; @@ -47,9 +46,8 @@ import com.intellij.psi.impl.source.tree.injected.InjectedLanguageManagerImpl; import com.intellij.testFramework.*; import com.intellij.testFramework.builders.ModuleFixtureBuilder; import com.intellij.testFramework.fixtures.HeavyIdeaTestFixture; +import com.intellij.util.ObjectUtils; import com.intellij.util.PathUtil; -import com.intellij.util.SmartList; -import com.intellij.util.lang.CompoundRuntimeException; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -61,7 +59,6 @@ import java.io.IOException; import java.io.PrintStream; import java.util.HashSet; import java.util.LinkedHashSet; -import java.util.List; import java.util.Set; /** @@ -102,49 +99,39 @@ class HeavyIdeaTestFixtureImpl extends BaseFixture implements HeavyIdeaTestFixtu @Override public void tearDown() throws Exception { final Project project = getProject(); - final List exceptions = new SmartList<>(); - try { - LightPlatformTestCase.doTearDown(project, myApplication, false, exceptions); - for (ModuleFixtureBuilder moduleFixtureBuilder : myModuleFixtureBuilders) { - moduleFixtureBuilder.getFixture().tearDown(); - } - - EdtTestUtil.runInEdtAndWait(() -> PlatformTestCase.closeAndDisposeProjectAndCheckThatNoOpenProjects(project, exceptions)); - myProject = null; - - for (File fileToDelete : myFilesToDelete) { - if (!FileUtil.delete(fileToDelete)) { - exceptions.add(new IOException("Can't delete " + fileToDelete)); + RunAll runAll = new RunAll() + .append(() -> LightPlatformTestCase.doTearDown(project, myApplication, false)) + .append(() -> { + for (ModuleFixtureBuilder moduleFixtureBuilder : myModuleFixtureBuilders) { + moduleFixtureBuilder.getFixture().tearDown(); } - } - } - catch (Throwable e) { - exceptions.add(e); + }) + .append(() -> EdtTestUtil.runInEdtAndWait(() -> PlatformTestCase.closeAndDisposeProjectAndCheckThatNoOpenProjects(project))) + .append(() -> myProject = null); + + for (File fileToDelete : myFilesToDelete) { + runAll = runAll.append(() -> { + if (!FileUtil.delete(fileToDelete)) { + throw new IOException("Can't delete " + fileToDelete); + } + }); } - try { - super.tearDown(); - } - catch (Throwable e) { - exceptions.add(e); - } - - try { - myEditorListenerTracker.checkListenersLeak(); - myThreadTracker.checkLeak(); - LightPlatformTestCase.checkEditorsReleased(exceptions); - PlatformTestCase.cleanupApplicationCaches(project); - InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project); - } - finally { - CompoundRuntimeException.throwIfNotEmpty(exceptions); - } + runAll + .append(super::tearDown) + .append(() -> myEditorListenerTracker.checkListenersLeak()) + .append(() -> myThreadTracker.checkLeak()) + .append(() -> LightPlatformTestCase.checkEditorsReleased()) + .append(() -> PlatformTestCase.cleanupApplicationCaches(project)) + .append(() -> InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project)) + .run(); } private void setUpProject() throws IOException { File tempDirectory = FileUtil.createTempDirectory(myName, ""); - PlatformTestCase.synchronizeTempDirVfs(LocalFileSystem.getInstance().refreshAndFindFileByIoFile(tempDirectory)); + PlatformTestCase + .synchronizeTempDirVfs(ObjectUtils.assertNotNull(LocalFileSystem.getInstance().refreshAndFindFileByIoFile(tempDirectory))); myFilesToDelete.add(tempDirectory); String projectPath = FileUtil.toSystemIndependentName(tempDirectory.getPath()) + "/" + myName + ProjectFileType.DOT_DEFAULT_EXTENSION; @@ -239,11 +226,6 @@ class HeavyIdeaTestFixtureImpl extends BaseFixture implements HeavyIdeaTestFixtu PsiDocumentManager.getInstance(getProject()).commitAllDocuments(); } }.execute(); - return ApplicationManager.getApplication().runReadAction(new Computable() { - @Override - public PsiFile compute() { - return PsiManager.getInstance(getProject()).findFile(virtualFile[0]); - } - }); + return ReadAction.compute(() -> PsiManager.getInstance(getProject()).findFile(virtualFile[0])); } } diff --git a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/LightIdeaTestFixtureImpl.java b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/LightIdeaTestFixtureImpl.java index 1a9f90d08620..467a0f1f88d4 100644 --- a/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/LightIdeaTestFixtureImpl.java +++ b/platform/testFramework/src/com/intellij/testFramework/fixtures/impl/LightIdeaTestFixtureImpl.java @@ -28,10 +28,6 @@ import com.intellij.psi.codeStyle.CodeStyleSettingsManager; import com.intellij.psi.impl.source.tree.injected.InjectedLanguageManagerImpl; import com.intellij.testFramework.*; import com.intellij.testFramework.fixtures.LightIdeaTestFixture; -import com.intellij.util.SmartList; -import com.intellij.util.lang.CompoundRuntimeException; - -import java.util.List; /** * @author mike @@ -65,19 +61,15 @@ public class LightIdeaTestFixtureImpl extends BaseFixture implements LightIdeaTe CodeStyleSettingsManager.getInstance(project).dropTemporarySettings(); CodeStyleSettings oldCodeStyleSettings = myOldCodeStyleSettings; myOldCodeStyleSettings = null; - List exceptions = new SmartList<>(); - try { - UsefulTestCase.doCheckForSettingsDamage(oldCodeStyleSettings, getCurrentCodeStyleSettings(), exceptions); - LightPlatformTestCase.doTearDown(project, LightPlatformTestCase.getApplication(), true, exceptions); - super.tearDown(); - InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project); - PersistentFS.getInstance().clearIdCache(); - PlatformTestCase.cleanupApplicationCaches(project); - } - finally { - CompoundRuntimeException.throwIfNotEmpty(exceptions); - } + new RunAll() + .append(() -> UsefulTestCase.doCheckForSettingsDamage(oldCodeStyleSettings, getCurrentCodeStyleSettings())) + .append(() -> LightPlatformTestCase.doTearDown(project, LightPlatformTestCase.getApplication(), true)) + .append(super::tearDown) + .append(() -> InjectedLanguageManagerImpl.checkInjectorsAreDisposed(project)) + .append(() -> PersistentFS.getInstance().clearIdCache()) + .append(() -> PlatformTestCase.cleanupApplicationCaches(project)) + .run(); } @Override diff --git a/platform/util/src/com/intellij/util/lang/CompoundRuntimeException.java b/platform/util/src/com/intellij/util/lang/CompoundRuntimeException.java index 4beb5afc9e09..caf014b8d5f3 100644 --- a/platform/util/src/com/intellij/util/lang/CompoundRuntimeException.java +++ b/platform/util/src/com/intellij/util/lang/CompoundRuntimeException.java @@ -37,6 +37,10 @@ public class CompoundRuntimeException extends RuntimeException { myExceptions = throwables; } + public List getExceptions() { + return myExceptions; + } + @Override public String getMessage() { return processAll(new Function() { From 513cb6943294d03566a80d820e9aa557ff930213 Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 29 Oct 2016 11:02:50 +0200 Subject: [PATCH 04/39] add diagnostics and make non-exceptional EA-83768 - assert: AstPath.invalidatePaths --- .../intellij/psi/impl/source/PsiFileImpl.java | 14 ++++++++++++-- .../intellij/psi/impl/source/tree/AstPath.java | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/PsiFileImpl.java b/platform/core-impl/src/com/intellij/psi/impl/source/PsiFileImpl.java index cd6dcfaf4de2..e019751e1be9 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/PsiFileImpl.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/PsiFileImpl.java @@ -74,6 +74,7 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF private boolean myInvalidated; private volatile boolean myAstLoaded; private volatile boolean myUseStrongRefs; + private volatile boolean mySwitchingToStrongRefs; private AstPathPsiMap myRefToPsi; private final ThreadLocal myFileElementBeingLoaded = new ThreadLocal(); protected final PsiManagerEx myManager; @@ -276,7 +277,7 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF StubBasedPsiElementBase psi = pair.first; AstPath path = pair.second; path.getNode().setPsi(psi); - myRefToPsi.cachePsi(path, psi); + associateAstPathWithPsi(path, psi); psi.setStubIndex(i + 1); } } @@ -1131,6 +1132,8 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF public final void beforeAstChange() { if (!useStrongRefs()) { + LOG.assertTrue(!mySwitchingToStrongRefs); + mySwitchingToStrongRefs = true; myRefToPsi.switchToStrongRefs(); FileElement element = getTreeElement(); @@ -1141,6 +1144,7 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF } myUseStrongRefs = true; + mySwitchingToStrongRefs = false; } } @@ -1155,10 +1159,16 @@ public abstract class PsiFileImpl extends ElementBase implements PsiFileEx, PsiF synchronized (PsiLock.LOCK) { psi = myRefToPsi.getCachedPsi(path); - return psi != null ? psi : myRefToPsi.cachePsi(path, creator.create()); + return psi != null ? psi : associateAstPathWithPsi(path, creator.create()); } } + @NotNull + private StubBasedPsiElementBase associateAstPathWithPsi(@NotNull AstPath path, @NotNull StubBasedPsiElementBase psi) { + LOG.assertTrue(!mySwitchingToStrongRefs); + return myRefToPsi.cachePsi(path, psi); + } + final AstPathPsiMap getRefToPsi() { return myRefToPsi; } diff --git a/platform/core-impl/src/com/intellij/psi/impl/source/tree/AstPath.java b/platform/core-impl/src/com/intellij/psi/impl/source/tree/AstPath.java index e96cc0679f5b..19c4d03e30c3 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/source/tree/AstPath.java +++ b/platform/core-impl/src/com/intellij/psi/impl/source/tree/AstPath.java @@ -16,6 +16,7 @@ package com.intellij.psi.impl.source.tree; import com.intellij.extapi.psi.StubBasedPsiElementBase; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Key; import com.intellij.openapi.vfs.VirtualFileWithId; import com.intellij.psi.PsiElement; @@ -39,6 +40,7 @@ import java.util.List; * @author peter */ public abstract class AstPath extends SubstrateRef { + private static final Logger LOG = Logger.getInstance("#com.intellij.psi.impl.source.tree.AstPath"); private static final Key PATH_CHILDREN = Key.create("PATH_CHILDREN"); private static final Key NODE_PATH = Key.create("NODE_PATH"); @@ -111,18 +113,22 @@ public abstract class AstPath extends SubstrateRef { scope.putUserData(PATH_CHILDREN, null); for (CompositeElement child : children) { child.putUserData(NODE_PATH, null); - PsiElement cachedPsi = child.getCachedPsi(); - if (cachedPsi instanceof StubBasedPsiElementBase) { - if (((StubBasedPsiElementBase)cachedPsi).getSubstrateRef() instanceof AstPath) { - throw new AssertionError(cachedPsi.hashCode()); - } - } + assertConsistency(child.getCachedPsi()); if (child instanceof LazyParseableElement) { invalidatePaths((LazyParseableElement)child); } } } + private static void assertConsistency(PsiElement cachedPsi) { + if (cachedPsi instanceof StubBasedPsiElementBase && + ((StubBasedPsiElementBase)cachedPsi).getSubstrateRef() instanceof AstPath) { + LOG.error("Expected strong reference at " + cachedPsi + + " of " + cachedPsi.getClass() + + " and " + ((StubBasedPsiElementBase)cachedPsi).getElementType()); + } + } + private static class ChildPath extends AstPath { private final AstPath myParent; private final int myIndex; From 5ade7757ecc50e7e1b6d80e58dff4f43eeb5cb1c Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 29 Oct 2016 11:03:08 +0200 Subject: [PATCH 05/39] fix compilation --- .../com/intellij/testFramework/RunAll.java | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 platform/testFramework/src/com/intellij/testFramework/RunAll.java diff --git a/platform/testFramework/src/com/intellij/testFramework/RunAll.java b/platform/testFramework/src/com/intellij/testFramework/RunAll.java new file mode 100644 index 000000000000..97f4d3a37304 --- /dev/null +++ b/platform/testFramework/src/com/intellij/testFramework/RunAll.java @@ -0,0 +1,70 @@ +/* + * Copyright 2000-2016 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.testFramework; + +import com.intellij.util.SmartList; +import com.intellij.util.ThrowableRunnable; +import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.lang.CompoundRuntimeException; +import org.jetbrains.annotations.Contract; +import org.jetbrains.annotations.NotNull; + +import java.util.Arrays; +import java.util.List; + +/** + * Runs all given runnables and throws all the caught exceptions at the end. + * + * @author peter + */ +public class RunAll implements Runnable { + private final List> myActions; + + public RunAll(ThrowableRunnable... actions) { + this(Arrays.asList(actions)); + } + + private RunAll(List> actions) { + myActions = actions; + } + + @Contract(pure=true) + public RunAll append(ThrowableRunnable... actions) { + return new RunAll(ContainerUtil.concat(myActions, ContainerUtil.newArrayList(actions))); + } + + @Override + public void run() { + CompoundRuntimeException.throwIfNotEmpty(collectExceptions()); + } + + @NotNull + public List collectExceptions() { + List errors = new SmartList<>(); + for (ThrowableRunnable action : myActions) { + try { + action.run(); + } + catch (CompoundRuntimeException e) { + errors.addAll(e.getExceptions()); + } + catch (Throwable e) { + errors.add(e); + } + } + return errors; + } +} From d5a1e152578d3aa044c2874839a99149faa1e12f Mon Sep 17 00:00:00 2001 From: Alexander Koshevoy Date: Sat, 29 Oct 2016 13:07:07 +0300 Subject: [PATCH 06/39] PY-21273 Fixing deadlock on debugging Python script in docker container Excessive synchronization on calling waitForConnect() of the main debugger removed. Operations with myOtherDebuggers made safe without going into critical sections if some uncontrolled methods are called within them, particularly in isConnected() method. --- .../pydev/ClientModeMultiProcessDebugger.java | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/python/pydevSrc/com/jetbrains/python/debugger/pydev/ClientModeMultiProcessDebugger.java b/python/pydevSrc/com/jetbrains/python/debugger/pydev/ClientModeMultiProcessDebugger.java index f3111c1bece2..49a7e210e016 100644 --- a/python/pydevSrc/com/jetbrains/python/debugger/pydev/ClientModeMultiProcessDebugger.java +++ b/python/pydevSrc/com/jetbrains/python/debugger/pydev/ClientModeMultiProcessDebugger.java @@ -30,7 +30,6 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { private final List myOtherDebuggers = Lists.newArrayList(); private ThreadRegistry myThreadRegistry = new ThreadRegistry(); - @NotNull private final Object mySocketsObject = new Object(); public ClientModeMultiProcessDebugger(@NotNull final IPyDebugProcess debugProcess, @NotNull String host, int port) { @@ -55,18 +54,14 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { @Override public boolean isConnected() { - synchronized (myOtherDebuggersObject) { - return myOtherDebuggers.stream().anyMatch(RemoteDebugger::isConnected); - } + return getOtherDebuggers().stream().anyMatch(RemoteDebugger::isConnected); } @Override public void waitForConnect() throws Exception { Thread.sleep(500L); - synchronized (mySocketsObject) { - myMainDebugger.waitForConnect(); - } + myMainDebugger.waitForConnect(); } private void connectToSubprocess() { @@ -172,7 +167,7 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { else { // thread is not found in registry - lets search for it in attached debuggers - for (ProcessDebugger d : myOtherDebuggers) { + for (ProcessDebugger d : getOtherDebuggers()) { for (PyThreadInfo thread : d.getThreads()) { if (threadId.equals(thread.getId())) { return d; @@ -223,7 +218,7 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { List threads = collectAllThreads(); - if (myOtherDebuggers.size() > 0) { + if (!isOtherDebuggersEmpty()) { //here we add process id to thread name in case there are more then one process return Collections.unmodifiableCollection(Collections2.transform(threads, new Function() { @Override @@ -260,11 +255,8 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { return result; } - private void cleanOtherDebuggers() { - synchronized (myOtherDebuggersObject) { - removeDisconnected(getOtherDebuggers()); - } + removeDisconnected(getOtherDebuggers()); } private void removeDisconnected(ArrayList debuggers) { @@ -295,6 +287,11 @@ public class ClientModeMultiProcessDebugger implements ProcessDebugger { } } + private boolean isOtherDebuggersEmpty() { + synchronized (myOtherDebuggersObject) { + return myOtherDebuggers.isEmpty(); + } + } @Override public void execute(@NotNull AbstractCommand command) { From baaaa8b004033830a1dfdfd9d74751175153dafb Mon Sep 17 00:00:00 2001 From: Alexander Koshevoy Date: Sat, 29 Oct 2016 13:52:45 +0300 Subject: [PATCH 07/39] PY-21273 Fixing deadlock on debugging Python script in docker container Critical sections in ClientModeDebuggerTransport under mySocketObject shortened, myState taken out of them completely. --- .../ClientModeDebuggerTransport.java | 172 +++++++++--------- 1 file changed, 82 insertions(+), 90 deletions(-) diff --git a/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ClientModeDebuggerTransport.java b/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ClientModeDebuggerTransport.java index 8cf033a84bdf..4deb6b3e6a06 100644 --- a/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ClientModeDebuggerTransport.java +++ b/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ClientModeDebuggerTransport.java @@ -69,7 +69,7 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { @NotNull private final String myHost; private final int myPort; - @NotNull private State myState = State.INIT; + @NotNull private volatile State myState = State.INIT; @Nullable private Socket mySocket; @Nullable private DebuggerReader myDebuggerReader; @@ -92,14 +92,13 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { catch (InterruptedException e) { throw new IOException(e); } - synchronized (mySocketObject) { - if (myState != State.INIT) { - throw new IllegalStateException( - "Inappropriate state of Python debugger for connecting to Python debugger: " + myState + "; " + State.INIT + " is expected"); - } - doConnect(); + if (myState != State.INIT) { + throw new IllegalStateException( + "Inappropriate state of Python debugger for connecting to Python debugger: " + myState + "; " + State.INIT + " is expected"); } + + doConnect(); } private void doConnect() throws IOException { @@ -115,72 +114,72 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { mySocket = null; } } - - int i = 0; - boolean connected = false; - while (!connected && i < MAX_CONNECTION_TRIES) { - i++; - try { - Socket clientSocket = new Socket(); - clientSocket.setSoTimeout(0); - clientSocket.connect(new InetSocketAddress(myHost, myPort)); - - try { - myDebuggerReader = new DebuggerReader(myDebugger, clientSocket.getInputStream()); - } - catch (IOException e) { - LOG.debug("Failed to create debugger reader", e); - throw e; - } - - mySocket = clientSocket; - connected = true; - } - catch (ConnectException e) { - if (i < MAX_CONNECTION_TRIES) { - try { - Thread.sleep(SLEEP_TIME_BETWEEN_CONNECTION_TRIES); - } - catch (InterruptedException e1) { - throw new IOException(e1); - } - } - } - } - - if (!connected) { - myState = State.DISCONNECTED; - throw new IOException("Failed to connect to debugging script"); - } - - myState = State.CONNECTED; - LOG.debug("Connected to Python debugger script on #" + i + " attempt"); - - - try { - myDebugProcess.init(); - myDebugger.run(); - } - catch (PyDebuggerException e) { - myState = State.DISCONNECTED; - throw new IOException("Failed to send run command", e); - } - - myScheduledExecutor.schedule(() -> { - synchronized (mySocketObject) { - if (myState == State.CONNECTED) { - try { - LOG.debug("Reconnecting..."); - doConnect(); - } - catch (IOException e) { - LOG.debug(e); - myDebugger.fireCommunicationError(); - } - } - } - }, CHECK_CONNECTION_APPROVED_DELAY, TimeUnit.MILLISECONDS); } + + int i = 0; + boolean connected = false; + while (!connected && i < MAX_CONNECTION_TRIES) { + i++; + try { + Socket clientSocket = new Socket(); + clientSocket.setSoTimeout(0); + clientSocket.connect(new InetSocketAddress(myHost, myPort)); + + try { + myDebuggerReader = new DebuggerReader(myDebugger, clientSocket.getInputStream()); + } + catch (IOException e) { + LOG.debug("Failed to create debugger reader", e); + throw e; + } + + synchronized (mySocketObject) { + mySocket = clientSocket; + } + connected = true; + } + catch (ConnectException e) { + if (i < MAX_CONNECTION_TRIES) { + try { + Thread.sleep(SLEEP_TIME_BETWEEN_CONNECTION_TRIES); + } + catch (InterruptedException e1) { + throw new IOException(e1); + } + } + } + } + + if (!connected) { + myState = State.DISCONNECTED; + throw new IOException("Failed to connect to debugging script"); + } + + myState = State.CONNECTED; + LOG.debug("Connected to Python debugger script on #" + i + " attempt"); + + + try { + myDebugProcess.init(); + myDebugger.run(); + } + catch (PyDebuggerException e) { + myState = State.DISCONNECTED; + throw new IOException("Failed to send run command", e); + } + + myScheduledExecutor.schedule(() -> { + if (myState == State.CONNECTED) { + try { + LOG.debug("Reconnecting..."); + doConnect(); + } + catch (IOException e) { + LOG.debug(e); + myDebugger.fireCommunicationError(); + } + } + }, CHECK_CONNECTION_APPROVED_DELAY, TimeUnit.MILLISECONDS); } @Override @@ -198,13 +197,13 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { @Override public void close() { - synchronized (mySocketObject) { - try { - if (myDebuggerReader != null) { - myDebuggerReader.stop(); - } + try { + if (myDebuggerReader != null) { + myDebuggerReader.stop(); } - finally { + } + finally { + synchronized (mySocketObject) { if (mySocket != null) { try { mySocket.close(); @@ -218,10 +217,7 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { @Override public boolean isConnected() { - synchronized (mySocketObject) { - return myState == State.APPROVED; - //return myConnected && mySocket != null && !mySocket.isClosed(); - } + return myState == State.APPROVED; } @Override @@ -231,10 +227,8 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { @Override public void messageReceived(@NotNull ProtocolFrame frame) { - synchronized (mySocketObject) { - if (myState == State.CONNECTED) { - myState = State.APPROVED; - } + if (myState == State.CONNECTED) { + myState = State.APPROVED; } } @@ -266,10 +260,8 @@ public class ClientModeDebuggerTransport extends BaseDebuggerTransport { } protected void onCommunicationError() { - synchronized (mySocketObject) { - if (myState == State.APPROVED) { - getDebugger().fireCommunicationError(); - } + if (myState == State.APPROVED) { + getDebugger().fireCommunicationError(); } } } From bb9812222514f2920f7659ee022fedbbef4e99bb Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 29 Oct 2016 12:35:28 +0200 Subject: [PATCH 08/39] handle MethodOrClosureScopeChooser selection in a write-safe context (EA-90852 - assert: PsiModificationTrackerImpl.fireEvent) --- .../groovy/refactoring/ui/MethodOrClosureScopeChooser.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/ui/MethodOrClosureScopeChooser.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/ui/MethodOrClosureScopeChooser.java index c977bbe66d61..5a1bfa1fd363 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/ui/MethodOrClosureScopeChooser.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/refactoring/ui/MethodOrClosureScopeChooser.java @@ -15,6 +15,7 @@ */ package org.jetbrains.plugins.groovy.refactoring.ui; +import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.editor.colors.EditorColors; @@ -151,7 +152,7 @@ public class MethodOrClosureScopeChooser { else { toSearchFor = superMethod.isEnabled() && superMethod.isSelected() ? ToSearchIn.getParent() : null; } - IdeFocusManager.findInstance().doWhenFocusSettlesDown(() -> callback.fun(ToSearchIn, toSearchFor)); + IdeFocusManager.findInstance().doWhenFocusSettlesDown(() -> callback.fun(ToSearchIn, toSearchFor), ModalityState.current()); } }, KeyStroke.getKeyStroke(KeyEvent.VK_ENTER, 0))); From bc84dbaa749ef58ef3691f21331d5d5bc0903d03 Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 29 Oct 2016 13:03:46 +0200 Subject: [PATCH 09/39] show git merge dialog in a write-safe context (EA-90653 - assert: FileDocumentManagerImpl.saveAllDocuments) --- plugins/git4idea/src/git4idea/merge/GitMergeUtil.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/git4idea/src/git4idea/merge/GitMergeUtil.java b/plugins/git4idea/src/git4idea/merge/GitMergeUtil.java index ac9c6d32da34..ddc285718ce2 100644 --- a/plugins/git4idea/src/git4idea/merge/GitMergeUtil.java +++ b/plugins/git4idea/src/git4idea/merge/GitMergeUtil.java @@ -18,6 +18,7 @@ package git4idea.merge; import com.intellij.history.Label; import com.intellij.history.LocalHistory; import com.intellij.ide.util.ElementsChooser; +import com.intellij.openapi.application.ModalityState; import com.intellij.openapi.project.Project; import com.intellij.openapi.vcs.AbstractVcsHelper; import com.intellij.openapi.vcs.ProjectLevelVcsManager; @@ -29,6 +30,7 @@ import com.intellij.openapi.vcs.update.UpdateInfoTree; import com.intellij.openapi.vcs.update.UpdatedFiles; import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.ui.GuiUtils; import com.intellij.util.ui.UIUtil; import git4idea.GitRevisionNumber; import git4idea.GitVcs; @@ -143,12 +145,12 @@ public class GitMergeUtil { Collection unmergedNames = files.getGroupById(FileGroup.MERGED_WITH_CONFLICT_ID).getFiles(); if (!unmergedNames.isEmpty()) { List unmerged = mapNotNull(unmergedNames, name -> LocalFileSystem.getInstance().findFileByPath(name)); - UIUtil.invokeLaterIfNeeded(() -> { + GuiUtils.invokeLaterIfNeeded(() -> { GitVcs vcs = GitVcs.getInstance(project); if (vcs != null) { AbstractVcsHelper.getInstance(project).showMergeDialog(unmerged, vcs.getMergeProvider()); } - }); + }, ModalityState.defaultModalityState()); } } } From 6963fdde8a6d8272a4cf42e116acbe854d8e3d6e Mon Sep 17 00:00:00 2001 From: Yaroslav Lepenkin Date: Fri, 28 Oct 2016 18:02:07 +0300 Subject: [PATCH 10/39] [Parameter Name Hints] add ToggleInlineHints action to parameter hints popup menu. In regular popup menu action will be shown only if hints are disabled and there is a hint to show at caret. --- .../inlays/ToggleInlineHintsActionTest.kt | 82 +++++++++++++++++++ .../codeInsight/hints/PopupActions.kt | 46 +++++++++-- .../src/idea/LangActions.xml | 1 + 3 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 java/java-tests/testSrc/com/intellij/codeInsight/daemon/inlays/ToggleInlineHintsActionTest.kt diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/inlays/ToggleInlineHintsActionTest.kt b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/inlays/ToggleInlineHintsActionTest.kt new file mode 100644 index 000000000000..132087a8626d --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/inlays/ToggleInlineHintsActionTest.kt @@ -0,0 +1,82 @@ +/* + * Copyright 2000-2016 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.codeInsight.daemon.inlays + +import com.intellij.codeInsight.hints.isPossibleHintNearOffset +import com.intellij.openapi.editor.ex.EditorSettingsExternalizable +import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase +import org.assertj.core.api.Assertions.assertThat + +class ToggleInlineHintsActionTest : LightCodeInsightFixtureTestCase() { + + private var before: Boolean = false + + override fun setUp() { + super.setUp() + before = EditorSettingsExternalizable.getInstance().isShowParameterNameHints + EditorSettingsExternalizable.getInstance().isShowParameterNameHints = false + } + + override fun tearDown() { + EditorSettingsExternalizable.getInstance().isShowParameterNameHints = before + super.tearDown() + } + + fun `test is enabled near method with possible hints`() { + myFixture.configureByText("A.java", """" +class Test { + Test(int time) {} + void initialize(int loadTime) {} + static void s_test() { + Test test = new Test(10); + test.initialize(10000); + } +} +""") + + val file = myFixture.file + val caretModel = myFixture.editor.caretModel + + assertThat(caretModel.caretCount).isEqualTo(4) + + editor.caretModel.allCarets.forEach { + val hintCanBeAtCaret = isPossibleHintNearOffset(file, it.offset) + assertThat(hintCanBeAtCaret).isTrue() + } + } + + fun `test is disabled in random places`() { + myFixture.configureByText("A.java", """" +class Test { + static void s_test() { + int a = 2; + List list = null; + } +} +""") + + val file = myFixture.file + val caretModel = myFixture.editor.caretModel + + assertThat(caretModel.caretCount).isEqualTo(3) + + editor.caretModel.allCarets.forEach { + val hintCanBeAtCaret = isPossibleHintNearOffset(file, it.offset) + assertThat(hintCanBeAtCaret).isFalse() + } + } + +} \ No newline at end of file diff --git a/platform/lang-impl/src/com/intellij/codeInsight/hints/PopupActions.kt b/platform/lang-impl/src/com/intellij/codeInsight/hints/PopupActions.kt index cbbe24034532..aba36f650a9e 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/hints/PopupActions.kt +++ b/platform/lang-impl/src/com/intellij/codeInsight/hints/PopupActions.kt @@ -23,6 +23,7 @@ import com.intellij.codeInsight.hints.settings.ParameterNameHintsSettings import com.intellij.codeInsight.intention.HighPriorityAction import com.intellij.codeInsight.intention.IntentionAction import com.intellij.injected.editor.EditorWindow +import com.intellij.openapi.actionSystem.ActionPlaces import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.AnActionEvent import com.intellij.openapi.actionSystem.CommonDataKeys @@ -98,16 +99,32 @@ class ToggleInlineHintsAction : AnAction() { } override fun update(e: AnActionEvent) { - if (InlayParameterHintsExtension.hasAnyExtensions()) { - e.presentation.isEnabledAndVisible = true - val isShow = EditorSettingsExternalizable.getInstance().isShowParameterNameHints - e.presentation.text = if (isShow) disableText else enableText + if (!InlayParameterHintsExtension.hasAnyExtensions()) { + e.presentation.isEnabledAndVisible = false + return } - else { - e.presentation.isEnabledAndVisible = false + + val isHintsShownNow = EditorSettingsExternalizable.getInstance().isShowParameterNameHints + e.presentation.text = if (isHintsShownNow) disableText else enableText + e.presentation.isEnabledAndVisible = true + + if (isInMainEditorPopup(e)) { + val file = CommonDataKeys.PSI_FILE.getData(e.dataContext) ?: return + val editor = CommonDataKeys.EDITOR.getData(e.dataContext) ?: return + val caretOffset = editor.caretModel.offset + e.presentation.isEnabledAndVisible = !isHintsShownNow && isPossibleHintNearOffset(file, caretOffset) } } + private fun isInMainEditorPopup(e: AnActionEvent): Boolean { + if (e.place != ActionPlaces.EDITOR_POPUP) return false + + val editor = CommonDataKeys.EDITOR.getData(e.dataContext) ?: return false + val offset = editor.caretModel.offset + + return !editor.inlayModel.hasInlineElementAt(offset) + } + override fun actionPerformed(e: AnActionEvent) { val settings = EditorSettingsExternalizable.getInstance() val before = settings.isShowParameterNameHints @@ -156,4 +173,19 @@ private fun addMethodAtCaretToBlackList(editor: Editor, file: PsiFile) { ParameterNameHintsSettings.getInstance().addIgnorePattern(file.language, pattern) refreshAllOpenEditors() -} \ No newline at end of file +} + +fun isPossibleHintNearOffset(file: PsiFile, offset: Int): Boolean { + val hintProvider = InlayParameterHintsExtension.forLanguage(file.language) ?: return false + + var element = file.findElementAt(offset) + for (i in 0..3) { + if (element == null) return false + + val hints = hintProvider.getParameterHints(element) + if (hints.isNotEmpty()) return true + element = element.parent + } + + return false +} diff --git a/platform/platform-resources/src/idea/LangActions.xml b/platform/platform-resources/src/idea/LangActions.xml index 79fc2e5f0658..a0c79c8bff5b 100644 --- a/platform/platform-resources/src/idea/LangActions.xml +++ b/platform/platform-resources/src/idea/LangActions.xml @@ -306,6 +306,7 @@ + From ac6615e2825a682493857ff1358c62690179a497 Mon Sep 17 00:00:00 2001 From: Sergey Ignatov Date: Sat, 29 Oct 2016 17:24:15 +0300 Subject: [PATCH 11/39] allow to override initial selection in open file action --- .../src/com/intellij/ide/actions/OpenFileAction.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java b/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java index 3ccf199c0dab..0e3bd6d60725 100644 --- a/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java +++ b/platform/platform-impl/src/com/intellij/ide/actions/OpenFileAction.java @@ -56,7 +56,7 @@ public class OpenFileAction extends AnAction implements DumbAware { final FileChooserDescriptor descriptor = showFiles ? new ProjectOrFileChooserDescriptor() : new ProjectOnlyFileChooserDescriptor(); descriptor.putUserData(PathChooserDialog.PREFER_LAST_OVER_EXPLICIT, showFiles); - FileChooser.chooseFiles(descriptor, project, VfsUtil.getUserHomeDir(), files -> { + FileChooser.chooseFiles(descriptor, project, getPathToSelect(), files -> { for (VirtualFile file : files) { if (!descriptor.isFileSelectable(file)) { String message = IdeBundle.message("error.dir.contains.no.project", file.getPresentableUrl()); @@ -68,6 +68,11 @@ public class OpenFileAction extends AnAction implements DumbAware { }); } + @Nullable + protected VirtualFile getPathToSelect() { + return VfsUtil.getUserHomeDir(); + } + @Override public void update(@NotNull AnActionEvent e) { if (NewWelcomeScreen.isNewWelcomeScreen(e)) { From d3f262672a1a0fea985e7bd6da9c369c43faacc6 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 18:29:04 +0300 Subject: [PATCH 12/39] Remove useless annotation --- .../openapi/vcs/impl/projectlevelman/NewMappings.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java index c57d3dc5e34d..67071915a69b 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java @@ -128,7 +128,6 @@ public class NewMappings { mappingsChanged(); } - @Modification public void setMapping(final String path, final String activeVcsName) { LOG.debug("setMapping path = '" + path + "' vcs = " + activeVcsName); final VcsDirectoryMapping newMapping = new VcsDirectoryMapping(path, activeVcsName); @@ -198,7 +197,6 @@ public class NewMappings { myFileWatchRequestsManager.ping(); } - @Modification public void setDirectoryMappings(final List items) { LOG.debug("setDirectoryMappings, size: " + items.size()); MySetMappingsPreProcessor setMappingsPreProcessor = new MySetMappingsPreProcessor(items); @@ -304,13 +302,11 @@ public class NewMappings { return result; } - @Modification public void disposeMe() { LOG.debug("dispose me"); clearImpl(); } - @Modification public void clear() { LOG.debug("clear"); clearImpl(); @@ -367,7 +363,6 @@ public class NewMappings { } } - @Modification public void removeDirectoryMapping(final VcsDirectoryMapping mapping) { LOG.debug("remove mapping: " + mapping.getDirectory()); @@ -565,7 +560,6 @@ public class NewMappings { } } - @Modification public void beingUnregistered(final String name) { synchronized (myLock) { keepActiveVcs(new Runnable() { @@ -601,9 +595,6 @@ public class NewMappings { } } - private @interface Modification { - } - public List getDefaultRoots() { synchronized (myLock) { final String defaultVcs = haveDefaultMapping(); From 6b12c785babb622ec3685368edd38aef73a40b0f Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 18:30:19 +0300 Subject: [PATCH 13/39] Lambdify --- .../vcs/impl/projectlevelman/NewMappings.java | 69 +++++++------------ 1 file changed, 26 insertions(+), 43 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java index 67071915a69b..0d4903655c0e 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java @@ -42,12 +42,7 @@ import java.util.*; public class NewMappings { - public static Comparator MAPPINGS_COMPARATOR = new Comparator() { - @Override - public int compare(@NotNull VcsDirectoryMapping o1, @NotNull VcsDirectoryMapping o2) { - return o1.getDirectory().compareTo(o2.getDirectory()); - } - }; + public static Comparator MAPPINGS_COMPARATOR = Comparator.comparing(VcsDirectoryMapping::getDirectory); private final static Logger LOG = Logger.getInstance("#com.intellij.openapi.vcs.impl.projectlevelman.NewMappings"); private final Object myLock; @@ -90,11 +85,9 @@ public class NewMappings { } myActivated = false; - vcsManager.addInitializationRequest(VcsInitObject.MAPPINGS, new DumbAwareRunnable() { - public void run() { - if (!myProject.isDisposed()) { - activateActiveVcses(); - } + vcsManager.addInitializationRequest(VcsInitObject.MAPPINGS, (DumbAwareRunnable)() -> { + if (!myProject.isDisposed()) { + activateActiveVcses(); } }); } @@ -142,15 +135,13 @@ public class NewMappings { } final Ref switched = new Ref<>(Boolean.FALSE); - keepActiveVcs(new Runnable() { - public void run() { - // sorted -> map. sorted mappings are NOT changed; - switched.set(trySwitchVcs(path, activeVcsName)); - if (!switched.get().booleanValue()) { - final List newList = listForVcsFromMap(newMapping.getVcs()); - newList.add(newMapping); - sortedMappingsByMap(); - } + keepActiveVcs(() -> { + // sorted -> map. sorted mappings are NOT changed; + switched.set(trySwitchVcs(path, activeVcsName)); + if (!switched.get().booleanValue()) { + final List newList = listForVcsFromMap(newMapping.getVcs()); + newList.add(newMapping); + sortedMappingsByMap(); } }); @@ -210,14 +201,12 @@ public class NewMappings { itemsCopy = items; } - keepActiveVcs(new Runnable() { - public void run() { - myVcsToPaths.clear(); - for (VcsDirectoryMapping mapping : itemsCopy) { - listForVcsFromMap(mapping.getVcs()).add(mapping); - } - sortedMappingsByMap(); + keepActiveVcs(() -> { + myVcsToPaths.clear(); + for (VcsDirectoryMapping mapping : itemsCopy) { + listForVcsFromMap(mapping.getVcs()).add(mapping); } + sortedMappingsByMap(); }); mappingsChanged(); @@ -318,12 +307,10 @@ public class NewMappings { // if vcses were not mapped, there's nothing to clear if ((myActiveVcses == null) || (myActiveVcses.length == 0)) return; - keepActiveVcs(new Runnable() { - public void run() { - myVcsToPaths.clear(); - myActiveVcses = new AbstractVcs[0]; - mySortedMappings = VcsDirectoryMapping.EMPTY_ARRAY; - } + keepActiveVcs(() -> { + myVcsToPaths.clear(); + myActiveVcses = new AbstractVcs[0]; + mySortedMappings = VcsDirectoryMapping.EMPTY_ARRAY; }); myFileWatchRequestsManager.ping(); } @@ -366,11 +353,9 @@ public class NewMappings { public void removeDirectoryMapping(final VcsDirectoryMapping mapping) { LOG.debug("remove mapping: " + mapping.getDirectory()); - keepActiveVcs(new Runnable() { - public void run() { - if (removeVcsFromMap(mapping, mapping.getVcs())) { - sortedMappingsByMap(); - } + keepActiveVcs(() -> { + if (removeVcsFromMap(mapping, mapping.getVcs())) { + sortedMappingsByMap(); } }); @@ -562,11 +547,9 @@ public class NewMappings { public void beingUnregistered(final String name) { synchronized (myLock) { - keepActiveVcs(new Runnable() { - public void run() { - myVcsToPaths.remove(name); - sortedMappingsByMap(); - } + keepActiveVcs(() -> { + myVcsToPaths.remove(name); + sortedMappingsByMap(); }); } From 494eff81f4e4e901b78a5228122a2ffe8750b9b9 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 18:31:28 +0300 Subject: [PATCH 14/39] Remove unused inner class --- .../vcs/impl/projectlevelman/NewMappings.java | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java index 0d4903655c0e..5029672fde3e 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java @@ -190,8 +190,6 @@ public class NewMappings { public void setDirectoryMappings(final List items) { LOG.debug("setDirectoryMappings, size: " + items.size()); - MySetMappingsPreProcessor setMappingsPreProcessor = new MySetMappingsPreProcessor(items); - setMappingsPreProcessor.invoke(); final List itemsCopy; if (items.isEmpty()) { @@ -556,28 +554,6 @@ public class NewMappings { mappingsChanged(); } - private static class MySetMappingsPreProcessor { - private final List myItems; - private List myItemsCopy; - - public MySetMappingsPreProcessor(final List items) { - myItems = items; - } - - public List getItemsCopy() { - return myItemsCopy; - } - - public void invoke() { - if (myItems.isEmpty()) { - myItemsCopy = Collections.singletonList(new VcsDirectoryMapping("", "")); - } - else { - myItemsCopy = myItems; - } - } - } - public List getDefaultRoots() { synchronized (myLock) { final String defaultVcs = haveDefaultMapping(); From 343d9a2ea6f43398f95a5ff71b1d08120f77ab1c Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 14 Oct 2016 18:40:52 +0300 Subject: [PATCH 15/39] [vcs-log] register caches invalidator for log so that Invalidate... action would remove system/vcs-log directory 1. On invalidateCaches create a "corruption.marker" in system/vcs-log. 2. Check existence of this marker before creating log storage and indexes. If marker exists, remove system/vcs-log. 3. Problem arises, however, when system/vcs-log could not be deleted. Nothing can be done here, so just display error balloon, fallback to memory storage and empty indexes. Loading log in background should be disabled in this case. --- .../vcs-log/impl/src/META-INF/vcs-log.xml | 2 + .../com/intellij/vcs/log/data/EmptyIndex.java | 59 +++++++++++++++++++ .../com/intellij/vcs/log/data/VcsLogData.java | 28 +++++++-- .../vcs/log/impl/FatalErrorConsumer.java | 2 + .../vcs/log/impl/VcsLogCachesInvalidator.java | 46 +++++++++++++++ .../intellij/vcs/log/impl/VcsLogManager.java | 5 ++ .../intellij/vcs/log/impl/VcsProjectLog.java | 6 +- .../intellij/vcs/log/util/PersistentUtil.java | 6 ++ .../vcs/log/data/VcsLogRefresherTest.java | 13 +++- .../vcs/log/data/VisiblePackBuilderTest.kt | 26 -------- 10 files changed, 160 insertions(+), 33 deletions(-) create mode 100644 platform/vcs-log/impl/src/com/intellij/vcs/log/data/EmptyIndex.java create mode 100644 platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java diff --git a/platform/vcs-log/impl/src/META-INF/vcs-log.xml b/platform/vcs-log/impl/src/META-INF/vcs-log.xml index 4f44100f5b9e..960c14f95090 100644 --- a/platform/vcs-log/impl/src/META-INF/vcs-log.xml +++ b/platform/vcs-log/impl/src/META-INF/vcs-log.xml @@ -25,6 +25,8 @@ + + diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/EmptyIndex.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/EmptyIndex.java new file mode 100644 index 000000000000..a83d4e16310b --- /dev/null +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/EmptyIndex.java @@ -0,0 +1,59 @@ +/* + * Copyright 2000-2016 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.vcs.log.data; + +import com.intellij.openapi.vfs.VirtualFile; +import com.intellij.vcs.log.VcsLogDetailsFilter; +import com.intellij.vcs.log.data.index.VcsLogIndex; +import org.jetbrains.annotations.NotNull; + +import java.util.List; +import java.util.Set; + +public class EmptyIndex implements VcsLogIndex { + @Override + public void scheduleIndex(boolean full) { + } + + @Override + public boolean isIndexed(int commit) { + return false; + } + + @Override + public boolean isIndexed(@NotNull VirtualFile root) { + return false; + } + + @Override + public void markForIndexing(int commit, @NotNull VirtualFile root) { + } + + @Override + public boolean canFilter(@NotNull List filters) { + return false; + } + + @NotNull + @Override + public Set filter(@NotNull List detailsFilters) { + throw new UnsupportedOperationException(); + } + + @Override + public void markCorrupted() { + } +} diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java index 16af2331ad5d..be451c68b38b 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java @@ -15,6 +15,7 @@ */ package com.intellij.vcs.log.data; +import com.intellij.ide.caches.CachesInvalidator; import com.intellij.openapi.Disposable; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.components.ServiceManager; @@ -35,6 +36,8 @@ import com.intellij.vcs.log.*; import com.intellij.vcs.log.data.index.VcsLogIndex; import com.intellij.vcs.log.data.index.VcsLogPersistentIndex; import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.VcsLogCachesInvalidator; +import com.intellij.vcs.log.util.PersistentUtil; import com.intellij.vcs.log.util.StopWatch; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -91,15 +94,30 @@ public class VcsLogData implements Disposable, VcsLogDataProvider { myUserRegistry = (VcsUserRegistryImpl)ServiceManager.getService(project, VcsUserRegistry.class); myFatalErrorsConsumer = fatalErrorsConsumer; - myHashMap = createLogHashMap(); + VcsLogProgress progress = new VcsLogProgress(); + Disposer.register(this, progress); + + VcsLogCachesInvalidator invalidator = CachesInvalidator.EP_NAME.findExtension(VcsLogCachesInvalidator.class); + if (invalidator.isValid()) { + myHashMap = createLogHashMap(); + myIndex = new VcsLogPersistentIndex(myProject, myHashMap, progress, logProviders, myFatalErrorsConsumer, this); + } + else { + // this is not recoverable + // restart won't help here + // and can not shut down ide because of this + // so use memory storage (probably leading to out of memory at some point) + no index + String message = "Could not delete " + PersistentUtil.LOG_CACHE + "\nDelete it manually and restart IDEA."; + LOG.error(message); + myFatalErrorsConsumer.displayFatalErrorMessage(message); + myHashMap = new InMemoryStorage(); + myIndex = new EmptyIndex(); + } + myTopCommitsDetailsCache = new TopCommitsCache(myHashMap); myMiniDetailsGetter = new MiniDetailsGetter(myHashMap, logProviders, myTopCommitsDetailsCache, this); myDetailsGetter = new CommitDetailsGetter(myHashMap, logProviders, this); - VcsLogProgress progress = new VcsLogProgress(); - Disposer.register(this, progress); - myIndex = new VcsLogPersistentIndex(myProject, myHashMap, progress, logProviders, myFatalErrorsConsumer, this); - myRefresher = new VcsLogRefresherImpl(myProject, myHashMap, myLogProviders, myUserRegistry, myIndex, progress, myTopCommitsDetailsCache, this::fireDataPackChangeEvent, FAILING_EXCEPTION_HANDLER, RECENT_COMMITS_COUNT); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java index 566ec3d80c1c..0573ed544ba6 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java @@ -20,4 +20,6 @@ import org.jetbrains.annotations.Nullable; public interface FatalErrorConsumer { void consume(@Nullable Object source, @NotNull Exception exception); + + void displayFatalErrorMessage(@NotNull String message); } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java new file mode 100644 index 000000000000..e0260bbf485e --- /dev/null +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java @@ -0,0 +1,46 @@ +/* + * Copyright 2000-2016 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.vcs.log.impl; + +import com.intellij.ide.caches.CachesInvalidator; +import com.intellij.openapi.util.io.FileUtil; +import com.intellij.util.ArrayUtil; +import com.intellij.vcs.log.util.PersistentUtil; + +public class VcsLogCachesInvalidator extends CachesInvalidator { + + public synchronized boolean isValid() { + if (PersistentUtil.getCorruptionMarkerFile().exists()) { + boolean deleted = FileUtil.delete(PersistentUtil.LOG_CACHE); + if (!deleted) { + // if could not delete caches, ensure that corruption marker is still there + FileUtil.createIfDoesntExist(PersistentUtil.getCorruptionMarkerFile()); + } + return deleted; + } + return true; + } + + @Override + public void invalidateCaches() { + if (PersistentUtil.LOG_CACHE.exists()) { + String[] children = PersistentUtil.LOG_CACHE.list(); + if (!ArrayUtil.isEmpty(children)) { + FileUtil.createIfDoesntExist(PersistentUtil.getCorruptionMarkerFile()); + } + } + } +} diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index 84f4b4bbf5a9..cb685aea87f3 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -240,5 +240,10 @@ public class VcsLogManager implements Disposable { myLogData.getIndex().markCorrupted(); } } + + @Override + public void displayFatalErrorMessage(@NotNull String message) { + VcsBalloonProblemNotifier.showOverChangesView(myProject, message, MessageType.ERROR); + } } } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsProjectLog.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsProjectLog.java index 3664dd8b9033..390b7cc1b0a0 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsProjectLog.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsProjectLog.java @@ -15,6 +15,7 @@ */ package com.intellij.vcs.log.impl; +import com.intellij.ide.caches.CachesInvalidator; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.project.Project; @@ -111,7 +112,10 @@ public class VcsProjectLog { logManager.scheduleInitialization(); } else if (PostponableLogRefresher.keepUpToDate()) { - HeavyAwareExecutor.executeOutOfHeavyProcessLater(logManager::scheduleInitialization, 5000); + VcsLogCachesInvalidator invalidator = CachesInvalidator.EP_NAME.findExtension(VcsLogCachesInvalidator.class); + if (invalidator.isValid()) { + HeavyAwareExecutor.executeOutOfHeavyProcessLater(logManager::scheduleInitialization, 5000); + } } } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/util/PersistentUtil.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/util/PersistentUtil.java index 407c77cce7c6..b39928c3b663 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/util/PersistentUtil.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/util/PersistentUtil.java @@ -33,6 +33,7 @@ import java.util.Map; public class PersistentUtil { @NotNull public static final File LOG_CACHE = new File(PathManager.getSystemPath(), "vcs-log"); + @NotNull private static final String CORRUPTION_MARKER = "corruption.marker"; @NotNull public static String calcLogId(@NotNull Project project, @NotNull Map logProviders) { @@ -83,4 +84,9 @@ public class PersistentUtil { new PersistentHashMap<>(storageFile, new IntInlineKeyDescriptor(), externalizer, Page.PAGE_SIZE), storageFile); } + + @NotNull + public static File getCorruptionMarkerFile() { + return new File(LOG_CACHE, CORRUPTION_MARKER); + } } diff --git a/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VcsLogRefresherTest.java b/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VcsLogRefresherTest.java index da9fe03e983b..d438daa36f4d 100644 --- a/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VcsLogRefresherTest.java +++ b/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VcsLogRefresherTest.java @@ -33,6 +33,7 @@ import com.intellij.vcs.log.graph.GraphCommit; import com.intellij.vcs.log.impl.*; import com.intellij.vcs.test.VcsPlatformTest; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.util.*; import java.util.concurrent.*; @@ -194,7 +195,17 @@ public class VcsLogRefresherTest extends VcsPlatformTest { } private VcsLogRefresherImpl createLoader(Consumer dataPackConsumer) { - myLogData = new VcsLogData(myProject, myLogProviders, (source, exception) -> LOG.error(exception)); + myLogData = new VcsLogData(myProject, myLogProviders, new FatalErrorHandler() { + @Override + public void consume(@Nullable Object source, @NotNull Exception exception) { + LOG.error(exception); + } + + @Override + public void displayFatalErrorMessage(@NotNull String message) { + LOG.error(message); + } + }); Disposer.register(myProject, myLogData); return new VcsLogRefresherImpl(myProject, myLogData.getHashMap(), myLogProviders, myLogData.getUserRegistry(), myLogData.getIndex(), new VcsLogProgress(), diff --git a/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VisiblePackBuilderTest.kt b/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VisiblePackBuilderTest.kt index 172b294d2b55..44071cfd9fbc 100644 --- a/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VisiblePackBuilderTest.kt +++ b/platform/vcs-log/impl/test/com/intellij/vcs/log/data/VisiblePackBuilderTest.kt @@ -247,31 +247,5 @@ class VisiblePackBuilderTest { } } - class EmptyIndex : VcsLogIndex { - override fun isIndexed(root: VirtualFile): Boolean { - return false - } - - override fun isIndexed(commit: Int): Boolean { - return false - } - - override fun canFilter(filters: MutableList): Boolean { - return false - } - - override fun scheduleIndex(full: Boolean) { - } - - override fun markForIndexing(index: Int, root: VirtualFile) { - } - - override fun filter(detailsFilters: MutableList): MutableSet { - throw UnsupportedOperationException() - } - - override fun markCorrupted() { - } - } } From 45334bc7d4eaa80752abb958215abed3f30f4220 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 14 Oct 2016 19:02:08 +0300 Subject: [PATCH 16/39] [vcs-log] add logging message when log caches deleted --- .../com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java index e0260bbf485e..efb2aa2118c2 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogCachesInvalidator.java @@ -16,11 +16,13 @@ package com.intellij.vcs.log.impl; import com.intellij.ide.caches.CachesInvalidator; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.io.FileUtil; import com.intellij.util.ArrayUtil; import com.intellij.vcs.log.util.PersistentUtil; public class VcsLogCachesInvalidator extends CachesInvalidator { + private static final Logger LOG = Logger.getInstance(VcsLogCachesInvalidator.class); public synchronized boolean isValid() { if (PersistentUtil.getCorruptionMarkerFile().exists()) { @@ -29,6 +31,9 @@ public class VcsLogCachesInvalidator extends CachesInvalidator { // if could not delete caches, ensure that corruption marker is still there FileUtil.createIfDoesntExist(PersistentUtil.getCorruptionMarkerFile()); } + else { + LOG.debug("Deleted VCS Log caches at " + PersistentUtil.LOG_CACHE); + } return deleted; } return true; From c640ee61e207a53944772be6531963b9d27a7193 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 21 Oct 2016 16:42:42 +0300 Subject: [PATCH 17/39] [vcs-log] rename FatalErrorConsumer -> FatalErrorHandler --- .../impl/src/com/intellij/vcs/log/data/VcsLogData.java | 6 +++--- .../src/com/intellij/vcs/log/data/VcsLogStorageImpl.java | 6 +++--- .../com/intellij/vcs/log/data/index/VcsLogPathsIndex.java | 6 +++--- .../intellij/vcs/log/data/index/VcsLogPersistentIndex.java | 6 +++--- .../com/intellij/vcs/log/data/index/VcsLogUserIndex.java | 4 ++-- .../{FatalErrorConsumer.java => FatalErrorHandler.java} | 2 +- .../impl/src/com/intellij/vcs/log/impl/VcsLogManager.java | 4 ++-- 7 files changed, 17 insertions(+), 17 deletions(-) rename platform/vcs-log/impl/src/com/intellij/vcs/log/impl/{FatalErrorConsumer.java => FatalErrorHandler.java} (95%) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java index be451c68b38b..9b74807d3b04 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogData.java @@ -35,7 +35,7 @@ import com.intellij.util.containers.ContainerUtil; import com.intellij.vcs.log.*; import com.intellij.vcs.log.data.index.VcsLogIndex; import com.intellij.vcs.log.data.index.VcsLogPersistentIndex; -import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.FatalErrorHandler; import com.intellij.vcs.log.impl.VcsLogCachesInvalidator; import com.intellij.vcs.log.util.PersistentUtil; import com.intellij.vcs.log.util.StopWatch; @@ -82,12 +82,12 @@ public class VcsLogData implements Disposable, VcsLogDataProvider { @NotNull private final VcsLogRefresherImpl myRefresher; @NotNull private final List myDataPackChangeListeners = ContainerUtil.createLockFreeCopyOnWriteList(); - @NotNull private final FatalErrorConsumer myFatalErrorsConsumer; + @NotNull private final FatalErrorHandler myFatalErrorsConsumer; @NotNull private final VcsLogIndex myIndex; public VcsLogData(@NotNull Project project, @NotNull Map logProviders, - @NotNull FatalErrorConsumer fatalErrorsConsumer) { + @NotNull FatalErrorHandler fatalErrorsConsumer) { myProject = project; myLogProviders = logProviders; myDataLoaderQueue = new BackgroundTaskQueue(project, "Loading history..."); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogStorageImpl.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogStorageImpl.java index 2ede93b681e1..2bc928310cd0 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogStorageImpl.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/VcsLogStorageImpl.java @@ -28,7 +28,7 @@ import com.intellij.util.io.IOUtil; import com.intellij.util.io.KeyDescriptor; import com.intellij.util.io.PersistentEnumeratorBase; import com.intellij.vcs.log.*; -import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.FatalErrorHandler; import com.intellij.vcs.log.impl.HashImpl; import com.intellij.vcs.log.impl.VcsRefImpl; import com.intellij.vcs.log.util.PersistentUtil; @@ -59,12 +59,12 @@ public class VcsLogStorageImpl implements Disposable, VcsLogStorage { @NotNull private final PersistentEnumeratorBase myCommitIdEnumerator; @NotNull private final PersistentEnumeratorBase myRefsEnumerator; - @NotNull private final FatalErrorConsumer myExceptionReporter; + @NotNull private final FatalErrorHandler myExceptionReporter; private volatile boolean myDisposed = false; public VcsLogStorageImpl(@NotNull Project project, @NotNull Map logProviders, - @NotNull FatalErrorConsumer exceptionReporter, + @NotNull FatalErrorHandler exceptionReporter, @NotNull Disposable parent) throws IOException { myExceptionReporter = exceptionReporter; diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPathsIndex.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPathsIndex.java index 90f0d76b5ce2..4488b35de094 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPathsIndex.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPathsIndex.java @@ -31,7 +31,7 @@ import com.intellij.util.indexing.StorageException; import com.intellij.util.io.*; import com.intellij.util.text.CaseInsensitiveStringHashingStrategy; import com.intellij.vcs.log.VcsFullCommitDetails; -import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.FatalErrorHandler; import com.intellij.vcs.log.impl.VcsChangesLazilyParsedDetails; import com.intellij.vcs.log.util.PersistentUtil; import gnu.trove.THashMap; @@ -58,7 +58,7 @@ public class VcsLogPathsIndex extends VcsLogFullDetailsIndex { public VcsLogPathsIndex(@NotNull String logId, @NotNull Set roots, - @NotNull FatalErrorConsumer fatalErrorConsumer, + @NotNull FatalErrorHandler fatalErrorHandler, @NotNull Disposable disposableParent) throws IOException { super(logId, NAME, VcsLogPersistentIndex.getVersion(), new PathsIndexer(createPathsEnumerator(logId), roots), new NullableIntKeyDescriptor(), disposableParent); @@ -67,7 +67,7 @@ public class VcsLogPathsIndex extends VcsLogFullDetailsIndex { VcsLogPersistentIndex.getVersion()); myPathsIndexer = (PathsIndexer)myIndexer; myPathsIndexer.setFatalErrorConsumer(e -> { - fatalErrorConsumer.consume(this, e); + fatalErrorHandler.consume(this, e); markCorrupted(); }); } diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java index 30dff4cd6d08..dbfa8b9fe7af 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogPersistentIndex.java @@ -39,7 +39,7 @@ import com.intellij.util.io.PersistentHashMap; import com.intellij.util.io.PersistentMap; import com.intellij.vcs.log.*; import com.intellij.vcs.log.data.*; -import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.FatalErrorHandler; import com.intellij.vcs.log.impl.VcsLogUtil; import com.intellij.vcs.log.impl.VcsLogUserFilterImpl; import com.intellij.vcs.log.util.PersistentUtil; @@ -61,7 +61,7 @@ public class VcsLogPersistentIndex implements VcsLogIndex, Disposable { private static final int VERSION = 0; @NotNull private final Project myProject; - @NotNull private final FatalErrorConsumer myFatalErrorsConsumer; + @NotNull private final FatalErrorHandler myFatalErrorsConsumer; @NotNull private final VcsLogProgress myProgress; @NotNull private final Map myProviders; @NotNull private final VcsLogStorage myHashMap; @@ -82,7 +82,7 @@ public class VcsLogPersistentIndex implements VcsLogIndex, Disposable { @NotNull VcsLogStorage hashMap, @NotNull VcsLogProgress progress, @NotNull Map providers, - @NotNull FatalErrorConsumer fatalErrorsConsumer, + @NotNull FatalErrorHandler fatalErrorsConsumer, @NotNull Disposable disposableParent) { myHashMap = hashMap; myProject = project; diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogUserIndex.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogUserIndex.java index 5338d9a2f79d..405b75f0b09d 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogUserIndex.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/index/VcsLogUserIndex.java @@ -26,7 +26,7 @@ import com.intellij.util.indexing.StorageException; import com.intellij.vcs.log.VcsFullCommitDetails; import com.intellij.vcs.log.VcsUser; import com.intellij.vcs.log.data.VcsUserRegistryImpl; -import com.intellij.vcs.log.impl.FatalErrorConsumer; +import com.intellij.vcs.log.impl.FatalErrorHandler; import gnu.trove.THashMap; import gnu.trove.TIntHashSet; import org.jetbrains.annotations.NotNull; @@ -42,7 +42,7 @@ public class VcsLogUserIndex extends VcsLogFullDetailsIndex { public VcsLogUserIndex(@NotNull String logId, @NotNull VcsUserRegistryImpl userRegistry, - @NotNull FatalErrorConsumer consumer, + @NotNull FatalErrorHandler consumer, @NotNull Disposable disposableParent) throws IOException { super(logId, "users", VcsLogPersistentIndex.getVersion(), new UserIndexer(userRegistry), ScalarIndexExtension.VOID_DATA_EXTERNALIZER, disposableParent); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorHandler.java similarity index 95% rename from platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java rename to platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorHandler.java index 0573ed544ba6..816b5a3ced63 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorConsumer.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/FatalErrorHandler.java @@ -18,7 +18,7 @@ package com.intellij.vcs.log.impl; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -public interface FatalErrorConsumer { +public interface FatalErrorHandler { void consume(@Nullable Object source, @NotNull Exception exception); void displayFatalErrorMessage(@NotNull String message); diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index cb685aea87f3..3e07fadd5621 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -78,7 +78,7 @@ public class VcsLogManager implements Disposable { myRecreateMainLogHandler = recreateHandler; Map logProviders = findLogProviders(roots, myProject); - myLogData = new VcsLogData(myProject, logProviders, new MyFatalErrorsConsumer()); + myLogData = new VcsLogData(myProject, logProviders, new MyFatalErrorsHandler()); myPostponableRefresher = new PostponableLogRefresher(myLogData); myTabsLogRefresher = new VcsLogTabsWatcher(myProject, myPostponableRefresher, myLogData); @@ -205,7 +205,7 @@ public class VcsLogManager implements Disposable { disposeLog(); } - private class MyFatalErrorsConsumer implements FatalErrorConsumer { + private class MyFatalErrorsHandler implements FatalErrorHandler { private boolean myIsBroken = false; @Override From 03e554e8c204d8c34a53aa82b0beec24ca5d1e2f Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 21 Oct 2016 16:51:50 +0300 Subject: [PATCH 18/39] [vcs-log] move isBroken check out of invokeLater --- .../intellij/vcs/log/impl/VcsLogManager.java | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index 3e07fadd5621..4e5a27873376 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -49,6 +49,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.Collection; import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; public class VcsLogManager implements Disposable { public static final ExtensionPointName LOG_PROVIDER_EP = ExtensionPointName.create("com.intellij.logProvider"); @@ -206,36 +207,36 @@ public class VcsLogManager implements Disposable { } private class MyFatalErrorsHandler implements FatalErrorHandler { - private boolean myIsBroken = false; + private final AtomicBoolean myIsBroken = new AtomicBoolean(false); @Override public void consume(@Nullable Object source, @NotNull final Exception e) { - ApplicationManager.getApplication().invokeLater(() -> { - if (!myIsBroken) { - myIsBroken = true; - processErrorFirstTime(source, e); - } - else { - LOG.debug(e); - } - }); + if (myIsBroken.compareAndSet(false, true)) { + processErrorFirstTime(source, e); + } + else { + LOG.debug(e); + } } protected void processErrorFirstTime(@Nullable Object source, @NotNull Exception e) { if (myRecreateMainLogHandler != null) { - String message = "Fatal error, VCS Log recreated: " + e.getMessage(); - if (isLogVisible()) { - LOG.info(e); - VcsBalloonProblemNotifier.showOverChangesView(myProject, message, MessageType.ERROR); - } - else { - LOG.error(message, e); - } - myRecreateMainLogHandler.run(); + ApplicationManager.getApplication().invokeLater(() -> { + String message = "Fatal error, VCS Log recreated: " + e.getMessage(); + if (isLogVisible()) { + LOG.info(e); + VcsBalloonProblemNotifier.showOverChangesView(myProject, message, MessageType.ERROR); + } + else { + LOG.error(message, e); + } + myRecreateMainLogHandler.run(); + }); } else { LOG.error(e); } + if (source instanceof VcsLogStorage) { myLogData.getIndex().markCorrupted(); } From 5f58996bab2a69ef2e0b35d71b5b5392e2b4af12 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 21 Oct 2016 16:54:13 +0300 Subject: [PATCH 19/39] [vcs-log] rename processErrorFirstTime -> processError --- .../impl/src/com/intellij/vcs/log/impl/VcsLogManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index 4e5a27873376..2fe923a29454 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -212,14 +212,14 @@ public class VcsLogManager implements Disposable { @Override public void consume(@Nullable Object source, @NotNull final Exception e) { if (myIsBroken.compareAndSet(false, true)) { - processErrorFirstTime(source, e); + processError(source, e); } else { LOG.debug(e); } } - protected void processErrorFirstTime(@Nullable Object source, @NotNull Exception e) { + protected void processError(@Nullable Object source, @NotNull Exception e) { if (myRecreateMainLogHandler != null) { ApplicationManager.getApplication().invokeLater(() -> { String message = "Fatal error, VCS Log recreated: " + e.getMessage(); From e83f306e030872e4ee1a94d72abe649d2f00bbbb Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Fri, 21 Oct 2016 16:55:03 +0300 Subject: [PATCH 20/39] [vcs-log] re-use displayFatalErrorMessage --- .../impl/src/com/intellij/vcs/log/impl/VcsLogManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index 2fe923a29454..a314f3f5ddd0 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -225,7 +225,7 @@ public class VcsLogManager implements Disposable { String message = "Fatal error, VCS Log recreated: " + e.getMessage(); if (isLogVisible()) { LOG.info(e); - VcsBalloonProblemNotifier.showOverChangesView(myProject, message, MessageType.ERROR); + displayFatalErrorMessage(message); } else { LOG.error(message, e); From eb6d8cf0be52b7c7a4026bb6e660ca2e9b9cd116 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sat, 22 Oct 2016 19:45:30 +0300 Subject: [PATCH 21/39] [vcs-log] fix spelling --- .../impl/src/com/intellij/vcs/log/impl/VcsLogManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java index a314f3f5ddd0..24a4868fb90e 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/impl/VcsLogManager.java @@ -222,7 +222,7 @@ public class VcsLogManager implements Disposable { protected void processError(@Nullable Object source, @NotNull Exception e) { if (myRecreateMainLogHandler != null) { ApplicationManager.getApplication().invokeLater(() -> { - String message = "Fatal error, VCS Log recreated: " + e.getMessage(); + String message = "Fatal error, VCS Log re-created: " + e.getMessage(); if (isLogVisible()) { LOG.info(e); displayFatalErrorMessage(message); From 50d645e256231ae80c83c74181df3619049dfb94 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 28 Oct 2016 13:32:24 +0200 Subject: [PATCH 22/39] Allow "Convert 'compareTo()' expression to 'equals()' call" intention in more cases --- .../ComparatorCombinatorsInspection.java | 9 +- .../ConvertCompareToToEqualsIntention.java | 128 ++++++++---------- .../convertCompareToToEquals/noQualifier.java | 9 ++ .../noQualifier_after.java | 9 ++ .../ConvertCompareToToEqualsTest.java | 13 +- .../siyeh/ig/psiutils/MethodCallUtils.java | 13 +- .../com/siyeh/ig/psiutils/MethodUtils.java | 14 -- .../description.html | 2 +- 8 files changed, 103 insertions(+), 94 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier.java create mode 100644 java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier_after.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java index fd8056a9fe12..e8f0efca53bd 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/ComparatorCombinatorsInspection.java @@ -25,7 +25,7 @@ import com.intellij.psi.codeStyle.SuggestedNameInfo; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.util.PsiTreeUtil; import com.siyeh.ig.psiutils.EquivalenceChecker; -import com.siyeh.ig.psiutils.MethodUtils; +import com.siyeh.ig.psiutils.MethodCallUtils; import one.util.streamex.StreamEx; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; @@ -55,8 +55,11 @@ public class ComparatorCombinatorsInspection extends BaseJavaBatchLocalInspectio if (body instanceof PsiMethodCallExpression) { PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; PsiExpression[] args = methodCall.getArgumentList().getExpressions(); - if (args.length == 1 && MethodUtils.isCompareToCall(methodCall)) { + if (args.length == 1 && MethodCallUtils.isCompareToCall(methodCall)) { PsiExpression left = methodCall.getMethodExpression().getQualifierExpression(); + if (left == null) { + return; + } PsiExpression right = args[0]; if (left instanceof PsiReferenceExpression && right instanceof PsiReferenceExpression) { PsiElement leftElement = ((PsiReferenceExpression)left).resolve(); @@ -163,7 +166,7 @@ public class ComparatorCombinatorsInspection extends BaseJavaBatchLocalInspectio String methodName = null; if (body instanceof PsiMethodCallExpression) { PsiMethodCallExpression methodCall = (PsiMethodCallExpression)body; - if (MethodUtils.isCompareToCall(methodCall)) { + if (MethodCallUtils.isCompareToCall(methodCall)) { methodName = "comparing"; keyExtractor = methodCall.getMethodExpression().getQualifierExpression(); if (keyExtractor instanceof PsiReferenceExpression) { diff --git a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java index a6f4fbb0f0e1..bba5c08bd123 100644 --- a/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java +++ b/java/java-impl/src/com/intellij/codeInsight/intention/impl/ConvertCompareToToEqualsIntention.java @@ -19,12 +19,12 @@ import com.intellij.codeInsight.FileModificationService; import com.intellij.codeInsight.intention.BaseElementAtCaretIntentionAction; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; -import com.intellij.openapi.util.Comparing; -import com.intellij.openapi.util.Pair; import com.intellij.psi.*; +import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.IncorrectOperationException; -import com.siyeh.ig.psiutils.MethodUtils; +import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.MethodCallUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -32,110 +32,100 @@ import org.jetbrains.annotations.Nullable; * @author Dmitry Batkovich */ public class ConvertCompareToToEqualsIntention extends BaseElementAtCaretIntentionAction { - public static final String TEXT = "Convert '.compareTo()' method to '.equals()' (may change semantics)"; @Override public void invoke(@NotNull Project project, Editor editor, @NotNull PsiElement element) throws IncorrectOperationException { if (!FileModificationService.getInstance().preparePsiElementsForWrite(element)) { return; } - final ResolveResult resolveResult = findCompareTo(element); - assert resolveResult != null; - final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(project).getElementFactory(); - final Pair qualifierAndParameter = getQualifierAndParameter(resolveResult.getCompareToCall()); - final PsiExpression newExpression = - elementFactory.createExpressionFromText(String.format((resolveResult.isEqEq() ? "" : "!") + "%s.equals(%s)", qualifierAndParameter.getFirst().getText(), qualifierAndParameter.getSecond().getText()), null); - final PsiElement result = resolveResult.getBinaryExpression().replace(newExpression); + final CompareToResult compareToResult = CompareToResult.findCompareTo(element); + assert compareToResult != null; + final PsiExpression qualifier = compareToResult.getQualifier(); + final PsiExpression argument = compareToResult.getArgument(); + final StringBuilder text = new StringBuilder(); + if (!compareToResult.isEqEq()) { + text.append('!'); + } + if (qualifier != null) { + text.append(qualifier.getText()).append('.'); + } + text.append("equals(").append(argument.getText()).append(')'); + final PsiExpression newExpression = JavaPsiFacade.getElementFactory(project).createExpressionFromText(text.toString(), null); + final PsiElement result = compareToResult.getBinaryExpression().replace(newExpression); editor.getCaretModel().moveToOffset(result.getTextOffset() + result.getTextLength()); } @Override public boolean isAvailable(@NotNull final Project project, final Editor editor, @NotNull final PsiElement element) { - return findCompareTo(element) != null; + return CompareToResult.findCompareTo(element) != null; } - private static Pair getQualifierAndParameter(PsiMethodCallExpression methodCallExpression) { - final PsiExpression qualifier = methodCallExpression.getMethodExpression().getQualifierExpression(); - assert qualifier != null; - final PsiExpression parameter = methodCallExpression.getArgumentList().getExpressions()[0]; - return Pair.create(qualifier, parameter); - } + private static class CompareToResult { - @Nullable - private static ResolveResult findCompareTo(PsiElement element) { - final PsiBinaryExpression binaryExpression = PsiTreeUtil.getParentOfType(element, PsiBinaryExpression.class); - if (binaryExpression == null) { - return null; - } - final PsiJavaToken operationSign = binaryExpression.getOperationSign(); - boolean isEqEq; - if (JavaTokenType.NE.equals(operationSign.getTokenType())) { - isEqEq = false; - } else if (JavaTokenType.EQEQ.equals(operationSign.getTokenType())) { - isEqEq = true; - } else { - return null; - } - PsiMethodCallExpression compareToExpression = null; - boolean hasZero = false; - for (PsiExpression psiExpression : binaryExpression.getOperands()) { - if (compareToExpression == null && MethodUtils.isCompareToCall(psiExpression)) { - compareToExpression = (PsiMethodCallExpression)psiExpression; - continue; - } - if (!hasZero && detectZero(psiExpression)) { - hasZero = true; - } - } - if (!hasZero || compareToExpression == null) { - return null; - } - getQualifierAndParameter(compareToExpression); - return new ResolveResult(binaryExpression, compareToExpression, isEqEq); - } - - private static boolean detectZero(final @NotNull PsiExpression expression) { - if (!(expression instanceof PsiLiteralExpression)) { - return false; - } - final Object value = ((PsiLiteralExpression)expression).getValue(); - return Comparing.equal(value, 0); - } - - private static class ResolveResult { private final PsiBinaryExpression myBinaryExpression; private final PsiMethodCallExpression myCompareToCall; - private final boolean myEqEq; - private ResolveResult(PsiBinaryExpression binaryExpression, PsiMethodCallExpression compareToCall, boolean eqEq) { + private CompareToResult(PsiBinaryExpression binaryExpression, PsiMethodCallExpression compareToCall) { myBinaryExpression = binaryExpression; myCompareToCall = compareToCall; - myEqEq = eqEq; } public PsiBinaryExpression getBinaryExpression() { return myBinaryExpression; } - public PsiMethodCallExpression getCompareToCall() { - return myCompareToCall; + public boolean isEqEq() { + return JavaTokenType.EQEQ.equals(myBinaryExpression.getOperationTokenType()); } - public boolean isEqEq() { - return myEqEq; + public PsiExpression getArgument() { + return myCompareToCall.getArgumentList().getExpressions()[0]; + } + + public PsiExpression getQualifier() { + return myCompareToCall.getMethodExpression().getQualifierExpression(); + } + + @Nullable + static CompareToResult findCompareTo(PsiElement element) { + final PsiBinaryExpression binaryExpression = PsiTreeUtil.getParentOfType(element, PsiBinaryExpression.class); + if (binaryExpression == null) { + return null; + } + final IElementType tokenType = binaryExpression.getOperationTokenType(); + if (!JavaTokenType.NE.equals(tokenType) && !JavaTokenType.EQEQ.equals(tokenType)) { + return null; + } + PsiMethodCallExpression compareToExpression; + final PsiExpression lhs = binaryExpression.getLOperand(); + final PsiExpression rhs = binaryExpression.getROperand(); + if (lhs instanceof PsiMethodCallExpression) { + compareToExpression = (PsiMethodCallExpression)lhs; + if (!MethodCallUtils.isCompareToCall(compareToExpression) || !ExpressionUtils.isZero(rhs)) { + return null; + } + } else if (rhs instanceof PsiMethodCallExpression) { + compareToExpression = (PsiMethodCallExpression)rhs; + if (!ExpressionUtils.isZero(lhs) || !MethodCallUtils.isCompareToCall(compareToExpression)) { + return null; + } + } else { + return null; + } + return new CompareToResult(binaryExpression, compareToExpression); } } @NotNull @Override public String getFamilyName() { - return TEXT; + return "Convert 'compareTo()' expression to 'equals()' call"; } @NotNull @Override public String getText() { - return getFamilyName(); + return "Convert 'compareTo()' expression to 'equals()' call (may change semantics)"; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier.java b/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier.java new file mode 100644 index 000000000000..4b20575d4dac --- /dev/null +++ b/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier.java @@ -0,0 +1,9 @@ +class X implements Comparable { + boolean m(X x) { + return compareTo(x) == ((int)0.0); + } + + public int compareTo(X x) { + return 0; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier_after.java b/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier_after.java new file mode 100644 index 000000000000..c0b7f15b9deb --- /dev/null +++ b/java/java-tests/testData/codeInsight/convertCompareToToEquals/noQualifier_after.java @@ -0,0 +1,9 @@ +class X implements Comparable { + boolean m(X x) { + return equals(x); + } + + public int compareTo(X x) { + return 0; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/intention/ConvertCompareToToEqualsTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/intention/ConvertCompareToToEqualsTest.java index 3945e414078b..70c784d97df7 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/intention/ConvertCompareToToEqualsTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/intention/ConvertCompareToToEqualsTest.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2014 JetBrains s.r.o. + * Copyright 2000-2016 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,7 +16,6 @@ package com.intellij.codeInsight.intention; import com.intellij.JavaTestUtil; -import com.intellij.codeInsight.intention.impl.ConvertCompareToToEqualsIntention; import com.intellij.testFramework.fixtures.CodeInsightTestUtil; import com.intellij.testFramework.fixtures.JavaCodeInsightFixtureTestCase; @@ -37,18 +36,22 @@ public class ConvertCompareToToEqualsTest extends JavaCodeInsightFixtureTestCase doTest(); } + public void testNoQualifier() { + doTest(); + } + public void testNotAvailable() { - doTestNotAvailable();; + doTestNotAvailable(); } private void doTest() { final String name = getTestName(true); - CodeInsightTestUtil.doIntentionTest(myFixture, ConvertCompareToToEqualsIntention.TEXT, name + ".java", name + "_after.java"); + CodeInsightTestUtil.doIntentionTest(myFixture, "Convert 'compareTo()' expression to 'equals()' call (may change semantics)", name + ".java", name + "_after.java"); } private void doTestNotAvailable() { myFixture.configureByFile(getTestName(true) + ".java"); - assertEmpty(myFixture.filterAvailableIntentions(ConvertCompareToToEqualsIntention.TEXT)); + assertEmpty(myFixture.filterAvailableIntentions("Convert 'compareTo()' expression to 'equals()' call (may change semantics)")); } } diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java index 31bf0b0c3d3f..c21f8817e624 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java @@ -55,14 +55,23 @@ public class MethodCallUtils { @Nullable public static PsiType getTargetType(@NotNull PsiMethodCallExpression expression) { - final PsiReferenceExpression method = expression.getMethodExpression(); - final PsiExpression qualifierExpression = method.getQualifierExpression(); + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); if (qualifierExpression == null) { return null; } return qualifierExpression.getType(); } + public static boolean isCompareToCall(@NotNull PsiMethodCallExpression expression) { + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + if (!HardcodedMethodConstants.COMPARE_TO.equals(methodExpression.getReferenceName())) { + return false; + } + final PsiMethod method = expression.resolveMethod(); + return MethodUtils.isCompareTo(method); + } + public static boolean isEqualsCall(PsiMethodCallExpression expression) { final PsiReferenceExpression methodExpression = expression.getMethodExpression(); final String name = methodExpression.getReferenceName(); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java index 03af8b2bf416..b5db5a04370d 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java @@ -363,18 +363,4 @@ public class MethodUtils { final PsiExpression returnValue = returnStatement.getReturnValue(); return returnValue instanceof PsiThisExpression; } - - @Contract("null -> false") - public static boolean isCompareToCall(final @Nullable PsiExpression expression) { - if (!(expression instanceof PsiMethodCallExpression)) { - return false; - } - final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; - if (methodCallExpression.getMethodExpression().getQualifierExpression() == null || - !HardcodedMethodConstants.COMPARE_TO.equals(methodCallExpression.getMethodExpression().getReferenceName())) { - return false; - } - final PsiMethod psiMethod = methodCallExpression.resolveMethod(); - return isCompareTo(psiMethod); - } } diff --git a/resources-en/src/intentionDescriptions/ConvertCompareToToEqualsIntention/description.html b/resources-en/src/intentionDescriptions/ConvertCompareToToEqualsIntention/description.html index c6fe60e4dacd..16770f22fbb4 100644 --- a/resources-en/src/intentionDescriptions/ConvertCompareToToEqualsIntention/description.html +++ b/resources-en/src/intentionDescriptions/ConvertCompareToToEqualsIntention/description.html @@ -15,6 +15,6 @@ --> -Converts call if compareTo method to call of equals method. +Converts a compareTo() method call expression to an equals() call. \ No newline at end of file From 024d0f5894d3f8359a71912a2cbbcdd0c216311e Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 28 Oct 2016 14:05:09 +0200 Subject: [PATCH 23/39] IG: warn on equalsIgnoreCase(), compareTo() and compareToIgnoreCase() call to itself (IDEA-162797) --- .../ig/bugs/EqualsWithItselfInspection.java | 8 ++++++-- .../siyeh/ig/psiutils/MethodCallUtils.java | 19 +++++++++++++++++++ .../com/siyeh/ig/psiutils/MethodUtils.java | 18 ++++++++++++++++++ .../EqualsWithItself.html | 6 +++--- .../equals_with_itself/EqualsWithItself.java | 16 ++++++++++++++++ 5 files changed, 62 insertions(+), 5 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWithItselfInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWithItselfInspection.java index 9c8a060a9ecb..08f016c860ab 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWithItselfInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWithItselfInspection.java @@ -16,7 +16,8 @@ package com.siyeh.ig.bugs; import com.intellij.psi.*; -import com.siyeh.InspectionGadgetsBundle;import com.siyeh.ig.BaseInspection; +import com.siyeh.InspectionGadgetsBundle; +import com.siyeh.ig.BaseInspection; import com.siyeh.ig.BaseInspectionVisitor; import com.siyeh.ig.psiutils.EquivalenceChecker; import com.siyeh.ig.psiutils.MethodCallUtils; @@ -52,7 +53,10 @@ public class EqualsWithItselfInspection extends BaseInspection { @Override public void visitMethodCallExpression(PsiMethodCallExpression expression) { super.visitMethodCallExpression(expression); - if (!MethodCallUtils.isEqualsCall(expression)) { + if (!MethodCallUtils.isEqualsCall(expression) && + !MethodCallUtils.isEqualsIgnoreCaseCall(expression) && + !MethodCallUtils.isCompareToCall(expression) && + !MethodCallUtils.isCompareToIgnoreCaseCall(expression)) { return; } final PsiReferenceExpression methodExpression = expression.getMethodExpression(); diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java index c21f8817e624..85cff482b0b8 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodCallUtils.java @@ -72,6 +72,15 @@ public class MethodCallUtils { return MethodUtils.isCompareTo(method); } + public static boolean isCompareToIgnoreCaseCall(@NotNull PsiMethodCallExpression expression) { + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + if (!"compareToIgnoreCase".equals(methodExpression.getReferenceName())) { + return false; + } + final PsiMethod method = expression.resolveMethod(); + return MethodUtils.isCompareToIgnoreCase(method); + } + public static boolean isEqualsCall(PsiMethodCallExpression expression) { final PsiReferenceExpression methodExpression = expression.getMethodExpression(); final String name = methodExpression.getReferenceName(); @@ -82,6 +91,16 @@ public class MethodCallUtils { return MethodUtils.isEquals(method); } + public static boolean isEqualsIgnoreCaseCall(PsiMethodCallExpression expression) { + final PsiReferenceExpression methodExpression = expression.getMethodExpression(); + final String name = methodExpression.getReferenceName(); + if (!HardcodedMethodConstants.EQUALS_IGNORE_CASE.equals(name)) { + return false; + } + final PsiMethod method = expression.resolveMethod(); + return MethodUtils.isEqualsIgnoreCase(method); + } + public static boolean isSimpleCallToMethod(@NotNull PsiMethodCallExpression expression, @NonNls @Nullable String calledOnClassName, @Nullable PsiType returnType, @NonNls @Nullable String methodName, @NonNls @Nullable String... parameterTypeStrings) { if (parameterTypeStrings == null) { diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java index b5db5a04370d..0a95789f96fd 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/psiutils/MethodUtils.java @@ -49,6 +49,15 @@ public class MethodUtils { && InheritanceUtil.isInheritor(method.getContainingClass(), CommonClassNames.JAVA_LANG_COMPARABLE); } + @Contract("null -> false") + public static boolean isCompareToIgnoreCase(@Nullable PsiMethod method) { + if (method == null) { + return false; + } + final PsiClassType stringType = TypeUtils.getStringType(method); + return methodMatches(method, "java.lang.String", PsiType.INT, "compareToIgnoreCase", stringType); + } + @Contract("null -> false") public static boolean isHashCode(@Nullable PsiMethod method) { return method != null && methodMatches(method, null, PsiType.INT, HardcodedMethodConstants.HASH_CODE); @@ -77,6 +86,15 @@ public class MethodUtils { return methodMatches(method, null, PsiType.BOOLEAN, HardcodedMethodConstants.EQUALS, objectType); } + @Contract("null -> false") + public static boolean isEqualsIgnoreCase(@Nullable PsiMethod method) { + if (method == null) { + return false; + } + final PsiClassType stringType = TypeUtils.getStringType(method); + return methodMatches(method, "java.lang.String", PsiType.BOOLEAN, HardcodedMethodConstants.EQUALS_IGNORE_CASE, stringType); + } + /** * @param method the method to compare to. * @param containingClassName the name of the class which contiains the diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsWithItself.html b/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsWithItself.html index 3095986ff668..e14b4780d055 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsWithItself.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsWithItself.html @@ -1,8 +1,8 @@ -Reports call to equals() were an object is compared for equality with itself. -This means that the argument and the qualifier to the call are identical. -In this case equals() will always return true. +Reports calls to equals() or compareTo() were an object is compared for equality with itself. +This means the argument and the qualifier to the call are identical, and it will always return true for equals() +or always 0 for compareTo().

diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_with_itself/EqualsWithItself.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_with_itself/EqualsWithItself.java index 850406b12534..6ec14731c05c 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_with_itself/EqualsWithItself.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_with_itself/EqualsWithItself.java @@ -24,4 +24,20 @@ class EqualsWithItself { public Object build() { return new Object(); } + + boolean string(String s) { + return s.equalsIgnoreCase(s); + } + + int compareTo(String s) { + return s.compareTo(s); + } + + int compareToIgnoreCase(String s) { + return s.compareToIgnoreCase(s); + } + + boolean safe(String a, String b) { + return a.equals(b) && a.equalsIgnoreCase(b) && a.compareTo(b) == 0; + } } \ No newline at end of file From 1eebf487f388897f0b32443099b2263a70eb890a Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Fri, 28 Oct 2016 17:34:13 +0200 Subject: [PATCH 24/39] IG: merge 3 inspections into one "Call to suspicious String method" inspection --- .../ex/InspectionProfileTest.java | 9 ++ .../src/META-INF/InspectionGadgets.xml | 19 +-- .../siyeh/InspectionGadgetsBundle.properties | 8 +- ...llToSuspiciousStringMethodInspection.java} | 92 ++++------- ...uspiciousStringMethodInspectionMerger.java | 47 ++++++ .../ig/internationalization/NonNlsUtils.java | 6 +- .../StringCompareToInspection.java | 146 ------------------ .../StringEqualsIgnoreCaseInspection.java | 142 ----------------- .../CallToSuspiciousStringMethod.html | 9 ++ .../StringCompareTo.html | 9 -- .../inspectionDescriptions/StringEquals.html | 9 -- .../StringEqualsIgnoreCase.html | 9 -- .../StringCompareToInspection.java | 13 -- .../StringEqualsIgnoreCaseInspection.java | 13 -- .../StringEqualsInspection.java | 13 -- .../CallToSuspiciousStringMethod.java | 18 +++ ...oSuspiciousStringMethodInspectionTest.java | 36 +++++ 17 files changed, 164 insertions(+), 434 deletions(-) rename plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/internationalization/{StringEqualsInspection.java => CallToSuspiciousStringMethodInspection.java} (51%) create mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/internationalization/CallToSuspiciousStringMethodInspectionMerger.java delete mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/internationalization/StringCompareToInspection.java delete mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/internationalization/StringEqualsIgnoreCaseInspection.java create mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/CallToSuspiciousStringMethod.html delete mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/StringCompareTo.html delete mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/StringEquals.html delete mode 100644 plugins/InspectionGadgets/src/inspectionDescriptions/StringEqualsIgnoreCase.html delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringCompareToInspection.java delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsIgnoreCaseInspection.java delete mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsInspection.java create mode 100644 plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/call_to_suspicious_string_method/CallToSuspiciousStringMethod.java create mode 100644 plugins/InspectionGadgets/testsrc/com/siyeh/ig/internationalization/CallToSuspiciousStringMethodInspectionTest.java diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/ex/InspectionProfileTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/ex/InspectionProfileTest.java index b209a4534582..096795247e23 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/ex/InspectionProfileTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/ex/InspectionProfileTest.java @@ -402,6 +402,15 @@ public class InspectionProfileTest extends LightIdeaTestCase { ""); } + public void testMergedCallToSuspiciousStringMethodInspections() throws Exception { + checkMergedNoChanges("\n" + + " " ); + } + public void testMergedMisspelledInspections() throws Exception { checkMergedNoChanges("\n" + "

+ + + \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/StringCompareTo.html b/plugins/InspectionGadgets/src/inspectionDescriptions/StringCompareTo.html deleted file mode 100644 index d4bf66f9ac8f..000000000000 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/StringCompareTo.html +++ /dev/null @@ -1,9 +0,0 @@ - - -Reports any call of compareTo() on String objects. Such calls are usually -incorrect in an internationalized environment. - -

- - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/StringEquals.html b/plugins/InspectionGadgets/src/inspectionDescriptions/StringEquals.html deleted file mode 100644 index d9ad1a0fb6bd..000000000000 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/StringEquals.html +++ /dev/null @@ -1,9 +0,0 @@ - - -Reports any call of equals() on String objects. Such calls are usually -incorrect in an internationalized environment. - -

- - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/StringEqualsIgnoreCase.html b/plugins/InspectionGadgets/src/inspectionDescriptions/StringEqualsIgnoreCase.html deleted file mode 100644 index 3556e6472e39..000000000000 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/StringEqualsIgnoreCase.html +++ /dev/null @@ -1,9 +0,0 @@ - - -Reports any call of equalsIgnoreCase() on String objects. Such calls are usually -incorrect in an internationalized environment. - -

- - - \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringCompareToInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringCompareToInspection.java deleted file mode 100644 index df6da4d5e838..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringCompareToInspection.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.siyeh.igtest.internationalization; - -public class StringCompareToInspection -{ - public StringCompareToInspection() - { - } - - public void foo() - { - "foo".compareTo("bar"); - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsIgnoreCaseInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsIgnoreCaseInspection.java deleted file mode 100644 index 1fab129ff45f..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsIgnoreCaseInspection.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.siyeh.igtest.internationalization; - -public class StringEqualsIgnoreCaseInspection -{ - public StringEqualsIgnoreCaseInspection() - { - } - - public void foo() - { - "foo".equalsIgnoreCase("bar"); - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsInspection.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsInspection.java deleted file mode 100644 index aa1754bdc48a..000000000000 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/StringEqualsInspection.java +++ /dev/null @@ -1,13 +0,0 @@ -package com.siyeh.igtest.internationalization; - -public class StringEqualsInspection -{ - public StringEqualsInspection() - { - } - - public void foo() - { - "foo".equals("bar"); - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/call_to_suspicious_string_method/CallToSuspiciousStringMethod.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/call_to_suspicious_string_method/CallToSuspiciousStringMethod.java new file mode 100644 index 000000000000..2404b3bd291f --- /dev/null +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/internationalization/call_to_suspicious_string_method/CallToSuspiciousStringMethod.java @@ -0,0 +1,18 @@ +class CallToSuspiciousStringMethod { + + void m(String a, String b) { + a.equals(b); + a.equalsIgnoreCase(b); + a.compareTo(b); + a.compareToIgnoreCase(b); + } + + @SuppressWarnings({"CallToStringCompareTo", "CallToStringEquals", "CallToStringEqualsIgnoreCase"}) + void n(String a, String b) { + a.equals(b); + a.equalsIgnoreCase(b); + a.compareTo(b); + //noinspection CallToSuspiciousStringMethod + a.compareToIgnoreCase(b); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/internationalization/CallToSuspiciousStringMethodInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/internationalization/CallToSuspiciousStringMethodInspectionTest.java new file mode 100644 index 000000000000..c9f4d22b0095 --- /dev/null +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/internationalization/CallToSuspiciousStringMethodInspectionTest.java @@ -0,0 +1,36 @@ +/* + * Copyright 2000-2016 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.siyeh.ig.internationalization; + +import com.intellij.codeInspection.InspectionProfileEntry; +import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.Nullable; + +/** + * @author Bas Leijdekkers + */ +public class CallToSuspiciousStringMethodInspectionTest extends LightInspectionTestCase { + + public void testCallToSuspiciousStringMethod() { + doTest(); + } + + @Nullable + @Override + protected InspectionProfileEntry getInspection() { + return new CallToSuspiciousStringMethodInspection(); + } +} \ No newline at end of file From 0e5a4664b373279f5838b89c43988e518494c878 Mon Sep 17 00:00:00 2001 From: Sergey Ignatov Date: Mon, 24 Oct 2016 22:40:24 +0300 Subject: [PATCH 25/39] flat welcome screen: use the whole page for the single project generator --- .../AbstractNewProjectDialog.java | 24 ++-- .../ProjectSettingsStepBase.java | 3 + .../impl/welcomeScreen/FlatWelcomeFrame.java | 133 +++++++++++------- 3 files changed, 100 insertions(+), 60 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/ide/util/projectWizard/AbstractNewProjectDialog.java b/platform/lang-impl/src/com/intellij/ide/util/projectWizard/AbstractNewProjectDialog.java index 9b631fd604d9..c4e7b86f8bf9 100644 --- a/platform/lang-impl/src/com/intellij/ide/util/projectWizard/AbstractNewProjectDialog.java +++ b/platform/lang-impl/src/com/intellij/ide/util/projectWizard/AbstractNewProjectDialog.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2015 JetBrains s.r.o. + * Copyright 2000-2016 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. @@ -26,10 +26,8 @@ import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.util.Pair; import com.intellij.openapi.wm.impl.welcomeScreen.FlatWelcomeFrame; import com.intellij.platform.DirectoryProjectGenerator; -import com.intellij.ui.ListSpeedSearch; import com.intellij.ui.ScrollingUtil; import com.intellij.ui.components.JBList; -import com.intellij.util.containers.Convertor; import com.intellij.util.ui.JBUI; import com.intellij.util.ui.update.UiNotifyConnector; import org.jetbrains.annotations.NotNull; @@ -43,7 +41,7 @@ import java.awt.event.KeyEvent; * @author Dennis.Ushakov */ public abstract class AbstractNewProjectDialog extends DialogWrapper { - private JBList myList; + private Pair myPair; public AbstractNewProjectDialog() { super(ProjectManager.getInstance().getDefaultProject()); @@ -53,13 +51,13 @@ public abstract class AbstractNewProjectDialog extends DialogWrapper { @Nullable @Override protected JComponent createCenterPanel() { - final DirectoryProjectGenerator[] generators = Extensions.getExtensions(DirectoryProjectGenerator.EP_NAME); + DirectoryProjectGenerator[] generators = Extensions.getExtensions(DirectoryProjectGenerator.EP_NAME); setTitle(generators.length == 0 ? "Create Project" : "New Project"); - final DefaultActionGroup root = createRootStep(); + DefaultActionGroup root = createRootStep(); - final Pair panel = FlatWelcomeFrame.createActionGroupPanel(root, getRootPane(), null); - final Dimension size = JBUI.size(666, 385); - final JPanel component = panel.first; + Pair pair = FlatWelcomeFrame.createActionGroupPanel(root, getRootPane(), null); + Dimension size = JBUI.size(666, 385); + JPanel component = pair.first; component.setMinimumSize(size); component.setPreferredSize(size); new AnAction() { @@ -68,17 +66,17 @@ public abstract class AbstractNewProjectDialog extends DialogWrapper { close(CANCEL_EXIT_CODE); } }.registerCustomShortcutSet(KeyEvent.VK_ESCAPE, 0, component); - myList = panel.second; - UiNotifyConnector.doWhenFirstShown(myList, () -> ScrollingUtil.ensureSelectionExists(myList)); + myPair = pair; + UiNotifyConnector.doWhenFirstShown(myPair.second, () -> ScrollingUtil.ensureSelectionExists(myPair.second)); - FlatWelcomeFrame.installQuickSearch(panel.second); + FlatWelcomeFrame.installQuickSearch(pair.second); return component; } @Nullable @Override public JComponent getPreferredFocusedComponent() { - return myList; + return FlatWelcomeFrame.getPreferredFocusedComponent(myPair); } @Nullable diff --git a/platform/lang-impl/src/com/intellij/ide/util/projectWizard/ProjectSettingsStepBase.java b/platform/lang-impl/src/com/intellij/ide/util/projectWizard/ProjectSettingsStepBase.java index deceb958b51c..dda254f7454c 100644 --- a/platform/lang-impl/src/com/intellij/ide/util/projectWizard/ProjectSettingsStepBase.java +++ b/platform/lang-impl/src/com/intellij/ide/util/projectWizard/ProjectSettingsStepBase.java @@ -48,6 +48,8 @@ import java.awt.event.ActionListener; import java.io.File; import java.util.List; +import static com.intellij.openapi.wm.impl.welcomeScreen.FlatWelcomeFrame.BOTTOM_PANEL; + public class ProjectSettingsStepBase extends AbstractActionWithPanel implements DumbAware { protected final DirectoryProjectGenerator myProjectGenerator; private final NullableConsumer myCallback; @@ -89,6 +91,7 @@ public class ProjectSettingsStepBase extends AbstractActionWithPanel implements mainPanel.add(scrollPane, BorderLayout.CENTER); final JPanel bottomPanel = new JPanel(new BorderLayout()); + bottomPanel.setName(BOTTOM_PANEL); bottomPanel.add(label, BorderLayout.NORTH); bottomPanel.add(button, BorderLayout.EAST); diff --git a/platform/platform-impl/src/com/intellij/openapi/wm/impl/welcomeScreen/FlatWelcomeFrame.java b/platform/platform-impl/src/com/intellij/openapi/wm/impl/welcomeScreen/FlatWelcomeFrame.java index ef0b5d91b4e9..a862713115c4 100644 --- a/platform/platform-impl/src/com/intellij/openapi/wm/impl/welcomeScreen/FlatWelcomeFrame.java +++ b/platform/platform-impl/src/com/intellij/openapi/wm/impl/welcomeScreen/FlatWelcomeFrame.java @@ -49,6 +49,7 @@ import com.intellij.ui.*; import com.intellij.ui.border.CustomLineBorder; import com.intellij.ui.components.JBList; import com.intellij.ui.components.JBSlidingPanel; +import com.intellij.ui.components.JBTextField; import com.intellij.ui.components.labels.ActionLink; import com.intellij.ui.components.panels.NonOpaquePanel; import com.intellij.ui.popup.PopupFactoryImpl; @@ -85,6 +86,7 @@ import java.util.List; * @author Konstantin Bulenkov */ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, AccessibleContextAccessor { + public static final String BOTTOM_PANEL = "BOTTOM_PANEL"; private static final String ACTION_GROUP_KEY = "ACTION_GROUP_KEY"; private BalloonLayout myBalloonLayout; private final FlatWelcomeScreen myScreen; @@ -202,6 +204,17 @@ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, Ac return "Welcome to " + ApplicationNamesInfo.getInstance().getFullProductName(); } + @Nullable + public static JComponent getPreferredFocusedComponent(@NotNull Pair pair) { + if (pair.second.getModel().getSize() == 1) { + JBTextField textField = UIUtil.uiTraverser(pair.first).filter(JBTextField.class).first(); + if (textField != null) { + return textField; + } + } + return pair.second; + } + private class FlatWelcomeScreen extends JPanel implements WelcomeScreen { private JBSlidingPanel mySlidingPanel = new JBSlidingPanel(); public ParameterizedRunnable> myEventListener; @@ -477,7 +490,10 @@ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, Ac for (ListSelectionListener listener : listeners) { listener.valueChanged(new ListSelectionEvent(list, list.getSelectedIndex(), list.getSelectedIndex(), true)); } - list.requestFocus(); + JComponent toFocus = FlatWelcomeFrame.getPreferredFocusedComponent(panel); + if (toFocus != null) { + toFocus.requestFocus(); + } }; final String name = action.getClass().getName(); mySlidingPanel.add(name, panel.first); @@ -777,7 +793,7 @@ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, Ac private JPanel root; private JPanel actions; } - + public static Pair createActionGroupPanel(final ActionGroup action, final JComponent parent, final Runnable backAction) { JPanel actionsListPanel = new JPanel(new BorderLayout()); actionsListPanel.setBackground(getProjectsBackground()); @@ -839,51 +855,51 @@ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, Ac JScrollPane pane = ScrollPaneFactory.createScrollPane(list, true); pane.setBackground(getProjectsBackground()); actionsListPanel.add(pane, BorderLayout.CENTER); - if (backAction != null) { - final JLabel back = new JLabel(AllIcons.Actions.Back); - back.setBorder(JBUI.Borders.empty(3, 7, 10, 7)); - back.setHorizontalAlignment(SwingConstants.LEFT); - new ClickListener() { - @Override - public boolean onClick(@NotNull MouseEvent event, int clickCount) { - backAction.run(); - return true; - } - }.installOn(back); - actionsListPanel.add(back, BorderLayout.SOUTH); - } + + boolean singleProjectGenerator = list.getModel().getSize() == 1; + final Ref selected = Ref.create(); final JPanel main = new JPanel(new BorderLayout()); main.add(actionsListPanel, BorderLayout.WEST); - ListSelectionListener selectionListener = new ListSelectionListener() { - @Override - public void valueChanged(ListSelectionEvent e) { - if (e.getValueIsAdjusting()) { - // Update when a change has been finalized. - // For instance, selecting an element with mouse fires two consecutive ListSelectionEvent events. - return; - } - if (!selected.isNull()) { - main.remove(selected.get()); - } - Object value = list.getSelectedValue(); - if (value instanceof AbstractActionWithPanel) { - JPanel panel = ((AbstractActionWithPanel)value).createPanel(); - panel.setBorder(JBUI.Borders.empty(7, 10)); - selected.set(panel); - main.add(selected.get()); + final JComponent back = createBackLabel(backAction, singleProjectGenerator); + + if (back != null && !singleProjectGenerator) { + actionsListPanel.add(back, BorderLayout.SOUTH); + } + + ListSelectionListener selectionListener = e -> { + if (e.getValueIsAdjusting()) { + // Update when a change has been finalized. + // For instance, selecting an element with mouse fires two consecutive ListSelectionEvent events. + return; + } + if (!selected.isNull()) { + main.remove(selected.get()); + } + Object value = list.getSelectedValue(); + if (value instanceof AbstractActionWithPanel) { + JPanel panel = ((AbstractActionWithPanel)value).createPanel(); + panel.setBorder(JBUI.Borders.empty(7, 10)); + selected.set(panel); + main.add(selected.get()); - for (JButton button : UIUtil.findComponentsOfType(main, JButton.class)) { - if (button.getClientProperty(DialogWrapper.DEFAULT_ACTION) == Boolean.TRUE) { - parent.getRootPane().setDefaultButton(button); - break; - } + if (singleProjectGenerator && back != null) { + JPanel first = UIUtil.uiTraverser(panel).traverse().filter(JPanel.class).filter((it) -> BOTTOM_PANEL.equals(it.getName())).first(); + if (first != null) { + first.add(back, BorderLayout.WEST); } - - main.revalidate(); - main.repaint(); } + + for (JButton button : UIUtil.findComponentsOfType(main, JButton.class)) { + if (button.getClientProperty(DialogWrapper.DEFAULT_ACTION) == Boolean.TRUE) { + parent.getRootPane().setDefaultButton(button); + break; + } + } + + main.revalidate(); + main.repaint(); } }; list.addListSelectionListener(selectionListener); @@ -896,18 +912,41 @@ public class FlatWelcomeFrame extends JFrame implements IdeFrame, Disposable, Ac }.registerCustomShortcutSet(KeyEvent.VK_ESCAPE, 0, main); } installQuickSearch(list); + + if (singleProjectGenerator) { + actionsListPanel.setPreferredSize(new Dimension(0, 0)); + } + return Pair.create(main, list); } - public static void installQuickSearch(JBList list) { - new ListSpeedSearch(list, new Convertor() { + @Nullable + private static JComponent createBackLabel(@Nullable Runnable backAction, boolean singleProjectGenerator) { + if (backAction == null) return null; + if (singleProjectGenerator) { + JButton button = new JButton("Back"); + button.addActionListener(e -> backAction.run()); + return button; + } + JLabel back = new JLabel(AllIcons.Actions.Back); + back.setBorder(JBUI.Borders.empty(3, 7, 10, 7)); + back.setHorizontalAlignment(SwingConstants.LEFT); + new ClickListener() { @Override - public String convert(Object o) { - if (o instanceof AbstractActionWithPanel) { //to avoid dependency mess with ProjectSettingsStepBase - return ((AbstractActionWithPanel)o).getTemplatePresentation().getText(); - } - return null; + public boolean onClick(@NotNull MouseEvent event, int clickCount) { + backAction.run(); + return true; } + }.installOn(back); + return back; + } + + public static void installQuickSearch(JBList list) { + new ListSpeedSearch(list, (Convertor)o -> { + if (o instanceof AbstractActionWithPanel) { //to avoid dependency mess with ProjectSettingsStepBase + return ((AbstractActionWithPanel)o).getTemplatePresentation().getText(); + } + return null; }); } From 7b6bf38ab7420ce06bcba1131feed8930b608b28 Mon Sep 17 00:00:00 2001 From: Alexander Koshevoy Date: Sun, 30 Oct 2016 02:19:21 +0300 Subject: [PATCH 26/39] PY-21278 Deadlock on closing PyCharm when Python debug server configuration is waiting for a script to be connected fixed --- .../ServerModeDebuggerTransport.java | 68 +++++++++---------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ServerModeDebuggerTransport.java b/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ServerModeDebuggerTransport.java index 5d299fe13237..18d73c7b26e0 100644 --- a/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ServerModeDebuggerTransport.java +++ b/python/pydevSrc/com/jetbrains/python/debugger/pydev/transport/ServerModeDebuggerTransport.java @@ -5,7 +5,6 @@ import com.intellij.openapi.vfs.CharsetToolkit; import com.jetbrains.python.debugger.pydev.ProtocolFrame; import com.jetbrains.python.debugger.pydev.RemoteDebugger; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.io.InputStream; @@ -20,10 +19,10 @@ public class ServerModeDebuggerTransport extends BaseDebuggerTransport { private static final Logger LOG = Logger.getInstance(ServerModeDebuggerTransport.class); @NotNull private final ServerSocket myServerSocket; - @Nullable private DebuggerReader myDebuggerReader; + private volatile DebuggerReader myDebuggerReader; private volatile boolean myConnected = false; - @Nullable private Socket mySocket; + private volatile Socket mySocket; private int myConnectionTimeout; public ServerModeDebuggerTransport(RemoteDebugger debugger, @NotNull ServerSocket socket, int connectionTimeout) { @@ -34,45 +33,44 @@ public class ServerModeDebuggerTransport extends BaseDebuggerTransport { @Override public void waitForConnect() throws IOException { + //noinspection SocketOpenedButNotSafelyClosed + myServerSocket.setSoTimeout(myConnectionTimeout); + synchronized (mySocketObject) { - //noinspection SocketOpenedButNotSafelyClosed - myServerSocket.setSoTimeout(myConnectionTimeout); - - Socket socket = myServerSocket.accept(); + mySocket = myServerSocket.accept(); myConnected = true; - try { - myDebuggerReader = new DebuggerReader(myDebugger, socket.getInputStream()); - } - catch (IOException e) { - try { - socket.close(); - } - catch (IOException ignore) { - } - throw e; - } - mySocket = socket; - - // mySocket is closed in close() method on process termination } + try { + synchronized (mySocketObject) { + myDebuggerReader = new DebuggerReader(myDebugger, mySocket.getInputStream()); + } + } + catch (IOException e) { + try { + mySocket.close(); + } + catch (IOException ignore) { + } + throw e; + } + + // mySocket is closed in close() method on process termination } @Override public void close() { - synchronized (mySocketObject) { - try { - if (myDebuggerReader != null) { - myDebuggerReader.stop(); - } + try { + if (myDebuggerReader != null) { + myDebuggerReader.stop(); } - finally { - if (!myServerSocket.isClosed()) { - try { - myServerSocket.close(); - } - catch (IOException e) { - LOG.warn("Error closing socket", e); - } + } + finally { + if (!myServerSocket.isClosed()) { + try { + myServerSocket.close(); + } + catch (IOException e) { + LOG.warn("Error closing socket", e); } } } @@ -80,7 +78,7 @@ public class ServerModeDebuggerTransport extends BaseDebuggerTransport { @Override public boolean isConnected() { - return myConnected && mySocket != null && !mySocket.isClosed(); + return myConnected && mySocket != null && !mySocket.isClosed(); } @Override From 17021e68847342e76463a226f2904c35477a7406 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 20:10:24 +0300 Subject: [PATCH 27/39] Remove unused methods --- .../vcs/impl/ProjectLevelVcsManagerImpl.java | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java index 55b580d9f0bc..d5859ffc4848 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java @@ -550,11 +550,6 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme return null; } - public boolean hasExplicitMapping(final FilePath f) { - VirtualFile vFile = ChangesUtil.findValidParentAccurately(f); - return vFile != null && hasExplicitMapping(vFile); - } - public boolean hasExplicitMapping(final VirtualFile vFile) { final VcsDirectoryMapping mapping = myMappings.getMappingFor(vFile); return mapping != null && !mapping.isDefaultMapping(); @@ -923,17 +918,6 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme return !file.isDirectory() && parent != null && parent.equals(myProject.getBaseDir()); } - // inner roots disclosed - public static List getRootsUnder(final List roots, final VirtualFile underWhat) { - final List result = new ArrayList<>(roots.size()); - for (VirtualFile root : roots) { - if (VfsUtilCore.isAncestor(underWhat, root, false)) { - result.add(root); - } - } - return result; - } - @Override public VcsHistoryCache getVcsHistoryCache() { return myVcsHistoryCache; From 3ad7ed3c9799e3d599b2a491161b1c231d0d2816 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 20:11:07 +0300 Subject: [PATCH 28/39] Lambdify --- .../vcs/impl/ProjectLevelVcsManagerImpl.java | 59 +++++++------------ 1 file changed, 22 insertions(+), 37 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java index d5859ffc4848..4d85ca3ee42f 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java @@ -257,14 +257,11 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme @Override public void projectOpened() { - addInitializationRequest(VcsInitObject.AFTER_COMMON, new Runnable() { - @Override - public void run() { - if (!ApplicationManager.getApplication().isUnitTestMode()) { - VcsRootChecker[] checkers = Extensions.getExtensions(VcsRootChecker.EXTENSION_POINT_NAME); - if (checkers.length != 0) { - VcsRootScanner.start(myProject, checkers); - } + addInitializationRequest(VcsInitObject.AFTER_COMMON, () -> { + if (!ApplicationManager.getApplication().isUnitTestMode()) { + VcsRootChecker[] checkers = Extensions.getExtensions(VcsRootChecker.EXTENSION_POINT_NAME); + if (checkers.length != 0) { + VcsRootScanner.start(myProject, checkers); } } }); @@ -306,17 +303,13 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme @Nullable public AbstractVcs getVcsFor(final FilePath file) { final VirtualFile vFile = ChangesUtil.findValidParentAccurately(file); - return ApplicationManager.getApplication().runReadAction(new Computable() { - @Override - @Nullable - public AbstractVcs compute() { - if (!ApplicationManager.getApplication().isUnitTestMode() && !myProject.isInitialized()) return null; - if (myProject.isDisposed()) throw new ProcessCanceledException(); - if (vFile != null) { - return getVcsFor(vFile); - } - return null; + return ApplicationManager.getApplication().runReadAction((Computable)() -> { + if (!ApplicationManager.getApplication().isUnitTestMode() && !myProject.isInitialized()) return null; + if (myProject.isDisposed()) throw new ProcessCanceledException(); + if (vFile != null) { + return getVcsFor(vFile); } + return null; }); } @@ -430,19 +423,16 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme return; } - ApplicationManager.getApplication().invokeLater(new Runnable() { - @Override - public void run() { - // for default and disposed projects the ContentManager is not available. - if (myProject.isDisposed() || myProject.isDefault()) return; - final ContentManager contentManager = getContentManager(); - if (contentManager == null) { - myPendingOutput.add(Pair.create(message, contentType)); - } - else { - getOrCreateConsoleContent(contentManager); - printToConsole(message, contentType); - } + ApplicationManager.getApplication().invokeLater(() -> { + // for default and disposed projects the ContentManager is not available. + if (myProject.isDisposed() || myProject.isDefault()) return; + final ContentManager contentManager = getContentManager(); + if (contentManager == null) { + myPendingOutput.add(Pair.create(message, contentType)); + } + else { + getOrCreateConsoleContent(contentManager); + printToConsole(message, contentType); } }, ModalityState.defaultModalityState()); } @@ -864,12 +854,7 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme } public void addInitializationRequest(final VcsInitObject vcsInitObject, final Runnable runnable) { - ApplicationManager.getApplication().runReadAction(new Runnable() { - @Override - public void run() { - myInitialization.add(vcsInitObject, runnable); - } - }); + ApplicationManager.getApplication().runReadAction(() -> myInitialization.add(vcsInitObject, runnable)); } @Override From d0c5b53c5ebd17dea14df56dded9f12ac4b05c49 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sat, 29 Oct 2016 20:25:53 +0300 Subject: [PATCH 29/39] Don't explicitly disconnect from project's message bus It is disconnected on dispose anyway. Also don't expect and pass the MessageBus in the constructor: it is just the project.getMessageBus() anyway. --- .../vcs/impl/ProjectLevelVcsManagerImpl.java | 20 +++++++------------ .../vcs/impl/projectlevelman/NewMappings.java | 8 +++----- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java index 4d85ca3ee42f..e32de562f801 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java @@ -67,7 +67,6 @@ import com.intellij.util.ContentUtilEx; import com.intellij.util.Processor; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.Convertor; -import com.intellij.util.messages.MessageBus; import com.intellij.util.messages.MessageBusConnection; import com.intellij.util.text.DateFormatUtil; import org.jdom.Attribute; @@ -91,7 +90,6 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme private final NewMappings myMappings; private final Project myProject; - private final MessageBus myMessageBus; private final MappingsToRoots myMappingsToRoots; private ContentManager myContentManager; @@ -127,20 +125,17 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme private final VcsHistoryCache myVcsHistoryCache; private final ContentRevisionCache myContentRevisionCache; - private final MessageBusConnection myConnect; private final FileIndexFacade myExcludedIndex; private final VcsFileListenerContextHelper myVcsFileListenerContextHelper; private final VcsAnnotationLocalChangesListenerImpl myAnnotationLocalChangesListener; public ProjectLevelVcsManagerImpl(Project project, final FileStatusManager manager, - MessageBus messageBus, final FileIndexFacade excludedFileIndex, ProjectManager projectManager, DefaultVcsRootPolicy defaultVcsRootPolicy, VcsFileListenerContextHelper vcsFileListenerContextHelper) { myProject = project; - myMessageBus = messageBus; mySerialization = new ProjectLevelVcsManagerSerialization(); myOptionsAndConfirmations = new OptionsAndConfirmations(); @@ -164,12 +159,11 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme } }); } - myMappings = new NewMappings(myProject, myMessageBus, this, manager); + myMappings = new NewMappings(myProject, this, manager); myMappingsToRoots = new MappingsToRoots(myMappings, myProject); myVcsHistoryCache = new VcsHistoryCache(); myContentRevisionCache = new ContentRevisionCache(); - myConnect = myMessageBus.connect(); myVcsFileListenerContextHelper = vcsFileListenerContextHelper; VcsListener vcsListener = new VcsListener() { @Override @@ -179,9 +173,10 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme } }; myExcludedIndex = excludedFileIndex; - myConnect.subscribe(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED, vcsListener); - myConnect.subscribe(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED_IN_PLUGIN, vcsListener); - myConnect.subscribe(UpdatedFilesListener.UPDATED_FILES, new UpdatedFilesListener() { + MessageBusConnection connection = myProject.getMessageBus().connect(); + connection.subscribe(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED, vcsListener); + connection.subscribe(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED_IN_PLUGIN, vcsListener); + connection.subscribe(UpdatedFilesListener.UPDATED_FILES, new UpdatedFilesListener() { @Override public void consume(Set strings) { myContentRevisionCache.clearCurrent(strings); @@ -239,7 +234,6 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme public void disposeComponent() { releaseConsole(); myMappings.disposeMe(); - myConnect.disconnect(); Disposer.dispose(myAnnotationLocalChangesListener); myContentManager = null; @@ -632,7 +626,7 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme @Override public void addVcsListener(VcsListener listener) { - final MessageBusConnection connection = myMessageBus.connect(); + MessageBusConnection connection = myProject.getMessageBus().connect(); connection.subscribe(VCS_CONFIGURATION_CHANGED, listener); myAdapters.put(listener, connection); } @@ -710,7 +704,7 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme @Override public void notifyDirectoryMappingChanged() { - myMessageBus.syncPublisher(VCS_CONFIGURATION_CHANGED).directoryMappingChanged(); + myProject.getMessageBus().syncPublisher(VCS_CONFIGURATION_CHANGED).directoryMappingChanged(); } public void readDirectoryMappings(final Element element) { diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java index 5029672fde3e..9fc7995fe053 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/projectlevelman/NewMappings.java @@ -34,7 +34,6 @@ import com.intellij.openapi.vfs.LocalFileSystem; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.Convertor; -import com.intellij.util.messages.MessageBus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -54,17 +53,16 @@ public class NewMappings { private FileWatchRequestsManager myFileWatchRequestsManager; private final DefaultVcsRootPolicy myDefaultVcsRootPolicy; - private final MessageBus myMessageBus; private final ProjectLevelVcsManager myVcsManager; private final FileStatusManager myFileStatusManager; private final Project myProject; private boolean myActivated; - public NewMappings(final Project project, final MessageBus messageBus, final ProjectLevelVcsManagerImpl vcsManager, + public NewMappings(Project project, + ProjectLevelVcsManagerImpl vcsManager, FileStatusManager fileStatusManager) { myProject = project; - myMessageBus = messageBus; myVcsManager = vcsManager; myFileStatusManager = fileStatusManager; myLock = new Object(); @@ -183,7 +181,7 @@ public class NewMappings { public void mappingsChanged() { if (myProject.isDisposed()) return; - myMessageBus.syncPublisher(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED).directoryMappingChanged(); + myProject.getMessageBus().syncPublisher(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED).directoryMappingChanged(); myFileStatusManager.fileStatusesChanged(); myFileWatchRequestsManager.ping(); } From 24a0e7a809c27e990a9cf4ae4fe0c73ccad13647 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 30 Oct 2016 12:47:28 +0300 Subject: [PATCH 30/39] Use recommended method instead of deprecated ProjectLevelVcsManager#add/removeVcsListener has been deprecated for a long time, but some usages were still not updated. --- .../com/intellij/openapi/vcs/ProjectLevelVcsManager.java | 2 ++ .../vcs-api/src/com/intellij/openapi/vcs/VcsListener.java | 4 ++-- .../openapi/vcs/changes/ChangeListManagerImpl.java | 8 ++++---- .../changes/committed/CommittedChangesViewManager.java | 3 +-- .../idea/svn/history/MergeInfoUpdatesListener.java | 5 +++-- 5 files changed, 12 insertions(+), 10 deletions(-) diff --git a/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java b/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java index 032ea0be76a9..b679df2d3f96 100644 --- a/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java +++ b/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java @@ -198,6 +198,7 @@ public abstract class ProjectLevelVcsManager { * @deprecated use {@link #VCS_CONFIGURATION_CHANGED} instead * @since 6.0 */ + @Deprecated public abstract void addVcsListener(VcsListener listener); /** @@ -207,6 +208,7 @@ public abstract class ProjectLevelVcsManager { * @deprecated use {@link #VCS_CONFIGURATION_CHANGED} instead * @since 6.0 */ + @Deprecated public abstract void removeVcsListener(VcsListener listener); /** diff --git a/platform/vcs-api/src/com/intellij/openapi/vcs/VcsListener.java b/platform/vcs-api/src/com/intellij/openapi/vcs/VcsListener.java index e8d3abbacb19..64fa6f38571d 100644 --- a/platform/vcs-api/src/com/intellij/openapi/vcs/VcsListener.java +++ b/platform/vcs-api/src/com/intellij/openapi/vcs/VcsListener.java @@ -25,9 +25,9 @@ package com.intellij.openapi.vcs; import java.util.EventListener; /** - * Allows to receive notifications about changes in VCS configuration for the project. + *

Allows to receive notifications about changes in VCS configuration for the project.

+ *

Use the {@link ProjectLevelVcsManager#VCS_CONFIGURATION_CHANGED} MessageBus topic to subscribe.

* - * @see ProjectLevelVcsManager#addVcsListener * @since 6.0 */ public interface VcsListener extends EventListener { diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ChangeListManagerImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ChangeListManagerImpl.java index 13c89e8f9a89..fda0d5762b9b 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ChangeListManagerImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/ChangeListManagerImpl.java @@ -63,6 +63,8 @@ import java.util.*; import java.util.concurrent.*; import java.util.concurrent.atomic.AtomicReference; +import static com.intellij.openapi.vcs.ProjectLevelVcsManager.*; + @State(name = "ChangeListManager", storages = @Storage(StoragePathMacros.WORKSPACE_FILE)) public class ChangeListManagerImpl extends ChangeListManagerEx implements ProjectComponent, ChangeListOwner, PersistentStateComponent { public static final Logger LOG = Logger.getInstance("#com.intellij.openapi.vcs.changes.ChangeListManagerImpl"); @@ -246,14 +248,14 @@ public class ChangeListManagerImpl extends ChangeListManagerEx implements Projec final ProjectLevelVcsManager vcsManager = ProjectLevelVcsManager.getInstance(myProject); if (ApplicationManager.getApplication().isUnitTestMode()) { myUpdater.initialized(); - vcsManager.addVcsListener(myVcsListener); + myProject.getMessageBus().connect().subscribe(VCS_CONFIGURATION_CHANGED, myVcsListener); } else { ((ProjectLevelVcsManagerImpl)vcsManager).addInitializationRequest( VcsInitObject.CHANGE_LIST_MANAGER, (DumbAwareRunnable)() -> { myUpdater.initialized(); broadcastStateAfterLoad(); - vcsManager.addVcsListener(myVcsListener); + myProject.getMessageBus().connect().subscribe(VCS_CONFIGURATION_CHANGED, myVcsListener); }); myConflictTracker.startTracking(); @@ -307,8 +309,6 @@ public class ChangeListManagerImpl extends ChangeListManagerEx implements Projec @Override public void projectClosed() { - ProjectLevelVcsManager.getInstance(myProject).removeVcsListener(myVcsListener); - synchronized (myDataLock) { myUpdateChangesProgressIndicator.cancel(); } diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/committed/CommittedChangesViewManager.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/committed/CommittedChangesViewManager.java index d33c9b63234b..e5880e6867fd 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/committed/CommittedChangesViewManager.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/committed/CommittedChangesViewManager.java @@ -87,8 +87,8 @@ public class CommittedChangesViewManager implements ChangesViewContentProvider { } public JComponent initContent() { - myVcsManager.addVcsListener(myVcsListener); myConnection = myBus.connect(); + myConnection.subscribe(ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED, myVcsListener); myConnection.subscribe(CommittedChangesCache.COMMITTED_TOPIC, new MyCommittedChangesListener()); updateChangesContent(); myComponent.refreshChanges(true); @@ -96,7 +96,6 @@ public class CommittedChangesViewManager implements ChangesViewContentProvider { } public void disposeContent() { - myVcsManager.removeVcsListener(myVcsListener); myConnection.disconnect(); Disposer.dispose(myComponent); myComponent = null; diff --git a/plugins/svn4idea/src/org/jetbrains/idea/svn/history/MergeInfoUpdatesListener.java b/plugins/svn4idea/src/org/jetbrains/idea/svn/history/MergeInfoUpdatesListener.java index 5f05a263adcf..36fc1206b7bd 100644 --- a/plugins/svn4idea/src/org/jetbrains/idea/svn/history/MergeInfoUpdatesListener.java +++ b/plugins/svn4idea/src/org/jetbrains/idea/svn/history/MergeInfoUpdatesListener.java @@ -18,7 +18,6 @@ package org.jetbrains.idea.svn.history; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.ZipperUpdater; -import com.intellij.openapi.vcs.ProjectLevelVcsManager; import com.intellij.openapi.vcs.VcsListener; import com.intellij.openapi.vcs.changes.committed.CommittedChangesTreeBrowser; import com.intellij.openapi.vcs.changes.committed.VcsConfigurationChangeListener; @@ -34,6 +33,8 @@ import org.jetbrains.idea.svn.mergeinfo.SvnMergeInfoCache; import java.util.ArrayList; import java.util.List; +import static com.intellij.openapi.vcs.ProjectLevelVcsManager.VCS_CONFIGURATION_CHANGED; + public class MergeInfoUpdatesListener { private final static int DELAY = 300; @@ -77,7 +78,7 @@ public class MergeInfoUpdatesListener { myConnection.subscribe(SvnVcs.ROOTS_RELOADED, reloadConsumer); - ProjectLevelVcsManager.getInstance(myProject).addVcsListener(new VcsListener() { + myConnection.subscribe(VCS_CONFIGURATION_CHANGED, new VcsListener() { public void directoryMappingChanged() { callReloadMergeInfo(); } From 69908b5f3c23d75535cc9e9c3e91adfb7bfbdeb9 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 30 Oct 2016 13:07:05 +0300 Subject: [PATCH 31/39] Suppress invokeLater warning since it is justified and explained --- .../src/com/intellij/openapi/vcs/impl/VcsInitialization.java | 1 + 1 file changed, 1 insertion(+) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java index 57e1e859735b..10cef7e7947f 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java @@ -120,6 +120,7 @@ public class VcsInitialization implements Disposable { if (ApplicationManager.getApplication().isWriteAccessAllowed()) { // dispose happens without prior project close (most likely light project case in tests) // get out of write action and wait there + //noinspection SSBasedInspection SwingUtilities.invokeLater(this::waitForCompletion); } else { From 8eb6907cb3fdf493a0c058824ddf825704d3ffb5 Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 30 Oct 2016 13:09:40 +0300 Subject: [PATCH 32/39] Cleanup & lambdify --- .../com/intellij/openapi/vcs/impl/VcsInitialization.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java index 10cef7e7947f..cb31a1cdc077 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/VcsInitialization.java @@ -21,6 +21,7 @@ import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Attachment; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProgressIndicator; +import com.intellij.openapi.progress.ProgressIndicatorProvider; import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.progress.Task; import com.intellij.openapi.progress.impl.ProgressManagerImpl; @@ -36,6 +37,7 @@ import org.jetbrains.annotations.TestOnly; import javax.swing.*; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.List; import java.util.concurrent.Future; @@ -85,11 +87,11 @@ public class VcsInitialization implements Disposable { list = myList; myInitStarted = true; // list would not be modified starting from this point Future future = myFuture; - if (future != null && future.isCancelled() || ProgressManager.getGlobalProgressIndicator().isCanceled()) { + if (future != null && future.isCancelled() || ProgressIndicatorProvider.getGlobalProgressIndicator().isCanceled()) { return; } } - Collections.sort(list, (o1, o2) -> o1.getFirst().getOrder() - o2.getFirst().getOrder()); + Collections.sort(list, Comparator.comparingInt(o -> o.getFirst().getOrder())); for (Pair pair : list) { ProgressManager.checkCanceled(); pair.getSecond().run(); @@ -136,7 +138,8 @@ public class VcsInitialization implements Disposable { TimeoutUtil.sleep(10); } if (myIndicator.isRunning()) { - LOG.error("Failed to wait for completion of VCS initialization for project "+myProject, new Attachment("thread dump", ThreadDumper.dumpThreadsToString())); + LOG.error("Failed to wait for completion of VCS initialization for project "+ myProject, + new Attachment("thread dump", ThreadDumper.dumpThreadsToString())); } } } From 38de4b523a347b6777e7390ccb23ee9203c0feea Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 30 Oct 2016 14:08:26 +0300 Subject: [PATCH 33/39] Move utility method from the API It is used only in the ShelveChangesManager, no external usages. --- .../intellij/openapi/vcs/ProjectLevelVcsManager.java | 2 -- .../vcs/changes/shelf/ShelveChangesManager.java | 12 +++++++++++- .../openapi/vcs/impl/ProjectLevelVcsManagerImpl.java | 11 ----------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java b/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java index b679df2d3f96..df6bec75a2ef 100644 --- a/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java +++ b/platform/vcs-api/src/com/intellij/openapi/vcs/ProjectLevelVcsManager.java @@ -282,8 +282,6 @@ public abstract class ProjectLevelVcsManager { public abstract boolean isFileInContent(final VirtualFile vf); public abstract boolean isIgnored(VirtualFile vf); - public abstract boolean dvcsUsedInProject(); - @NotNull public abstract VcsAnnotationLocalChangesListener getAnnotationLocalChangesListener(); } diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java index 36752a4e4f5e..686a36f1f1c5 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java @@ -360,7 +360,7 @@ public class ShelveChangesManager extends AbstractProjectComponent implements JD private void baseRevisionsOfDvcsIntoContext(List textChanges, CommitContext commitContext) { ProjectLevelVcsManager vcsManager = ProjectLevelVcsManager.getInstance(myProject); - if (vcsManager.dvcsUsedInProject() && VcsConfiguration.getInstance(myProject).INCLUDE_TEXT_INTO_SHELF) { + if (dvcsUsedInProject() && VcsConfiguration.getInstance(myProject).INCLUDE_TEXT_INTO_SHELF) { final Set big = SelectFilesToAddTextsToPatchPanel.getBig(textChanges); final ArrayList toKeep = new ArrayList<>(); for (Change change : textChanges) { @@ -377,6 +377,16 @@ public class ShelveChangesManager extends AbstractProjectComponent implements JD } } + private boolean dvcsUsedInProject() { + AbstractVcs[] allActiveVcss = ProjectLevelVcsManager.getInstance(myProject).getAllActiveVcss(); + for (AbstractVcs activeVcs : allActiveVcss) { + if (VcsType.distributed.equals(activeVcs.getType())) { + return true; + } + } + return false; + } + public ShelvedChangeList importFilePatches(final String fileName, final List patches, final PatchEP[] patchTransitExtensions) throws IOException { try { diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java index e32de562f801..735d10f8e536 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/impl/ProjectLevelVcsManagerImpl.java @@ -874,17 +874,6 @@ public class ProjectLevelVcsManagerImpl extends ProjectLevelVcsManagerEx impleme } } - @Override - public boolean dvcsUsedInProject() { - AbstractVcs[] allActiveVcss = getAllActiveVcss(); - for (AbstractVcs activeVcs : allActiveVcss) { - if (VcsType.distributed.equals(activeVcs.getType())) { - return true; - } - } - return false; - } - private boolean isInDirectoryBasedRoot(@Nullable VirtualFile file) { if (file != null && ProjectKt.isDirectoryBased(myProject)) { return ProjectKt.getStateStore(myProject).isProjectFile(file); From 44583db83110633ed37ec73c403c42c31bc5f98c Mon Sep 17 00:00:00 2001 From: Kirill Likhodedov Date: Sun, 30 Oct 2016 14:10:12 +0300 Subject: [PATCH 34/39] Streamify instead of for --- .../openapi/vcs/changes/shelf/ShelveChangesManager.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java index 686a36f1f1c5..dc9449616b4c 100644 --- a/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java +++ b/platform/vcs-impl/src/com/intellij/openapi/vcs/changes/shelf/ShelveChangesManager.java @@ -378,13 +378,8 @@ public class ShelveChangesManager extends AbstractProjectComponent implements JD } private boolean dvcsUsedInProject() { - AbstractVcs[] allActiveVcss = ProjectLevelVcsManager.getInstance(myProject).getAllActiveVcss(); - for (AbstractVcs activeVcs : allActiveVcss) { - if (VcsType.distributed.equals(activeVcs.getType())) { - return true; - } - } - return false; + return Arrays.stream(ProjectLevelVcsManager.getInstance(myProject).getAllActiveVcss()). + anyMatch(vcs -> VcsType.distributed.equals(vcs.getType())); } public ShelvedChangeList importFilePatches(final String fileName, final List patches, final PatchEP[] patchTransitExtensions) From c0d85551ac6c40eb1dd3bd29ada62d7b849f35cf Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sat, 29 Oct 2016 19:33:24 +0300 Subject: [PATCH 35/39] [git] do not mix normal output with error output; check exit code before parsing remaining buffer on process terminated EA-90861 --- .../src/git4idea/history/GitHistoryUtils.java | 119 ++++++++++-------- 1 file changed, 68 insertions(+), 51 deletions(-) diff --git a/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java b/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java index 118cc1d54aa8..39c24c130ec0 100644 --- a/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java +++ b/plugins/git4idea/src/git4idea/history/GitHistoryUtils.java @@ -15,6 +15,7 @@ */ package git4idea.history; +import com.intellij.execution.process.ProcessOutputTypes; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.components.ServiceManager; import com.intellij.openapi.diagnostic.Logger; @@ -55,6 +56,7 @@ import git4idea.history.browser.SHAHash; import git4idea.history.browser.SymbolicRefs; import git4idea.history.browser.SymbolicRefsI; import git4idea.history.wholeTree.AbstractHash; +import git4idea.i18n.GitBundle; import git4idea.log.GitLogProvider; import git4idea.log.GitRefManager; import org.jetbrains.annotations.NotNull; @@ -537,70 +539,85 @@ public class GitHistoryUtils { @NotNull Consumer recordConsumer, int bufferSize) throws VcsException { - final StringBuilder buffer = new StringBuilder(); + final StringBuilder output = new StringBuilder(); + final StringBuilder errors = new StringBuilder(); final Ref foundRecordEnd = Ref.create(false); final Ref ex = new Ref<>(); final AtomicInteger records = new AtomicInteger(); handler.addLineListener(new GitLineHandlerListener() { @Override public void onLineAvailable(String line, Key outputType) { - try { - // format of the record is .*.* - // then next record goes - // (rather inconveniently, after RECORD_END there is a list of modified files) - // so here I'm trying to find text between two RECORD_START symbols - // that simultaneously contains a RECORD_END - // this helps to deal with commits like a929478f6720ac15d949117188cd6798b4a9c286 in linux repo that have RECORD_START symbols in the message - // wont help with RECORD_END symbols in the message however (have not seen those yet) - - String tail = null; - if (!foundRecordEnd.get()) { - int recordEnd = line.indexOf(GitLogParser.RECORD_END); - if (recordEnd != -1) { - foundRecordEnd.set(true); - buffer.append(line.substring(0, recordEnd + 1)); - line = line.substring(recordEnd + 1); - } - else { - buffer.append(line).append("\n"); - } - } - - if (foundRecordEnd.get()) { - int nextRecordStart = line.indexOf(GitLogParser.RECORD_START); - if (nextRecordStart == -1) { - buffer.append(line).append("\n"); - } - else if (nextRecordStart == 0) { - tail = line + "\n"; - } - else { - buffer.append(line.substring(0, nextRecordStart)); - tail = line.substring(nextRecordStart) + "\n"; - } - } - - if (tail != null) { - if (records.incrementAndGet() > bufferSize) { - recordConsumer.consume(buffer); - buffer.setLength(0); - } - buffer.append(tail); - foundRecordEnd.set(false); - } + if (outputType == ProcessOutputTypes.STDERR) { + errors.append(line).append("\n"); } - catch (Exception e) { - ex.set(new VcsException(e)); + else if (outputType == ProcessOutputTypes.STDOUT) { + try { + // format of the record is .*.* + // then next record goes + // (rather inconveniently, after RECORD_END there is a list of modified files) + // so here I'm trying to find text between two RECORD_START symbols + // that simultaneously contains a RECORD_END + // this helps to deal with commits like a929478f6720ac15d949117188cd6798b4a9c286 in linux repo that have RECORD_START symbols in the message + // wont help with RECORD_END symbols in the message however (have not seen those yet) + + String tail = null; + if (!foundRecordEnd.get()) { + int recordEnd = line.indexOf(GitLogParser.RECORD_END); + if (recordEnd != -1) { + foundRecordEnd.set(true); + output.append(line.substring(0, recordEnd + 1)); + line = line.substring(recordEnd + 1); + } + else { + output.append(line).append("\n"); + } + } + + if (foundRecordEnd.get()) { + int nextRecordStart = line.indexOf(GitLogParser.RECORD_START); + if (nextRecordStart == -1) { + output.append(line).append("\n"); + } + else if (nextRecordStart == 0) { + tail = line + "\n"; + } + else { + output.append(line.substring(0, nextRecordStart)); + tail = line.substring(nextRecordStart) + "\n"; + } + } + + if (tail != null) { + if (records.incrementAndGet() > bufferSize) { + recordConsumer.consume(output); + output.setLength(0); + } + output.append(tail); + foundRecordEnd.set(false); + } + } + catch (Exception e) { + ex.set(new VcsException(e)); + } } } @Override public void processTerminated(int exitCode) { - try { - recordConsumer.consume(buffer); + if (exitCode != 0) { + String errorMessage = errors.toString(); + if (errorMessage.isEmpty()) { + errorMessage = GitBundle.message("git.error.exit", exitCode); + } + ex.set(new VcsException(errorMessage)); } - catch (Exception e) { - ex.set(new VcsException(e)); + else { + try { + recordConsumer.consume(output); + } + catch (Exception e) { + ex.set(new VcsException(e)); + } } } From a11b22b90e3289e706d0aaf8af0b97607eb30776 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sun, 30 Oct 2016 19:50:58 +0300 Subject: [PATCH 36/39] [vcs-log] minor: inline parameter that is always true --- .../src/com/intellij/vcs/log/data/AbstractDataGetter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java index 28e4891b5158..6dbcf4600da2 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java @@ -68,7 +68,7 @@ abstract class AbstractDataGetter implements Di Disposer.register(parentDisposable, this); myLoader = new SequentialLimitedLifoExecutor<>(this, MAX_LOADING_TASKS, task -> { - preLoadCommitData(task.myCommits, true); + preLoadCommitData(task.myCommits); notifyLoaded(); }); } @@ -141,7 +141,7 @@ abstract class AbstractDataGetter implements Di public void run(@NotNull final ProgressIndicator indicator) { indicator.checkCanceled(); try { - TIntObjectHashMap map = preLoadCommitData(toLoad, true); + TIntObjectHashMap map = preLoadCommitData(toLoad); map.forEachValue(value -> { result.add(value); return true; @@ -237,7 +237,7 @@ abstract class AbstractDataGetter implements Di } @NotNull - public TIntObjectHashMap preLoadCommitData(@NotNull TIntHashSet commits, boolean saveInCache) throws VcsException { + public TIntObjectHashMap preLoadCommitData(@NotNull TIntHashSet commits) throws VcsException { TIntObjectHashMap result = new TIntObjectHashMap<>(); final MultiMap rootsAndHashes = MultiMap.create(); commits.forEach(commit -> { @@ -256,7 +256,7 @@ abstract class AbstractDataGetter implements Di int index = myHashMap.getCommitIndex(data.getId(), data.getRoot()); result.put(index, data); } - if (saveInCache) saveInCache(result); + saveInCache(result); } else { LOG.error("No log provider for root " + entry.getKey().getPath() + ". All known log providers " + myLogProviders); From 56069016965cf655c4464e6aec07d8e01cda3043 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sun, 30 Oct 2016 19:51:39 +0300 Subject: [PATCH 37/39] [vcs-log] minor: lambdify --- .../vcs/log/data/AbstractDataGetter.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java index 6dbcf4600da2..9e338f3cd312 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/AbstractDataGetter.java @@ -74,12 +74,9 @@ abstract class AbstractDataGetter implements Di } private void notifyLoaded() { - UIUtil.invokeAndWaitIfNeeded(new Runnable() { - @Override - public void run() { - for (Runnable loadingFinishedListener : myLoadingFinishedListeners) { - loadingFinishedListener.run(); - } + UIUtil.invokeAndWaitIfNeeded((Runnable)() -> { + for (Runnable loadingFinishedListener : myLoadingFinishedListeners) { + loadingFinishedListener.run(); } }); } @@ -267,15 +264,10 @@ abstract class AbstractDataGetter implements Di } public void saveInCache(@NotNull TIntObjectHashMap details) { - UIUtil.invokeAndWaitIfNeeded(new Runnable() { - @Override - public void run() { - details.forEachEntry((key, value) -> { - myCache.put(key, value); - return true; - }); - } - }); + UIUtil.invokeAndWaitIfNeeded((Runnable)() -> details.forEachEntry((key, value) -> { + myCache.put(key, value); + return true; + })); } @NotNull From 6ac189e6e02415ab26ccb3eaaa52c4e3d426998d Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sun, 30 Oct 2016 20:10:49 +0300 Subject: [PATCH 38/39] [vcs-log] minor: lambdify, use Comparator.comparingInt --- .../log/graph/impl/facade/SimpleGraphInfo.java | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/SimpleGraphInfo.java b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/SimpleGraphInfo.java index ec8f9ce678c4..fe3a20094693 100644 --- a/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/SimpleGraphInfo.java +++ b/platform/vcs-log/graph/src/com/intellij/vcs/log/graph/impl/facade/SimpleGraphInfo.java @@ -82,12 +82,9 @@ public class SimpleGraphInfo implements PermanentGraphInfo { CommitId commit = permanentCommitsInfo.getCommitId(nodeId); List parents = ContainerUtil.newSmartList(); parents.addAll(ContainerUtil.mapNotNull(asLiteLinearGraph(linearGraph).getNodes(row, LiteLinearGraph.NodeFilter.DOWN), - new Function() { - @Override - public CommitId fun(Integer row) { - if (row < start || row >= end) return null; - return permanentCommitsInfo.getCommitId(linearGraph.getNodeId(row)); - } + row1 -> { + if (row1 < start || row1 >= end) return null; + return permanentCommitsInfo.getCommitId(linearGraph.getNodeId(row1)); })); graphCommits.add(new GraphCommitImpl<>(commit, parents, permanentCommitsInfo.getTimestamp(nodeId))); commitsIdMap.add(commit); @@ -110,12 +107,7 @@ public class SimpleGraphInfo implements PermanentGraphInfo { } } - ContainerUtil.sort(headNodeIndexes, new Comparator() { - @Override - public int compare(Integer o1, Integer o2) { - return layoutIndexes[o1] - layoutIndexes[o2]; - } - }); + ContainerUtil.sort(headNodeIndexes, Comparator.comparingInt(o -> layoutIndexes[o])); int[] starts = new int[headNodeIndexes.size()]; for (int i = 0; i < starts.length; i++) { starts[i] = layoutIndexes[headNodeIndexes.get(i)]; From d20dd17bc1603f4590ddb1e236e9ea2d99d198d1 Mon Sep 17 00:00:00 2001 From: Julia Beliaeva Date: Sun, 30 Oct 2016 20:25:44 +0300 Subject: [PATCH 39/39] [vcs-log] create empty fake visible pack for empty visible graph EA-87115 --- .../src/com/intellij/vcs/log/data/FakeVisiblePackBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/FakeVisiblePackBuilder.java b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/FakeVisiblePackBuilder.java index 398bcc198d4d..4b8868140bcc 100644 --- a/platform/vcs-log/impl/src/com/intellij/vcs/log/data/FakeVisiblePackBuilder.java +++ b/platform/vcs-log/impl/src/com/intellij/vcs/log/data/FakeVisiblePackBuilder.java @@ -41,7 +41,7 @@ public class FakeVisiblePackBuilder { @NotNull public VisiblePack build(@NotNull VisiblePack visiblePack) { - if (visiblePack.getVisibleGraph() instanceof VisibleGraphImpl) { + if (visiblePack.getVisibleGraph() instanceof VisibleGraphImpl && visiblePack.getVisibleGraph().getVisibleCommitCount() > 0) { return build(visiblePack.getDataPack(), ((VisibleGraphImpl)visiblePack.getVisibleGraph()), visiblePack.getFilters()); } else {