Skip to content

Conversation

SentryMan
Copy link
Collaborator

@SentryMan SentryMan commented Jan 8, 2024

  • all builder fields will generate with a Jspecify @nullable annotation
  • if any form of @NonNull is detected on a component, the generated build method will validate that the value isn't null
  • adds support for Jspecify @NullMarked and @NullUnmarked

Resolves #43 (At least for the checker framework)

also has changes from #45

@SentryMan SentryMan requested a review from rbygrave January 8, 2024 18:51
@SentryMan SentryMan self-assigned this Jan 8, 2024
@SentryMan SentryMan added the enhancement New feature or request label Jan 8, 2024
@SentryMan SentryMan added this to the 1.1 milestone Jan 8, 2024
@SentryMan
Copy link
Collaborator Author

Alright, we're now good with both checker framework and nullaway.

@SentryMan SentryMan enabled auto-merge January 8, 2024 22:31
@commonquail
Copy link

Yep, Just Works™. Very cool, thank you. :)

@SentryMan
Copy link
Collaborator Author

Traverse up to the record type and look for @NullMarked or @NullUnmarked, if neither are present then .

hmm, yeah wasn't that hard to add

@SentryMan SentryMan requested a review from rob-bygrave January 18, 2024 05:57
@rbygrave
Copy link
Contributor

I'm not keen on aspects of this PR because it adds a bunch of features that I'm not sure we want long term.

  • enforceNullSafety() I don't think this is justified, I think people should just use JSpecify
  • builderInterfaces I haven't seen any justification for this yet
  • GlobalSettings - I'd prefer if we can live without this
  • We might even look to only support JSpecify annotations

How do we feel about those features?

@SentryMan
Copy link
Collaborator Author

SentryMan commented Aug 30, 2024

just use JSpecify

I thought that was for static analysis. At runtime, the jspecify annotations do do much, so it'll be possible to slip nulls in undetected.

EDIT: nevermind let's just jspecify

builderInterfaces I haven't seen any justification for this yet

Forgot why I had it

GlobalSettings - I'd prefer if we can live without this

ok

We might even look to only support JSpecify annotations

ok

@SentryMan
Copy link
Collaborator Author

SentryMan commented Aug 30, 2024

@rbygrave please review when you get time.

EDIT: wait hang on
EDIT2: nvm we're good

@SentryMan SentryMan marked this pull request as draft August 30, 2024 14:09
auto-merge was automatically disabled August 30, 2024 14:09

Pull request was converted to draft

@SentryMan SentryMan marked this pull request as ready for review August 30, 2024 16:53
@SentryMan
Copy link
Collaborator Author

@rbygrave please review and deploy an RC when you get time

@SentryMan SentryMan enabled auto-merge (squash) August 30, 2024 17:18
* Return the AnnotationValue corresponding to the enforceNullSafety() member of the annotation,
* or null when the default value is implied.
*/
AnnotationValue enforceNullSafety() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rob to check this after merge.

* Return the AnnotationValue corresponding to the builderInterfaces() member of the annotation,
* or null when the default value is implied.
*/
AnnotationValue builderInterfaces() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rob to check this after merge.

this.components = components;
importTypes.add("io.avaje.recordbuilder.Generated");
importTypes.add("java.util.function.Consumer");
importTypes.add("java.util.function.Consumer");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double import line. Rob to check this after merge.

@rbygrave rbygrave disabled auto-merge August 31, 2024 20:45
@rbygrave rbygrave enabled auto-merge (squash) August 31, 2024 20:45
@rbygrave rbygrave disabled auto-merge August 31, 2024 20:46
@rbygrave rbygrave merged commit 3e05643 into avaje:main Aug 31, 2024
3 checks passed
@SentryMan SentryMan deleted the nullability branch August 31, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JSpecify 0.3.0, Checker Framework nullness, and avaje-record-builder in Maven
4 participants