From dfdceddc5cc4ff11b3605c4d88b787c4c591cf1f Mon Sep 17 00:00:00 2001 From: Tamas Miklossy Date: Wed, 29 Apr 2020 09:16:14 +0200 Subject: [PATCH] [https://github.com/eclipse/xtext-eclipse/issues/1221] Serializer. - Improve the serializeReplacement implementation by modifying the ReplaceRegion length calculation so that it takes not only the ICompositeNode.getTotalLength() into account, but considers if the new text contains additional whitespaces and the old text is also followed by white spaces, than the whitespaces contained by the original document is also consumed by the quickfix. - Implement corresponding SerializerReplacementCalculationTest test case based on the NoJdtTestLanguage. Signed-off-by: Tamas Miklossy --- .../NoJdtTestLanguageFormatter.xtend | 7 ++- .../NoJdtTestLanguageFormatter.java | 34 +++++++---- .../SerializerReplacementCalculationTest.java | 46 ++++++++++++++ .../xtext/serializer/impl/Serializer.java | 60 ++++++++++++++++++- 4 files changed, 131 insertions(+), 16 deletions(-) create mode 100644 org.eclipse.xtext.tests/src/org/eclipse/xtext/serializer/SerializerReplacementCalculationTest.java diff --git a/org.eclipse.xtext.testlanguages/src/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.xtend b/org.eclipse.xtext.testlanguages/src/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.xtend index ecd2b712f..bbb9d34f3 100644 --- a/org.eclipse.xtext.testlanguages/src/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.xtend +++ b/org.eclipse.xtext.testlanguages/src/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.xtend @@ -11,11 +11,12 @@ import org.eclipse.xtext.testlanguages.noJdt.noJdt.Model class NoJdtTestLanguageFormatter extends AbstractFormatter2 { def dispatch void format(Model model, extension IFormattableDocument document) { - // TODO: format HiddenRegions around keywords, attributes, cross references, etc. for (Greeting greeting : model.getGreetings()) { greeting.format; } } - - // TODO: implement for + + def dispatch void format(Greeting greeting, extension IFormattableDocument document) { + greeting.append[setNewLines(1, 1, 2)] + } } diff --git a/org.eclipse.xtext.testlanguages/xtend-gen/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.java b/org.eclipse.xtext.testlanguages/xtend-gen/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.java index b73bd69cb..9f7c20acb 100644 --- a/org.eclipse.xtext.testlanguages/xtend-gen/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.java +++ b/org.eclipse.xtext.testlanguages/xtend-gen/org/eclipse/xtext/testlanguages/noJdt/formatting2/NoJdtTestLanguageFormatter.java @@ -8,10 +8,12 @@ import org.eclipse.emf.common.util.EList; import org.eclipse.emf.ecore.EObject; import org.eclipse.xtext.formatting2.AbstractFormatter2; import org.eclipse.xtext.formatting2.IFormattableDocument; +import org.eclipse.xtext.formatting2.IHiddenRegionFormatter; import org.eclipse.xtext.resource.XtextResource; import org.eclipse.xtext.testlanguages.noJdt.noJdt.Greeting; import org.eclipse.xtext.testlanguages.noJdt.noJdt.Model; import org.eclipse.xtext.xbase.lib.Extension; +import org.eclipse.xtext.xbase.lib.Procedures.Procedure1; @SuppressWarnings("all") public class NoJdtTestLanguageFormatter extends AbstractFormatter2 { @@ -22,25 +24,35 @@ public class NoJdtTestLanguageFormatter extends AbstractFormatter2 { } } - public void format(final Object model, final IFormattableDocument document) { - if (model instanceof XtextResource) { - _format((XtextResource)model, document); + protected void _format(final Greeting greeting, @Extension final IFormattableDocument document) { + final Procedure1 _function = (IHiddenRegionFormatter it) -> { + it.setNewLines(1, 1, 2); + }; + document.append(greeting, _function); + } + + public void format(final Object greeting, final IFormattableDocument document) { + if (greeting instanceof XtextResource) { + _format((XtextResource)greeting, document); return; - } else if (model instanceof Model) { - _format((Model)model, document); + } else if (greeting instanceof Greeting) { + _format((Greeting)greeting, document); return; - } else if (model instanceof EObject) { - _format((EObject)model, document); + } else if (greeting instanceof Model) { + _format((Model)greeting, document); return; - } else if (model == null) { + } else if (greeting instanceof EObject) { + _format((EObject)greeting, document); + return; + } else if (greeting == null) { _format((Void)null, document); return; - } else if (model != null) { - _format(model, document); + } else if (greeting != null) { + _format(greeting, document); return; } else { throw new IllegalArgumentException("Unhandled parameter types: " + - Arrays.asList(model, document).toString()); + Arrays.asList(greeting, document).toString()); } } } diff --git a/org.eclipse.xtext.tests/src/org/eclipse/xtext/serializer/SerializerReplacementCalculationTest.java b/org.eclipse.xtext.tests/src/org/eclipse/xtext/serializer/SerializerReplacementCalculationTest.java new file mode 100644 index 000000000..9fb90e4ad --- /dev/null +++ b/org.eclipse.xtext.tests/src/org/eclipse/xtext/serializer/SerializerReplacementCalculationTest.java @@ -0,0 +1,46 @@ +/******************************************************************************* + * Copyright (c) 2020 itemis AG (http://www.itemis.eu) and others. + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *******************************************************************************/ +package org.eclipse.xtext.serializer; + +import org.eclipse.xtext.resource.SaveOptions; +import org.eclipse.xtext.testlanguages.noJdt.NoJdtTestLanguageStandaloneSetup; +import org.eclipse.xtext.testlanguages.noJdt.noJdt.Greeting; +import org.eclipse.xtext.testlanguages.noJdt.noJdt.Model; +import org.eclipse.xtext.tests.AbstractXtextTests; +import org.eclipse.xtext.util.ReplaceRegion; +import org.junit.Test; + +/** + * @author miklossy - Initial contribution and API + */ +public class SerializerReplacementCalculationTest extends AbstractXtextTests { + + @Override + public void setUp() throws Exception { + super.setUp(); + with(NoJdtTestLanguageStandaloneSetup.class); + } + + @Test + public void testSerializeReplacement001() throws Exception { + // Given + String textModel = "Hello Xtext!" + System.lineSeparator(); + StringBuilder stringBuilder = new StringBuilder(textModel); + Model model = (Model) getModel(textModel); + Greeting greeting = model.getGreetings().get(0); + ReplaceRegion replacement = getSerializer().serializeReplacement(greeting, SaveOptions.defaultOptions()); + + // When + replacement.applyTo(stringBuilder); + + // Then + assertEquals(textModel, stringBuilder.toString()); + } + +} diff --git a/org.eclipse.xtext/src/org/eclipse/xtext/serializer/impl/Serializer.java b/org.eclipse.xtext/src/org/eclipse/xtext/serializer/impl/Serializer.java index 58ed9db01..309e2a9eb 100644 --- a/org.eclipse.xtext/src/org/eclipse/xtext/serializer/impl/Serializer.java +++ b/org.eclipse.xtext/src/org/eclipse/xtext/serializer/impl/Serializer.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2011, 2017 itemis AG (http://www.itemis.eu) and others. + * Copyright (c) 2011, 2020 itemis AG (http://www.itemis.eu) and others. * This program and the accompanying materials are made available under the * terms of the Eclipse Public License 2.0 which is available at * http://www.eclipse.org/legal/epl-2.0. @@ -23,6 +23,8 @@ import org.eclipse.xtext.formatting2.regionaccess.ITextRegionAccess; import org.eclipse.xtext.formatting2.regionaccess.ITextReplacement; import org.eclipse.xtext.formatting2.regionaccess.TextRegionAccessBuilder; import org.eclipse.xtext.nodemodel.ICompositeNode; +import org.eclipse.xtext.nodemodel.ILeafNode; +import org.eclipse.xtext.nodemodel.INode; import org.eclipse.xtext.nodemodel.util.NodeModelUtils; import org.eclipse.xtext.parsetree.reconstr.ITokenStream; import org.eclipse.xtext.parsetree.reconstr.impl.TokenStringBuffer; @@ -43,6 +45,7 @@ import org.eclipse.xtext.util.EmfFormatter; import org.eclipse.xtext.util.ReplaceRegion; import org.eclipse.xtext.validation.IConcreteSyntaxValidator; +import com.google.common.base.CharMatcher; import com.google.inject.Inject; import com.google.inject.Provider; @@ -210,7 +213,60 @@ public class Serializer implements ISerializer { throw new IllegalStateException("Cannot replace an obj that has no associated node"); } String text = serialize(obj, options); - return new ReplaceRegion(node.getTotalOffset(), node.getTotalLength(), text); + int replaceRegionLength = calculateReplaceRegionLength(node, text); + return new ReplaceRegion(node.getTotalOffset(), replaceRegionLength, text); } + /** + * @since 2.22 + */ + protected int calculateReplaceRegionLength(ICompositeNode node, String text) { + int oldTextLength = node.getTotalLength(); + int newTextLength = text.length(); + + if (newTextLength > oldTextLength) { + String remainingText = text.substring(oldTextLength); + if (isWhitespace(remainingText) && hiddenNodeFollows(node)) { + return newTextLength; + } + } + return oldTextLength; + } + + /** + * @since 2.22 + */ + protected boolean hiddenNodeFollows(ICompositeNode node) { + INode followingNode = getFollowingNode(node); + if (followingNode instanceof ILeafNode) { + return ((ILeafNode)followingNode).isHidden(); + } + return false; + } + + /** + * Returns the node that follows the node, independently, if they have the same parent. + * @since 2.22 + */ + protected INode getFollowingNode(ICompositeNode node) { + if (node != null) { + if (node.hasNextSibling()) { + INode nextSibling = node.getNextSibling(); + Iterator nextSiblingLeafNodes = nextSibling.getLeafNodes().iterator(); + if (nextSiblingLeafNodes.hasNext()) { + return nextSiblingLeafNodes.next(); + } else { + return getFollowingNode(node.getParent()); + } + } + } + return null; + } + + /** + * @since 2.22 + */ + protected boolean isWhitespace(String text) { + return CharMatcher.whitespace().matchesAllOf(text); + } }