-
Notifications
You must be signed in to change notification settings - Fork 118
Add compile-time null safety #1256
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
base: main
Are you sure you want to change the base?
Conversation
902821c to
428f2b1
Compare
…across multiple classes to improve null-safety.
428f2b1 to
3671534
Compare
| } | ||
|
|
||
| @Property | ||
| fun keyNullThrows() { |
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.
Should it be removed? It would be better if it remained.
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.
In the following code, mapKey is recognized as not null, which causes a compilation error in Kotlin, so I removed the null check.
private void setMapKey(Object mapKey) {
if (mapKey == null) {
throw new IllegalArgumentException(
"Map key cannot be null."
);
}
setValue(
this.declarativeExpression.element(entrySize - 1).key(),
mapKey
);
}Would it be better to declare mapKey as nullable to handle this issue?
I have one question: since passing null to the key would result in an error, I thought it would be better to prevent null from being passed at compile time. Is there a specific reason why null is allowed in this case?
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.
Would it be better to declare mapKey as nullable to handle this issue?
Yes, that would be nice.
Is there a specific reason why null is allowed in this case?
As the InnerSpec is the public API, users can enter any value, including null. Therefore, all parameters in the InnerSpec should be marked as @Nullable.
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.
Done!
044e6ec to
71fdac5
Compare
… and InnerSpec methods
71fdac5 to
535b3f0
Compare
Summary
resolves #1251
Added CheckerFramework to enable compile-time null-safety checking and improve null safety validation.
Added
@Nullableannotations across the codebase to explicitly express nullability and strengthen null-safety.(Optional): Description
Added SuppressWarnings to skip null checks for existing code that wasn't handling null validation properly.
Added
@Nullableannotations where it seemed safe to do so - please review to make sure this won't cause any issues.Disabled nullability checks for test files.
Changed some Kotlin types from nullable to non-null - please double-check that these changes are safe.
After applying checkFramework, the compiler now shows warnings when
@Nullableannotations aren't placed directly next to the type:I actually addressed this in a previous PR, but it seems the commit got lost during conflict resolution. I'll handle this in a separate PR.
How Has This Been Tested?
Run tests
Is the Document updated?
No