Skip to content

Commit f14f44e

Browse files
naren2605naren2605
committed
updated changes addressing some review comments
1 parent 8ca245a commit f14f44e

File tree

2 files changed

+42
-45
lines changed

2 files changed

+42
-45
lines changed

‎build.xml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
patches/upgrade-lsp4j.diff
8484
patches/updated-show-input-params.diff
8585
patches/java-notebooks.diff
86-
patches/jvsce-361-draft.diff
86+
patches/jvsce-361-draft-v2.diff
8787
</string>
8888
<filterchain>
8989
<tokenfilter delimoutput=" ">

‎patches/jvsce-361-draft.diff‎ renamed to ‎patches/jvsce-361-draft-v2.diff‎

Lines changed: 41 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
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
22
new file mode 100644
3-
index 0000000000..6c6b1881f5
3+
index 0000000000..5d1b1f2fde
44
--- /dev/null
55
+++ b/java/java.hints/src/org/netbeans/modules/java/hints/bugs/MutableMethodsOnImmutableCollections.java
6-
@@ -0,0 +1,132 @@
6+
@@ -0,0 +1,129 @@
77
+/*
88
+ * Licensed to the Apache Software Foundation (ASF) under one
99
+ * or more contributor license agreements. See the NOTICE file
@@ -24,16 +24,17 @@ index 0000000000..6c6b1881f5
2424
+ */
2525
+package org.netbeans.modules.java.hints.bugs;
2626
+
27-
+import com.sun.source.tree.CompilationUnitTree;
27+
+import com.sun.source.tree.IdentifierTree;
2828
+import com.sun.source.tree.MemberSelectTree;
2929
+import com.sun.source.tree.MethodInvocationTree;
3030
+import com.sun.source.tree.Tree;
3131
+import com.sun.source.util.TreePath;
3232
+import java.util.ArrayList;
33-
+import java.util.Collection;
34-
+import java.util.HashSet;
3533
+import java.util.List;
34+
+import java.util.Optional;
3635
+import java.util.Set;
36+
+import java.util.function.Function;
37+
+import java.util.stream.Collectors;
3738
+import org.netbeans.api.java.source.CompilationInfo;
3839
+import org.netbeans.modules.java.hints.introduce.Flow;
3940
+import org.netbeans.modules.java.hints.introduce.Flow.FlowResult;
@@ -52,7 +53,7 @@ index 0000000000..6c6b1881f5
5253
+@Hint(displayName = "Track mutable methods on immutable collections",
5354
+ description = "Track mutable methods on immutable collections",
5455
+ category = "bugs",
55-
+ id = "ImmutableListCreation",
56+
+ id = "MutableMethodsOnImmutableCollections",
5657
+ severity = Severity.WARNING,
5758
+ options = Options.QUERY)
5859
+
@@ -66,8 +67,6 @@ index 0000000000..6c6b1881f5
6667
+ "add", "addAll", "remove", "removeAll", "retainAll", "clear"
6768
+ );
6869
+
69-
+ public static final String SUPPRESS_WARNING_KEY = "immutable-collection-mutation";
70-
+
7170
+ @TriggerPattern(value = "java.util.List.of($args$)")
7271
+ public static List<ErrorDescription> immutableList(HintContext ctx) {
7372
+ return checkForMutableMethodInvocations(ctx, MUTATING_METHODS_IN_LIST, "Attempting to modify an immutable List created via List.of()");
@@ -80,8 +79,10 @@ index 0000000000..6c6b1881f5
8079
+
8180
+ private static List<ErrorDescription> checkForMutableMethodInvocations(HintContext ctx, Set<String> mutatingMethods, String warningMessage) {
8281
+ List<ErrorDescription> errors = new ArrayList<>();
82+
+ FlowResult flow = Flow.assignmentsForUse(ctx.getInfo(), () -> ctx.isCanceled());
83+
+ List<MemberSelectTree> invocations = checkForUsagesAndMarkInvocations(ctx.getInfo(), flow, ctx.getPath());
8384
+
84-
+ for (MemberSelectTree mst : getInvocations(ctx.getInfo(), ctx.getPath().getLeaf(), () -> ctx.isCanceled())) {
85+
+ for (MemberSelectTree mst : invocations) {
8586
+ String method = mst.getIdentifier().toString();
8687
+ if (mutatingMethods.contains(method)) {
8788
+ errors.add(ErrorDescriptionFactory.forName(
@@ -95,43 +96,39 @@ index 0000000000..6c6b1881f5
9596
+ return errors;
9697
+ }
9798
+
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) {
99+
+ private static List<MemberSelectTree> checkForUsagesAndMarkInvocations(CompilationInfo info, FlowResult flow, TreePath initPattern) {
117100
+ 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);
101+
+ Function<Tree, Set<TreePath>> findIdentifierTreePaths = (Tree tree) -> {
102+
+ return flow.getValueUsers(tree)
103+
+ .stream()
104+
+ .filter(IdentifierTree.class::isInstance)
105+
+ .map(t -> flow.findPath(t, info.getCompilationUnit()))
106+
+ .filter(treePath -> treePath.getLeaf() instanceof IdentifierTree)
107+
+ .collect(Collectors.toSet());
108+
+ };
109+
+ Set<TreePath> identfiersPointingToInitializer = Optional.of(initPattern.getLeaf())
110+
+ .map(findIdentifierTreePaths)
111+
+ .orElse(Set.of());
112+
+ while (!identfiersPointingToInitializer.isEmpty()) {
113+
+ identfiersPointingToInitializer.forEach(indentifierPath -> {
114+
+ var ancestorPath = Optional.of(indentifierPath)
115+
+ .map(tpath -> tpath.getParentPath())
116+
+ .map(tpath -> tpath.getParentPath())
117+
+ .map(tpath -> tpath.getLeaf());
118+
+ ancestorPath.ifPresent(ancestor -> {
119+
+ if (ancestor instanceof MethodInvocationTree mit && mit.getMethodSelect() instanceof MemberSelectTree mst) {
120+
+ usedInvocationsWithIdentifier.add(mst);
121+
+ }
122+
+ });
123+
+ });
124+
+ identfiersPointingToInitializer = identfiersPointingToInitializer
125+
+ .parallelStream()
126+
+ .map(tpath->tpath.getLeaf())
127+
+ .map(findIdentifierTreePaths)
128+
+ .flatMap(tpaths->tpaths.parallelStream())
129+
+ .collect(Collectors.toSet());
134130
+ }
131+
+ return usedInvocationsWithIdentifier;
135132
+ }
136133
+
137134
+

0 commit comments

Comments
 (0)