From 8a7402bb235fee1426c8a1d34c2fdcfbb6f9e1e8 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 23 Jul 2014 19:46:55 +0200 Subject: [PATCH] restore the assertion on modifying PSI for uncommitted document (WEB-12791) --- .../testSrc/com/intellij/psi/MiscPsiTest.java | 16 ++++++++++++++++ .../intellij/pom/core/impl/PomModelImpl.java | 19 +++++++++++++++++-- .../psi/impl/PsiDocumentManagerBase.java | 2 +- .../psi/impl/PsiToDocumentSynchronizer.java | 8 +++----- 4 files changed, 37 insertions(+), 8 deletions(-) diff --git a/java/java-tests/testSrc/com/intellij/psi/MiscPsiTest.java b/java/java-tests/testSrc/com/intellij/psi/MiscPsiTest.java index 096547c4f4db..61414ab0b416 100644 --- a/java/java-tests/testSrc/com/intellij/psi/MiscPsiTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/MiscPsiTest.java @@ -260,4 +260,20 @@ public class MiscPsiTest extends LightCodeInsightFixtureTestCase { assertEquals("some.unknown.Foo", type.getCanonicalText()); } + public void testNoPsiModificationsInUncommittedDocument() { + final PsiJavaFile file = (PsiJavaFile)myFixture.addFileToProject("a.java", "class A{}"); + Document document = file.getViewProvider().getDocument(); + document.insertString(0, " "); + + PsiClass psiClass = file.getClasses()[0]; + try { + psiClass.addBefore(PsiParserFacade.SERVICE.getInstance(getProject()).createWhiteSpaceFromText(" "), psiClass.getLBrace()); + fail(); + } + catch (IllegalStateException e) { + assertEquals("Attempt to modify PSI for non-committed Document!", e.getMessage()); + } + assertEquals("class A{}", psiClass.getText()); + assertEquals(" class A{}", document.getText()); + } } diff --git a/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java b/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java index ba5858c52993..2ca35e4227e9 100644 --- a/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java +++ b/platform/core-impl/src/com/intellij/pom/core/impl/PomModelImpl.java @@ -301,15 +301,30 @@ public class PomModelImpl extends UserDataHolderBase implements PomModel { final PsiToDocumentSynchronizer synchronizer = manager.getSynchronizer(); final PsiElement changeScope = transaction.getChangeScope(); LOG.assertTrue(changeScope != null); - BlockSupportImpl.sendBeforeChildrenChangeEvent((PsiManagerImpl)PsiManager.getInstance(myProject), changeScope, true); - final PsiFile containingFileByTree = getContainingFileByTree(changeScope); + final PsiFile containingFileByTree = getContainingFileByTree(changeScope); + if (changeScope.isPhysical() && synchronizer.toProcessPsiEvent() && isDocumentUncommitted(containingFileByTree)) { + // fail-fast to prevent any psi modifications that would cause psi/document text mismatch + // PsiToDocumentSynchronizer assertions happen inside event processing and are logged by PsiManagerImpl.fireEvent instead of being rethrown + // so it's important to throw something outside event processing + throw new IllegalStateException("Attempt to modify PSI for non-committed Document!"); + } + + BlockSupportImpl.sendBeforeChildrenChangeEvent((PsiManagerImpl)PsiManager.getInstance(myProject), changeScope, true); Document document = containingFileByTree == null ? null : manager.getCachedDocument(containingFileByTree); if(document != null) { synchronizer.startTransaction(myProject, document, changeScope); } } + private boolean isDocumentUncommitted(@Nullable PsiFile file) { + if (file == null) return false; + + PsiDocumentManager manager = PsiDocumentManager.getInstance(myProject); + Document cachedDocument = manager.getCachedDocument(file); + return cachedDocument != null && manager.isUncommited(cachedDocument); + } + @Nullable private static PsiFile getContainingFileByTree(@NotNull final PsiElement changeScope) { // there could be pseudo physical trees (JSPX/JSP/etc.) which must not translate diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java index 3583af212528..8a8f7221d962 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiDocumentManagerBase.java @@ -699,7 +699,7 @@ public abstract class PsiDocumentManagerBase extends PsiDocumentManager implemen } // we can end up outside write action here if the document has forUseInNonAWTThread=true - ApplicationManager.getApplication().runWriteAction(new Runnable() { + ApplicationManager.getApplication().runWriteAction(new ExternalChangeAction() { @Override public void run() { psiFile.getViewProvider().beforeContentsSynchronized(); diff --git a/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java b/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java index 7611dd11162a..13c43571decb 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java @@ -74,10 +74,8 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { final PsiFile psiFile = event.getFile(); if (psiFile == null || psiFile.getNode() == null) return; - final Document document = getCachedDocument(psiFile, force); - if (document == null) return; - - if (myPsiDocumentManager.isUncommited(document)) { + final Document document = myPsiDocumentManager.getCachedDocument(psiFile); + if (document != null && myPsiDocumentManager.isUncommited(document)) { throw new IllegalStateException("Attempt to modify PSI for non-committed Document!"); } } @@ -207,7 +205,7 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { return myIgnorePsiEvents; } - private boolean toProcessPsiEvent() { + public boolean toProcessPsiEvent() { return !myIgnorePsiEvents && !ApplicationManager.getApplication().hasWriteAction(IgnorePsiEventsMarker.class); }