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

FII_USE_METHOD_REFERENCE false positives #282

Closed
boris-petrov opened this issue May 12, 2018 · 5 comments
Closed

FII_USE_METHOD_REFERENCE false positives #282

boris-petrov opened this issue May 12, 2018 · 5 comments

Comments

@boris-petrov
Copy link

boris-petrov commented May 12, 2018

This gives the warning:

map.computeIfAbsent("key", _unused -> object.getValue());

Which is wrong. Also this:

ImmutableMap.Builder<Object, ImmutableList.Builder<Object>> bar =
	ImmutableMap.builder();

IImmutableMap<Object, IImmutableList<Object>> foo =
	bar.map(builder -> (IImmutableList<Object>) builder.build()).build();

In the second line, in the lambda, the code builder.build() returns an ImmutableList<Object> rather than an IImmutableList<Object> and hence removing the cast makes the code not compile. The cast is needed and therefore this lambda cannot be made a method-reference.

P.S. You can imagine some implementations for class Immutable* and interface IImmutable*.

@mebigfatguy
Copy link
Owner

mebigfatguy commented May 12, 2018

Thanks for the report. I believe the first issue is now fixed.

As for the second report, I get it that a cast will eliminate the possibility of methodreference use, but i am unclear as to how to reproduce your example, as mine doesn't report.

I would use yours directly, but not sure how to produce IImmutableMap, etc, (assuming that ImmutableMap is from guava) as i can't create that class because of constructor visibility... unless i'm confused what's going on.

Could you show an example of what those classes are?

@boris-petrov
Copy link
Author

Sorry, I should have noted that the second example is not using Guava. Anyway, here is a complete reproduction:

import java.util.function.Function;

public class Test {
	public IImmutableMap<Object, IImmutableList<Object>> test() {
		ImmutableMap.Builder<Object, ImmutableList.Builder<Object>> bar =
			ImmutableMap.builder();

		return bar.map(builder -> (IImmutableList<Object>) builder.build()).build();
	}
}

@SuppressWarnings("unused")
interface IImmutableMap<K, V> {
}

class ImmutableMap<K, V> implements IImmutableMap<K, V> {
	public static <K, V> Builder<K, V> builder() {
		return new Builder<>();
	}

	public static final class Builder<K, V> {
		public ImmutableMap<K, V> build() {
			return new ImmutableMap<>();
		}

		public <L> Builder<K, L> map(@SuppressWarnings("unused") Function<V, L> function) {
			return new Builder<>();
		}
	}
}

@SuppressWarnings("unused")
interface IImmutableList<E> {
}

class ImmutableList<E> implements IImmutableList<E> {
	public static <E> Builder<E> builder() {
		return new Builder<>();
	}

	public static final class Builder<E> {
		public ImmutableList<E> build() {
			return new ImmutableList<>();
		}
	}
}

@mebigfatguy
Copy link
Owner

mebigfatguy commented May 13, 2018

Thanks!

that's an interesting case, i'm not sure why the cast is necessary (i agree that it is), as in the generated code no cast is produced

private static IImmutableList lambda$testBB$0(ImmutableList$Builder);
descriptor: (LImmutableList$Builder;)LIImmutableList;
flags: ACC_PRIVATE, ACC_STATIC, ACC_SYNTHETIC
Code:
stack=1, locals=1, args_size=1
0: aload_0
1: invokevirtual #6 // Method ImmutableList$Builder.build:()LImmutableList;
4: areturn
LineNumberTable:
line 8: 0

@boris-petrov
Copy link
Author

A cast is needed according to the Eclipse compiler, not sure about javac. I'm not sure either and I don't know if you can actually handle that case. :)

mebigfatguy added a commit that referenced this issue May 13, 2018
@mebigfatguy
Copy link
Owner

Thanks, should be fixed in head, the fix might over correct, but basically if the return value is not the same as the method declared return type, don't report. I don't really understand what javac is doing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants