From 1e27ed86639323477d0cfd24fbd800cafdda1ba2 Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 5 Sep 2015 09:36:40 +0200 Subject: [PATCH] smart pointers should survive document replaceText that's actually minimized to insertText (e.g. foo renamed to foo2) --- .../source/tree/injected/MyTestInjector.java | 2 ++ .../SmartPsiElementPointersTest.java | 14 +++++++++ .../openapi/editor/impl/DocumentImpl.java | 25 ++++++++------- .../openapi/editor/impl/RangeMarkerImpl.java | 19 ++++++++++-- .../editor/impl/event/DocumentEventImpl.java | 31 +++++++++++++++++++ .../psi/impl/PsiToDocumentSynchronizer.java | 31 +++++++++++++++---- .../psi/impl/smartPointers/MarkerCache.java | 3 +- .../MoveDirectoryWithClassesHelper.java | 11 +++---- .../openapi/editor/impl/RangeMarkerTest.java | 14 +++++++++ .../psi/impl/source/xml/XmlTagImpl.java | 23 +++++++++----- 10 files changed, 138 insertions(+), 35 deletions(-) diff --git a/java/java-impl/src/com/intellij/psi/impl/source/tree/injected/MyTestInjector.java b/java/java-impl/src/com/intellij/psi/impl/source/tree/injected/MyTestInjector.java index 5621240378e0..31d31efdcf1a 100644 --- a/java/java-impl/src/com/intellij/psi/impl/source/tree/injected/MyTestInjector.java +++ b/java/java-impl/src/com/intellij/psi/impl/source/tree/injected/MyTestInjector.java @@ -29,6 +29,7 @@ import com.intellij.lang.injection.ConcatenationAwareInjector; import com.intellij.lang.injection.InjectedLanguageManager; import com.intellij.lang.injection.MultiHostInjector; import com.intellij.lang.injection.MultiHostRegistrar; +import com.intellij.lang.java.JavaLanguage; import com.intellij.openapi.Disposable; import com.intellij.openapi.extensions.ExtensionPoint; import com.intellij.openapi.extensions.Extensions; @@ -72,6 +73,7 @@ public class MyTestInjector { registerForStringVarInitializer(parent, project, Language.findLanguageByID("Oracle"), "oracle", null, null); registerForParameterValue(parent, project, Language.findLanguageByID("Groovy"), "groovy"); + registerForStringVarInitializer(parent, project, JavaLanguage.INSTANCE, "java", "", ""); } private static void registerForParameterValue(Disposable parent, final Project project, final Language language, final String paramName) { diff --git a/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java b/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java index 9a7d7bc32d51..efb42a5d521b 100644 --- a/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java +++ b/java/java-tests/testSrc/com/intellij/psi/impl/smartPointers/SmartPsiElementPointersTest.java @@ -783,4 +783,18 @@ public class SmartPsiElementPointersTest extends CodeInsightTestCase { assertInstanceOf(pointer.getElement(), PsiTypeElement.class); } + public void testPointerToReferenceSurvivesRename() { + PsiFile file = configureByText(JavaFileType.INSTANCE, "class Foo extends Bar {}"); + PsiJavaCodeReferenceElement ref = PsiTreeUtil.findElementOfClassAtOffset(file, file.getText().indexOf("Bar"), PsiJavaCodeReferenceElement.class, false); + SmartPointerEx pointer = (SmartPointerEx)SmartPointerManager.getInstance(myProject).createSmartPsiElementPointer(ref); + ref = null; + + PlatformTestUtil.tryGcSoftlyReachableObjects(); + assertNull(pointer.getCachedElement()); + + ref = PsiTreeUtil.findElementOfClassAtOffset(file, file.getText().indexOf("Bar"), PsiJavaCodeReferenceElement.class, false); + ref.handleElementRename("BarImpl"); + assertNotNull(pointer.getElement()); + } + } diff --git a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java index e1945af84b99..dbe7cf828cf0 100644 --- a/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/editor/impl/DocumentImpl.java @@ -448,7 +448,7 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { myText = myText.ensureChunked(); ImmutableText newText = myText.insert(offset, ImmutableText.valueOf(s)); - updateText(newText, offset, null, newText.subtext(offset, offset + s.length()), false, LocalTimeCounter.currentTime()); + updateText(newText, offset, null, newText.subtext(offset, offset + s.length()), false, LocalTimeCounter.currentTime(), offset, 0); trimToSize(); } @@ -472,7 +472,7 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { } myText = myText.ensureChunked(); - updateText(myText.delete(startOffset, endOffset), startOffset, myText.subtext(startOffset, endOffset), null, false, LocalTimeCounter.currentTime()); + updateText(myText.delete(startOffset, endOffset), startOffset, myText.subtext(startOffset, endOffset), null, false, LocalTimeCounter.currentTime(), startOffset, endOffset - startOffset); } @Override @@ -517,8 +517,11 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { throw new ReadOnlyModificationException(this); } + int initialStartOffset = startOffset; + int initialOldLength = endOffset - startOffset; + final int newStringLength = s.length(); - final CharSequence chars = getCharsSequence(); + final CharSequence chars = myText; int newStartInString = 0; int newEndInString = newStringLength; while (newStartInString < newStringLength && @@ -555,7 +558,7 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { newText = myText.delete(startOffset, endOffset).insert(startOffset, changedPart); changedPart = newText.subtext(startOffset, startOffset + changedPart.length()); } - updateText(newText, startOffset, sToDelete, changedPart, wholeTextReplaced, newModificationStamp); + updateText(newText, startOffset, sToDelete, changedPart, wholeTextReplaced, newModificationStamp, initialStartOffset, initialOldLength); trimToSize(); } @@ -671,14 +674,16 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { @Nullable CharSequence oldString, @Nullable CharSequence newString, boolean wholeTextReplaced, - long newModificationStamp) { + long newModificationStamp, + int initialStartOffset, + int initialOldLength) { assertNotNestedModification(); boolean enableRecursiveModifications = Registry.is("enable.recursive.document.changes"); // temporary property, to remove in IDEA 16 myChangeInProgress = true; try { - final DocumentEvent event; + DocumentEvent event = new DocumentEventImpl(this, offset, oldString, newString, myModificationStamp, wholeTextReplaced, initialStartOffset, initialOldLength); try { - event = doBeforeChangedUpdate(offset, oldString, newString, wholeTextReplaced); + doBeforeChangedUpdate(event); } finally { if (enableRecursiveModifications) { @@ -697,8 +702,7 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { } } - @NotNull - private DocumentEvent doBeforeChangedUpdate(int offset, CharSequence oldString, CharSequence newString, boolean wholeTextReplaced) { + private void doBeforeChangedUpdate(DocumentEvent event) { Application app = ApplicationManager.getApplication(); if (app != null) { FileDocumentManager manager = FileDocumentManager.getInstance(); @@ -711,8 +715,6 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { getLineSet(); // initialize line set to track changed lines - DocumentEvent event = new DocumentEventImpl(this, offset, oldString, newString, myModificationStamp, wholeTextReplaced); - if (!ShutDownTracker.isShutdownHookRunning()) { DocumentListener[] listeners = getCachedListeners(); for (int i = listeners.length - 1; i >= 0; i--) { @@ -726,7 +728,6 @@ public class DocumentImpl extends UserDataHolderBase implements DocumentEx { } myEventsHandling = true; - return event; } private void assertInsideCommand() { diff --git a/platform/core-impl/src/com/intellij/openapi/editor/impl/RangeMarkerImpl.java b/platform/core-impl/src/com/intellij/openapi/editor/impl/RangeMarkerImpl.java index 6e6197068774..3ccefb207053 100644 --- a/platform/core-impl/src/com/intellij/openapi/editor/impl/RangeMarkerImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/editor/impl/RangeMarkerImpl.java @@ -19,6 +19,7 @@ import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.editor.event.DocumentEvent; import com.intellij.openapi.editor.ex.DocumentEx; import com.intellij.openapi.editor.ex.RangeMarkerEx; +import com.intellij.openapi.editor.impl.event.DocumentEventImpl; import com.intellij.openapi.util.ProperTextRange; import com.intellij.openapi.util.UserDataHolderBase; import com.intellij.util.Processor; @@ -192,12 +193,26 @@ public class RangeMarkerImpl extends UserDataHolderBase implements RangeMarkerEx final int newLength = e.getNewLength(); // changes after the end. - if (intervalEnd < offset || !isGreedyToRight && intervalEnd == offset) { + if (intervalEnd < offset) { + return new ProperTextRange(intervalStart, intervalEnd); + } + if (!isGreedyToRight && intervalEnd == offset) { + // handle replaceString that was minimized and resulted in insertString at the range end + if (e instanceof DocumentEventImpl && oldLength == 0 && ((DocumentEventImpl)e).getInitialStartOffset() < offset) { + return new ProperTextRange(intervalStart, intervalEnd + newLength); + } return new ProperTextRange(intervalStart, intervalEnd); } // changes before start - if (intervalStart > offset + oldLength || !isGreedyToLeft && intervalStart == offset + oldLength) { + if (intervalStart > offset + oldLength) { + return new ProperTextRange(intervalStart + newLength - oldLength, intervalEnd + newLength - oldLength); + } + if (!isGreedyToLeft && intervalStart == offset + oldLength) { + // handle replaceString that was minimized and resulted in insertString at the range start + if (e instanceof DocumentEventImpl && oldLength == 0 && ((DocumentEventImpl)e).getInitialStartOffset() + ((DocumentEventImpl)e).getInitialOldLength() > offset) { + return new ProperTextRange(intervalStart - oldLength, intervalEnd + newLength - oldLength); + } return new ProperTextRange(intervalStart + newLength - oldLength, intervalEnd + newLength - oldLength); } diff --git a/platform/core-impl/src/com/intellij/openapi/editor/impl/event/DocumentEventImpl.java b/platform/core-impl/src/com/intellij/openapi/editor/impl/event/DocumentEventImpl.java index 993dda9c0535..78f3e58fd03a 100644 --- a/platform/core-impl/src/com/intellij/openapi/editor/impl/event/DocumentEventImpl.java +++ b/platform/core-impl/src/com/intellij/openapi/editor/impl/event/DocumentEventImpl.java @@ -46,6 +46,8 @@ public class DocumentEventImpl extends DocumentEvent { private int myOptimizedOldLineShift = -1; private boolean myOptimizedOldLineShiftCalculated; + private final int myInitialStartOffset; + private final int myInitialOldLength; public DocumentEventImpl(@NotNull Document document, int offset, @@ -53,6 +55,16 @@ public class DocumentEventImpl extends DocumentEvent { CharSequence newString, long oldTimeStamp, boolean wholeTextReplaced) { + this(document, offset, oldString, newString, oldTimeStamp, wholeTextReplaced, offset, oldString == null ? 0 : oldString.length()); + } + public DocumentEventImpl(@NotNull Document document, + int offset, + CharSequence oldString, + CharSequence newString, + long oldTimeStamp, + boolean wholeTextReplaced, + int initialStartOffset, + int initialOldLength) { super(document); myOffset = offset; @@ -62,6 +74,9 @@ public class DocumentEventImpl extends DocumentEvent { myNewString = newString == null ? "" : newString; myNewLength = myNewString.length(); + myInitialStartOffset = initialStartOffset; + myInitialOldLength = initialOldLength; + myOldTimeStamp = oldTimeStamp; if (getDocument().getTextLength() == 0) { @@ -107,6 +122,22 @@ public class DocumentEventImpl extends DocumentEvent { return (Document)getSource(); } + /** + * @return initial start offset as requested in {@link Document#replaceString(int, int, CharSequence)} call, before common prefix and + * suffix were removed from the changed range. + */ + public int getInitialStartOffset() { + return myInitialStartOffset; + } + + /** + * @return initial "old fragment" length (endOffset - startOffset) as requested in {@link Document#replaceString(int, int, CharSequence)} call, before common prefix and + * suffix were removed from the changed range. + */ + public int getInitialOldLength() { + return myInitialOldLength; + } + public int getStartOldIndex() { if (isStartOldIndexCalculated) return myStartOldIndex; 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 22861d1d4ff6..5c34ed2c4b95 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java +++ b/platform/core-impl/src/com/intellij/psi/impl/PsiToDocumentSynchronizer.java @@ -184,7 +184,7 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { public void syncDocument(@NotNull Document document, @NotNull PsiTreeChangeEventImpl event) { int oldLength = event.getOldChild() instanceof ForeignLeafPsiElement ? 0 : event.getOldLength(); String newText = event.getNewChild() instanceof ForeignLeafPsiElement ? "" : event.getNewChild().getText(); - replaceString(document, event.getOffset(), event.getOffset() + oldLength, newText); + replaceString(document, event.getOffset(), event.getOffset() + oldLength, newText, event.getNewChild()); } }); } @@ -194,7 +194,7 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { doSync(event, false, new DocSyncAction() { @Override public void syncDocument(@NotNull Document document, @NotNull PsiTreeChangeEventImpl event) { - replaceString(document, event.getOffset(), event.getOffset() + event.getOldLength(), event.getParent().getText()); + replaceString(document, event.getOffset(), event.getOffset() + event.getOldLength(), event.getParent().getText(), event.getParent()); } }); } @@ -212,24 +212,28 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { return !myIgnorePsiEvents && !ApplicationManager.getApplication().hasWriteAction(IgnorePsiEventsMarker.class); } + @TestOnly public void replaceString(Document document, int startOffset, int endOffset, String s) { + replaceString(document, startOffset, endOffset, s, null); + } + private void replaceString(Document document, int startOffset, int endOffset, String s, @Nullable PsiElement replacement) { final DocumentChangeTransaction documentChangeTransaction = getTransaction(document); if(documentChangeTransaction != null) { - documentChangeTransaction.replace(startOffset, endOffset - startOffset, s); + documentChangeTransaction.replace(startOffset, endOffset - startOffset, s, replacement); } } public void insertString(Document document, int offset, String s) { final DocumentChangeTransaction documentChangeTransaction = getTransaction(document); if(documentChangeTransaction != null){ - documentChangeTransaction.replace(offset, 0, s); + documentChangeTransaction.replace(offset, 0, s, null); } } private void deleteString(Document document, int startOffset, int endOffset){ final DocumentChangeTransaction documentChangeTransaction = getTransaction(document); if(documentChangeTransaction != null){ - documentChangeTransaction.replace(startOffset, endOffset - startOffset, ""); + documentChangeTransaction.replace(startOffset, endOffset - startOffset, "", null); } } @@ -332,7 +336,7 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { return myAffectedFragments; } - public void replace(int psiStart, int length, @NotNull String replace) { + public void replace(int psiStart, int length, @NotNull String replace, @Nullable PsiElement replacement) { // calculating fragment // minimize replace int start = 0; @@ -354,6 +358,21 @@ public class PsiToDocumentSynchronizer extends PsiTreeChangeAdapter { end--; } + // increase the changed range to start and end on PSI token boundaries + // this will help to survive smart pointers with the same boundaries + if (replacement != null && (newStartInReplace > 0 || newEndInReplace < replaceLength)) { + PsiElement startLeaf = replacement.findElementAt(newStartInReplace); + PsiElement endLeaf = replacement.findElementAt(newEndInReplace - 1); + if (startLeaf != null && endLeaf != null) { + int leafStart = startLeaf.getTextRange().getStartOffset() - replacement.getTextRange().getStartOffset(); + int leafEnd = endLeaf.getTextRange().getEndOffset() - replacement.getTextRange().getStartOffset(); + start += leafStart - newStartInReplace; + end += leafEnd - newEndInReplace; + newStartInReplace = leafStart; + newEndInReplace = leafEnd; + } + } + // optimization: when delete fragment from the middle of the text, prefer split at the line boundaries if (newStartInReplace == newEndInReplace && start > 0 && start < end && StringUtil.indexOf(chars, '\n', start, end) != -1) { // try to align to the line boundaries diff --git a/platform/core-impl/src/com/intellij/psi/impl/smartPointers/MarkerCache.java b/platform/core-impl/src/com/intellij/psi/impl/smartPointers/MarkerCache.java index de2a27ca84cb..9c6cfeec8771 100644 --- a/platform/core-impl/src/com/intellij/psi/impl/smartPointers/MarkerCache.java +++ b/platform/core-impl/src/com/intellij/psi/impl/smartPointers/MarkerCache.java @@ -93,7 +93,8 @@ class MarkerCache { else { frozen = frozen.applyEvent(event, 0); corrected = new DocumentEventImpl(frozen, event.getOffset(), event.getOldFragment(), event.getNewFragment(), event.getOldTimeStamp(), - event.isWholeTextReplaced()); + event.isWholeTextReplaced(), + ((DocumentEventImpl) event).getInitialStartOffset(), ((DocumentEventImpl) event).getInitialOldLength()); } map.forEachEntry(new TLongObjectProcedure() { diff --git a/platform/lang-impl/src/com/intellij/refactoring/move/moveClassesOrPackages/MoveDirectoryWithClassesHelper.java b/platform/lang-impl/src/com/intellij/refactoring/move/moveClassesOrPackages/MoveDirectoryWithClassesHelper.java index 5a31ec093867..c354dddf5c15 100644 --- a/platform/lang-impl/src/com/intellij/refactoring/move/moveClassesOrPackages/MoveDirectoryWithClassesHelper.java +++ b/platform/lang-impl/src/com/intellij/refactoring/move/moveClassesOrPackages/MoveDirectoryWithClassesHelper.java @@ -132,13 +132,10 @@ public abstract class MoveDirectoryWithClassesHelper { @Nullable public PsiReference getReference() { PsiElement element = getElement(); - if (element == null) { - return null; - } - else { - final ProperTextRange rangeInElement = getRangeInElement(); - return rangeInElement != null ? element.findReferenceAt(rangeInElement.getStartOffset()) : element.getReference(); - } + if (element == null) return null; + final ProperTextRange rangeInElement = getRangeInElement(); + PsiReference reference = rangeInElement != null ? element.findReferenceAt(rangeInElement.getStartOffset()) : element.getReference(); + return reference != null && reference.getRangeInElement().equals(rangeInElement) ? reference : null; } } } diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java index 40b50616c332..13b55d4bcc0f 100644 --- a/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java +++ b/platform/platform-tests/testSrc/com/intellij/openapi/editor/impl/RangeMarkerTest.java @@ -1186,4 +1186,18 @@ public class RangeMarkerTest extends LightPlatformTestCase { marker.dispose(); assertEmpty(LazyRangeMarkerFactoryImpl.getMarkers(virtualFile)); } + + public void testNonGreedyMarkersGrowOnAppendingReplace() { + Document doc = new DocumentImpl("foo"); + RangeMarker marker = doc.createRangeMarker(0, 3); + assertFalse(marker.isGreedyToLeft()); + assertFalse(marker.isGreedyToRight()); + + doc.replaceString(0, 3, "foobar"); + assertValidMarker(marker, 0, 6); + + doc.replaceString(0, 3, "goofoo"); + assertValidMarker(marker, 0, 9); + } + } diff --git a/xml/xml-psi-impl/src/com/intellij/psi/impl/source/xml/XmlTagImpl.java b/xml/xml-psi-impl/src/com/intellij/psi/impl/source/xml/XmlTagImpl.java index 35fb5f08b580..728df741bb88 100644 --- a/xml/xml-psi-impl/src/com/intellij/psi/impl/source/xml/XmlTagImpl.java +++ b/xml/xml-psi-impl/src/com/intellij/psi/impl/source/xml/XmlTagImpl.java @@ -31,6 +31,7 @@ import com.intellij.pom.PomManager; import com.intellij.pom.PomModel; import com.intellij.pom.event.PomModelEvent; import com.intellij.pom.impl.PomTransactionBase; +import com.intellij.pom.tree.events.TreeChangeEvent; import com.intellij.pom.xml.XmlAspect; import com.intellij.pom.xml.impl.events.XmlAttributeSetImpl; import com.intellij.pom.xml.impl.events.XmlTagNameChangedImpl; @@ -1133,13 +1134,21 @@ public class XmlTagImpl extends XmlElementImpl implements XmlTag { treeNext.getElementType() == XmlElementType.XML_TEXT) { final XmlText prevText = (XmlText)treePrev.getPsi(); final XmlText nextText = (XmlText)treeNext.getPsi(); - try { - prevText.setValue(prevText.getValue() + nextText.getValue()); - nextText.delete(); - } - catch (IncorrectOperationException e) { - LOG.error(e); - } + + final String newValue = prevText.getValue() + nextText.getValue(); + + // merging two XmlText-s should be done in one transaction to preserve smart pointers + ChangeUtil.prepareAndRunChangeAction(new ChangeUtil.ChangeAction() { + @Override + public void makeChange(TreeChangeEvent destinationTreeChange) { + PsiElement anchor = prevText.getPrevSibling(); + prevText.delete(); + nextText.delete(); + XmlText text = (XmlText)addAfter(XmlElementFactory.getInstance(getProject()).createDisplayText("x"), anchor); + text.setValue(newValue); + } + }, this); + } } }