From e9194d2ab97a56923d7129d8dbf8ba97cc82974c Mon Sep 17 00:00:00 2001
From: Thomas Heilbronner <thomas.heilbronner@gmail.com>
Date: Tue, 25 Dec 2018 22:35:35 +0100
Subject: [PATCH] fix the remove modifier implementation

The current one throws a ConcurrentModificationException if the modifier
that should be removed exists on the node
---
 .../ast/nodeTypes/NodeWithModifiersTest.java  | 32 +++++++++++++++++++
 .../ast/nodeTypes/NodeWithModifiers.java      | 17 +++++-----
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/javaparser-core-testing/src/test/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiersTest.java b/javaparser-core-testing/src/test/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiersTest.java
index bc5c1f2a2f..a50bbedeeb 100644
--- a/javaparser-core-testing/src/test/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiersTest.java
+++ b/javaparser-core-testing/src/test/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiersTest.java
@@ -21,6 +21,7 @@
 
 package com.github.javaparser.ast.nodeTypes;
 
+import com.github.javaparser.ast.Modifier;
 import com.github.javaparser.ast.Node;
 import com.github.javaparser.ast.NodeList;
 import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
@@ -31,7 +32,10 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import static com.github.javaparser.ast.Modifier.Keyword.PRIVATE;
 import static com.github.javaparser.ast.Modifier.Keyword.PUBLIC;
+import static com.github.javaparser.ast.Modifier.Keyword.STATIC;
+import static com.github.javaparser.ast.Modifier.Keyword.SYNCHRONIZED;
 import static com.github.javaparser.ast.Modifier.createModifierList;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -61,4 +65,32 @@ public void propertyChange(Node observedNode, ObservableProperty property, Objec
         assertEquals("property MODIFIERS is changed to [public ]", changes.get(0));
     }
 
+    @Test
+    void removeExistingModifier() {
+        NodeWithModifiers node = anythingWithModifiers(PUBLIC);
+        node.removeModifier(PUBLIC);
+        assertEquals(0, node.getModifiers().size());
+    }
+
+    @Test
+    void ignoreNotExistingModifiersOnRemove() {
+        NodeWithModifiers node = anythingWithModifiers(PUBLIC);
+        node.removeModifier(PRIVATE);
+
+        assertEquals(createModifierList(PUBLIC), node.getModifiers());
+    }
+
+    @Test
+    void keepModifiersThatShouldNotBeRemoved() {
+        NodeWithModifiers node = anythingWithModifiers(PUBLIC, STATIC, SYNCHRONIZED);
+        node.removeModifier(PUBLIC, PRIVATE, STATIC);
+
+        assertEquals(createModifierList(SYNCHRONIZED), node.getModifiers());
+    }
+
+    private NodeWithModifiers anythingWithModifiers(Modifier.Keyword ... keywords) {
+        ClassOrInterfaceDeclaration foo = new ClassOrInterfaceDeclaration(new NodeList<>(), false, "Foo");
+        foo.addModifier(keywords);
+        return foo;
+    }
 }
\ No newline at end of file
diff --git a/javaparser-core/src/main/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiers.java b/javaparser-core/src/main/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiers.java
index 39834a6d8d..957b7d13c8 100644
--- a/javaparser-core/src/main/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiers.java
+++ b/javaparser-core/src/main/java/com/github/javaparser/ast/nodeTypes/NodeWithModifiers.java
@@ -26,6 +26,9 @@
 import com.github.javaparser.ast.NodeList;
 
 import java.util.Arrays;
+import java.util.List;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import static com.github.javaparser.ast.NodeList.toNodeList;
 
@@ -61,15 +64,11 @@ default N addModifier(Modifier.Keyword... newModifiers) {
 
     @SuppressWarnings("unchecked")
     default N removeModifier(Modifier.Keyword... modifiersToRemove) {
-        NodeList<Modifier> existingModifiers = new NodeList<>(getModifiers());
-        for (Modifier.Keyword modifierToRemove : modifiersToRemove) {
-            for (Modifier existingModifier : existingModifiers) {
-                if (existingModifier.getKeyword() == modifierToRemove) {
-                    existingModifiers.remove(existingModifier);
-                }
-            }
-        }
-        setModifiers(existingModifiers);
+        List<Modifier.Keyword> modifiersToRemoveAsList = Arrays.asList(modifiersToRemove);
+        NodeList<Modifier> remaining = getModifiers().stream()
+                .filter(existingModifier -> !modifiersToRemoveAsList.contains(existingModifier.getKeyword()))
+                .collect(toNodeList());
+        setModifiers(remaining);
         return (N) this;
     }