From a055c5ed69e8922795eb91434859b5df0ed75488 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 27 Jan 2017 18:09:03 +0100 Subject: [PATCH] assert that finishLookup is called outside write action, because it can display "clear read-only status" dialog --- .../LightFixtureCompletionTestCase.java | 8 +----- .../intellij/json/JsonLiveTemplateTest.java | 11 +++----- .../codeInsight/lookup/impl/LookupImpl.java | 28 +++++++++++-------- .../lookup/impl/LookupTypedHandler.java | 2 +- .../fixtures/CodeInsightTestUtil.java | 13 +++------ .../GrCompletionWithLibraryTest.groovy | 14 ++-------- .../lang/GroovyLiveTemplatesTest.groovy | 10 ++----- .../PyLiveTemplatesExpandingTest.java | 14 +++------- .../completion/XmlCompletionTest.java | 13 --------- .../completion/XmlSyncTagTest.java | 4 +-- 10 files changed, 36 insertions(+), 81 deletions(-) diff --git a/java/testFramework/src/com/intellij/codeInsight/completion/LightFixtureCompletionTestCase.java b/java/testFramework/src/com/intellij/codeInsight/completion/LightFixtureCompletionTestCase.java index 4ca03ab7d9e5..583391f45acc 100644 --- a/java/testFramework/src/com/intellij/codeInsight/completion/LightFixtureCompletionTestCase.java +++ b/java/testFramework/src/com/intellij/codeInsight/completion/LightFixtureCompletionTestCase.java @@ -19,7 +19,6 @@ import com.intellij.codeInsight.lookup.LookupElement; import com.intellij.codeInsight.lookup.LookupEvent; import com.intellij.codeInsight.lookup.LookupManager; import com.intellij.codeInsight.lookup.impl.LookupImpl; -import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.util.text.StringUtil; import com.intellij.testFramework.LightProjectDescriptor; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; @@ -77,12 +76,7 @@ public abstract class LightFixtureCompletionTestCase extends LightCodeInsightFix final LookupImpl lookup = getLookup(); lookup.setCurrentItem(item); if (LookupEvent.isSpecialCompletionChar(completionChar)) { - new WriteCommandAction.Simple(getProject()) { - @Override - protected void run() throws Throwable { - lookup.finishLookup(completionChar); - } - }.execute().throwException(); + lookup.finishLookup(completionChar); } else { type(completionChar); } diff --git a/json/tests/test/com/intellij/json/JsonLiveTemplateTest.java b/json/tests/test/com/intellij/json/JsonLiveTemplateTest.java index 30f9b4408f3b..8dfdb0394a6b 100644 --- a/json/tests/test/com/intellij/json/JsonLiveTemplateTest.java +++ b/json/tests/test/com/intellij/json/JsonLiveTemplateTest.java @@ -25,7 +25,6 @@ import com.intellij.codeInsight.template.impl.TemplateImpl; import com.intellij.codeInsight.template.impl.TemplateManagerImpl; import com.intellij.codeInsight.template.impl.actions.ListTemplatesAction; import com.intellij.json.liveTemplates.JsonContextType; -import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Editor; import com.intellij.testFramework.fixtures.CodeInsightTestUtil; import com.intellij.util.containers.ContainerUtil; @@ -82,12 +81,10 @@ public class JsonLiveTemplateTest extends JsonTestCase { createJsonTemplate("foo", "foo", templateContent); myFixture.configureByText(JsonFileType.INSTANCE, "foo"); final Editor editor = myFixture.getEditor(); - WriteCommandAction.runWriteCommandAction(null, () -> { - new ListTemplatesAction().actionPerformedImpl(getProject(), editor); - final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(editor); - assertNotNull(lookup); - lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); - }); + new ListTemplatesAction().actionPerformedImpl(getProject(), editor); + final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(editor); + assertNotNull(lookup); + lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); myFixture.checkResult(templateContent.replaceAll("\\$.*?\\$", "")); } } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupImpl.java index 281b731689b0..a02085bcc328 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupImpl.java @@ -461,6 +461,22 @@ public class LookupImpl extends LightweightHint implements LookupEx, Disposable } public void finishLookup(char completionChar, @Nullable final LookupElement item) { + LOG.assertTrue(!ApplicationManager.getApplication().isWriteAccessAllowed(), "finishLookup should be called without a write action"); + final PsiFile file = getPsiFile(); + boolean writableOk = file == null || FileModificationService.getInstance().prepareFileForWrite(file); + if (myDisposed) { // ensureFilesWritable could close us by showing a dialog + return; + } + + if (!writableOk) { + doHide(false, true); + fireItemSelected(null, completionChar); + return; + } + CommandProcessor.getInstance().executeCommand(myProject, () -> finishLookupInWritableFile(completionChar, item), null, null); + } + + void finishLookupInWritableFile(char completionChar, @Nullable LookupElement item) { //noinspection deprecation,unchecked if (item == null || !item.isValid() || @@ -477,18 +493,6 @@ public class LookupImpl extends LightweightHint implements LookupEx, Disposable return; } - final PsiFile file = getPsiFile(); - boolean writableOk = file == null || FileModificationService.getInstance().prepareFileForWrite(file); - if (myDisposed) { // ensureFilesWritable could close us by showing a dialog - return; - } - - if (!writableOk) { - doHide(false, true); - fireItemSelected(null, completionChar); - return; - } - final String prefix = itemPattern(item); boolean plainMatch = ContainerUtil.or(item.getAllLookupStrings(), s -> StringUtil.containsIgnoreCase(s, prefix)); if (!plainMatch) { diff --git a/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupTypedHandler.java b/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupTypedHandler.java index 4e4f8d4ecaf2..f27342593833 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupTypedHandler.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/lookup/impl/LookupTypedHandler.java @@ -144,7 +144,7 @@ public class LookupTypedHandler extends TypedActionHandlerBase { } FeatureUsageTracker.getInstance().triggerFeatureUsed(CodeCompletionFeatures.EDITING_COMPLETION_FINISH_BY_DOT_ETC); - lookup.finishLookup(charTyped); + lookup.finishLookupInWritableFile(charTyped, item); return true; } } diff --git a/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightTestUtil.java b/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightTestUtil.java index 708e37ce9c88..d8ada560c849 100644 --- a/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightTestUtil.java +++ b/platform/testFramework/src/com/intellij/testFramework/fixtures/CodeInsightTestUtil.java @@ -157,15 +157,10 @@ public class CodeInsightTestUtil { public static void doLiveTemplateTest(@NotNull final CodeInsightTestFixture fixture, @NotNull final String before, @NotNull final String after) { fixture.configureByFile(before); - new WriteCommandAction(fixture.getProject()) { - @Override - protected void run(@NotNull Result result) throws Throwable { - new ListTemplatesAction().actionPerformedImpl(fixture.getProject(), fixture.getEditor()); - final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(fixture.getEditor()); - assert lookup != null; - lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); - } - }.execute(); + new ListTemplatesAction().actionPerformedImpl(fixture.getProject(), fixture.getEditor()); + final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(fixture.getEditor()); + assert lookup != null; + lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); fixture.checkResultByFile(after, false); } diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/completion/GrCompletionWithLibraryTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/completion/GrCompletionWithLibraryTest.groovy index 6f5ecd60b611..1f2355f34743 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/completion/GrCompletionWithLibraryTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/completion/GrCompletionWithLibraryTest.groovy @@ -14,11 +14,9 @@ * limitations under the License. */ package org.jetbrains.plugins.groovy.completion + import com.intellij.codeInsight.completion.CompletionType import com.intellij.codeInsight.lookup.LookupElement -import com.intellij.codeInsight.lookup.impl.LookupImpl -import com.intellij.openapi.application.Result -import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.module.Module import com.intellij.openapi.roots.ContentEntry import com.intellij.openapi.roots.ModifiableRootModel @@ -266,15 +264,7 @@ use\ LookupElement groovyUtilTuple = elements.find { it.psiElement instanceof PsiClass && it.psiElement.qualifiedName == 'groovy.lang.Tuple'} assertNotNull(elements as String, groovyUtilTuple) - LookupImpl lookup = getLookup() - lookup.setCurrentItem(tuple) - - new WriteCommandAction(myFixture.project, file) { - @Override - protected void run(@NotNull Result result) throws Throwable { - lookup.finishLookup('\n' as char) - } - }.execute() + lookup.finishLookup('\n' as char, tuple) myFixture.checkResult('''\ import p.Tuple diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyLiveTemplatesTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyLiveTemplatesTest.groovy index daaf408c0177..48487dfc01e7 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyLiveTemplatesTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/GroovyLiveTemplatesTest.groovy @@ -22,7 +22,6 @@ import com.intellij.codeInsight.template.impl.TemplateImpl import com.intellij.codeInsight.template.impl.TemplateManagerImpl import com.intellij.codeInsight.template.impl.TemplateSettings import com.intellij.codeInsight.template.impl.actions.ListTemplatesAction -import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.editor.Editor import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase import org.jetbrains.plugins.groovy.util.TestUtils @@ -68,13 +67,8 @@ void usage(int num, boolean someBoolean, List args){ } static void expandTemplate(final Editor editor) { - WriteCommandAction.runWriteCommandAction(null, new Runnable() { - @Override - void run() { - new ListTemplatesAction().actionPerformedImpl(editor.getProject(), editor) - ((LookupImpl)LookupManager.getActiveLookup(editor)).finishLookup(Lookup.NORMAL_SELECT_CHAR) - } - }) + new ListTemplatesAction().actionPerformedImpl(editor.getProject(), editor) + ((LookupImpl)LookupManager.getActiveLookup(editor)).finishLookup(Lookup.NORMAL_SELECT_CHAR) } void testGroovyStatementContext() throws Exception { diff --git a/python/testSrc/com/jetbrains/python/codeInsight/liveTemplates/PyLiveTemplatesExpandingTest.java b/python/testSrc/com/jetbrains/python/codeInsight/liveTemplates/PyLiveTemplatesExpandingTest.java index ddf4de749f68..f7edf9232bbd 100644 --- a/python/testSrc/com/jetbrains/python/codeInsight/liveTemplates/PyLiveTemplatesExpandingTest.java +++ b/python/testSrc/com/jetbrains/python/codeInsight/liveTemplates/PyLiveTemplatesExpandingTest.java @@ -19,7 +19,6 @@ import com.intellij.codeInsight.lookup.Lookup; import com.intellij.codeInsight.lookup.LookupManager; import com.intellij.codeInsight.lookup.impl.LookupImpl; import com.intellij.codeInsight.template.impl.actions.ListTemplatesAction; -import com.intellij.openapi.command.WriteCommandAction; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.jetbrains.python.fixtures.PyTestCase; @@ -42,15 +41,10 @@ public class PyLiveTemplatesExpandingTest extends PyTestCase { final Editor editor = myFixture.getEditor(); final Project project = myFixture.getProject(); - WriteCommandAction.runWriteCommandAction( - project, - () -> { - new ListTemplatesAction().actionPerformedImpl(project, editor); - final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(editor); - assertNotNull(lookup); - lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); - } - ); + new ListTemplatesAction().actionPerformedImpl(project, editor); + final LookupImpl lookup = (LookupImpl)LookupManager.getActiveLookup(editor); + assertNotNull(lookup); + lookup.finishLookup(Lookup.NORMAL_SELECT_CHAR); myFixture.checkResultByFile(getTestName(false) + "/a_after.py"); } diff --git a/xml/tests/src/com/intellij/codeInsight/completion/XmlCompletionTest.java b/xml/tests/src/com/intellij/codeInsight/completion/XmlCompletionTest.java index 4666772a78da..6af47dbcde58 100644 --- a/xml/tests/src/com/intellij/codeInsight/completion/XmlCompletionTest.java +++ b/xml/tests/src/com/intellij/codeInsight/completion/XmlCompletionTest.java @@ -26,15 +26,12 @@ import com.intellij.codeInsight.template.impl.TemplateManagerImpl; import com.intellij.javaee.ExternalResourceManager; import com.intellij.javaee.ExternalResourceManagerEx; import com.intellij.javaee.ExternalResourceManagerExImpl; -import com.intellij.openapi.application.Result; -import com.intellij.openapi.command.WriteCommandAction; import com.intellij.psi.codeStyle.CodeStyleSchemes; import com.intellij.psi.codeStyle.CodeStyleSettings; import com.intellij.psi.statistics.StatisticsManager; import com.intellij.psi.statistics.impl.StatisticsManagerImpl; import com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase; import com.intellij.xml.util.XmlUtil; -import org.jetbrains.annotations.NotNull; import java.util.List; @@ -81,16 +78,6 @@ public class XmlCompletionTest extends LightCodeInsightFixtureTestCase { ExternalResourceManagerExImpl.addTestResource(url, location, getTestRootDisposable()); } - @Override - protected void runTest() throws Throwable { - new WriteCommandAction(getProject()) { - @Override - protected void run(@NotNull Result result) throws Throwable { - XmlCompletionTest.super.runTest(); - } - }.execute(); - } - public void testCompleteWithAnyInSchema() throws Exception { String location = "29.xsd"; addResource("aaa",location); diff --git a/xml/tests/src/com/intellij/codeInsight/completion/XmlSyncTagTest.java b/xml/tests/src/com/intellij/codeInsight/completion/XmlSyncTagTest.java index 1c4a7368e622..da49693bc34d 100644 --- a/xml/tests/src/com/intellij/codeInsight/completion/XmlSyncTagTest.java +++ b/xml/tests/src/com/intellij/codeInsight/completion/XmlSyncTagTest.java @@ -58,10 +58,10 @@ public abstract class XmlSyncTagTest extends LightPlatformCodeInsightFixtureTest final String toType, final String result) { myFixture.configureByText(fileType, text); - CommandProcessor.getInstance().executeCommand(getProject(), () -> ApplicationManager.getApplication().runWriteAction(() -> { + CommandProcessor.getInstance().executeCommand(getProject(), () -> { myFixture.completeBasic(); if (toType != null) myFixture.type(toType); - }), "Typing", DocCommandGroupId.noneGroupId(myFixture.getEditor().getDocument()), myFixture.getEditor().getDocument()); + }, "Typing", DocCommandGroupId.noneGroupId(myFixture.getEditor().getDocument()), myFixture.getEditor().getDocument()); myFixture.checkResult(result); } }