#2947: fix for wildcard bounded values.

[Zegveld]

Got the given example situation working and all other tests still pass.

• Add testcase with super wildcard.
• rename test package to be more generic for extends/super wildcard usage. maybe typebound instead of wildcardextends.

Already created the PR so that the fix is already reviewable. (Only planning on adding more tests not changing more code)

Fixes #2947

[filiphr]

I also had a look at this one when I was analysing it and it was tricky, because on the first step like you've done now it is being fixed. However, I think that if you do @Mapping( target = "object", source = "object.contents" ) it will no longer work, or something similar to that. I think once you do without bounds for <?> you get Object type for ?. However, it is worth exploring ways to fix this properly.

[filiphr]

@Zegveld I really dislike the fix in this PR. It is way to intrusive and changes a lot of things that shouldn't really be affected. I'll give it a go and try to see if I can come up with another fix for this one.

[Zegveld]

The one bit I don't like myself about the current change is the "?" comparison bit at the Filters class.
Though the changes at Type feel logical to me.

As a result of the changes an parameter at TypeFactory.getReturnType(...) became obsolete which resulted in the changes there.

[filiphr]

Though the changes at Type feel logical to me.

All that looks way too error prone. We should create a correct accessor with a correct type and not pass this things down to the Type to resolve it. When @Mapping is not used everything works fine, so we need to only fix it in the creation of the SourceReference. Perhaps we need to tackle #2070 once and for all :)

[Zegveld]

Seeing as tackling #2070 would indeed mean that the code there would change again, I've done some more rework and moved all the required actions onto the Filters class. At the same time I've refactored the "?" comparison away. Still not 100% happy with the new result but it is improving.

[sjaakd]

I guess I need to dig in to this material as well. I'll try to have a look sometime this week..

[Zegveld]

I believe the current state is the best I can do atm. if you have any idea's for improvements I'm open to them.

[sjaakd]

Its a while ago.. but for me this: http://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html was really useful. I'll have a better look at the case soon.

[filiphr]

I really do not feel comfortable removing one of the methods from the TypeFactory. Imagine if we have something like:

public interface OtherWildCardInterface extends WildcardInterface {
}

@Mapper
public interface WildcardNestedInheritedExtendsMapper {
    WildcardNestedInheritedExtendsMapper INSTANCE = Mappers.getMapper( WildcardNestedInheritedExtendsMapper.class );

    @Mapping( target = "object", source = "contained.value")
    Target map(SourceContainerInherited<? extends OtherWildCardInterface> source);

    default String mapToString(WildcardInterface rawData) {
        return rawData.getContents();
    }

    default String mapToString(OtherWildCardInterface rawData) {
        return rawData.getContents() + "other";
    }
}

Or perhaps another raw types that need to be returned in the scope of the currently declared type. There might be some other edge cases where we resolve these things.

---

In addition to that in the Filters we have

        if ( returnType.getKind() == TypeKind.WILDCARD ) {
            if ( ( (WildcardType) returnType ).getExtendsBound() != null ) {
                return ( (WildcardType) returnType ).getExtendsBound();
            }
            return ( (TypeVariable) executableElement.getReturnType() ).getUpperBound();
        }

I am confused by that. We check the returnType and if that does not have an extends bound we assume that we can cast the executable element to TypeVariable.

---

I'll need some more time to process this changes and try to wrap my head around them

[Zegveld]

with SourceContainerInherited<? extends OtherWildCardInterface> then the type returned is OtherWildCardInterface, because the returnType of the container internal method is a WILDCARD with an extendsBound, so that extendsBound is chosen.

I'll move the Issue2677Mapper and corresponding test to the location where the new tests are as well. Then this case is also visible in the code-review.

---

concerning the Filters bit returnType.getKind() == TypeKind.WILDCARD can only be true when executable.getReturnType() is a TypeVariable.

It however is not true that because executable.getReturnType() is a TypeVariable then the returnType.getKind() must be a wildcard. There are other types that also fit the TypeVariable grouping, like TypeKind.TYPEVAR.

---

Think I'll also add some inline comments as to how and why the code works like this, and which situation is covered by what line.

[sjaakd]

LGTM.. some naming remarks..

[sjaakd]

I guess this can only be if the method is generic. Because the getWithinContext would already have tackled the class defined type-var's right?

[Zegveld]

That is correct.

[sjaakd]

I understand that this is the base for a wildcard.. But the interface itself does not contain wildcards. Hence the name is somewhat confusing. Would UpperBound be a better name?

[sjaakd]

Or WildCardBase or just WildCard

[Zegveld]

went with WildCard and WildCardImpl

[sjaakd]

same here: UpperBoundImpl?

[Zegveld]

went with WildCard and WildCardImpl