Skip to content

Commit 552d872

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

File tree

2 files changed

+265
-0
lines changed

2 files changed

+265
-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: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
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..cd5275e628
4+
--- /dev/null
5+
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollections.java
6+
@@ -0,0 +1,140 @@
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.HashSet;
34+
+import java.util.List;
35+
+import java.util.Set;
36+
+import java.util.stream.Collectors;
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+
+ List<MemberSelectTree> usedInvocationsWithIdentifier = new ArrayList<>();
109+
+ var initializerMit = getMit(info.getCompilationUnit(), patternTriggered);
110+
+ if (initializerMit != null) {
111+
+ FlowResult flow = Flow.assignmentsForUse(info, cancel);
112+
+ var variablesAssginedToPattern = flow.getValueUsers(initializerMit).stream().collect(Collectors.toSet());
113+
+ for (var variableIdent : variablesAssginedToPattern) {
114+
+ var tree = TreePath.getPath(info.getCompilationUnit(), variableIdent).getParentPath().getParentPath().getLeaf();
115+
+ if (tree instanceof MethodInvocationTree mit && mit.getMethodSelect() instanceof MemberSelectTree mst) {
116+
+ usedInvocationsWithIdentifier.add(mst);
117+
+ }
118+
+
119+
+ }
120+
+ checkForAssignmentsAndMarkInvocations(info, flow, variablesAssginedToPattern, usedInvocationsWithIdentifier);
121+
+ }
122+
+ return usedInvocationsWithIdentifier;
123+
+ }
124+
+
125+
+ private static void checkForAssignmentsAndMarkInvocations(CompilationInfo info, FlowResult flow, Set<Tree> variablesAssginedToPattern, List<MemberSelectTree> usedInvocationsWithIdentifier) {
126+
+ Set<Tree> valuesAssignedTo = flow.getAssignmentsForUse().keySet();
127+
+ Set<Tree> variablesToCheck = variablesAssginedToPattern;
128+
+ do {
129+
+ Set<Tree> nextSetOfVariablesToCheck = new HashSet<>();
130+
+ for (var valueAssignedTo : valuesAssignedTo) {
131+
+ Iterable<? extends TreePath> valuesAssignedFrom = flow.getAssignmentsForUse().get(valueAssignedTo);
132+
+ for (var valueAssignedFrom : valuesAssignedFrom) {
133+
+ if (variablesToCheck.contains(valueAssignedFrom.getLeaf())) {
134+
+ var tree = TreePath.getPath(info.getCompilationUnit(), valueAssignedTo).getParentPath().getParentPath().getLeaf();
135+
+ if (tree instanceof MethodInvocationTree mit && mit.getMethodSelect() instanceof MemberSelectTree mst) {
136+
+ usedInvocationsWithIdentifier.add(mst);
137+
+ }
138+
+ nextSetOfVariablesToCheck.add(valueAssignedTo);
139+
+ }
140+
+ }
141+
+ }
142+
+ variablesToCheck = nextSetOfVariablesToCheck;
143+
+ } while (!variablesToCheck.isEmpty());
144+
+ }
145+
+
146+
+}
147+
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
148+
new file mode 100644
149+
index 0000000000..229b730615
150+
--- /dev/null
151+
+++ b/java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollectionsTest.java
152+
@@ -0,0 +1,111 @@
153+
+/*
154+
+ * Licensed to the Apache Software Foundation (ASF) under one
155+
+ * or more contributor license agreements. See the NOTICE file
156+
+ * distributed with this work for additional information
157+
+ * regarding copyright ownership. The ASF licenses this file
158+
+ * to you under the Apache License, Version 2.0 (the
159+
+ * "License"); you may not use this file except in compliance
160+
+ * with the License. You may obtain a copy of the License at
161+
+ *
162+
+ * http://www.apache.org/licenses/LICENSE-2.0
163+
+ *
164+
+ * Unless required by applicable law or agreed to in writing,
165+
+ * software distributed under the License is distributed on an
166+
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
167+
+ * KIND, either express or implied. See the License for the
168+
+ * specific language governing permissions and limitations
169+
+ * under the License.
170+
+ */
171+
+package org.netbeans.modules.java.hints.bugs;
172+
+
173+
+import org.netbeans.junit.NbTestCase;
174+
+import org.netbeans.modules.java.hints.test.api.HintTest;
175+
+
176+
+/**
177+
+ *
178+
+ * @author nbalyamm
179+
+ */
180+
+public class MutableMethodsOnImmutableCollectionsTest extends NbTestCase {
181+
+
182+
+ public MutableMethodsOnImmutableCollectionsTest(String name) {
183+
+ super(name);
184+
+ }
185+
+ public void testCaseWithMutlipleVariablesAndNoAssigmentChange() throws Exception{
186+
+
187+
+ HintTest
188+
+ .create()
189+
+ .input("""
190+
+ package test;
191+
+
192+
+ import java.util.*;
193+
+
194+
+ public class Test {
195+
+ private void test () {
196+
+ var l=List.of("foo","bar");
197+
+ var l2=List.of("fool2","barl2");
198+
+ l.add("bar2");
199+
+ l.clear();
200+
+ l2.clear();
201+
+ }
202+
+
203+
+ }
204+
+ """)
205+
+ .sourceLevel(10)
206+
+ .run(MutableMethodsOnImmutableCollections.class)
207+
+ .assertWarnings(
208+
+ "8:13-8:16:warning:Attempting to modify an immutable List created via List.of()",
209+
+ "9:13-9:18:warning:Attempting to modify an immutable List created via List.of()",
210+
+ "10:14-10:19:warning:Attempting to modify an immutable List created via List.of()");
211+
+ }
212+
+
213+
+ public void testCaseWithAssignmentChange() throws Exception{
214+
+
215+
+ HintTest
216+
+ .create()
217+
+ .input("""
218+
+ package test;
219+
+
220+
+ import java.util.*;
221+
+
222+
+ public class Test {
223+
+ private void test () {
224+
+ var l=List.of("foo","bar");
225+
+ var l2=List.of("foo2","bar2");
226+
+ l.add("bar2");
227+
+ l.clear();
228+
+ l2.clear();
229+
+ l2 = new ArrayList();
230+
+ l2.clear();
231+
+ l2 = List.of("foo3","bar3");
232+
+ l2.clear();
233+
+ l2 = l;
234+
+ l2.clear();
235+
+ if(true){
236+
+ l.clear();
237+
+ }
238+
+ List<String> l3 = new ArrayList<String>();
239+
+ l3 = l2;
240+
+ l3.clear();
241+
+ List<String> l4 = new ArrayList<String>();
242+
+ l4 = l3;
243+
+ l4.clear();
244+
+ var s1 = Set.of("sfoo1","sbar1");
245+
+ s1.clear();
246+
+ }
247+
+ }
248+
+ """)
249+
+ .sourceLevel(10)
250+
+ .run(MutableMethodsOnImmutableCollections.class)
251+
+ .assertWarnings(
252+
+ "8:13-8:16:warning:Attempting to modify an immutable List created via List.of()",
253+
+ "9:13-9:18:warning:Attempting to modify an immutable List created via List.of()",
254+
+ "10:14-10:19:warning:Attempting to modify an immutable List created via List.of()",
255+
+ "14:14-14:19:warning:Attempting to modify an immutable List created via List.of()",
256+
+ "16:14-16:19:warning:Attempting to modify an immutable List created via List.of()",
257+
+ "18:15-18:20:warning:Attempting to modify an immutable List created via List.of()",
258+
+ "22:14-22:19:warning:Attempting to modify an immutable List created via List.of()",
259+
+ "25:14-25:19:warning:Attempting to modify an immutable List created via List.of()",
260+
+ "27:14-27:19:warning:Attempting to modify an immutable Set created via Set.of()"
261+
+ );
262+
+ }
263+
+}
264+
\ No newline at end of file

0 commit comments

Comments
 (0)