Rule description & motivation S4276 checks for uses of functional interfaces such as Function<Integer, R> that could be replaced with IntFunction<R> which avoids autoboxing.
However, apart from such cases, developers who are not yet familiar with all the functional interfaces in the JDK will use general functional interfaces instead of specialised ones. In a large code base where the team recently began using lambdas, I found several such cases and was surprised that SonarQube did not detect them.
I came up with this list of possible replacements that should be detected by a new rule or by S4276:
Current Interface
Preferred Interface
Remarks
Function<X, X>
UnaryOperator<X>
the original pattern I found
BiFunction<X, X, X>
BinaryOperator<X>
Predicate<File>
FileFilter
Predicate<Path>
PathMatcher
Callable<X>
Supplier<X>
if it does not throw a checked exception
Function<File, Boolean>
FileFilter
if possible
Function<Path, Boolean>
PathMatcher
if possible
Function<X, Boolean>
Predicate<X>
if possible
BiFunction<X Y, Boolean>
BiPredicate<X,Y>
if possible
Impact to keep this code as it is
The general interface arguably does not capture the author’s intention as well as the specialised one.
Using the general *Function interfaces makes it impossible to use the and(), or(), negate() methods offered by *Predicate.
Notes S4276 is titled “Functional Interfaces should be as specialised as possible”, which makes no mention of autoboxing. If a new rule is added for my suggestion, S4276 title should be rephrased to differentiate it from the new one.
thank you for your suggestion. I think these additional interfaces can be handled by the same rule and we don’t need to create a new rule for them. Also, cases Function<X,X> and BiFunction<X,X,X> should already be handled by the rule, do you have an example code where this doesn’t work?
However, I am not convinced about your proposal to replace Callable with Supplier. Callable is used extensively within java.util.concurrent framework and it is not possible to use Supplier with this API. Such change would break a lot of existing code.
Also changing Predicate<File> with FileFilter means that API which is working with generic Predicate<?> will stop to work. Same for PathMatcher.
Come to think of it, I agree with your reasoning regarding Predicate vs FileFilter/PathMatcher. Also, although you didn’t mention it, a similar case can be made for Function<File, Boolean> to FileFilter and Function<Path, Boolean> to PathMatcher as that would mean losing the compose()/andThen() functionality. So I’m fine with leaving all that out.
Yes, but not in a public code base. I verified that the quality gate includes S4276 and that no issue was reported on the class (we’re using SonarCloud), so this seems like a false negative that should be fixed. The class looks like the following; I only changed names and removed irrelevant members.
Note: When I read the introductory paragraphs in the rule description, I concluded it only covers boxing-related replacements and never bothered to check the table for this case. Independently of fixing the false negative, you should definitively add a paragraph about those other replacements, probably similar to the rationale I gave above.
Are you providing bytecode for your analysis? Bytecode for Foo needs to be provided (unless Foo is defined in the same source file), otherwise analyzer cannot deduce that type arguments are the same symbol.
How can I verify this? I would assume bytecode is available to the scanner due to these points:
We’re using the Maven plugin in the same way as the docs recommend.
Foo is part of another package that is compiled in another Maven module which is part of the same aggregator build, and the current module has a regular dependency to that module.
Note, however, that Foo also uses Lombok (specifically @Value, @Builder(toBuilder = true) and @NonNull) - then again, it still ends up as bytecode in a jar, so that fact should not really be relevant when scanning FooModifier, right?
You can run your maven command with -X , this will print more debug information (it’s quite verbose for big projects). When .class file is not found for some class it will print something like
[DEBUG] 09:47:18.688 .class not found for test.System
(note that currently there is minor issue that it prints such statements for packages, you can ignore that).
Also it is important how you launch sonar:sonar goal. It should be one of these two ways
[ edit: I forgot to post the -X, but I did include it in the command ]
No issue was raised with FooModifier, and no debug message indicated bytecode for Foo was missing (though I did see the harmless .class not found for com.example.s4276 messages you mentioned).
(Our automated build uses the same, just with deploy instead of verify, and without -X.)
Sorry, I did a mistake and overlooked the fact that we are talking about method parameter. The rule as it is currently implemented won’t raise issue on method parameter, because it only consider implements clause and variables with initializer.