Skip to content

Commit b1631ce

Browse files
SONARJAVA-5101 Fix FP on S5860 when Regex are used in lambdas (#5119)
There is an existing mechanism to track regex that escape analysis when they are passed in a constructor to another class or returned. We use the same mechanism to exclude regex used in lambdas and method references.
1 parent c3ae6b5 commit b1631ce

2 files changed

Lines changed: 102 additions & 0 deletions

File tree

java-checks-test-sources/default/src/main/java/checks/regex/UnusedGroupNamesCheck.java

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
package checks.regex;
22

3+
import java.util.List;
4+
import java.util.Map;
35
import java.util.regex.Matcher;
46
import java.util.regex.Pattern;
7+
import java.util.stream.Collectors;
8+
import java.util.stream.Stream;
59

610
abstract class UnusedGroupNamesCheck {
711

@@ -296,4 +300,78 @@ void useMatcher() {
296300
@org.hibernate.validator.constraints.URL(regexp = "(?<group>[a-z])") // Noncompliant
297301
String url;
298302

303+
static class GroupUsedViaMethodReference {
304+
private static final Pattern NAME_WITH_QUOTED_VALUE =
305+
Pattern.compile("^(?<name>[a-zA-Z_:][a-zA-Z0-9_:]*)=\"(?<value>.*)\"$"); // Compliant
306+
307+
public static Map<String, String> hiddenUsage(List<String> strings) {
308+
return strings.stream()
309+
.map(NAME_WITH_QUOTED_VALUE::matcher)
310+
.filter(Matcher::matches)
311+
.collect(Collectors.toMap(
312+
nv -> nv.group("name"),
313+
nv -> nv.group("value"))
314+
);
315+
}
316+
}
317+
318+
// Do not consider RE escaping if the method reference is not "leaking" it
319+
// (matcher replaced with flags).
320+
static class GroupNotUsedViaMethodReference {
321+
private static final Pattern NAME_WITH_QUOTED_VALUE =
322+
Pattern.compile("^(?<name>[a-zA-Z_:][a-zA-Z0-9_:]*)=\"(?<value>.*)\"$"); // Noncompliant
323+
324+
public static int noUsage(List<String> strings) {
325+
return Stream.generate(NAME_WITH_QUOTED_VALUE::flags)
326+
.limit(1)
327+
.findFirst()
328+
.get();
329+
}
330+
}
331+
332+
static class GroupUsedViaLambda {
333+
private static final Pattern NAME_WITH_QUOTED_VALUE =
334+
Pattern.compile("^(?<name>[a-zA-Z_:][a-zA-Z0-9_:]*)=\"(?<value>.*)\"$"); // Compliant
335+
336+
public static Map<String, String> hiddenUsage(List<String> strings) {
337+
return strings.stream()
338+
.map(s -> NAME_WITH_QUOTED_VALUE.matcher(s))
339+
.filter(Matcher::matches)
340+
.collect(Collectors.toMap(
341+
nv -> nv.group("name"),
342+
nv -> nv.group("value"))
343+
);
344+
}
345+
}
346+
347+
static class GroupUsedViaBlockInLambda {
348+
private static final Pattern NAME_WITH_QUOTED_VALUE =
349+
Pattern.compile("^(?<name>[a-zA-Z_:][a-zA-Z0-9_:]*)=\"(?<value>.*)\"$"); // Compliant
350+
351+
public static Map<String, String> hiddenUsage(List<String> strings) {
352+
return strings.stream()
353+
.map(s -> {
354+
System.out.println("testing usage inside a block in a lambda");
355+
return NAME_WITH_QUOTED_VALUE.matcher(s);
356+
})
357+
.filter(Matcher::matches)
358+
.collect(Collectors.toMap(
359+
nv -> nv.group("name"),
360+
nv -> nv.group("value"))
361+
);
362+
}
363+
}
364+
365+
// Do not consider RE escaping if the method in lambda is not "leaking" it
366+
// (matcher replaced with hashCode).
367+
static class GroupNotUsedViaLambda {
368+
private static final Pattern NAME_WITH_QUOTED_VALUE =
369+
Pattern.compile("^(?<name>[a-zA-Z_:][a-zA-Z0-9_:]*)=\"(?<value>.*)\"$"); // Noncompliant
370+
371+
public static Map<String, String> noUsage(List<String> strings) {
372+
return strings.stream()
373+
.map(s -> NAME_WITH_QUOTED_VALUE.hashCode())
374+
.collect(Collectors.toMap(hc -> hc.toString(), unused -> "s"));
375+
}
376+
}
299377
}

java-checks/src/main/java/org/sonar/java/checks/regex/AbstractRegexCheckTrackingMatchers.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.sonar.plugins.java.api.tree.IdentifierTree;
3636
import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree;
3737
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
38+
import org.sonar.plugins.java.api.tree.MethodReferenceTree;
3839
import org.sonar.plugins.java.api.tree.NewClassTree;
3940
import org.sonar.plugins.java.api.tree.ReturnStatementTree;
4041
import org.sonar.plugins.java.api.tree.Tree;
@@ -114,6 +115,7 @@ public List<Tree.Kind> nodesToVisit() {
114115
nodes.add(Tree.Kind.NEW_CLASS);
115116
nodes.add(Tree.Kind.RETURN_STATEMENT);
116117
nodes.add(Tree.Kind.COMPILATION_UNIT);
118+
nodes.add(Tree.Kind.METHOD_REFERENCE);
117119
return nodes;
118120
}
119121

@@ -137,6 +139,8 @@ public void leaveNode(Tree tree) {
137139
if (PATTERN_OR_MATCHER_ARGUMENT.matches((NewClassTree) tree)) {
138140
onConstructorFound((NewClassTree) tree);
139141
}
142+
} else if (tree.is(Tree.Kind.METHOD_REFERENCE)) {
143+
onMethodReferenceFound((MethodReferenceTree) tree);
140144
} else {
141145
super.visitNode(tree);
142146
}
@@ -176,6 +180,20 @@ protected void onMethodInvocationFound(MethodInvocationTree mit) {
176180
}
177181
}
178182

183+
/**
184+
* When method reference is used on a pattern, we can no longer analyze it,
185+
* and therefore must add it to the {@link #escapingRegexes}.
186+
*/
187+
private void onMethodReferenceFound(MethodReferenceTree methodReference) {
188+
Optional.of(methodReference)
189+
.filter(matchers::matches)
190+
.map(MethodReferenceTree::expression)
191+
.filter(ExpressionTree.class::isInstance)
192+
.map(ExpressionTree.class::cast)
193+
.flatMap(this::getRegex)
194+
.ifPresent(escapingRegexes::add);
195+
}
196+
179197
private Optional<RegexParseResult> getRegex(ExpressionTree tree) {
180198
if (tree.is(Tree.Kind.METHOD_INVOCATION)) {
181199
MethodInvocationTree mit = (MethodInvocationTree) tree;
@@ -223,6 +241,10 @@ public void checkRegex(RegexParseResult regexForLiterals, ExpressionTree methodI
223241
}
224242
}
225243

244+
/**
245+
* Track ownership of pattern when they are assigned to variables and
246+
* add them to {@link #escapingRegexes}, when we cannot track them.
247+
*/
226248
private void handleAssignment(MethodInvocationTree mit, RegexParseResult regex) {
227249
Tree parent = mit.parent();
228250
if (parent.is(Tree.Kind.VARIABLE, Tree.Kind.ASSIGNMENT)) {
@@ -237,6 +259,8 @@ private void handleAssignment(MethodInvocationTree mit, RegexParseResult regex)
237259
if (!grandParent.is(Tree.Kind.METHOD_INVOCATION) || !trackedMethodMatchers().matches((MethodInvocationTree) grandParent)) {
238260
escapingRegexes.add(regex);
239261
}
262+
} else if (parent.is(Tree.Kind.LAMBDA_EXPRESSION)) {
263+
escapingRegexes.add(regex);
240264
}
241265
}
242266

0 commit comments

Comments
 (0)