LexicalPreservingPrinter: Unsupported operation - remove()

[ThLeu]

Not sure about this one, if it even is supposed to work like that, but I just put together some quick test of a specific use case I would like to use the LexicalPreservingPrinter for.

Basically "moving" some Annotations around by first removing them and adding them again.
The test I put together (copy of LexicalPreservingPrinterTest.handleOverrideAnnotation) fails with...

java.lang.UnsupportedOperationException: removed child(MarkerAnnotationExpr) vs TokenTextElement(73) {protected}

	at com.github.javaparser.printer.lexicalpreservation.Difference.apply(Difference.java:520)
	at com.github.javaparser.printer.lexicalpreservation.LexicalPreservingPrinter$1.concreteListChange(LexicalPreservingPrinter.java:164)
....

The test case

    @Test
    public void moveOverrideAnnotations() {
        String code = "public class TestPage extends Page {" + EOL +
                EOL +
                "   protected void test() {}" + EOL +
                EOL +
                "   protected @Override void initializePage() {}" + EOL +
                "}";

        Pair<ParseResult<CompilationUnit>, LexicalPreservingPrinter> result = LexicalPreservingPrinter
                .setup(ParseStart.COMPILATION_UNIT, Providers.provider(code));

        CompilationUnit cu = result.a.getResult().get();

        cu.getTypes().stream()
                .forEach(type -> {
                    type.getMembers().stream()
                            .forEach(member -> {
                                if (member instanceof MethodDeclaration) {
                                    MethodDeclaration methodDeclaration = (MethodDeclaration) member;
                                    if (methodDeclaration.getAnnotationByName("Override").isPresent()) {
                                        methodDeclaration.getAnnotations().stream()
                                                .forEach(anno -> anno.remove());

                                        methodDeclaration.addMarkerAnnotation("Override");
                                    }
                                }
                            });
                });
        assertEquals("public class TestPage extends Page {" + EOL +
                EOL +
                "   @Override" + EOL +
                "   protected void test() {}" + EOL +
                EOL +
                "   @Override" + EOL +
                "   protected void initializePage() {}" + EOL +
                "}", result.b.print(cu));
    }

Kind regards

[ftomassetti]

The first thing I noticed is that looking at the calculated different it would expect to find some annotation to delete before the protected keyword.

However it find the protected keyword in first position and not the annotation so it complains

[ftomassetti]

This happens because looking at the CSM it expects to find the annotation before the protected keyword, while it is after

This is something that would require special handling I think, because it is a case in which some elements can be in an unexpected order.

[ThLeu]

Yes the "unexpected order" to say it nicely with your words :) is kind of the problem with our legacy code (problably not only ours) that is why I would like to refactor some things mostly automatically but keep everyhting else in place, hence it would be nice to use the LexicalPreservingPrinter for this.

[ftomassetti]

I think we have two options to resolver this:

1. An ad-hoc solution for MethodDeclaration (and ConstructorDeclaration?)
2. A generalized solution to permite mixed elements in the CSM

The first one should be relatively easy. What do you think?

[matozoid]

I think modifiers can be found in many places (about ten,) so it should be fixed everywhere. I can't think of another place where things can be mixed like this, so it can be a solution specific to modifiers and annotations.

[ftomassetti]

Ok, a solution is almost ready. The only issue is now related to whitespace reconciliation: I get an extra whitespace... I introduced a new concept named CsmMix to handle CsmElement that can be in random order. It is quite tricky because we want to understand how the position of new elements is related to position of old elements to determine the right order.