Merge pull request #3600 from jlerbsc/master

Fix issue #3588 Modifier is removed when removing an annotation
javaparser · Jun 2, 2022 · cc0e50a · cc0e50a
2 parents 06eb620 + ed79beb
commit cc0e50a
Show file tree

Hide file tree

Showing 6 changed files with 158 additions and 48 deletions.
diff --git a/.../java/com/github/javaparser/printer/lexicalpreservation/LexicalPreservingPrinterTest.java b/.../java/com/github/javaparser/printer/lexicalpreservation/LexicalPreservingPrinterTest.java
@@ -57,24 +57,7 @@
 import com.github.javaparser.ast.body.Parameter;
 import com.github.javaparser.ast.body.VariableDeclarator;
 import com.github.javaparser.ast.comments.LineComment;
-import com.github.javaparser.ast.expr.AnnotationExpr;
-import com.github.javaparser.ast.expr.ArrayCreationExpr;
-import com.github.javaparser.ast.expr.AssignExpr;
-import com.github.javaparser.ast.expr.BinaryExpr;
-import com.github.javaparser.ast.expr.BooleanLiteralExpr;
-import com.github.javaparser.ast.expr.CharLiteralExpr;
-import com.github.javaparser.ast.expr.DoubleLiteralExpr;
-import com.github.javaparser.ast.expr.Expression;
-import com.github.javaparser.ast.expr.FieldAccessExpr;
-import com.github.javaparser.ast.expr.IntegerLiteralExpr;
-import com.github.javaparser.ast.expr.LongLiteralExpr;
-import com.github.javaparser.ast.expr.MethodCallExpr;
-import com.github.javaparser.ast.expr.NameExpr;
-import com.github.javaparser.ast.expr.SimpleName;
-import com.github.javaparser.ast.expr.StringLiteralExpr;
-import com.github.javaparser.ast.expr.TextBlockLiteralExpr;
-import com.github.javaparser.ast.expr.ThisExpr;
-import com.github.javaparser.ast.expr.VariableDeclarationExpr;
+import com.github.javaparser.ast.expr.*;
 import com.github.javaparser.ast.stmt.BlockStmt;
 import com.github.javaparser.ast.stmt.CatchClause;
 import com.github.javaparser.ast.stmt.ExpressionStmt;
@@ -1729,5 +1712,49 @@ void testClassOrInterfacePreservationWithFullyQualifiedName_MultipleVariablesDec
         assertTransformedToString(expectedCode, cu);
 
     }
+
+    // issue 3588 Modifier is removed when removing an annotation. 
+    @Test
+    void testRemovingInlinedAnnotation() {
+        // Given
+        considerCode("public class Foo{\n"
+                + "     protected @Nullable Object bar;\n"
+                + "}");
+
+        // When
+        FieldDeclaration fd = cu.findFirst(FieldDeclaration.class).get();
+        // modification of the AST
+        AnnotationExpr ae = fd.getAnnotations().get(0);
+        ae.remove();
+
+        // Assert
+        String expectedCode =   "public class Foo{\n"
+                + "     protected Object bar;\n"
+                + "}";
+        assertTransformedToString(expectedCode, cu);
+
+    }
+
+    // issue 3588 Modifier is removed when removing an annotation. 
+    @Test
+    void testRemovingInlinedAnnotation_alternate_case() {
+        // Given
+        considerCode("public class Foo{\n"
+                + "     @Nullable protected Object bar;\n"
+                + "}");
+
+        // When
+        FieldDeclaration fd = cu.findFirst(FieldDeclaration.class).get();
+        // modification of the AST
+        AnnotationExpr ae = fd.getAnnotations().get(0);
+        ae.remove();
+
+        // Assert
+        String expectedCode =   "public class Foo{\n"
+                + "     protected Object bar;\n"
+                + "}";
+        assertTransformedToString(expectedCode, cu);
+
+    }
 
 }
diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/ConcreteSyntaxModel.java b/javaparser-core/src/main/java/com/github/javaparser/printer/ConcreteSyntaxModel.java
@@ -21,12 +21,37 @@
 
 package com.github.javaparser.printer;
 
+import static com.github.javaparser.GeneratedJavaParserConstants.*;
+import static com.github.javaparser.ast.observer.ObservableProperty.*;
+import static com.github.javaparser.printer.concretesyntaxmodel.CsmConditional.Condition.FLAG;
+import static com.github.javaparser.printer.concretesyntaxmodel.CsmConditional.Condition.IS_EMPTY;
+import static com.github.javaparser.printer.concretesyntaxmodel.CsmConditional.Condition.IS_NOT_EMPTY;
+import static com.github.javaparser.printer.concretesyntaxmodel.CsmConditional.Condition.IS_PRESENT;
+import static com.github.javaparser.printer.concretesyntaxmodel.CsmElement.*;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
 import com.github.javaparser.GeneratedJavaParserConstants;
