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

@Nullable / @NonNull #73

Closed
BenoitDuffez opened this Issue Apr 19, 2017 · 19 comments

Comments

Projects
None yet
2 participants
@BenoitDuffez

I use these annotations a LOT and I don't think you use them. Would you consider adding them?

For example, Box#put supports null collections but doesn't say so:

public void put(Collection<T> entities) {
    if(entities != null && !entities.isEmpty()) { // yay!
        Cursor cursor = getWriter();
        try {
            Iterator i = entities.iterator();
            while(i.hasNext()) {
                cursor.put(i.next());
            }
           commitWriter(cursor);
        } finally {
           releaseWriter(cursor);
        }
    }
}
@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 21, 2017

Member

I guess you are referring to Android support annotations. The thing is that the project itself is a plain Java project without any Android dependencies.

An alternative would be IntelliJ's annotations, however. (Or JSR-305 implementation by FindBugs).

Member

greenrobot commented Apr 21, 2017

I guess you are referring to Android support annotations. The thing is that the project itself is a plain Java project without any Android dependencies.

An alternative would be IntelliJ's annotations, however. (Or JSR-305 implementation by FindBugs).

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 21, 2017

Yup, I forgot about that. Nevermind then.

Yup, I forgot about that. Nevermind then.

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 21, 2017

Member

@BenoitDuffez Actually, it's a good thing to do. I'm going with JSR-305 implementation by FindBugs ('com.google.code.findbugs:jsr305:3.0.2'), which should indicate problems also in IntelliJ/AS.

Member

greenrobot commented Apr 21, 2017

@BenoitDuffez Actually, it's a good thing to do. I'm going with JSR-305 implementation by FindBugs ('com.google.code.findbugs:jsr305:3.0.2'), which should indicate problems also in IntelliJ/AS.

@greenrobot greenrobot added this to the 0.9.11 milestone Apr 21, 2017

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 21, 2017

Great news! Maybe reopen?
Cheers

Great news! Maybe reopen?
Cheers

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 21, 2017

Member

@BenoitDuffez I'm just finishing this, so no need to reopen. Will ship with next release.

Member

greenrobot commented Apr 21, 2017

@BenoitDuffez I'm just finishing this, so no need to reopen. Will ship with next release.

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 27, 2017

Member

PS.: @BenoitDuffez Did you check the release? Does it work well for you?

Member

greenrobot commented Apr 27, 2017

PS.: @BenoitDuffez Did you check the release? Does it work well for you?

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 27, 2017

Didn't notice the release. It looks great! Will reopen if I have more suggestions regarding this.

Thanks a lot!!!

Didn't notice the release. It looks great! Will reopen if I have more suggestions regarding this.

Thanks a lot!!!

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 27, 2017

gradle started to complain about a version mismatch for the findbugs lib.
Is it a dependency of your lib?

I had to add this because it wouldn't build:

    resolutionStrategy.force 'com.google.code.findbugs:jsr305:1.3.9'

Since I think it's related, I have reopened. Please close if it's not accurate.

gradle started to complain about a version mismatch for the findbugs lib.
Is it a dependency of your lib?

I had to add this because it wouldn't build:

    resolutionStrategy.force 'com.google.code.findbugs:jsr305:1.3.9'

Since I think it's related, I have reopened. Please close if it's not accurate.

@BenoitDuffez BenoitDuffez reopened this Apr 27, 2017

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 27, 2017

Here are my test dependencies, if that's relevant:

testCompile 'junit:junit:4.12'
testCompile 'org.powermock:powermock:1.6.5'
testCompile 'org.powermock:powermock-module-junit4:1.6.5'
testCompile 'org.powermock:powermock-api-mockito:1.6.5'
testCompile 'com.squareup.okhttp3:mockwebserver:3.6.0'

The IDE complained about a mismatch between release/debug builds and test builds.
Here's the full message:

Error:Conflict with dependency 'com.google.code.findbugs:jsr305' in project ':app'. Resolved versions for app (3.0.2) and test app (2.0.1) differ. See http://g.co/androidstudio/app-test-app-conflict for details.

