jborgers / PMD-jPinpoint-rules

PMD rule set for responsible Java and Kotlin coding: performance, sustainability, multi-threading, data mixup and more.
Apache License 2.0
43 stars 10 forks source link

pmd7 fix request #392 AvoidImplicitlyRecompilingRegex assume external constants bad? #393

Open stokpop opened 2 days ago

stokpop commented 2 days ago

assume constants for regexps are not simple

For discussion: pmd6 rule had a hit, but that might be a bug as in unit tests other Contant.SEP was assumed good, but should we maybe turn it around? If it is a constant assume it to be not a simple regex?

import my.app.FooConstant;
class Foo {
      public static String checkUser(String userId){
        if (!userId.matches(FooConstant.USER_REGEX)) { // bad, missing violation or assume good?
            throw new RuntimeException("Invalid userId");
        }
    }
}

This was here already, should we turn this into "assumed bad, external"?

public class Try {
    private static final String SMALL = "-.";
    private static final String LARGE = "------.-";
    public static void foo(String query) {
        query.split(Constants.SEP); //assumed good, external
        query.split(SMALL); //good
        query.split(LARGE); //bad
    }
}

note:

public static final String USER_REGEX = "^USR\\d+$";
sonarcloud[bot] commented 2 days ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

jborgers commented 2 days ago

Hmm, we like to avoid false positives. I think it is better to assume simple/short. We might want to make a new rule on lower severity level (info?) for the cases we cannot determine, external constant.