SimonCockx
commented
1 year ago Repro: https://github.com/SimonCockx/xtext-assertAllWhitespaceIsFormatted/tree/conditionalreplacer-bug
(on branch conditionalreplacer-bug)
The language features expressions that are preceded by Expression:, e.g.,
Expression:
if True then True exists
Expression:
TrueGrammar:
Model:
('Expression' ':'
exprs+=Expression)*
;
Expression:
ExistsOperation
;
ExistsOperation returns Expression:
Primary =>({ExistsOperation.argument=current} 'exists')*
;
Primary returns Expression:
{BooleanLiteral} 'True'
| {ConditionalExpression} 'if' condition=Expression 'then' then=Expression
;Formatting does the following:
- The whole model is indented once. (this is a good illustration of the issue. Because of a leftover
indentDec, this rule will be broken) - Each expression section starts on a new line.
- Each expression starts on a new line and is indented.
- If an
if-thenexpression is too long, put thethenbranch on a new line. - If an
existsoperation is too long, put the operation on a new line and indent.
Formatter:
class MyDslFormatter extends AbstractFormatter2 {
def dispatch void format(Model model, extension IFormattableDocument document) {
model.surround[indent]
model.regionFor.keywords('Expression').forEach[
prepend[newLine]
append[noSpace]
]
model.regionFor.keywords(':').forEach[
append[newLine]
]
model.exprs.forEach[
surround[indent]
format
]
}
def dispatch void format(ExistsOperation existsOperation, extension IFormattableDocument document) {
val existsRegion = existsOperation.regionForEObject
val existsRegionAndNext = existsRegion.merge(existsRegion.nextHiddenRegion)
formatConditionally(existsRegionAndNext.offset, existsRegionAndNext.length,
[doc | // case: long expression
val extension singleLineDoc = doc.requireFitsInLine
existsOperation.regionFor.keyword('exists')
.prepend[oneSpace]
existsOperation.argument
.format
],
[extension doc | // case: short expression
existsOperation.regionFor.keyword('exists')
.prepend[newLine]
set(
existsOperation.argument.nextHiddenRegion,
existsOperation.nextHiddenRegion,
[indent]
)
existsOperation.argument
.format
]
)
}
def dispatch void format(ConditionalExpression conditional, extension IFormattableDocument document) {
conditional.regionFor.keywords('if', 'then').forEach[
append[oneSpace]
]
val conditionalRegion = conditional.regionForEObject
val conditionalRegionAndNext = conditionalRegion.merge(conditionalRegion.nextHiddenRegion)
formatConditionally(conditionalRegionAndNext.offset, conditionalRegionAndNext.length,
[doc | // case: short conditional
val extension singleLineDoc = doc.requireFitsInLine
conditional.regionFor.keyword('then')
.prepend[oneSpace]
conditional.condition.format
conditional.then.format
],
[extension doc | // case: long conditional
conditional.regionFor.keyword('then')
.prepend[newLine]
conditional.condition.format
conditional.then.format
]
)
}
}As a test, I try to format the following code.
Expression: if True then True exists
Expression : TrueI would expect the result to be the following.
Expression:
if True
then True
exists
Expression:
TrueThe actual result is the following:
Expression:
if True
then True
exists
Expression:
TrueNote that a region without indentation should never exist, because I've indented the whole model once.
Failing test:
class FormattingTest {
@Inject
FormatterTestHelper helper;
@Test
def void testWhiteSpaceIsFormattedBug() {
helper.assertFormatted[
preferences[
put(FormatterPreferenceKeys.maxLineWidth, 10);
]
toBeFormatted = '''
Expression: if True then True exists
Expression : True
'''
expectation = '''
«""»
Expression:
if True
then True
exists
Expression:
True
'''
]
}
}Patching the HiddenRegionFormattingMerger fixes this issue.
cdietrich
szarnekow
I stumbled upon an issue where branches of
IFormattableDocument::formatConditionallyactually modify the text replacers of the parent document, leading to leftover formatting in the other branches.I managed to track down the code responsible for modifying the
IHiddenRegionFormatters to this method in the classHiddenRegionFormattingMerger:See the scary
TODOcomment.Removing those first lines solves my issue. As a workaround I'm now using a custom
IMerger<IHiddenRegionFormatting>.