Here are my test dependencies, if that's relevant:

testCompile 'junit:junit:4.12'
testCompile 'org.powermock:powermock:1.6.5'
testCompile 'org.powermock:powermock-module-junit4:1.6.5'
testCompile 'org.powermock:powermock-api-mockito:1.6.5'
testCompile 'com.squareup.okhttp3:mockwebserver:3.6.0'

The IDE complained about a mismatch between release/debug builds and test builds.
Here's the full message:

Error:Conflict with dependency 'com.google.code.findbugs:jsr305' in project ':app'. Resolved versions for app (3.0.2) and test app (2.0.1) differ. See http://g.co/androidstudio/app-test-app-conflict for details.
@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 28, 2017

Member

Would your tests work if you would put this into your deps?
testCompile 'com.google.code.findbugs:jsr305:3.0.2'

Member

greenrobot commented Apr 28, 2017

Would your tests work if you would put this into your deps?
testCompile 'com.google.code.findbugs:jsr305:3.0.2'

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 28, 2017

Actually the tests work, there is just a gradle warning about that. The problem is that the IDE (Android Studio) considers this blocking enough to stop the compilation and apk deployment.

e.g. ./gradlew clean compileDebugSources test is successful: compilation yields the warning and the tests run and are successful.
Here's the warning in the gradle wrapper console:

WARNING: Conflict with dependency 'com.google.code.findbugs:jsr305' in project ':app'. Resolved versions for app (3.0.2) and test app (2.0.1) differ. See http://g.co/androidstudio/app-test-app-conflict for details.

The findbugs lib is embedded in espresso:

androidTestCompile('com.android.support.test.espresso:espresso-core:2.2.2', {
    exclude group: 'com.android.support', module: 'support-annotations'
})

and they use 2.0.1:

androidTestCompile - Classpath for compiling the androidTest sources.
\--- com.android.support.test.espresso:espresso-core:2.2.2
     +--- com.squareup:javawriter:2.1.1
     +--- com.android.support.test:rules:0.5
     |    \--- com.android.support.test:runner:0.5
     |         +--- junit:junit:4.12
     |         |    \--- org.hamcrest:hamcrest-core:1.3
     |         \--- com.android.support.test:exposed-instrumentation-api-publish:0.5
     +--- com.android.support.test:runner:0.5 (*)
     +--- javax.inject:javax.inject:1
     +--- org.hamcrest:hamcrest-library:1.3
     |    \--- org.hamcrest:hamcrest-core:1.3
     +--- com.android.support.test.espresso:espresso-idling-resource:2.2.2
     +--- org.hamcrest:hamcrest-integration:1.3
     |    \--- org.hamcrest:hamcrest-library:1.3 (*)
     +--- com.google.code.findbugs:jsr305:2.0.1
     \--- javax.annotation:javax.annotation-api:1.2

Adding the line you gave me to my app gradle build script does not remove the warning.

Actually the tests work, there is just a gradle warning about that. The problem is that the IDE (Android Studio) considers this blocking enough to stop the compilation and apk deployment.

e.g. ./gradlew clean compileDebugSources test is successful: compilation yields the warning and the tests run and are successful.
Here's the warning in the gradle wrapper console:

WARNING: Conflict with dependency 'com.google.code.findbugs:jsr305' in project ':app'. Resolved versions for app (3.0.2) and test app (2.0.1) differ. See http://g.co/androidstudio/app-test-app-conflict for details.

The findbugs lib is embedded in espresso:

androidTestCompile('com.android.support.test.espresso:espresso-core:2.2.2', {
    exclude group: 'com.android.support', module: 'support-annotations'
})

and they use 2.0.1:

