review: fix issue related with overridden methods retrieved from getAllMethods

[surli]

getAllMethods should not retrieve a parent method if an overridden method is defined, even if the signature is different.

For example, if a class implements the method public int compareTo(T o); with object it will have the following signature: public int compareTo(Object o) and getAllMethod should only retrieve this one.

[surli]

I may use your help on this one @pvojtechovsky : I'm trying to use ClassTypingContext but I get a lot of errors that I do not fully understand...

[pvojtechovsky]

I will have a look at it today evening

[pvojtechovsky]

There were bugs in ClassTypingContext. I have fixed them in #1379.

Rebase this PR after #1379 is merged, then the tests should pass.

[pvojtechovsky]

I suggest to move creation of ClassTypingContext above (out of the cycle). It will perform faster, because creation of ClassTypingContext is expensive and it is better to use cached information about super type hierarchy, then to create it always again.

Like this:

	@Override
	public Set<CtMethod<?>> getAllMethods() {
		final Set<String> distinctSignatures = new HashSet<>();
		final Set<CtMethod<?>> l = new SignatureBasedSortedSet<>();
		final ClassTypingContext ctc = new ClassTypingContext(CtTypeImpl.this);
		map(new AllTypeMembersFunction(CtMethod.class)).forEach(new CtConsumer<CtMethod<?>>() {
			@Override
			public void accept(CtMethod<?> method) {

				for (CtMethod<?> methods : l) {
					if (ctc.isSameSignature(methods, method)) {
						return;
					}
				}

				if (distinctSignatures.add(method.getSignature())) {
					l.add(method);
				}
			}
		});
		return Collections.unmodifiableSet(l);
	}

I would commit into this branch, but I have no permission.

[surli]

I suggest to move creation of ClassTypingContext above (out of the cycle).

Yes, this was my first move but as I got a lot of errors in tests, I tried inside the loop to be sure of the state of ClassTypingContext I will change that :)

[surli]

There were bugs in ClassTypingContext. I have fixed them in #1379.

Ok, I wasn't sure but I had this feeling indeed. I'll look at your PR right now.

[pvojtechovsky]

If typeParamRef.getDeclaration(); returns null then something is wrong ... may be it should throw SpoonException?

In what situtation it happens? Is it really correct to ignore that null?

[surli]

It was happening in LambdaTest.testCompileLambdaGeneratedBySpoon and MainTest.testTest. Apparently the problem is it tries to resolve R in public abstract R apply(T arg0); but it never reach the interface Function where it's defined.

[pvojtechovsky]

So it sounds like another bug of CtTypeParameterReference#getDeclaration()
And then I do not like the idea to ignore null here. It should throw SpoonExpeception instead.

[surli]

So it sounds like another bug of CtTypeParameterReference#getDeclaration()

Ok I'm working on it, it seems to be related with my recent changes with CtTypeParameter on method parameters.

And then I do not like the idea to ignore null here. It should throw SpoonExpeception instead.

I'll change that.

[surli]

This set should not be needed anymore.

[pvojtechovsky]

You are right! Please remove it too

[surli]

Actually I may have found an issue which prevent me from removing the set, see #1407. I propose to keep it as it is (it works well now) and to remove it later if needed following the change made for #1407

[monperrus]

I would first merge this, with a clean implementation, and maybe a temporary behavioral change highlighted in the test diff. And then, we can concentrate on #1407.