GCX-HCI / ThirtyInch

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

ThirtyInch Lint Checks #100

Closed mannodermaus closed 5 years ago

mannodermaus commented 7 years ago

ThirtyInch has quickly become my favourite MVP implementation. Still, and I guess it's mostly due to my forgetful mind, nearly every time I implement a new Ti-based Activity, connect everything & run, I encounter the dreaded error I've grown so accustomed to: java.lang.IllegalArgumentException: This Activity doesn't implement a TiView interface. This is the default behaviour. Override provideView() to explicitly change this.

I'd like to propose some custom Lint checks that facilitate the daily workflow with ThirtyInch. I'm unsure how feasible this would be, especially since you allow this default behaviour to be changed on a per-Activity basis. The error would ideally be triggered on TiActivity & CompositeActivity sub-classes with the TiActivityPlugin applied, if no TiView interface is implemented.

Again, I'm probably the only one oblivious enough to always forget this step, and maybe the effort outweighs the benefit by a long shot.

passsy commented 7 years ago

Yeah that would be a great addition. This lint check could check if provideView() is overriden, if not it should check if the Activity/Fragment implements the View interface.

I must admit I have no idea how to write custom lint rule and bundle it within the aar.

passsy commented 7 years ago

For inspirations see timber lint checks

//cc @winterDroid @vanniktech

winterDroid commented 7 years ago

Can you provide some more information what issues should be found? I'm not really familiar with this library

vanniktech commented 7 years ago

Why was I cc'd into this?

passsy commented 7 years ago

@vanniktech You're familiar with lint and maybe interested in contributing 😉, ignore otherwise

@winterDroid This is the minimal setup when using a TiActivity

// Lint warning because MyActivity doesn't implement MyView 
public class MyActivity extends TiActivity<MyPresenter, MyView> {

    @Override
    public MyPresenter createPresenter() {
        return new MyPresenter();
    }
}

public class MyPresenter extends TiPresenter<MyView> {
    // boilerplate
}

interface MyView extends TiView {
    // boilerplate
}

It compiles fine but crashes at runtime because MyActivity doesn't implement MyView. Lint should warn here.

Correct MyActivity implementation:

// No warning, MyActivity implements MyView
public class MyActivity extends TiActivity<MyPresenter, MyView> implements MyView {

    @Override
    public MyPresenter createPresenter() {
        return new MyPresenter();
    }
}

But lint should not warn when the user has overridden provideView()

public class MyActivity extends TiActivity<MyPresenter, MyView> {

    @Override
    public MyPresenter createPresenter() {
        return new MyPresenter();
    }

    @Override
    public MyView provideView() {
        // provideView implemented, no warning required that MyActivity doesn't implement MyView
        return new MyView() { };
    }
}
vanniktech commented 7 years ago

I've got some own lint rules - https://github.com/vanniktech/lint-rules - the setup is pretty basic and straightforward. Happy to answer any questions. Also have a look at the tests to see how you can test your rule easily without having to build your checks and run them on a real project.

StefMa commented 6 years ago

I am just going to leave that here “Writing your first Lint check” @vanniktech https://medium.com/@vanniktech/writing-your-first-lint-check-39ad0e90b9e6

vanniktech commented 6 years ago

Ping me when you've got your first check for a review.