Feature: Lint Checks - Githubissues

GCX-HCI / ThirtyInch

a MVP library for Android favoring a stateful Presenter

Apache License 2.0

1.03k stars 101 forks source link

Feature: Lint Checks #158

Closed jreehuis closed 5 years ago

jreehuis commented 6 years ago

Description

This implementation adopts and updates #111. Fixes #100

The updates include:

Using the now stable Lint version to use UAST
Bundles the Lint checks with TI
Fixing the detection scope so Android Studio will report the error in the editor
Creating an own TI Lint Category
TiIssue is now a sealed class
some Kotlin tweaks

How to Test

Play with the following scenarios:

Remove the implements HelloWorldView in the HelloWorldActivity of the sample app
Remove the implements HelloWorldView but override provideView() in the HelloWorldActivity of the sample app
Use TI v0.9.1-SNAPSHOT of this branch in another project and have an TiActivity which does not implement the given TiView subclass

jreehuis commented 5 years ago

@StefMa I addressed all your comments. Would be great if you could have another look on this PR. :)

StefMa commented 5 years ago

Hey, sure. After I am back from my sick leave I will take a look into it 👍

StefMa commented 5 years ago

Works 🎉 bildschirmfoto 2018-09-17 um 10 23 26

StefMa commented 5 years ago

What does not work:

abstract class GalleryDetailsActivity : TiActivity<GalleryDetailsPresenter, GalleryDetailsView>()

class CameraGalleryDetailsActivity : GalleryDetailsActivity() {

No Lint issue will be shown :(

StefMa commented 5 years ago

Works also with Java 🤘 bildschirmfoto 2018-09-17 um 10 31 16

StefMa commented 5 years ago

Something like this shouldn't be an issue: bildschirmfoto 2018-09-17 um 10 33 05

Ok, maybe this is not an issue because it will never happen in real world

StefMa commented 5 years ago

Other than that it looks great 🎉 Thanks @jreehuis

jreehuis commented 5 years ago

Thanks for the extensive tests! 😃 Especially the abstract class scenario will get complicated... 😡 I will see to solve it. But definitely a real world case. Thanks for pointing to it.

jreehuis commented 5 years ago

Something like this shouldn't be an issue:

Ok, maybe this is not an issue because it will never happen in real world

In my view this is correct. StuffActivity could be also used separately and so we should highlight this error.

jreehuis commented 5 years ago

What does not work:

abstract class GalleryDetailsActivity : TiActivity<GalleryDetailsPresenter, GalleryDetailsView>()

class CameraGalleryDetailsActivity : GalleryDetailsActivity() {

No Lint issue will be shown :(

OK, wasn't that hard than expected. This scenario should also be covered now.

jreehuis commented 5 years ago

All done.

StefMa commented 5 years ago

Issue created as discussed. Merged ✅

Thank you @jreehuis 🤘