2.x: Feature/#4876 more null checks #5055
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add assertion to help static code analysis
jschneider
force-pushed
the
feature/#4876-more-null-checks
branch
from
49d7a0c
to
79e9855
Compare
akarnokd
changed the title
Codecov Report@@ Coverage Diff @@
## 2.x #5055 +/- ##
============================================
+ Coverage 95.53% 95.58% +0.05%
- Complexity 5540 5544 +4
============================================
Files 612 612
Lines 39576 39577 +1
Branches 5553 5554 +1
============================================
+ Hits 37807 37831 +24
+ Misses 775 754 -21
+ Partials 994 992 -2
Continue to review full report at Codecov.
|
akarnokd
requested changes
Feb 3, 2017
| @@ -73,6 +76,7 @@ public void subscribeActual(Subscriber<? super R> s) { | |||
| Publisher<? extends T>[] a = array; | |||
| int n; | |||
| if (a == null) { | |||
| assert iterable != null; //either array or iterable are initialized with non null values | |||
There was a problem hiding this comment.
Asserts are off by default but they increase the bytecode size; besides we don't use assert it anyway.
There was a problem hiding this comment.
ok. removed it.
Do you have a standard way to suppress false positives?
| @@ -246,6 +246,7 @@ public void onNext(T t) { | |||
|
|
|||
| if (i == size) { | |||
| window = null; | |||
| assert w != null; | |||
| super(source); | ||
| this.otherArray = otherArray; | ||
| this.otherIterable = null; | ||
| this.combiner = combiner; | ||
| } | ||
|
|
||
| public FlowableWithLatestFromMany(Publisher<T> source, Iterable<? extends Publisher<?>> otherIterable, Function<? super Object[], R> combiner) { | ||
| public FlowableWithLatestFromMany(@NonNull Publisher<T> source, @NonNull Iterable<? extends Publisher<?>> otherIterable, Function<? super Object[], R> combiner) { |
| @@ -115,4 +115,18 @@ public void run() { | |||
|
|
|||
| assertEquals(0, calls[0]); | |||
| } | |||
|
|
|||
| @Test | |||
| public void npe() throws Exception { | |||
There was a problem hiding this comment.
This could be named a bit more informatively.
There was a problem hiding this comment.
yep. added some comments
jschneider
force-pushed
the
feature/#4876-more-null-checks
branch
from
05d5986
to
339b6b8
Compare
akarnokd
approved these changes
Feb 3, 2017
This was referenced Feb 8, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have added some more null checks to help the static code analysis.
I think I have also fixed one possible NPE in NewThreadWorker