Fix buggy update of host document when editing in fragment editor (#42)

monogon-dev · Mar 15, 2021 · a496d6c · a496d6c
1 parent 4da6dd4
commit a496d6c
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 43 deletions.
diff --git a/...java/dev/monogon/cue/lang/elementManipulator/AbstractStringLiteralElementManipulator.java b/...java/dev/monogon/cue/lang/elementManipulator/AbstractStringLiteralElementManipulator.java
@@ -31,29 +31,28 @@ abstract class AbstractStringLiteralElementManipulator<T extends CueStringLitera
     public final T handleContentChange(@NotNull T element,
                                        @NotNull TextRange range,
                                        String newRangeContent) throws IncorrectOperationException {
-        var contentRange = getRangeInElement(element);
-        var content = contentRange.substring(element.getText());
+        var maxRange = getRangeInElement(element);
+        var maxContent = maxRange.substring(element.getText());
         var escapedRangeContent = CueEscaperUtil.escapeCueString(newRangeContent, true, !(element instanceof CueMultilineLiteral), true,
                                                                  element instanceof CueSimpleBytesLit,
                                                                  element instanceof CueSimpleStringLit,
                                                                  element instanceof CueMultilineBytesLit,
                                                                  element instanceof CueMultilineStringLit,
                                                                  element.getEscapePaddingSize());
-        // it's possible that the current content range is smaller than the passed in range
-        // this could happen when an empty multiline string is updated with non-empty content
-        var fixedRange = !contentRange.contains(range) ? contentRange.intersection(range) : range;
-        var updatedContent = fixedRange.shiftLeft(contentRange.getStartOffset()).replace(content, escapedRangeContent);
+        // shiftLeft range to become relative to maxContent
+        var updatedContent = range.shiftLeft(maxRange.getStartOffset()).replace(maxContent, escapedRangeContent);
+        if (element instanceof CueMultilineLiteral && !updatedContent.isEmpty() && !updatedContent.endsWith("\n")) {
+            updatedContent = updatedContent + "\n";
+        }
 
+        // replace children, keep original parent element
+        // language injection in 2020.3 assumes that the host remains the same element (fixed in 2021.1?)
         var replacement = createStringLiteral(element, updatedContent, element.getEscapePaddingSize());
         if (replacement == null) {
             throw new IncorrectOperationException("unable to create simple string literal for content:" + newRangeContent);
         }
-
-        var replaced = element.replace(replacement);
-        if (replaced == null) {
-            throw new IncorrectOperationException("unable to replace with new simple string literal for content:" + newRangeContent);
-        }
-        return (T)replaced;
+        element.getNode().replaceAllChildrenToChildrenOf(replacement.getNode());
+        return element;
     }
 
     @Override

diff --git a/src/main/java/dev/monogon/cue/lang/psi/CueMultilineLiteral.java b/src/main/java/dev/monogon/cue/lang/psi/CueMultilineLiteral.java
@@ -1,4 +1,16 @@
 package dev.monogon.cue.lang.psi;
 
+import com.intellij.psi.impl.source.tree.LeafPsiElement;
+import dev.monogon.cue.lang.CueTypes;
+
 public interface CueMultilineLiteral extends CueStringLiteral {
+    @Override
+    default boolean isValidHost() {
+        var opening = getOpeningQuote();
+        var next = opening.getNextSibling();
+        if (!(next instanceof LeafPsiElement) || ((LeafPsiElement)next).getElementType() != CueTypes.NEWLINE) {
+            return false;
+        }
+        return getClosingQuote() != null;
+    }
 }
diff --git a/src/main/java/dev/monogon/cue/lang/psi/CuePsiElementFactory.java b/src/main/java/dev/monogon/cue/lang/psi/CuePsiElementFactory.java
@@ -23,31 +23,37 @@ public static CueSimpleStringLit createSimpleStringLiteral(@NotNull Project proj
     }
 
     @Nullable
-    public static CueMultilineStringLit createMultilineStringLiteral(@NotNull Project project,
-                                                                     @NotNull String unquotedContent,
-                                                                     int paddingSize) {
+    public static CueSimpleBytesLit createSimpleBytesLiteral(@NotNull Project project, @NotNull String unquotedContent, int paddingSize) {
         var padding = "#".repeat(paddingSize);
-        var lfContent = unquotedContent.isEmpty() ? "\n" : "\n" + unquotedContent + "\n";
-        var text = String.format("%s\"\"\"%s\"\"\"%s", padding, lfContent, padding);
+        var text = String.format("%s'%s'%s", padding, unquotedContent, padding);
         var file = PsiFileFactory.getInstance(project).createFileFromText("__.cue", CueFileType.INSTANCE,
                                                                           text, LocalTimeCounter.currentTime(), true);
 
-        return PsiTreeUtil.getParentOfType(file.findElementAt(1), CueMultilineStringLit.class);
+        return PsiTreeUtil.getParentOfType(file.findElementAt(1), CueSimpleBytesLit.class);
     }
 
     @Nullable
-    public static CueSimpleBytesLit createSimpleBytesLiteral(@NotNull Project project, @NotNull String unquotedContent, int paddingSize) {
+    public static CueMultilineStringLit createMultilineStringLiteral(@NotNull Project project,
+                                                                     @NotNull String unquotedContent,
+                                                                     int paddingSize) {
+        assert unquotedContent.isEmpty() || unquotedContent.endsWith("\n");
+
         var padding = "#".repeat(paddingSize);
-        var text = String.format("%s'%s'%s", padding, unquotedContent, padding);
+        var lfContent = "\n" + unquotedContent;
+        var text = String.format("%s\"\"\"%s\"\"\"%s", padding, lfContent, padding);
         var file = PsiFileFactory.getInstance(project).createFileFromText("__.cue", CueFileType.INSTANCE,
                                                                           text, LocalTimeCounter.currentTime(), true);
 
-        return PsiTreeUtil.getParentOfType(file.findElementAt(1), CueSimpleBytesLit.class);
+        return PsiTreeUtil.getParentOfType(file.findElementAt(1), CueMultilineStringLit.class);
     }
 
-    public static CueMultilineBytesLit createMultilineBytesLiteral(Project project, String unquotedContent, int paddingSize) {
+    public static CueMultilineBytesLit createMultilineBytesLiteral(@NotNull Project project,
+                                                                   @NotNull String unquotedContent,
+                                                                   int paddingSize) {
+        assert unquotedContent.isEmpty() || unquotedContent.endsWith("\n");
+
         var padding = "#".repeat(paddingSize);
-        var lfContent = unquotedContent.isEmpty() ? "\n" : "\n" + unquotedContent + "\n";
+        var lfContent = "\n" + unquotedContent;
         var text = String.format("%s'''%s'''%s", padding, lfContent, padding);
         var file = PsiFileFactory.getInstance(project).createFileFromText("__.cue", CueFileType.INSTANCE,
                                                                           text, LocalTimeCounter.currentTime(), true);

diff --git a/src/main/java/dev/monogon/cue/lang/psi/CueStringLiteral.java b/src/main/java/dev/monogon/cue/lang/psi/CueStringLiteral.java
@@ -31,13 +31,6 @@ default PsiLanguageInjectionHost updateText(@NotNull String text) {
 
     @Override
     default boolean isValidHost() {
-        if (isMultilineLiteral()) {
-            var opening = getOpeningQuote();
-            var next = opening.getNextSibling();
-            if (!(next instanceof LeafPsiElement) || ((LeafPsiElement)next).getElementType() != CueTypes.NEWLINE) {
-                return false;
-            }
-        }
         return getClosingQuote() != null;
     }
 
@@ -48,18 +41,18 @@ default TextRange getLiteralContentRange() {
         var openingQuote = getOpeningQuote();
         var openingNext = openingQuote.getNextSibling();
         var closingQuote = getClosingQuote();
-        var closingPrev = closingQuote != null ? closingQuote.getPrevSibling() : null;
         var lf = isMultilineLiteral()
                  && openingNext instanceof LeafPsiElement
                  && ((LeafPsiElement)openingNext).getElementType() == CueTypes.NEWLINE
                  ? 1 : 0;
-        var lfClosing = isMultilineLiteral()
-                        && closingPrev instanceof LeafPsiElement
-                        && ((LeafPsiElement)closingPrev).getElementType() == CueTypes.NEWLINE
-                        && !openingQuote.isEquivalentTo(closingPrev.getPrevSibling())
-                        ? 1 : 0;
+
+        // for multiline strings the content range is including the trailing linefeed
+        // because otherwise IntelliJ will incorrectly update the content when
+        // changing content "abc" to "abc\nxyz". For unknown reasons in this specific case
+        // IntelliJ isn't updating the injected host ranges properly
+        // Including the trailing linefeed is a workaround
         return TextRange.create(openingQuote.getStartOffsetInParent() + openingQuote.getTextLength() + lf,
-                                closingQuote == null ? getTextLength() : closingQuote.getStartOffsetInParent() - lfClosing);
+                                closingQuote == null ? getTextLength() : closingQuote.getStartOffsetInParent());
     }
 
     /**

diff --git a/src/test/java/dev/monogon/cue/lang/injection/CueMultiHostInjectorTest.java b/src/test/java/dev/monogon/cue/lang/injection/CueMultiHostInjectorTest.java
@@ -1,13 +1,22 @@
 package dev.monogon.cue.lang.injection;
 
+import com.intellij.codeInsight.intention.impl.QuickEditAction;
+import com.intellij.openapi.application.ApplicationManager;
+import com.intellij.openapi.command.CommandProcessor;
+import com.intellij.openapi.editor.Document;
 import com.intellij.openapi.util.TextRange;
+import com.intellij.psi.PsiDocumentManager;
+import com.intellij.psi.PsiFile;
 import dev.monogon.cue.CueLightTest;
 import dev.monogon.cue.lang.psi.CueSimpleBytesLit;
 import dev.monogon.cue.lang.psi.CueStringLiteral;
+import org.intellij.plugins.intelliLang.inject.InjectedLanguage;
+import org.intellij.plugins.intelliLang.inject.TemporaryPlacesRegistry;
 import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.function.Consumer;
 
 public class CueMultiHostInjectorTest extends CueLightTest {
     @Test
@@ -67,4 +76,55 @@ public void interpolationMultiple() {
             new InjectionData(TextRange.create(16, 16), "\\(3)", null));
         assertEquals(expected, ranges);
     }
+
+    @Test
+    public void fragmentEditorEmpty() {
+        var file = myFixture.configureByText("a.cue", "'''\n<caret>'''");
+        withInjectedContent(file, fragmentFile -> {
+            edit(fragmentFile, doc -> {
+                doc.insertString(0, "a");
+                doc.insertString(1, "b");
+                doc.insertString(2, "c");
+                doc.insertString(3, "\n");
+            });
+
+            myFixture.checkResult("'''\nabc\n'''");
+        });
+    }
+
+    @Test
+    public void fragmentEditorAppending() {
+        var file = myFixture.configureByText("a.cue", "'''\ntest<caret>\n'''");
+        withInjectedContent(file, fragmentFile -> {
+            edit(fragmentFile, doc -> {
+                doc.insertString(4, "\n");
+                doc.insertString(5, "a");
+                doc.insertString(6, "b");
+                doc.insertString(7, "c");
+            });
+
+            myFixture.checkResult("'''\ntest\nabc\n'''");
+        });
+    }
+
+    private void withInjectedContent(PsiFile file, Consumer<PsiFile> action) {
+        var host = findTypedElement(CueStringLiteral.class);
+        TemporaryPlacesRegistry.getInstance(getProject()).addHostWithUndo(host, InjectedLanguage.create("JSON"));
+        disposeOnTearDown(() -> TemporaryPlacesRegistry.getInstance(getProject()).removeHostWithUndo(getProject(), host));
+
+        var quickEdit = new QuickEditAction();
+        var handler = quickEdit.invokeImpl(getProject(), myFixture.getEditor(), file);
+        var fragmentFile = handler.getNewFile();
+
+        action.accept(fragmentFile);
+    }
+
+    private void edit(PsiFile file, Consumer<Document> action) {
+        CommandProcessor.getInstance().executeCommand(file.getProject(), () -> {
+            ApplicationManager.getApplication().runWriteAction(() -> {
+                var doc = PsiDocumentManager.getInstance(getProject()).getDocument(file);
+                action.accept(doc);
+            });
+        }, "Change Doc", "Change doc");
+    }
 }
diff --git a/src/test/java/dev/monogon/cue/lang/psi/CuePsiElementFactoryTest.java b/src/test/java/dev/monogon/cue/lang/psi/CuePsiElementFactoryTest.java
@@ -15,10 +15,10 @@ public void simpleString() {
 
     @Test
     public void multilineString() {
-        var string = CuePsiElementFactory.createMultilineStringLiteral(getProject(), "my content", 0);
+        var string = CuePsiElementFactory.createMultilineStringLiteral(getProject(), "my content\n", 0);
         assertEquals("\"\"\"\nmy content\n\"\"\"", string.getText());
 
-        string = CuePsiElementFactory.createMultilineStringLiteral(getProject(), "my content", 1);
+        string = CuePsiElementFactory.createMultilineStringLiteral(getProject(), "my content\n", 1);
         assertEquals("#\"\"\"\nmy content\n\"\"\"#", string.getText());
     }
 
@@ -33,10 +33,10 @@ public void simpleBytes() {
 
     @Test
     public void multilineBytes() {
-        var bytes = CuePsiElementFactory.createMultilineBytesLiteral(getProject(), "my content", 0);
+        var bytes = CuePsiElementFactory.createMultilineBytesLiteral(getProject(), "my content\n", 0);
         assertEquals("'''\nmy content\n'''", bytes.getText());
 
-        bytes = CuePsiElementFactory.createMultilineBytesLiteral(getProject(), "my content", 1);
+        bytes = CuePsiElementFactory.createMultilineBytesLiteral(getProject(), "my content\n", 1);
         assertEquals("#'''\nmy content\n'''#", bytes.getText());
     }
 }
diff --git a/src/test/java/dev/monogon/cue/lang/psi/impl/CueMultilineStringLiteralMixinTest.java b/src/test/java/dev/monogon/cue/lang/psi/impl/CueMultilineStringLiteralMixinTest.java
@@ -14,7 +14,7 @@ public void contentRange() {
         myFixture.configureByText("a.cue", "\"\"\"\ncontent<caret>\n\"\"\"");
         var string = findTypedElement(CueMultilineStringLit.class);
         assertTrue(string.isValidHost());
-        assertEquals(TextRange.create(4, 11), string.getLiteralContentRange());
+        assertEquals(TextRange.create(4, 12), string.getLiteralContentRange());
     }
 
     @Test
@@ -37,7 +37,7 @@ public void contentRangePadded() {
         myFixture.configureByText("a.cue", "##\"\"\"\ncontent<caret>\n\"\"\"##");
         var string = findTypedElement(CueMultilineStringLit.class);
         assertTrue(string.isValidHost());
-        assertEquals(TextRange.create(6, 13), string.getLiteralContentRange());
+        assertEquals(TextRange.create(6, 14), string.getLiteralContentRange());
     }
 
     @Test