-
Notifications
You must be signed in to change notification settings - Fork 236
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate lint check to new PSI API #130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @felipecsl!
I made a few comments on code style and the fact that we can't upgrade the gradle build plugin to 2.2.2 yet.
@@ -5,7 +5,7 @@ buildscript { | |||
jcenter() | |||
} | |||
dependencies { | |||
classpath 'com.android.tools.build:gradle:2.1.3' | |||
classpath 'com.android.tools.build:gradle:2.2.2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This upgrade is not possible at the moment, Robolectric 3.0 does not work with gradle build plugin 2.2.2 and upgrading Robolectric to 3.1.4 causes a conflict with PowerMock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm weird, we have Robolectric 3.0 working fine with the plugin v2.2.2, anyways this is unrelated so I'll just revert it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is probably an Android application vs library. The location of the merged manifest changed in 2.2.2 and Robolectric has the path hard coded to the old path in 3.0 for libraries.
|
||
private static final List<String> METHODS = Collections.emptyList(); | ||
|
||
public class BetaDetector extends Detector implements Detector.JavaPsiScanner { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline after this.
|
||
public class BetaDetector extends Detector implements Detector.JavaPsiScanner { | ||
// This would normally be private final, but has exposed here for testing purposes, since there | ||
// seems to be no way to inject it via the Detector constructor using LintDetectorTest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems fine, I don't think it really needs a comment.
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
|
||
public class BetaDetectorTest extends LintDetectorTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newline here.
PR updated |
would it be possible to get this out in a SNAPSHOT release of the library? |
I've released this in |
Since the latest release is |
Also I can't find |
Sorry about the wait @felipecsl, I've just returned after some leave. No, we don't currently version snapshots like that since we don't know the next version number at the time, but it is a bit backwards compared to most libraries' snapshot versioning. I've released I'll make a full release soon. |
Running lint with this change spits out quite a bit of noise because of missing classes and results in the custom rules not being loaded.
Since the custom rules aren't currently being used for anything and I haven't seen any workable fix for this, I've just removed them for now. The 2.4.0 release is going out right now with these changes. |
Custom lint checks using the old and deprecated Lombok AST API cause an incompatibility between the Android lint and Retrolambda as described here evant/gradle-retrolambda#96
Migrating to the new API fixes the issue.