Skip to content

Commit a041f8d

Browse files
naren2605naren2605
committed
#361 | adding warning messages on mutable method invocations on immutable collection
1 parent 78f4858 commit a041f8d

File tree

2 files changed

+257
-0
lines changed

2 files changed

+257
-0
lines changed

‎build.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
patches/nb-telemetry.diff
7575
patches/change-method-parameters-refactoring-qualified-names.diff
7676
patches/javavscode-375.diff
77+
patches/jvsce-361-draft.diff
7778
</string>
7879
<filterchain>
7980
<tokenfilter delimoutput=" ">

‎patches/jvsce-361-draft.diff

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
diff --git a/java/java.hints/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollections.java b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollections.java
2+
new file mode 100644
3+
index 0000000000..6c6b1881f5
4+
--- /dev/null
5+
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollections.java
6+
@@ -0,0 +1,132 @@
7+
+/*
8+
+ * Licensed to the Apache Software Foundation (ASF) under one
9+
+ * or more contributor license agreements. See the NOTICE file
10+
+ * distributed with this work for additional information
11+
+ * regarding copyright ownership. The ASF licenses this file
12+
+ * to you under the Apache License, Version 2.0 (the
13+
+ * "License"); you may not use this file except in compliance
14+
+ * with the License. You may obtain a copy of the License at
15+
+ *
16+
+ * http://www.apache.org/licenses/LICENSE-2.0
17+
+ *
18+
+ * Unless required by applicable law or agreed to in writing,
19+
+ * software distributed under the License is distributed on an
20+
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
21+
+ * KIND, either express or implied. See the License for the
22+
+ * specific language governing permissions and limitations
23+
+ * under the License.
24+
+ */
25+
+package org.netbeans.modules.java.hints.bugs;
26+
+
27+
+import com.sun.source.tree.CompilationUnitTree;
28+
+import com.sun.source.tree.MemberSelectTree;
29+
+import com.sun.source.tree.MethodInvocationTree;
30+
+import com.sun.source.tree.Tree;
31+
+import com.sun.source.util.TreePath;
32+
+import java.util.ArrayList;
33+
+import java.util.Collection;
34+
+import java.util.HashSet;
35+
+import java.util.List;
36+
+import java.util.Set;
37+
+import org.netbeans.api.java.source.CompilationInfo;
38+
+import org.netbeans.modules.java.hints.introduce.Flow;
39+
+import org.netbeans.modules.java.hints.introduce.Flow.FlowResult;
40+
+import org.netbeans.spi.editor.hints.ErrorDescription;
41+
+import org.netbeans.spi.editor.hints.Severity;
42+
+import org.netbeans.spi.java.hints.HintContext;
43+
+import org.netbeans.spi.java.hints.Hint;
44+
+import org.netbeans.spi.java.hints.Hint.Options;
45+
+import org.netbeans.spi.java.hints.TriggerPattern;
46+
+import org.netbeans.spi.java.hints.ErrorDescriptionFactory;
47+
+
48+
+/**
49+
+ *
50+
+ * @author nbalyam
51+
+ */
52+
+@Hint(displayName = "Track mutable methods on immutable collections",
53+
+ description = "Track mutable methods on immutable collections",
54+
+ category = "bugs",
55+
+ id = "ImmutableListCreation",
56+
+ severity = Severity.WARNING,
57+
+ options = Options.QUERY)
58+
+
59+
+public class MutableMethodsOnImmutableCollections {
60+
+
61+
+ private static final Set<String> MUTATING_METHODS_IN_LIST = Set.of(
62+
+ "add", "addAll", "remove", "removeAll", "clear", "set", "replaceAll", "sort"
63+
+ );
64+
+
65+
+ private static final Set<String> MUTATING_METHODS_IN_SET = Set.of(
66+
+ "add", "addAll", "remove", "removeAll", "retainAll", "clear"
67+
+ );
68+
+
69+
+ public static final String SUPPRESS_WARNING_KEY = "immutable-collection-mutation";
70+
+
71+
+ @TriggerPattern(value = "java.util.List.of($args$)")
72+
+ public static List<ErrorDescription> immutableList(HintContext ctx) {
73+
+ return checkForMutableMethodInvocations(ctx, MUTATING_METHODS_IN_LIST, "Attempting to modify an immutable List created via List.of()");
74+
+ }
75+
+
76+
+ @TriggerPattern(value = "java.util.Set.of($args$)")
77+
+ public static List<ErrorDescription> immutableSet(HintContext ctx) {
78+
+ return checkForMutableMethodInvocations(ctx, MUTATING_METHODS_IN_SET, "Attempting to modify an immutable Set created via Set.of()");
79+
+ }
80+
+
81+
+ private static List<ErrorDescription> checkForMutableMethodInvocations(HintContext ctx, Set<String> mutatingMethods, String warningMessage) {
82+
+ List<ErrorDescription> errors = new ArrayList<>();
83+
+
84+
+ for (MemberSelectTree mst : getInvocations(ctx.getInfo(), ctx.getPath().getLeaf(), () -> ctx.isCanceled())) {
85+
+ String method = mst.getIdentifier().toString();
86+
+ if (mutatingMethods.contains(method)) {
87+
+ errors.add(ErrorDescriptionFactory.forName(
88+
+ ctx,
89+
+ TreePath.getPath(ctx.getInfo().getCompilationUnit(), mst),
90+
+ warningMessage
91+
+ ));
92+
+
93+
+ }
94+
+ }
95+
+ return errors;
96+
+ }
97+
+
98+
+ private static Tree getMit(CompilationUnitTree cut, Tree patternTriggered) {
99+
+ if (patternTriggered instanceof MethodInvocationTree mit) {
100+
+ return TreePath.getPath(cut, mit).getLeaf();
101+
+ } else {
102+
+ return null;
103+
+ }
104+
+
105+
+ }
106+
+
107+
+ private static List<MemberSelectTree> getInvocations(CompilationInfo info, Tree patternTriggered, Flow.Cancel cancel) {
108+
+ var initializerMit = getMit(info.getCompilationUnit(), patternTriggered);
109+
+ if (initializerMit != null) {
110+
+ FlowResult flow = Flow.assignmentsForUse(info, cancel);
111+
+ return checkForUsagesAndMarkInvocations(info, flow, initializerMit);
112+
+ }
113+
+ return List.of();
114+
+ }
115+
+
116+
+ private static List<MemberSelectTree> checkForUsagesAndMarkInvocations(CompilationInfo info, FlowResult flow, Tree invokedMethodPattern) {
117+
+ List<MemberSelectTree> usedInvocationsWithIdentifier = new ArrayList<>();
118+
+ Collection<Tree> variablesToCheck = Set.of(invokedMethodPattern);
119+
+ do {
120+
+ Set<Tree> nextSetOfVariablesToCheck = new HashSet<>();
121+
+ for(var variable:variablesToCheck){
122+
+ markMethodInvocation(info, variable, usedInvocationsWithIdentifier);
123+
+ nextSetOfVariablesToCheck.addAll(flow.getValueUsers(variable));
124+
+ }
125+
+ variablesToCheck = nextSetOfVariablesToCheck;
126+
+ } while (!variablesToCheck.isEmpty());
127+
+ return usedInvocationsWithIdentifier;
128+
+ }
129+
+
130+
+ private static void markMethodInvocation(CompilationInfo info, Tree tree, List<MemberSelectTree> usedInvocationsWithIdentifier) {
131+
+ var ancestor = TreePath.getPath(info.getCompilationUnit(), tree).getParentPath().getParentPath().getLeaf();
132+
+ if (ancestor instanceof MethodInvocationTree mit && mit.getMethodSelect() instanceof MemberSelectTree mst) {
133+
+ usedInvocationsWithIdentifier.add(mst);
134+
+ }
135+
+ }
136+
+
137+
+
138+
+}
139+
diff --git a/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollectionsTest.java b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollectionsTest.java
140+
new file mode 100644
141+
index 0000000000..229b730615
142+
--- /dev/null
143+
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollectionsTest.java
144+
@@ -0,0 +1,111 @@
145+
+/*
146+
+ * Licensed to the Apache Software Foundation (ASF) under one
147+
+ * or more contributor license agreements. See the NOTICE file
148+
+ * distributed with this work for additional information
149+
+ * regarding copyright ownership. The ASF licenses this file
150+
+ * to you under the Apache License, Version 2.0 (the
151+
+ * "License"); you may not use this file except in compliance
152+
+ * with the License. You may obtain a copy of the License at
153+
+ *
154+
+ * http://www.apache.org/licenses/LICENSE-2.0
155+
+ *
156+
+ * Unless required by applicable law or agreed to in writing,
157+
+ * software distributed under the License is distributed on an
158+
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
159+
+ * KIND, either express or implied. See the License for the
160+
+ * specific language governing permissions and limitations
161+
+ * under the License.
162+
+ */
163+
+package org.netbeans.modules.java.hints.bugs;
164+
+
165+
+import org.netbeans.junit.NbTestCase;
166+
+import org.netbeans.modules.java.hints.test.api.HintTest;
167+
+
168+
+/**
169+
+ *
170+
+ * @author nbalyamm
171+
+ */
172+
+public class MutableMethodsOnImmutableCollectionsTest extends NbTestCase {
173+
+
174+
+ public MutableMethodsOnImmutableCollectionsTest(String name) {
175+
+ super(name);
176+
+ }
177+
+ public void testCaseWithMutlipleVariablesAndNoAssigmentChange() throws Exception{
178+
+
179+
+ HintTest
180+
+ .create()
181+
+ .input("""
182+
+ package test;
183+
+
184+
+ import java.util.*;
185+
+
186+
+ public class Test {
187+
+ private void test () {
188+
+ var l=List.of("foo","bar");
189+
+ var l2=List.of("fool2","barl2");
190+
+ l.add("bar2");
191+
+ l.clear();
192+
+ l2.clear();
193+
+ }
194+
+
195+
+ }
196+
+ """)
197+
+ .sourceLevel(10)
198+
+ .run(MutableMethodsOnImmutableCollections.class)
199+
+ .assertWarnings(
200+
+ "8:13-8:16:warning:Attempting to modify an immutable List created via List.of()",
201+
+ "9:13-9:18:warning:Attempting to modify an immutable List created via List.of()",
202+
+ "10:14-10:19:warning:Attempting to modify an immutable List created via List.of()");
203+
+ }
204+
+
205+
+ public void testCaseWithAssignmentChange() throws Exception{
206+
+
207+
+ HintTest
208+
+ .create()
209+
+ .input("""
210+
+ package test;
211+
+
212+
+ import java.util.*;
213+
+
214+
+ public class Test {
215+
+ private void test () {
216+
+ var l=List.of("foo","bar");
217+
+ var l2=List.of("foo2","bar2");
218+
+ l.add("bar2");
219+
+ l.clear();
220+
+ l2.clear();
221+
+ l2 = new ArrayList();
222+
+ l2.clear();
223+
+ l2 = List.of("foo3","bar3");
224+
+ l2.clear();
225+
+ l2 = l;
226+
+ l2.clear();
227+
+ if(true){
228+
+ l.clear();
229+
+ }
230+
+ List<String> l3 = new ArrayList<String>();
231+
+ l3 = l2;
232+
+ l3.clear();
233+
+ List<String> l4 = new ArrayList<String>();
234+
+ l4 = l3;
235+
+ l4.clear();
236+
+ var s1 = Set.of("sfoo1","sbar1");
237+
+ s1.clear();
238+
+ }
239+
+ }
240+
+ """)
241+
+ .sourceLevel(10)
242+
+ .run(MutableMethodsOnImmutableCollections.class)
243+
+ .assertWarnings(
244+
+ "8:13-8:16:warning:Attempting to modify an immutable List created via List.of()",
245+
+ "9:13-9:18:warning:Attempting to modify an immutable List created via List.of()",
246+
+ "10:14-10:19:warning:Attempting to modify an immutable List created via List.of()",
247+
+ "14:14-14:19:warning:Attempting to modify an immutable List created via List.of()",
248+
+ "16:14-16:19:warning:Attempting to modify an immutable List created via List.of()",
249+
+ "18:15-18:20:warning:Attempting to modify an immutable List created via List.of()",
250+
+ "22:14-22:19:warning:Attempting to modify an immutable List created via List.of()",
251+
+ "25:14-25:19:warning:Attempting to modify an immutable List created via List.of()",
252+
+ "27:14-27:19:warning:Attempting to modify an immutable Set created via Set.of()"
253+
+ );
254+
+ }
255+
+}
256+
\ No newline at end of file

0 commit comments

Comments
 (0)