-import com.github.javaparser.ast.*;
+import com.github.javaparser.ast.ArrayCreationLevel;
+import com.github.javaparser.ast.CompilationUnit;
+import com.github.javaparser.ast.ImportDeclaration;
+import com.github.javaparser.ast.Modifier;
+import com.github.javaparser.ast.Node;
+import com.github.javaparser.ast.PackageDeclaration;
 import com.github.javaparser.ast.body.*;
 import com.github.javaparser.ast.comments.Comment;
 import com.github.javaparser.ast.expr.*;
-import com.github.javaparser.ast.modules.*;
+import com.github.javaparser.ast.modules.ModuleDeclaration;
+import com.github.javaparser.ast.modules.ModuleExportsDirective;
+import com.github.javaparser.ast.modules.ModuleOpensDirective;
+import com.github.javaparser.ast.modules.ModuleProvidesDirective;
+import com.github.javaparser.ast.modules.ModuleRequiresDirective;
+import com.github.javaparser.ast.modules.ModuleUsesDirective;
 import com.github.javaparser.ast.observer.ObservableProperty;
 import com.github.javaparser.ast.stmt.*;
 import com.github.javaparser.ast.type.*;
@@ -35,14 +60,6 @@
 import com.github.javaparser.printer.concretesyntaxmodel.CsmElement;
 import com.github.javaparser.printer.concretesyntaxmodel.CsmMix;
 
-import java.util.*;
-import java.util.stream.Collectors;
-
-import static com.github.javaparser.GeneratedJavaParserConstants.*;
-import static com.github.javaparser.ast.observer.ObservableProperty.*;
-import static com.github.javaparser.printer.concretesyntaxmodel.CsmConditional.Condition.*;
-import static com.github.javaparser.printer.concretesyntaxmodel.CsmElement.*;
-
 /**
  * The Concrete Syntax Model for a single node type. It knows the syntax used to represent a certain element in Java
  * code.
@@ -222,8 +239,7 @@ private static CsmElement typeArguments() {
         concreteSyntaxModelByClass.put(FieldDeclaration.class, sequence(
                 orphanCommentsBeforeThis(),
                 comment(),
-                annotations(),
-                modifiers(),
+                mix(annotations(), modifiers()),
                 conditional(ObservableProperty.VARIABLES, IS_NOT_EMPTY, child(ObservableProperty.MAXIMUM_COMMON_TYPE)),
                 space(),
                 list(ObservableProperty.VARIABLES, sequence(comma(), space())),

diff --git a/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Added.java b/javaparser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Added.java
@@ -71,6 +71,8 @@ public boolean isRemoved() {
     public boolean isIndent() { return element instanceof CsmIndent; }
 
     public boolean isUnindent() { return element instanceof CsmUnindent; }
+
+    private boolean isToken() { return element instanceof CsmToken; }
 
     public TextElement toTextElement() {
         if (element instanceof LexicalDifferenceCalculator.CsmChild) {
@@ -81,4 +83,21 @@ public TextElement toTextElement() {
             throw new UnsupportedOperationException(element.getClass().getSimpleName());
         }
     }
+
+    /*
+     * If the {@code DifferenceElement} wraps an EOL token then this method returns a new wrapped {@code CsmElement}
+     * with the specified line separator. The line separator parameter must be a CsmToken with a valid line separator.
+     */
+    @Override
+    public DifferenceElement replaceEolTokens(CsmElement lineSeparator) {
+        return isNewLineToken() ? new Added(lineSeparator) : this;
+    }
+
+    /*
+     * Return true if the wrapped {@code CsmElement} is a new line token
+     */
+    private boolean isNewLineToken() {
+        return isToken() && ((CsmToken) element).isNewLine();
+    }
+
 }
diff --git a/...re/src/main/java/com/github/javaparser/printer/lexicalpreservation/DifferenceElement.java b/...re/src/main/java/com/github/javaparser/printer/lexicalpreservation/DifferenceElement.java
@@ -48,4 +48,13 @@ static DifferenceElement kept(CsmElement element) {
     default boolean isChild() { 
         return getElement() instanceof LexicalDifferenceCalculator.CsmChild; 
     }
+
+    /*
+     * If the {@code DifferenceElement} wraps an EOL token then this method returns a new wrapped {@code CsmElement}
+     * with the specified line separator. The line separator parameter must be a {@code CsmToken} with a valid line
+     * separator. By default this method returns the instance itself.
+     */
+    default DifferenceElement replaceEolTokens(CsmElement lineSeparator) {
+        return this;
+    }
 }
diff --git a/...n/java/com/github/javaparser/printer/lexicalpreservation/LexicalDifferenceCalculator.java b/...n/java/com/github/javaparser/printer/lexicalpreservation/LexicalDifferenceCalculator.java
@@ -21,6 +21,15 @@
 
 package com.github.javaparser.printer.lexicalpreservation;
 
