-
Notifications
You must be signed in to change notification settings - Fork 580
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
Add android nullness annotations #182
Conversation
1835823
to
d26e1e1
Compare
spotbugs-tests/build.gradle
Outdated
|
||
compileOnly 'com.apple:AppleJavaExtensions:1.4' | ||
compile 'junit:junit:4.12' |
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 dependency will come from test-harness
spotbugs-tests/build.gradle
Outdated
// This seem to work with most tests except the new one based on AnalysisRunner | ||
// TODO fix AnalysisRunner or the test classpath | ||
classpath = project.sourceSets.main.runtimeClasspath \ | ||
+ project(':spotbugs').sourceSets.main.runtimeClasspath \ |
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 true way of referencing classes is adding dependency on a project
@@ -214,7 +214,6 @@ private void setUpEngine(String... analyzeMe) throws IOException { | |||
} | |||
} | |||
|
|||
project.addAuxClasspathEntry("lib/junit.jar"); |
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 is not necessary as soon as junit is in classpath via project dependencies
@@ -236,7 +235,9 @@ private void setUpEngine(String... analyzeMe) throws IOException { | |||
String[] cpParts = classpath.split(File.pathSeparator); | |||
for (String cpStr : cpParts) { | |||
File file = new File(cpStr); | |||
project.addAuxClasspathEntry(file.getCanonicalPath()); |
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.
classpath might contain empty directories witch make project unhappy
Changes Unknown when pulling 8fda833 on kzaikin:android-nullness into ** on spotbugs:master**. |
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.
- update CHANGELOG.md
- merge latest
master
branch and resolve conflict
@@ -4,6 +4,10 @@ This is the changelog for SpotBugs. This follows [Keep a Changelog v0.3](http:// | |||
|
|||
## Unreleased (2017/??/??) | |||
|
|||
### Added | |||
|
|||
* Make TypeQualifierResolver recognize android.support.annotation.NonNull and Nullable ([#182](https://github.com/spotbugs/spotbugs/pull/182)) |
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.
added it
Should anything else be done to have this merged? |
@Retention(CLASS) | ||
@Target({METHOD, PARAMETER, FIELD, ANNOTATION_TYPE, PACKAGE}) | ||
public @interface NonNull { | ||
} |
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.
Confirmed that this code has same Retention policy with https://github.com/android/platform_frameworks_support/blob/master/annotations/src/android/support/annotation/NonNull.java
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.
LGTM
This change is really great. Unfortunately the existing gradle plugin in version 1.1 still points to RC2 which makes this change unavailable to Android builds, right? |
@scheffield You can use http://spotbugs.readthedocs.io/en/latest/migration.html#findbugs-gradle-plugin |
The pull request is built from presumably small commits
Commit 1 adds tests on existing NullnessAnnotations inside spotbugs gradle project
Commit 2 adds android annotations to NullnessAnnotations along with a small refactoring (we already have tests so it's safe)
Commit 3 adds android annotations to TypeQualifierResolver and a few integration tests using SpotBugsRule approach
Fix #96