Skip to content

Commit ee1bfad

Browse files
alban-auzeillquentin-jaquier-sonarsource
alban-auzeill
authored and
quentin-jaquier-sonarsource
committed
SONARJAVA-3688 FP on S5860(UnusedGroupNamesCheck) for name referenced by dollar curly braces
1 parent ab3b22e commit ee1bfad

File tree

2 files changed

+81
-30
lines changed

2 files changed

+81
-30
lines changed

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,33 @@ private Matcher someOtherMethod() {
203203
return null;
204204
}
205205

206+
void replacement(String input) {
207+
Matcher m1 = Pattern.compile("a(?<foo>.)").matcher(input);
208+
m1.replaceAll("x${foo}"); // Compliant, foo is used
209+
210+
Matcher m2 = Pattern.compile("a(?<foo>.)").matcher(input);
211+
m2.replaceAll("${foo}x${bar}"); // Noncompliant {{There is no group named 'bar' in the regular expression.}}
212+
213+
Matcher m3 = Pattern.compile("a(?<bar>.)").matcher(input);
214+
m3.replaceFirst("x$1"); // Noncompliant {{Directly use '${bar}' instead of its group number.}}
215+
216+
Matcher m4 = Pattern.compile("a(?<foo>.)").matcher(input);
217+
m4.replaceFirst("x${bar}x"); // Noncompliant {{There is no group named 'bar' in the regular expression.}}
218+
219+
Matcher m5 = Pattern.compile("a(?<foo>.)").matcher(input);
220+
StringBuffer buffer = new StringBuffer();
221+
m5.appendReplacement(buffer, "${bar}x"); // Noncompliant {{There is no group named 'bar' in the regular expression.}}
222+
m5.appendReplacement(buffer, "x$1x"); // Noncompliant {{Directly use '${foo}' instead of its group number.}}
223+
224+
Matcher m6 = Pattern.compile("a(.)(?<foo>.)").matcher(input);
225+
m6.appendReplacement(buffer, "$1x${foo}x$1"); // Compliant, the group 1 has no name
226+
m6.appendReplacement(buffer, "xx${bar}xx"); // Noncompliant {{There is no group named 'bar' in the regular expression.}}
227+
m6.appendReplacement(buffer, "\\${bar}xx"); // Compliant, dollar is escaped
228+
m6.appendReplacement(buffer, "$1x$2x${foo}"); // Noncompliant {{Directly use '${foo}' instead of its group number.}}
229+
m6.appendReplacement(buffer, "\\$2x${foo}"); // Compliant, dollar is escaped
230+
m6.appendReplacement(buffer, "${2}"); // Compliant, 2 is not a valid group name
231+
}
232+
206233
static class UsingConstant {
207234
private static final Pattern CONSTANT_PATTERN = Pattern.compile("(?<group>[a-z])");
208235

‎java-checks/src/main/java/org/sonar/java/checks/regex/UnusedGroupNamesCheck.java‎

Lines changed: 54 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,18 @@
2424
import java.util.HashMap;
2525
import java.util.List;
2626
import java.util.Map;
27-
import java.util.Optional;
2827
import java.util.function.Function;
28+
import java.util.regex.Matcher;
29+
import java.util.regex.Pattern;
2930
import java.util.stream.Collectors;
3031
import org.sonar.check.Rule;
32+
import org.sonar.java.model.ExpressionUtils;
3133
import org.sonar.java.regex.RegexCheck;
3234
import org.sonar.java.regex.RegexParseResult;
3335
import org.sonar.java.regex.ast.BackReferenceTree;
3436
import org.sonar.java.regex.ast.CapturingGroupTree;
3537
import org.sonar.java.regex.ast.RegexBaseVisitor;
3638
import org.sonar.plugins.java.api.semantic.MethodMatchers;
37-
import org.sonar.plugins.java.api.semantic.Type;
3839
import org.sonar.plugins.java.api.tree.ExpressionTree;
3940
import org.sonar.plugins.java.api.tree.MethodInvocationTree;
4041

@@ -47,13 +48,27 @@ public class UnusedGroupNamesCheck extends AbstractRegexCheckTrackingMatchers {
4748

4849
private static final String JAVA_UTIL_REGEX_MATCHER = "java.util.regex.Matcher";
4950

50-
private static final MethodMatchers MATCHER_GROUP = MethodMatchers.create()
51-
.ofTypes(JAVA_UTIL_REGEX_MATCHER)
52-
.names("group")
53-
// covers both 'group(String)' and 'group(int)'
54-
.addParametersMatcher(JAVA_LANG_STRING)
55-
.addParametersMatcher("int")
56-
.build();
51+
private static final Pattern GROUP_NUMBER_REPLACEMENT_REGEX = Pattern.compile("(?<!\\\\)\\$(?<number>\\d++)");
52+
private static final Pattern GROUP_NAME_REPLACEMENT_REGEX = Pattern.compile("(?<!\\\\)\\$\\{(?<name>[A-Za-z][0-9A-Za-z]*+)\\}");
53+
54+
private static final MethodMatchers MATCHER_GROUP = MethodMatchers.or(
55+
MethodMatchers.create()
56+
.ofTypes(JAVA_UTIL_REGEX_MATCHER)
57+
.names("group")
58+
// covers both 'group(String)' and 'group(int)'
59+
.addParametersMatcher(JAVA_LANG_STRING)
60+
.addParametersMatcher("int")
61+
.build(),
62+
MethodMatchers.create()
63+
.ofTypes(JAVA_UTIL_REGEX_MATCHER)
64+
.names("appendReplacement")
65+
.addParametersMatcher(MethodMatchers.ANY, MethodMatchers.ANY)
66+
.build(),
67+
MethodMatchers.create()
68+
.ofTypes(JAVA_UTIL_REGEX_MATCHER)
69+
.names("replaceAll", "replaceFirst")
70+
.addParametersMatcher(MethodMatchers.ANY)
71+
.build());
5772

5873
@Override
5974
protected MethodMatchers trackedMethodMatchers() {
@@ -76,38 +91,47 @@ protected void checkRegex(RegexParseResult regexForLiterals, List<MethodInvocati
7691
}
7792

7893
private void checkGroupUsage(MethodInvocationTree mit, KnownGroupsCollector knownGroups) {
79-
ExpressionTree arg0 = mit.arguments().get(0);
80-
Type arg0Type = arg0.symbolType();
81-
if (arg0Type.is("int")) {
82-
checkUsingNumberInsteadOfName(knownGroups, arg0);
94+
String methodName = ExpressionUtils.methodName(mit).name();
95+
if ("group".equals(methodName)) {
96+
ExpressionTree arg0 = mit.arguments().get(0);
97+
if (arg0.symbolType().is("int")) {
98+
arg0.asConstant(Integer.class).ifPresent(index -> checkUsingNumberInsteadOfName(knownGroups, arg0, index, false));
99+
} else {
100+
arg0.asConstant(String.class).ifPresent(name -> checkNoSuchName(knownGroups, arg0, name));
101+
}
83102
} else {
84-
checkNoSuchName(knownGroups, arg0);
103+
int argIndex = "appendReplacement".equals(methodName) ? 1 : 0;
104+
ExpressionTree arg = mit.arguments().get(argIndex);
105+
arg.asConstant(String.class).ifPresent(replacement -> checkUsingReplacementString(knownGroups, arg, replacement));
85106
}
86107
}
87108

88-
private void checkUsingNumberInsteadOfName(KnownGroupsCollector knownGroups, ExpressionTree arg0) {
89-
Optional<Integer> groupNumber = arg0.asConstant(Integer.class);
90-
if (!groupNumber.isPresent()) {
91-
return;
109+
private void checkUsingReplacementString(KnownGroupsCollector knownGroups, ExpressionTree arg, String replacement) {
110+
Matcher indexMatcher = GROUP_NUMBER_REPLACEMENT_REGEX.matcher(replacement);
111+
while (indexMatcher.find()) {
112+
int groupNumber = Integer.parseInt(indexMatcher.group("number"));
113+
checkUsingNumberInsteadOfName(knownGroups, arg, groupNumber, true);
114+
}
115+
Matcher nameMatcher = GROUP_NAME_REPLACEMENT_REGEX.matcher(replacement);
116+
while (nameMatcher.find()) {
117+
checkNoSuchName(knownGroups, arg, nameMatcher.group("name"));
92118
}
93-
Integer groupNumberValue = groupNumber.get();
94-
CapturingGroupTree capturingGroupTree = knownGroups.groupsByNumber.get(groupNumberValue);
119+
}
120+
121+
private void checkUsingNumberInsteadOfName(KnownGroupsCollector knownGroups, ExpressionTree arg0, int groupNumber, boolean dollarReference) {
122+
CapturingGroupTree capturingGroupTree = knownGroups.groupsByNumber.get(groupNumber);
95123
if (capturingGroupTree == null) {
96124
return;
97125
}
98-
String message = String.format(ISSUE_USE_NAME_INSTEAD_OF_NUMBER, capturingGroupTree.getName().orElse("?"));
99-
RegexIssueLocation secondary = toLocation(capturingGroupTree, "Group %d", g -> groupNumberValue);
126+
String groupName = capturingGroupTree.getName().map(name -> dollarReference ? ("${" + name + "}") : name).orElse("?");
127+
String message = String.format(ISSUE_USE_NAME_INSTEAD_OF_NUMBER, groupName);
128+
RegexIssueLocation secondary = toLocation(capturingGroupTree, "Group %d", g -> groupNumber);
100129
reportIssue(arg0, message, null, Collections.singletonList(secondary));
101130
}
102131

103-
private void checkNoSuchName(KnownGroupsCollector knownGroups, ExpressionTree arg0) {
104-
Optional<String> groupName = arg0.asConstant(String.class);
105-
if (!groupName.isPresent()) {
106-
return;
107-
}
108-
String groupNameValue = groupName.get();
109-
if (!knownGroups.groupsByName.keySet().contains(groupNameValue)) {
110-
String message = String.format(ISSUE_NO_GROUP_WITH_SUCH_NAME, groupNameValue);
132+
private void checkNoSuchName(KnownGroupsCollector knownGroups, ExpressionTree arg0, String groupName) {
133+
if (!knownGroups.groupsByName.keySet().contains(groupName)) {
134+
String message = String.format(ISSUE_NO_GROUP_WITH_SUCH_NAME, groupName);
111135
List<RegexIssueLocation> secondaries = knownGroups.groupsByName.values()
112136
.stream()
113137
.map(group -> toLocation(group, "Named group '%s'", g -> g.getName().get()))

0 commit comments

Comments
 (0)