androidTestCompile - Classpath for compiling the androidTest sources.
\--- com.android.support.test.espresso:espresso-core:2.2.2
     +--- com.squareup:javawriter:2.1.1
     +--- com.android.support.test:rules:0.5
     |    \--- com.android.support.test:runner:0.5
     |         +--- junit:junit:4.12
     |         |    \--- org.hamcrest:hamcrest-core:1.3
     |         \--- com.android.support.test:exposed-instrumentation-api-publish:0.5
     +--- com.android.support.test:runner:0.5 (*)
     +--- javax.inject:javax.inject:1
     +--- org.hamcrest:hamcrest-library:1.3
     |    \--- org.hamcrest:hamcrest-core:1.3
     +--- com.android.support.test.espresso:espresso-idling-resource:2.2.2
     +--- org.hamcrest:hamcrest-integration:1.3
     |    \--- org.hamcrest:hamcrest-library:1.3 (*)
     +--- com.google.code.findbugs:jsr305:2.0.1
     \--- javax.annotation:javax.annotation-api:1.2

Adding the line you gave me to my app gradle build script does not remove the warning.

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 29, 2017

Member

Ah right it's "androidTestCompile", how about this then?

androidTestCompile 'com.google.code.findbugs:jsr305:3.0.2'
Member

greenrobot commented Apr 29, 2017

Ah right it's "androidTestCompile", how about this then?

androidTestCompile 'com.google.code.findbugs:jsr305:3.0.2'
@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Apr 29, 2017

Member

Anyway dependency conflicts are always bad - I am almost considering removing the annotations again...

Member

greenrobot commented Apr 29, 2017

Anyway dependency conflicts are always bad - I am almost considering removing the annotations again...

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez Apr 29, 2017

No I just needed to understand the dependencies I use.

Do you require findbugs v3 or would you be fine with either v2 or v3? I'm guessing you have a hard requirement of v3+.

I'm not on my computer right now so I won't be able to test with the new instructions. I'll keep you updated today (hopefully).

No I just needed to understand the dependencies I use.

Do you require findbugs v3 or would you be fine with either v2 or v3? I'm guessing you have a hard requirement of v3+.

I'm not on my computer right now so I won't be able to test with the new instructions. I'll keep you updated today (hopefully).

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez May 1, 2017

Sorry about the delay. This makes the warning go away:

androidTestCompile 'com.google.code.findbugs:jsr305:3.0.2'

That's fine with me. Thanks for your support.

Sorry about the delay. This makes the warning go away:

androidTestCompile 'com.google.code.findbugs:jsr305:3.0.2'

That's fine with me. Thanks for your support.

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez May 2, 2017

I guess we can close. Thanks.

I guess we can close. Thanks.

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot May 5, 2017

Member

Not sure about that jsr305 version conflict - maybe I'll just go with custom objectbox annotations. That would be documentation only without IDE/FindBugs support.

Member

greenrobot commented May 5, 2017

Not sure about that jsr305 version conflict - maybe I'll just go with custom objectbox annotations. That would be documentation only without IDE/FindBugs support.

@greenrobot greenrobot reopened this May 5, 2017

@BenoitDuffez

This comment has been minimized.

Show comment
Hide comment
@BenoitDuffez

BenoitDuffez May 5, 2017

One can teach the IDE to consider annotation to be null/nonnull, so that it's more than just documentation, but also warnings.

One can teach the IDE to consider annotation to be null/nonnull, so that it's more than just documentation, but also warnings.

@greenrobot

This comment has been minimized.

Show comment
Hide comment
@greenrobot

greenrobot Jun 2, 2017

Member

I tried to use compileOnly for jsr305. This seems to break kapt rather quickly. Also, annotations are already used in generated code / super classes of generated code. Thus, it seems simpler to stick with a compile dependency. Added a troubleshooting entry in the FAQ for this.

Member

greenrobot commented Jun 2, 2017

I tried to use compileOnly for jsr305. This seems to break kapt rather quickly. Also, annotations are already used in generated code / super classes of generated code. Thus, it seems simpler to stick with a compile dependency. Added a troubleshooting entry in the FAQ for this.

@greenrobot greenrobot closed this Jun 2, 2017

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