Skip to content

Commit

Permalink
Rewrote "String::replaceAll with dot" inspection to apply to more met…
Browse files Browse the repository at this point in the history 
…hods and regex control characters.
  • Loading branch information
@mbien
mbien committed Nov 24, 2021
1 parent 9a25ef5 commit ddf958d
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 18 deletions.
9 changes: 5 additions & 4 deletions java/java.hints/src/org/netbeans/modules/java/hints/bugs/Bundle.properties
View file
Expand Up @@ -25,10 +25,11 @@ DN_AnnotationsNotRuntime_instanceof={0} does not have runtime Retention, the con
DN_org.netbeans.modules.java.hints.bugs.AnnotationsNotRuntime=Annotations without runtime Retention
DESC_org.netbeans.modules.java.hints.bugs.AnnotationsNotRuntime=Warns about reflective access to annotations with CLASS or SOURCE retentions

DN_org.netbeans.modules.java.hints.bugs.Tiny.stringReplaceAllDot=String.replaceAll(".", )
DESC_org.netbeans.modules.java.hints.bugs.Tiny.stringReplaceAllDot=Finds occurrences of calls to String.replaceAll(".", $target), which would replace all characters of the source string with $target.
ERR_string-replace-all-dot=Call to String.replaceAll(".", $target) is probably undesired
FIX_string-replace-all-dot=Replace with call to String.replaceAll("\\.", $target)
DN_org.netbeans.modules.java.hints.bugs.Tiny.singleCharRegex=Single Char Regex
DESC_org.netbeans.modules.java.hints.bugs.Tiny.singleCharRegex=Finds occurrences of single regex control characters \
used as parameter for methods expecting regular expressions and quotes those.
ERR_single-char-regex=Using a single regex control character as regex is probably undesired
FIX_single-char-regex=Escape character for regex usage

