GCX-HCI / ThirtyInch

a MVP library for Android favoring a stateful Presenter
Apache License 2.0
1.03k stars 101 forks source link

Added Lint Check for missing TiView implementation #111

Closed mannodermaus closed 5 years ago

mannodermaus commented 7 years ago

This is an implementation based on the proposal made in #100. It introduces custom lint checks for ThirtyInch with a UAST-based Detector that seeks to point out illegal configuration of TiActivity, TiFragment, CompositeActivity & CompositeFragment sub-classes. Basically, it works like this:

Note that there is an exception to this rule for Ti- classes, wherein a user can override TiViewProvider#provideView() to deviate from the default behaviour. The Detector verifies that, and doesn't report an issue on sub-classes that also override this method.

Integration with CompositeAndroid works in a more low-level fashion, where the default constructor of the Composite- sub-class is analyzed for its registration of a TiPlugin via addPlugin(). If found, the TiView is extracted from there.

Please take a close look at the behaviour of the Detector, and also at potential loopholes inside the unit tests.

mannodermaus commented 7 years ago

Thanks for the initial feedback! I'd love to rework the module into Kotlin, as it would absolutely allow for cleaner structure in several places.

mannodermaus commented 7 years ago

I have separated the Detector into two distinct cases for Ti and CompositeAndroid. The implementations themselves are in Kotlin now, as well. The only remaining piece of Java code is present in the unit tests for the Detectors, pretty much only because of IJ's nice syntax highlighting on @Language("JAVA") strings. Ready for review!

mannodermaus commented 7 years ago

Unsure about the NoClassDefFoundError, but it seems like the bundled Lint API in that project is running on an older (and most likely, stable) version, either because of the IDE itself or the Gradle plugin. What versions of AS and the Android Gradle Plugin are you experiencing that on?

StefMa commented 7 years ago

I am not on my work macbook anymore. So I don't know it exactly. But I use the latest stable of android studio 2.3.3 I think 🤔?! And I use the same gradle plugin as you do since it is defined in the build.gradle ☺️

Sent from my Google Nexus 5X using FastHub

StefMa commented 6 years ago

@aurae I've tried to it again with AS 3.0-Beta5. I don't get the NoClassDefFoundError anymore.

But unfortunately I don't see the lint warning. Which I've already mentioned here https://github.com/grandcentrix/ThirtyInch/pull/111#pullrequestreview-54343610

I've also tested it with AS 3.0-Beta5 with the new plugin (3.0-Beta5 of course :)): And it doesn't work...

passsy commented 6 years ago

Also interesting: https://github.com/googlesamples/android-custom-lint-rules/pull/10

mannodermaus commented 6 years ago

Great find @passsy, let's keep an eye on that.

Unfortunately, I'm currently unable to attend to this PR myself, so I'm unsure as to why the rule wouldn't trigger in its current state. I'm sure we will be able to fix that soon, though!

mannodermaus commented 5 years ago

Superseded by #158