diff --git a/README.md b/README.md index 5742fee8..3c9cb58e 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ ## Description / Features -This plugin requires the [SonarJava Plugin](https://docs.sonarqube.org/display/PLUG/SonarJava), and uses [SpotBugs](https://spotbugs.github.io), [fb-contrib](http://fb-contrib.sourceforge.net/) and [Find Security Bugs](http://h3xstream.github.io/find-sec-bugs/) to provide coding rules. +This plugin requires the [SonarJava Plugin](https://docs.sonarqube.org/display/PLUG/SonarJava), and uses [SpotBugs](https://spotbugs.github.io), [fb-contrib](https://github.com/mebigfatguy/fb-contrib) and [Find Security Bugs](https://find-sec-bugs.github.io/) to provide coding rules. ### Supported Languages diff --git a/generate_profiles/BuildXmlFiles.groovy b/generate_profiles/BuildXmlFiles.groovy index 85233aaf..82e562f3 100644 --- a/generate_profiles/BuildXmlFiles.groovy +++ b/generate_profiles/BuildXmlFiles.groovy @@ -8,13 +8,13 @@ import groovy.json.JsonSlurper; @Grapes([ @Grab(group='com.github.spotbugs', module='spotbugs', version='4.7.3'), - @Grab(group='com.mebigfatguy.sb-contrib', module='sb-contrib', version='7.4.7'), + @Grab(group='com.mebigfatguy.sb-contrib', module='sb-contrib', version='7.6.0'), @Grab(group='com.h3xstream.findsecbugs' , module='findsecbugs-plugin', version='1.12.0')] ) FB = new Plugin(groupId: 'com.github.spotbugs', artifactId: 'spotbugs', version: '4.7.3') -CONTRIB = new Plugin(groupId: 'com.mebigfatguy.sb-contrib', artifactId: 'sb-contrib', version: '7.4.7') +CONTRIB = new Plugin(groupId: 'com.mebigfatguy.sb-contrib', artifactId: 'sb-contrib', version: '7.6.0') FSB = new Plugin(groupId: 'com.h3xstream.findsecbugs', artifactId: 'findsecbugs-plugin', version: '1.12.0') def destDir() { diff --git a/pom.xml b/pom.xml index 2d3c5dd8..c250dc5e 100644 --- a/pom.xml +++ b/pom.xml @@ -53,7 +53,7 @@ Update the version table and the rules count badge in README.md --> 4.7.3 - 7.4.7 + 7.6.0 1.12.0 1.8 diff --git a/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java index 1bab518b..bbc07746 100644 --- a/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java +++ b/src/main/java/org/sonar/plugins/findbugs/rules/FbContribRulesDefinition.java @@ -27,8 +27,8 @@ public class FbContribRulesDefinition implements RulesDefinition { public static final String REPOSITORY_KEY = "fb-contrib"; public static final String REPOSITORY_NAME = "FindBugs Contrib"; - public static final int RULE_COUNT = 307; - public static final int DEACTIVED_RULE_COUNT = 0; + public static final int RULE_COUNT = 312; + public static final int DEACTIVED_RULE_COUNT = 1; @Override public void define(Context context) { diff --git a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-and-fb-contrib.xml b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-and-fb-contrib.xml index 0db7696a..ca6818a7 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-and-fb-contrib.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-and-fb-contrib.xml @@ -1634,9 +1634,6 @@ - - - @@ -1658,6 +1655,9 @@ + + + @@ -2216,6 +2216,9 @@ + + + @@ -2243,6 +2246,9 @@ + + + @@ -2273,9 +2279,6 @@ - - - @@ -2303,4 +2306,16 @@ + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/org/sonar/plugins/findbugs/rules-fbcontrib.xml b/src/main/resources/org/sonar/plugins/findbugs/rules-fbcontrib.xml index 26d35407..c1785cb1 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/rules-fbcontrib.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/rules-fbcontrib.xml @@ -714,7 +714,7 @@ if (!date1.equals( date2 )) a potential cause of memory bloat.</p> <p> - If this collection is a list, set or otherwise of static things (e.g. a List&gt;String&gt; for month names), consider + If this collection is a list, set or otherwise of static things (e.g. a List&lt;String&gt; for month names), consider adding all of the elements in a static initializer, which can only be called once:<br/> <pre><code> private static List&lt;String&gt; monthNames = new ArrayList&lt;String&gt;(); @@ -1111,15 +1111,6 @@ if (myString.indexOf('e') != -1) { performance bug - - Correctness - Method calls equals on an enum instance - SPP_EQUALS_ON_ENUM - <p>This method calls the equals(Object) method on an enum instance. Since enums values are singletons, - you can use == to safely compare two enum values. In fact, the implementation for Enum.equals does just - that.</p> - correctness - bug - Correctness - Method uses invalid C++ style null check on Boolean SPP_INVALID_BOOLEAN_NULL_CHECK @@ -1215,6 +1206,18 @@ if ({{FLAWED_TEST_LOGIC}}) { correctness bug + + Correctness - Method call passes object that the method is called on as a parameter + SPP_PASSING_THIS_AS_PARM + <p>This method calls an instance method passing the object that the method is called on as a parameter, such as + <code> + foo.someMethod(foo); + </code> + As you already have access to this object thru this, you don't need to pass it. + </p> + correctness + bug + Correctness - Method calls keySet() just to call contains, use containsKey instead MUI_USE_CONTAINSKEY @@ -3275,6 +3278,15 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { } correctness bug + + Correctness - Static field is autowired + WI_WIRING_OF_STATIC_FIELD + <p>Autowiring of static fields does not work using simple @Autowire annotations, not should you attempt to do + so as it's an anti pattern. Use PostConstruct methods to initialize static fields if you must do something + like this.</p> + correctness + bug + Correctness - Method gets and sets a value of a ConcurrentHashMap in a racy manner CCI_CONCURRENT_COLLECTION_ISSUES_USE_PUT_IS_RACY @@ -3377,6 +3389,28 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { } correctness bug + + Correctness - Method uses Optional.equals(Optional.empty()), when Optional.isPresent is more readable + OI_OPTIONAL_ISSUES_ISPRESENT_PREFERRED + <p>This method uses Optional.equals(Optional.empty()). It is more readable and more clear just to use the Optional.isPresent() + method to determine whether the reference exists or not. Use + <br/> + <pre><code> + Optional f = getSomeOptional(); + if (!f.isPresent()) { + } + </code></pre> + rather than + <br/> + <pre><code> + Optional f = getSomeOptional(); + if (f.equals(Optional.empty()) { + } + </code></pre> + </p> + correctness + bug + Correctness - Method constructs a Date object, merely to convert it to an Instant object UAC_UNNECESSARY_API_CONVERSION_DATE_TO_INSTANT @@ -3560,8 +3594,8 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { } correctness bug - - Correctness - Method calls contains() on a collected lambda expression + + Experimental - Method calls contains() on a collected lambda expression FII_AVOID_CONTAINS_ON_COLLECTED_STREAM <p>This method builds a collection using lambda expressions with a collect terminal operation. It then immediately calls the contains() method on it, to see if an item is present. This is sub optimal as the lambda still needs to @@ -3578,8 +3612,7 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { } baubles.stream().anyMatch(b -> name.equals(b.getName())) </pre></code> </p> - correctness - bug + experimental Correctness - Method creates an anonymous lambda expression instead of specifying a method reference @@ -3729,4 +3762,41 @@ if (shouldCalcHalting && (calculateHaltingProbability() &gt; 0) { } correctness bug + + Correctness - Method calls equals on an enum instance + ENMI_EQUALS_ON_ENUM + <p>This method calls the equals(Object) method on an enum instance. Since enums values are singletons, + you can use == to safely compare two enum values. In fact, the implementation for Enum.equals does just + that.</p> + correctness + bug + + + Correctness - Method sets an enum reference to null + ENMI_NULL_ENUM_VALUE + <p>This method sets the value of an enum reference to null. An enum should never have a null value. + If there is a state where you do not know what the value of an enum should be, than that should be one of the + proper enum value. So add a MyEnum.UNKNOWN or such. This keeps the logic of switch statements, etc, much simpler. + correctness + bug + + + Correctness - Enum class only declares one enum value + ENMI_ONE_ENUM_VALUE + <p>This enum class only declares one value (or perhaps 0!). As such it is pointless, as its value will always be the same thing. + Therefore use of this enum is just bloating the code base. One exception is if you are using a null value as a second value. + This is a mistake, and should be replaced with a second enum value, even if it's NULL, or UNKNOWN, or NON_INTITIALIZED or some other + sentinel value. + correctness + bug + + + Correctness - Method specifies superfluous routes thru route() or concat() + AKI_SUPERFLUOUS_ROUTE_SPECIFICATION + <p>This method uses the route() or concat() method to build optional routes but only passes 1 route to the method. + This just causes an extra route specification to be created for no reason. Only use route() or concat() when you have more than + one route to combine into one. + correctness + bug + \ No newline at end of file