From 097a8a1dd3ca6c16511df3f645f87f64a2caf9f6 Mon Sep 17 00:00:00 2001 From: Moritz Eysholdt Date: Thu, 16 Apr 2015 16:33:26 +0200 Subject: [PATCH] [formatter] format leftover undefined hidden regions Signed-off-by: Moritz Eysholdt --- .../junit4/formatter/FormatterTester.java | 35 +--------- .../xtext/formatting2/AbstractFormatter2.java | 53 ++++++++++++--- .../internal/HiddenRegionReplacer.java | 45 +++--------- .../regionaccess/IHiddenRegion.java | 16 ++++- .../internal/AbstractHiddenRegion.java | 46 ++++++++++++- .../regionaccess/internal/TextRegions.java | 68 +++++++++++++++++++ 6 files changed, 183 insertions(+), 80 deletions(-) create mode 100644 plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/TextRegions.java diff --git a/plugins/org.eclipse.xtext.junit4/src/org/eclipse/xtext/junit4/formatter/FormatterTester.java b/plugins/org.eclipse.xtext.junit4/src/org/eclipse/xtext/junit4/formatter/FormatterTester.java index d19f6d366..f922c2a35 100644 --- a/plugins/org.eclipse.xtext.junit4/src/org/eclipse/xtext/junit4/formatter/FormatterTester.java +++ b/plugins/org.eclipse.xtext.junit4/src/org/eclipse/xtext/junit4/formatter/FormatterTester.java @@ -10,7 +10,6 @@ package org.eclipse.xtext.junit4.formatter; import static com.google.common.base.Preconditions.*; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Set; @@ -20,12 +19,11 @@ import org.eclipse.xtext.formatting2.IFormatter2; import org.eclipse.xtext.formatting2.debug.TextRegionAccessToString; import org.eclipse.xtext.formatting2.debug.TextRegionsToString; import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegion; -import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegionPart; import org.eclipse.xtext.formatting2.regionaccess.ITextRegionAccess; import org.eclipse.xtext.formatting2.regionaccess.ITextReplacement; import org.eclipse.xtext.formatting2.regionaccess.ITextSegment; -import org.eclipse.xtext.formatting2.regionaccess.IWhitespace; import org.eclipse.xtext.formatting2.regionaccess.TextRegionAccessBuilder; +import org.eclipse.xtext.formatting2.regionaccess.internal.TextRegions; import org.eclipse.xtext.junit4.util.ParseHelper; import org.eclipse.xtext.nodemodel.INode; import org.eclipse.xtext.nodemodel.SyntaxErrorMessage; @@ -66,40 +64,13 @@ public class FormatterTester { private Serializer serializer; protected void assertAllWhitespaceIsFormatted(ITextRegionAccess access, List replacements) { - List actual = Lists.newArrayList(replacements); - Collections.sort(actual); List expected = Lists.newArrayList(); IHiddenRegion current = access.regionForRootEObject().getPreviousHiddenRegion(); while (current != null) { - if (current.getLength() == 0) { - expected.add(current); - } else { - for (IHiddenRegionPart part : current.getParts()) - if (part instanceof IWhitespace) - expected.add(part); - } + expected.addAll(current.getMergedSpaces()); current = current.getNextHiddenRegion(); } - int e = 0, a = 0; - List missing = Lists.newArrayList(); - while (e < expected.size() && a < actual.size()) { - ITextSegment hidden = expected.get(e); - ITextReplacement replacement = actual.get(a); - int compareTo = hidden.compareTo(replacement); - if (compareTo == 0) { - e++; - a++; - } else if (compareTo < 1) { - missing.add(hidden); - e++; - } else { - a++; - } - } - while (e < expected.size()) { - missing.add(expected.get(e)); - e++; - } + List missing = TextRegions.difference(expected, replacements); if (!missing.isEmpty()) { TextRegionsToString toString = new TextRegionsToString().setTextRegionAccess(access); for (ITextSegment region : missing) diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/AbstractFormatter2.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/AbstractFormatter2.java index 3b3da97d0..a35badc3b 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/AbstractFormatter2.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/AbstractFormatter2.java @@ -30,10 +30,12 @@ import org.eclipse.xtext.formatting2.internal.TextReplacerMerger; import org.eclipse.xtext.formatting2.internal.WhitespaceReplacer; import org.eclipse.xtext.formatting2.regionaccess.IComment; import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegion; +import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegionPart; import org.eclipse.xtext.formatting2.regionaccess.ISemanticRegion; import org.eclipse.xtext.formatting2.regionaccess.ITextRegionAccess; import org.eclipse.xtext.formatting2.regionaccess.ITextReplacement; import org.eclipse.xtext.formatting2.regionaccess.ITextSegment; +import org.eclipse.xtext.formatting2.regionaccess.internal.TextRegions; import org.eclipse.xtext.formatting2.regionaccess.internal.TextReplacement; import org.eclipse.xtext.grammaranalysis.impl.GrammarElementTitleSwitch; import org.eclipse.xtext.preferences.ITypedPreferenceValues; @@ -41,6 +43,8 @@ import org.eclipse.xtext.preferences.TypedPreferenceKey; import org.eclipse.xtext.resource.XtextResource; import org.eclipse.xtext.xbase.lib.Extension; +import com.google.common.collect.Lists; + /** *

* This is an abstract base class for language-specific formatters. @@ -204,12 +208,12 @@ public abstract class AbstractFormatter2 implements IFormatter2 { throw new IllegalStateException("No " + ITextReplacer.class.getSimpleName() + " configured for " + elementName); } - public IFormattableSubDocument createFormattableSubDocument(ITextSegment region, IFormattableDocument parent) { - return new SubDocument(region, parent); + public IFormattableDocument createFormattableRootDocument() { + return new RootDocument(this); } - public IHiddenRegionFormatting createHiddenRegionFormatting() { - return new HiddenRegionFormatting(this); + public IFormattableSubDocument createFormattableSubDocument(ITextSegment region, IFormattableDocument parent) { + return new SubDocument(region, parent); } public IHiddenRegionFormatter createHiddenRegionFormatter(IHiddenRegionFormatting formatting) { @@ -220,6 +224,10 @@ public abstract class AbstractFormatter2 implements IFormatter2 { return new DoubleHiddenRegionFormatter(f1, f2); } + public IHiddenRegionFormatting createHiddenRegionFormatting() { + return new HiddenRegionFormatting(this); + } + public IMerger createHiddenRegionFormattingMerger() { return new HiddenRegionFormattingMerger(this); } @@ -240,10 +248,6 @@ public abstract class AbstractFormatter2 implements IFormatter2 { return new WhitespaceReplacer(hiddens, formatting); } - public IFormattableDocument createFormattableRootDocument() { - return new RootDocument(this); - } - @Override public final List format(FormatterRequest request) { try { @@ -251,8 +255,9 @@ public abstract class AbstractFormatter2 implements IFormatter2 { IFormattableDocument document = createFormattableRootDocument(); XtextResource xtextResource = request.getTextRegionAccess().getResource(); format(xtextResource, document); - List replacements = document.renderToTextReplacements(); - return replacements; + List rendered = document.renderToTextReplacements(); + List postprocessed = postProcess(document, rendered); + return postprocessed; } finally { reset(); } @@ -300,6 +305,34 @@ public abstract class AbstractFormatter2 implements IFormatter2 { this.regionAccess = request.getTextRegionAccess(); } + protected List postProcess(IFormattableDocument document, List replacements) { + List expected = Lists.newArrayList(); + IHiddenRegion current = regionAccess.regionForRootEObject().getPreviousHiddenRegion(); + while (current != null) { + if (current.isUndefined()) + expected.addAll(current.getMergedSpaces()); + current = current.getNextHiddenRegion(); + } + if (expected.isEmpty()) + return replacements; + List missing = TextRegions.difference(expected, replacements); + if (missing.isEmpty()) + return replacements; + List result = Lists.newArrayList(replacements); + for (ITextSegment seg : missing) { + IHiddenRegion h = null; + if (seg instanceof IHiddenRegion) + h = (IHiddenRegion) seg; + if (seg instanceof IHiddenRegionPart) + h = ((IHiddenRegionPart) seg).getHiddenRegion(); + if (h != null && (h.getNextSemanticRegion() == null || h.getPreviousSemanticRegion() == null)) + result.add(seg.replaceWith("")); + else + result.add(seg.replaceWith(" ")); + } + return result; + } + /** * Overwrite this method to clean up member fields after {@link #format(Object, IFormattableDocument)} has been * called. diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/internal/HiddenRegionReplacer.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/internal/HiddenRegionReplacer.java index 4fd36548f..836918969 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/internal/HiddenRegionReplacer.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/internal/HiddenRegionReplacer.java @@ -19,7 +19,6 @@ import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegion; import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegionPart; import org.eclipse.xtext.formatting2.regionaccess.ITextSegment; import org.eclipse.xtext.formatting2.regionaccess.IWhitespace; -import org.eclipse.xtext.formatting2.regionaccess.internal.TextSegment; import com.google.common.collect.Lists; @@ -91,12 +90,12 @@ public class HiddenRegionReplacer implements ITextReplacer { public ITextReplacerContext createReplacements(ITextReplacerContext context) { AbstractFormatter2 formatter = context.getFormatter(); List hiddens = region.getParts(); - if (region.isUndefined() || hiddens.isEmpty()) { + if (hiddens.isEmpty()) { return formatter.createWhitespaceReplacer(region, formatting).createReplacements(context); } else if ((hiddens.size() == 1 && hiddens.get(0) instanceof IWhitespace)) { return formatter.createWhitespaceReplacer(hiddens.get(0), formatting).createReplacements(context); } else { - List replacers = createReplacers(formatter, hiddens); + List replacers = createReplacers(formatter); applyHiddenRegionFormatting(replacers); ITextReplacerContext current = context; current.setNextReplacerIsChild(); @@ -106,38 +105,16 @@ public class HiddenRegionReplacer implements ITextReplacer { } } - /** - * returns a list that starts with whitespace, ends with whitespace and contains a sequence of strictly alternating - * whitespace- and comment-regions. - */ - protected List createReplacers(AbstractFormatter2 formatter, List parts) { - ITextSegment last = null; - List result = Lists.newArrayList(); - for (IHiddenRegionPart part : parts) { - if (part instanceof IWhitespace) { - if (last == null || last instanceof IComment) { - result.add(formatter.createWhitespaceReplacer(part, formatter.createHiddenRegionFormatting())); - } else { - int mergedLength = last.getLength() + part.getLength(); - TextSegment merged = new TextSegment(part.getTextRegionAccess(), last.getOffset(), mergedLength); - IHiddenRegionFormatting formatting2 = formatter.createHiddenRegionFormatting(); - result.set(result.size() - 1, formatter.createWhitespaceReplacer(merged, formatting2)); - } - } - if (part instanceof IComment) { - if (last == null || last instanceof IComment) { - TextSegment region = new TextSegment(part.getTextRegionAccess(), part.getOffset(), 0); - result.add(formatter.createWhitespaceReplacer(region, formatter.createHiddenRegionFormatting())); - } - result.add(formatter.createCommentReplacer((IComment) part)); - } - last = part; + protected List createReplacers(AbstractFormatter2 formatter) { + List regions = region.getAlternatingMergedSpaceAndComments(); + List replacers = Lists.newArrayListWithCapacity(regions.size()); + for (ITextSegment region : regions) { + if (region instanceof IComment) + replacers.add(formatter.createCommentReplacer((IComment) region)); + else + replacers.add(formatter.createWhitespaceReplacer(region, formatter.createHiddenRegionFormatting())); } - if (last instanceof IComment) { - TextSegment region = new TextSegment(last.getTextRegionAccess(), last.getOffset() + last.getLength(), 0); - result.add(formatter.createWhitespaceReplacer(region, formatter.createHiddenRegionFormatting())); - } - return result; + return replacers; } protected WhitespaceReplacer findWhitespaceThatSeparatesSemanticRegions(List replacers) { diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/IHiddenRegion.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/IHiddenRegion.java index 9c0acd02e..7919a1561 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/IHiddenRegion.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/IHiddenRegion.java @@ -10,8 +10,9 @@ package org.eclipse.xtext.formatting2.regionaccess; import java.util.List; /** - *

Represents and groups all {@link IWhitespace} and {@link IComment comments} between two {@link ISemanticRegion semantic regions}. May be - * empty. + *

+ * Represents and groups all {@link IWhitespace} and {@link IComment comments} between two {@link ISemanticRegion + * semantic regions}. May be empty. * * @author Moritz Eysholdt - Initial contribution and API * @@ -30,7 +31,8 @@ public interface IHiddenRegion extends ISequentialRegion { boolean containsComment(); /** - * @return all {@link IWhitespace white spaces} and {@link IComment comments} that belong to this {@link IHiddenRegion}. + * @return all {@link IWhitespace white spaces} and {@link IComment comments} that belong to this + * {@link IHiddenRegion}. */ List getParts(); @@ -40,4 +42,12 @@ public interface IHiddenRegion extends ISequentialRegion { * semantic model has been constructed or modified programmatically. */ boolean isUndefined(); + + /** + * @return returns a list that starts with whitespace, ends with whitespace and contains a sequence of strictly + * alternating whitespace- and comment-regions. + */ + List getAlternatingMergedSpaceAndComments(); + + List getMergedSpaces(); } \ No newline at end of file diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/AbstractHiddenRegion.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/AbstractHiddenRegion.java index b7adeb82e..a0f76e8c4 100644 --- a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/AbstractHiddenRegion.java +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/AbstractHiddenRegion.java @@ -7,6 +7,7 @@ *******************************************************************************/ package org.eclipse.xtext.formatting2.regionaccess.internal; +import java.util.Collections; import java.util.List; import org.eclipse.xtext.formatting2.debug.TextRegionAccessToString; @@ -15,6 +16,8 @@ import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegion; import org.eclipse.xtext.formatting2.regionaccess.IHiddenRegionPart; import org.eclipse.xtext.formatting2.regionaccess.ISemanticRegion; import org.eclipse.xtext.formatting2.regionaccess.ITextRegionAccess; +import org.eclipse.xtext.formatting2.regionaccess.ITextSegment; +import org.eclipse.xtext.formatting2.regionaccess.IWhitespace; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -23,10 +26,10 @@ import com.google.common.collect.Lists; * @author Moritz Eysholdt - Initial contribution and API */ public abstract class AbstractHiddenRegion extends AbstractTextSegment implements IHiddenRegion { + private final ITextRegionAccess access; private final List hiddens = Lists.newArrayList(); private ISemanticRegion next; private ISemanticRegion previous; - private final ITextRegionAccess access; protected AbstractHiddenRegion(ITextRegionAccess access) { super(); @@ -37,6 +40,37 @@ public abstract class AbstractHiddenRegion extends AbstractTextSegment implement this.hiddens.add(part); } + protected List collectAlternatingSpaceAndComments(boolean includeComments) { + List parts = getParts(); + if (parts.isEmpty()) { + return Collections. singletonList(this); + } else { + ITextSegment last = null; + List result = Lists.newArrayList(); + for (IHiddenRegionPart part : parts) { + if (part instanceof IWhitespace) { + if (last == null || last instanceof IComment) { + result.add(part); + } else { + int mergedLength = last.getLength() + part.getLength(); + result.add(new TextSegment(part.getTextRegionAccess(), last.getOffset(), mergedLength)); + } + } else if (part instanceof IComment) { + if (last == null || last instanceof IComment) { + result.add(new TextSegment(part.getTextRegionAccess(), part.getOffset(), 0)); + } + if (includeComments) + result.add(part); + } + last = part; + } + if (last instanceof IComment) { + result.add(new TextSegment(last.getTextRegionAccess(), last.getOffset() + last.getLength(), 0)); + } + return ImmutableList.copyOf(result); + } + } + @Override public boolean containsComment() { for (IHiddenRegionPart hidden : hiddens) @@ -45,6 +79,11 @@ public abstract class AbstractHiddenRegion extends AbstractTextSegment implement return false; } + @Override + public List getAlternatingMergedSpaceAndComments() { + return collectAlternatingSpaceAndComments(true); + } + @Override public int getLength() { if (hiddens.isEmpty()) @@ -90,6 +129,11 @@ public abstract class AbstractHiddenRegion extends AbstractTextSegment implement return previous; } + @Override + public List getMergedSpaces() { + return collectAlternatingSpaceAndComments(false); + } + @Override public ITextRegionAccess getTextRegionAccess() { return access; diff --git a/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/TextRegions.java b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/TextRegions.java new file mode 100644 index 000000000..07e7211df --- /dev/null +++ b/plugins/org.eclipse.xtext/src/org/eclipse/xtext/formatting2/regionaccess/internal/TextRegions.java @@ -0,0 +1,68 @@ +/******************************************************************************* + * Copyright (c) 2015 itemis AG (http://www.itemis.eu) and others. + * All rights reserved. This program and the accompanying materials + * are made available under the terms of the Eclipse Public License v1.0 + * which accompanies this distribution, and is available at + * http://www.eclipse.org/legal/epl-v10.html + *******************************************************************************/ +package org.eclipse.xtext.formatting2.regionaccess.internal; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; + +import org.eclipse.xtext.util.ITextRegion; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; + +/** + * @author Moritz Eysholdt - Initial contribution and API + * @since 2.9 + */ +public class TextRegions { + + private static enum Comp implements Comparator { + INSTANCE; + + @Override + public int compare(ITextRegion o1, ITextRegion o2) { + int cmp1 = o1.getOffset() - o2.getOffset(); + if (cmp1 != 0) + return cmp1; + int cmp2 = o1.getLength() - o2.getLength(); + if (cmp2 != 0) + return cmp2; + return 0; + } + } + + public static List difference(Iterable r1, Iterable r2) { + ArrayList regions1 = Lists.newArrayList(r1); + ArrayList regions2 = Lists.newArrayList(r2); + Collections.sort(regions1, Comp.INSTANCE); + Collections.sort(regions2, Comp.INSTANCE); + List missing = Lists.newArrayList(); + int i1 = 0, i2 = 0; + while (i1 < regions1.size() && i2 < regions2.size()) { + T t1 = regions1.get(i1); + ITextRegion t2 = regions2.get(i2); + int compareTo = Comp.INSTANCE.compare(t1, t2); + if (compareTo == 0) { + i1++; + i2++; + } else if (compareTo < 1) { + missing.add(t1); + i1++; + } else { + i2++; + } + } + while (i1 < regions1.size()) { + missing.add(regions1.get(i1)); + i1++; + } + return ImmutableList.copyOf(missing); + } +}