Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

ThirtyInch Lint Checks #100

Closed
mannodermaus opened this issue Jun 15, 2017 · 8 comments
Closed

ThirtyInch Lint Checks #100

mannodermaus opened this issue Jun 15, 2017 · 8 comments

Comments

@mannodermaus
Copy link

mannodermaus commented Jun 15, 2017

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
Copy link
Contributor

passsy commented Jun 15, 2017

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
Copy link
Contributor

passsy commented Jun 15, 2017

For inspirations see timber lint checks

//cc @winterDroid @vanniktech

@winterDroid
Copy link

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

@vanniktech
Copy link

Why was I cc'd into this?

@passsy
Copy link
Contributor

passsy commented Jun 15, 2017

@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
Copy link

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
Copy link
Contributor

StefMa commented Dec 23, 2017

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
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants