Skip to content
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

Address existing UnknownNullness lint violations #395

Merged
merged 17 commits into from
Dec 17, 2018

Conversation

fractalwrench
Copy link
Contributor

@fractalwrench fractalwrench commented Dec 11, 2018

Goal

The newly added UnknownNullness lint check has identified several sites in bugsnag-android's public API where the nullability is not explicitly defined. We should annotate these as either NonNull, or Nullable, so that Kotlin users can gain the benefit of compile-time safety.

For example, adding the NonNull annotation to the Client constructor would have the following effect:

Client(@NonNull String apiKey) {}

Client client = new Client(null); // java code will fail at runtime
val client = Client(null) // kotlin code will fail at compile time, because an optional String is not allowed

Further reading: https://developer.android.com/kotlin/interop#java_for_kotlin_consumption

Changeset

Due to the number of violations, this PR will serve as an integration branch for other PRs that address the lint violations. This PR also has regenerated the lint_baseline used to suppress warnings, as its line numbers/warnings are out of date due to changes in #389.

WIP

After all the PRs have been integrated into this branch, the lint_baseline check will be run once more to ensure that no UnknownNullness violations were missed.

Additionally, code inspection will be run to ensure there aren't any inconsistencies in the nullability annotations (for example, a NonNull method which can return null). This should also identify some potential bugs around null checks which can be moved out into separate tickets.

@fractalwrench
Copy link
Contributor Author

Merging in feature branches to address any remaining comments in one place, and check that:

  • CI passes
  • Every getter/setter pair uses the same annotation, and doesn't mix-n-match, as this generates a separate field + setter

@fractalwrench fractalwrench changed the title [WIP] Address existing UnknownNullness lint violations Address existing UnknownNullness lint violations Dec 14, 2018
@fractalwrench
Copy link
Contributor Author

I've now double-checked that each getter/setter is consistent with the annotation used, which should address all remaining feedback.

@fractalwrench fractalwrench merged commit 59b42bd into next Dec 17, 2018
@fractalwrench fractalwrench deleted the resolve-unknown-nullness-issues branch January 15, 2019 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants