auto-matter
auto-matter copied to clipboard
danielnorberg
Refactor processor to make extension easier
This delegates most of the field-specific processing to FieldProcessor,
which can be implemented to customize handling of different types.
Collections are handled by CollectionProcessor, optionals by
OptionalProcessor. Everything else is handled by DefaultProcessor.
A data-driven approach was taken, with FieldProcessor taking the
descriptor and field as input and outputting Javapoet specs or
statements. However, in this patch CollectionProcessor is dependent on
AutoMatterProcessor for the implementation of singular.
There is also some duplication and inefficiencies in the processor as a result of this refactor. Since doing away with them would change text order in some places and thus require edits to most of the test files, I prefer leaving that work to a followup commit to reduce review burden.
Other notes:
-
It's not clear where common code should live. For now most of it is in default methods on
FieldProcessor, but I'm open to suggestions. -
Due to the above use of default, source level 1.8 is now required
-
I wasn't sure how to organize the classes, so I've left them all at top level. It might become clearer once common code is centralized and other processors are added.
-
Not sure if this is feasible, but it could be interesting to allow users to supply their own processors via some annotation, or to register them somehow with AutoMatter. This would allow extensions to live in separate packages.
This is a much-needed refactoring. Kudos!
Looks great overall, just a few small comments.
- Eliminate circular imports and dependencies.
- Everything should be package-private or private.
- Leave everything at top level for now.
Allowing extensions in separate packages at sounds great. AutoMatterProcessor can use j.u.ServiceLoader to discover extensions and extensions can use AutoService to register themselves. Should be straightforward but let's do it in a separate PR.
Re: using Map<Class, FieldProcessor> instead of string, that would require having the necessary classes in the classpath wouldn't it? Maybe we can do ClassName instead?