DN_org.netbeans.modules.java.hints.bugs.CastVSInstanceOf=Incompatible cast/instanceof
DESC_org.netbeans.modules.java.hints.bugs.CastVSInstanceOf=Incompatible cast surrounded with incompatible instanceof
Expand Down
52 changes: 44 additions & 8 deletions java/java.hints/src/org/netbeans/modules/java/hints/bugs/Tiny.java
View file
Expand Up @@ -29,6 +29,7 @@
import com.sun.source.tree.IfTree;
import com.sun.source.tree.LabeledStatementTree;
import com.sun.source.tree.LineMap;
import com.sun.source.tree.LiteralTree;
import com.sun.source.tree.MemberSelectTree;
import com.sun.source.tree.MethodInvocationTree;
import com.sun.source.tree.SwitchTree;
Expand Down Expand Up @@ -91,18 +92,53 @@
})
public class Tiny {

@Hint(displayName = "#DN_org.netbeans.modules.java.hints.bugs.Tiny.stringReplaceAllDot", description = "#DESC_org.netbeans.modules.java.hints.bugs.Tiny.stringReplaceAllDot", category="bugs", suppressWarnings="ReplaceAllDot")
@TriggerPattern(value="$str.replaceAll(\".\", $to)",
constraints=@ConstraintVariableType(variable="$str", type="java.lang.String"))
public static ErrorDescription stringReplaceAllDot(HintContext ctx) {
@Hint(displayName = "#DN_org.netbeans.modules.java.hints.bugs.Tiny.singleCharRegex", description = "#DESC_org.netbeans.modules.java.hints.bugs.Tiny.singleCharRegex", category="bugs", suppressWarnings="SingleCharRegex")
@TriggerPatterns({
@TriggerPattern(value="$str.replaceAll($pattern, $to)",
constraints = {
@ConstraintVariableType(variable="$str", type="java.lang.String"),
@ConstraintVariableType(variable="$pattern", type="java.lang.String") }),
@TriggerPattern(value = "$str.replaceFirst($pattern, $repl)",
constraints = {
@ConstraintVariableType(variable = "$str", type = "java.lang.String"),
@ConstraintVariableType(variable = "$pattern", type = "java.lang.String"),
@ConstraintVariableType(variable = "$repl", type = "java.lang.String")
}),
@TriggerPattern(value="$str.split($pattern)",
constraints = {
@ConstraintVariableType(variable="$str", type="java.lang.String"),
@ConstraintVariableType(variable="$pattern", type="java.lang.String") }),
@TriggerPattern(value = "$str.split($pattern, $limit)",
constraints = {
@ConstraintVariableType(variable = "$str", type = "java.lang.String"),
@ConstraintVariableType(variable = "$pattern", type = "java.lang.String"),
@ConstraintVariableType(variable = "$limit", type = "int")
})
})
public static ErrorDescription singleCharRegex(HintContext ctx) {
Tree constant = ((MethodInvocationTree) ctx.getPath().getLeaf()).getArguments().get(0);
TreePath constantTP = new TreePath(ctx.getPath(), constant);

String fixDisplayName = NbBundle.getMessage(Tiny.class, "FIX_string-replace-all-dot");
Fix fix = JavaFixUtilities.rewriteFix(ctx, fixDisplayName, constantTP, "\"\\\\.\"");
String displayName = NbBundle.getMessage(Tiny.class, "ERR_string-replace-all-dot");
if (constantTP.getLeaf().getKind() == Kind.STRING_LITERAL) {

String value = (String) ((LiteralTree) constantTP.getLeaf()).getValue();

if (value != null && value.length() == 1 && isRegExControlCharacter(value.charAt(0))) {

String fixDisplayName = NbBundle.getMessage(Tiny.class, "FIX_single-char-regex");
String displayName = NbBundle.getMessage(Tiny.class, "ERR_single-char-regex");

Fix fix = JavaFixUtilities.rewriteFix(ctx, fixDisplayName, constantTP, "\"\\\\"+value.charAt(0)+"\"");
return ErrorDescriptionFactory.forTree(ctx, constant, displayName, fix);
}
}

return null;
}

return ErrorDescriptionFactory.forTree(ctx, constant, displayName, fix);
private static boolean isRegExControlCharacter(char c) {
return c == '.' || c == '$' || c == '|' || c == '^' || c == '?' || c == '*' || c == '+' || c == '\\'
|| c == '(' || c == ')' || c == '[' || c == ']' || c == '{' || c == '}';
}

@Hint(displayName = "#DN_org.netbeans.modules.java.hints.bugs.Tiny.newObject", description = "#DESC_org.netbeans.modules.java.hints.bugs.Tiny.newObject", category="bugs", suppressWarnings="ResultOfObjectAllocationIgnored", options=Options.QUERY)
Expand Down
4 changes: 2 additions & 2 deletions java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/Bundle_test.properties
View file
Expand Up @@ -22,8 +22,8 @@ DN_AnnotationsNotRuntime_instanceof={0} instanceof
DN_RegExp=Invalid regular expression
ERR_CastVSInstanceOf=CastVSInstanceOf

ERR_string-replace-all-dot=ERR_string-replace-all-dot
FIX_string-replace-all-dot=FIX_string-replace-all-dot
ERR_single-char-regex=ERR_single-char-regex
FIX_single-char-regex=FIX_single-char-regex

ERR_newObject=new Object

Expand Down
32 changes: 28 additions & 4 deletions java/java.hints/test/unit/src/org/netbeans/modules/java/hints/bugs/TinyTest.java
View file
Expand Up @@ -38,7 +38,7 @@ public TinyTest(String name) {
super(name);
}

public void testPositive1() throws Exception {
public void testSingleCharRegexPositive1() throws Exception {
HintTest
.create()
.input("package test;\n" +
Expand All @@ -48,8 +48,8 @@ public void testPositive1() throws Exception {
" }\n" +
"}\n")
.run(Tiny.class)
.findWarning("3:23-3:26:verifier:ERR_string-replace-all-dot")
.applyFix("FIX_string-replace-all-dot")
.findWarning("3:23-3:26:verifier:ERR_single-char-regex")
.applyFix("FIX_single-char-regex")
.assertCompilable()
.assertOutput("package test;\n" +
"public class Test {\n" +
Expand All @@ -59,13 +59,37 @@ public void testPositive1() throws Exception {
"}\n");
}

public void testNegative1() throws Exception {
public void testSingleCharRegexPositive2() throws Exception {
HintTest
.create()
.input("package test;\n" +
"public class Test {\n" +
" public void test(String[] args) {\n" +
" \"a\".split(\"$\");\n" +
" }\n" +
"}\n")
.run(Tiny.class)
.findWarning("3:18-3:21:verifier:ERR_single-char-regex")
.applyFix("FIX_single-char-regex")
.assertCompilable()
.assertOutput("package test;\n" +
"public class Test {\n" +
" public void test(String[] args) {\n" +
" \"a\".split(\"\\\\$\");\n" +
" }\n" +
"}\n");
}

public void testSingleCharRegexNegative1() throws Exception {
HintTest
.create()
.input("package test;\n" +
"public class Test {\n" +
" public void test(String[] args) {\n" +
" \"a\".replaceAll(\",\", \"/\");\n" +
" \"a\".replaceFirst(\"$$\", \"/\");\n" +
" String foo = \"foo\";\n" +
" \"a\".split(foo);\n" +
" }\n" +
"}\n")
.run(Tiny.class)
Expand Down

0 comments on commit ddf958d

Please sign in to comment.