Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jackpot] Rewrite "String::replaceAll with dot" inspection to apply to more cases and methods #3218

Merged
merged 1 commit into from Nov 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
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
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
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