mebigfatguy / fb-contrib

a FindBugs/SpotBugs plugin for doing static code analysis for java code bases
http://fb-contrib.sf.net
GNU Lesser General Public License v2.1
157 stars 45 forks source link

FII_USE_FUNCTION_IDENTITY false positive #471

Open nmatt opened 7 months ago

nmatt commented 7 months ago

FII_USE_FUNCTION_IDENTITY is shown on the STRING constant in the following example:

interface Parser<T>
{
    T parse(String value);

    static final Parser<String> STRING = s -> s;
}

Although specifying

    static final Parser<String> STRING = Function.<String> identity()::apply;

would work, I'd argue that this doesn't make sense here.

ThrawnCA commented 7 months ago

I'd argue that this doesn't make sense here.

It would create one less object, so...present your argument, I guess?

nmatt commented 7 months ago

@ThrawnCA: In both cases, an instance of the (super)class Parser has to be created. So I don't understand why there would be a difference in the number of objects created.

FII_USE_FUNCTION_IDENTITY makes sense for cases like:

    static final Function<String, String> STRING = Function.identity();

i.e. where the existing Function object can be assigned as-is.

To illustrate, the following prints "4":

    Parser<String> a = s -> s;
    Parser<String> b = Function.<String> identity()::apply;
    Function<String, String> c = Function.identity();
    Function<String, String> d = Function.<String> identity()::apply;
    System.out.println(new HashSet<>(asList(a, b, c, d)).size());

This also means that even in d a separate object is created, due to the ::apply indirection. This indirection is necessary in the Parser case, because Function is not assignable to Parser.