#1742 refactoring and making builder optional

[sjaakd]

Currently the accessor type is defined in PropertyMapper while it should be an intrinsic property of Accessor. Types are fetched and derived in context (again) all-over-the-place.. Sometimes that requires the parent type again and even the mapping method (to derive in context and resolve generics).

Next, builderType is part of type. One could say there is a relation from builderType to Type but not the other way around. A builderType could be in a completely different package. It's still not as I want.. But I managed to move the builderType (mostly) to the place were its relevant: the BeanMappingMethod.

Done!

I'm still not completely happy with the number of places were I do need to consult the typeFactory for the effectiveType or the builderType. This indicates a bad separation of concerns. Probably stemming from the fact that we do analyse a nested target and try to find the associated types in TargetReference. I think that in the end TargetReference and SourceReference should be handled in the BeanMappingMethod and not in Mapping upfront but in the context of the BeanMappingMethod.

I experimented with making annotations and annotation fields Optional. I think we should apply this pattern, because we keep on doing null checks all over the place.

Fixes #1742

[filiphr]

Wow @sjaakd this is an amazing PR 😄.

All your points are on point.

Regarding the SourceReference and TargetReference they are already used mostly within a BeanMappingMethod context. They are derived from the @Mapping that is mostly used for bean mapping methods. Or am I missing something? In any case you are completely right about the separation of the concerns.

I experimented with making annotations and annotation fields Optional. I think we should apply this pattern, because we keep on doing null checks all over the place.

IMP doing a not null check vs Optional#isPresent there is not a big difference.

I've added some comments to the PR. My "biggest" concern is the naming of the noBuilder. For some reason it doesn't stick with me. How about disableBuilder? Maybe something else, I am open to suggestions.

[filiphr]

I forgot to mention. This PR would also solve #1661, right?

[sjaakd]

I forgot to mention. This PR would also solve #1661, right?

it does..

[filiphr]

For #1661 let's also add an annotation processor option. So this would partially solve it.

[filiphr]

Thanks for the updates. LGTM for merging

[chris922]

I just had a quick glance at your changes, looks awesome. Thanks @sjaakd !