Add nullability annotations to sentry module.
#1439
Conversation
|
Excited about this change! |
Codecov Report
@@ Coverage Diff @@
## main #1439 +/- ##
============================================
- Coverage 75.87% 75.61% -0.27%
- Complexity 1901 1905 +4
============================================
Files 189 189
Lines 6499 6532 +33
Branches 640 659 +19
============================================
+ Hits 4931 4939 +8
- Misses 1272 1277 +5
- Partials 296 316 +20
Continue to review full report at Codecov.
|
sentry-android-core/src/main/java/io/sentry/android/core/AndroidLogger.java
Outdated
Show resolved
Hide resolved
|
@marandaneto have a look please. Since this is a breaking change anyway, perhaps we can re-evaluate if some parameters should/should not be nullable |
| @@ -191,11 +191,10 @@ private void setThreads(final @NotNull SentryEvent event) { | |||
| // crashed properly | |||
| List<Long> mechanismThreadIds = null; | |||
|
|
|||
| final boolean hasExceptions = | |||
| event.getExceptions() != null && !event.getExceptions().isEmpty(); | |||
| final List<SentryException> eventExceptions = event.getExceptions(); | |||
There was a problem hiding this comment.
wondering why we've changed the hasExceptions instead of checking NPE and emptiness 2 times
There was a problem hiding this comment.
NullAway is not able to recognize this check. If we want to keep using it, we would either have to do it as in PR, or disable warning for this method.
| @@ -26,7 +26,7 @@ public SendFireAndForgetEnvelopeSender( | |||
| Objects.requireNonNull(options, "SentryOptions is required"); | |||
|
|
|||
| final String dirPath = sendFireAndForgetDirPath.getDirPath(); | |||
| if (!hasValidPath(dirPath, options.getLogger())) { | |||
| if (dirPath == null || !hasValidPath(dirPath, options.getLogger())) { | |||
There was a problem hiding this comment.
hasValidPath already checks for NPE
There was a problem hiding this comment.
Same reason as here #1439 (comment)
| @@ -26,7 +26,7 @@ public SendFireAndForgetOutboxSender( | |||
| Objects.requireNonNull(options, "SentryOptions is required"); | |||
|
|
|||
| final String dirPath = sendFireAndForgetDirPath.getDirPath(); | |||
| if (!hasValidPath(dirPath, options.getLogger())) { | |||
| if (dirPath == null || !hasValidPath(dirPath, options.getLogger())) { | |||
There was a problem hiding this comment.
Same reason as here #1439 (comment)
|
Co-authored-by: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com>
|
Few comments left to resolve and take decision. The rest is fixed. |
|
|
|
Merging this to move things! Looks like nothing major pending, but lets work on follow up PRs if something comes up |
📜 Description
Adds
@Nullable&@NotNullannotations to classes and interfaces insentrymodule.💡 Motivation and Context
Better Kotlin interoperatibility and cleaner API.
💚 How did you test it?
With
NullAwayErrorprone plugin.I did not unfortunately find a way to ensure that API is 100% documented. Errorprone triggers violation if the null safety is violated in the code, but if the code is not used, it won't trigger an error.
📝 Checklist
Breaking changes affect mainly Kotlin users.
🔮 Next steps
Add nullability annotations to other modules - as PRs to
nullabilitybranch.