+import static com.github.javaparser.TokenTypes.eolTokenKind;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Optional;
+
 import com.github.javaparser.GeneratedJavaParserConstants;
 import com.github.javaparser.ast.Modifier;
 import com.github.javaparser.ast.Node;
@@ -31,8 +40,8 @@
 import com.github.javaparser.ast.observer.ObservableProperty;
 import com.github.javaparser.ast.stmt.ExpressionStmt;
 import com.github.javaparser.printer.ConcreteSyntaxModel;
-import com.github.javaparser.printer.Stringable;
 import com.github.javaparser.printer.SourcePrinter;
+import com.github.javaparser.printer.Stringable;
 import com.github.javaparser.printer.concretesyntaxmodel.*;
 import com.github.javaparser.printer.lexicalpreservation.changes.Change;
 import com.github.javaparser.printer.lexicalpreservation.changes.ListAdditionChange;
@@ -42,15 +51,6 @@
 import com.github.javaparser.printer.lexicalpreservation.changes.PropertyChange;
 import com.github.javaparser.utils.LineSeparator;
 
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Optional;
-
-import static com.github.javaparser.TokenTypes.eolTokenKind;
-
 class LexicalDifferenceCalculator {
 
     /**
@@ -144,19 +144,24 @@ List<DifferenceElement> calculateListAdditionDifference(ObservableProperty obser
         return differenceElements;
     }
 
+    /*
+     * Replace EOL token in the list of {@code DifferenceElement} by the specified line separator
+     */
     private void replaceEolTokens(List<DifferenceElement> differenceElements, LineSeparator lineSeparator) {
+        CsmElement eol = getNewLineToken(lineSeparator);
         for (int i = 0; i < differenceElements.size(); i++) {
             DifferenceElement differenceElement = differenceElements.get(i);
-            if (differenceElement.isAdded()) {
-                CsmElement element = differenceElement.getElement();
-                boolean isWhitespaceToken = element instanceof CsmToken && ((CsmToken) element).isNewLine();
-                if (isWhitespaceToken) {
-                    differenceElements.set(i, new Added(CsmElement.newline(lineSeparator)));
-                }
-            }
+            differenceElements.set(i, differenceElement.replaceEolTokens(eol));
         }
     }
-
+
+    /*
+     * Returns a new line token 
+     */
+    private CsmElement getNewLineToken(LineSeparator lineSeparator) {
+        return CsmElement.newline(lineSeparator);
+    }
+
     List<DifferenceElement> calculateListReplacementDifference(ObservableProperty observableProperty, NodeList<?> nodeList, int index, Node newValue) {
         Node container = nodeList.getParentNodeForChildren();
         CsmElement element = ConcreteSyntaxModel.forClass(container.getClass());

diff --git a/...rser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Reshuffled.java b/...rser-core/src/main/java/com/github/javaparser/printer/lexicalpreservation/Reshuffled.java
@@ -21,7 +21,12 @@
 
 package com.github.javaparser.printer.lexicalpreservation;
 
+import java.util.List;
+import java.util.stream.Collectors;
+
+import com.github.javaparser.printer.concretesyntaxmodel.CsmElement;
 import com.github.javaparser.printer.concretesyntaxmodel.CsmMix;
+import com.github.javaparser.printer.concretesyntaxmodel.CsmToken;
 
 /**
  * Elements in a CsmMix have been reshuffled. It could also mean that
@@ -81,4 +86,33 @@ public boolean isAdded() {
     public boolean isRemoved() {
         return false;
     }
+
+    /*
+     * If the {@code DifferenceElement} wraps an EOL token then this method returns a new {@code DifferenceElement}
+     * with all eof token replaced by the specified line separator. The line separator parameter must be a CsmToken with a valid line separator.
+     */
+    @Override
+    public DifferenceElement replaceEolTokens(CsmElement lineSeparator) {
+        CsmMix modifiedNextOrder = new CsmMix(replaceTokens(nextOrder.getElements(), lineSeparator));
+        CsmMix modifiedPreviousOrder = new CsmMix(replaceTokens(previousOrder.getElements(), lineSeparator));
+        return new Reshuffled(modifiedPreviousOrder, modifiedNextOrder);
+    }
+
+    /*
+     * Replaces all eol tokens in the list by the specified line separator token 
+     */
+    private List<CsmElement> replaceTokens(List<CsmElement> elements, CsmElement lineSeparator) {
+        return elements.stream()
+                .map(element -> isNewLineToken(element) ? lineSeparator : element)
+                .collect(Collectors.toList());
+    }
+
+    /*
+     * Return true if the wrapped {@code CsmElement} is a new line token
+     */
+    private boolean isNewLineToken(CsmElement element) {
+        return isToken(element) && ((CsmToken) element).isNewLine();
+    }
+
+    private boolean isToken(CsmElement element) { return element instanceof CsmToken; }
 }