Skip to content

Print only @Nullable type use annotations in error messages#1507

Merged
msridhar merged 4 commits intomasterfrom
dont-print-nonnull-in-types
Apr 3, 2026
Merged

Print only @Nullable type use annotations in error messages#1507
msridhar merged 4 commits intomasterfrom
dont-print-nonnull-in-types

Conversation

@msridhar
Copy link
Copy Markdown
Collaborator

@msridhar msridhar commented Apr 1, 2026

See #1499 (comment). This should make our error messages more consistent and less confusing. We now only include @Nullable type use annotations.

Eventually, we may want to adopt JSpecify shorthand when printing types, but we'd have to think more carefully about that.

Summary by CodeRabbit

  • Refactor

    • Diagnostic type annotations are printed more concisely: most inline annotations (e.g., @nonnull) are suppressed while @nullable remains visible once; array spacing normalized for clearer output.
  • Tests

    • Updated expected diagnostic texts in several tests to match the slimmer annotation formatting.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.43%. Comparing base (6e5983b) to head (a46a465).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1507      +/-   ##
============================================
- Coverage     88.44%   88.43%   -0.01%     
  Complexity     2813     2813              
============================================
  Files            99       99              
  Lines          9414     9410       -4     
  Branches       1890     1888       -2     
============================================
- Hits           8326     8322       -4     
  Misses          529      529              
  Partials        559      559              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

Walkthrough

Reworked pretty-printing of type-use nullability in GenericTypePrettyPrintingVisitor: removed per-annotation emission and added a private helper appendNullableAnnotationIfPresent(Type t, StringBuilder sb) that scans annotation mirrors and appends exactly @Nullable once when present (ignoring other annotations). visitClassType and visitTypeVar now call this helper. visitArrayType prints the element, calls the helper, and uses [] for the array suffix (adjusting spacing so output becomes "<elem> @nullable []" when nullable). Two jspecify tests had expected diagnostic strings updated to match the new rendered types.

Possibly related PRs

Suggested labels

jspecify

Suggested reviewers

  • yuxincs
  • lazaroclapp
🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: filtering type-use annotations in error messages to show only @Nullable while suppressing @NonNull.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dont-print-nonnull-in-types

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java`:
- Around line 68-80: The array type pretty-printer still prints annotations
directly in visitArrayType, allowing "@NonNull" to appear; update visitArrayType
to use the same filtering helper by invoking
appendAnnotation(Attribute.TypeCompound compound, StringBuilder sb) for each
annotation on the array (or element) instead of calling
compound.type.accept(this, null) directly, so "@NonNull" is elided consistently;
locate the visitArrayType method and replace direct annotation rendering with
calls to appendAnnotation for each Attribute.TypeCompound found.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9e8187e7-5394-4d03-ae9e-c0417315817b

📥 Commits

Reviewing files that changed from the base of the PR and between 6e5983b and d4c0e01.

📒 Files selected for processing (3)
  • nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java
  • nullaway/src/test/java/com/uber/nullaway/jspecify/GenericMethodTests.java
  • nullaway/src/test/java/com/uber/nullaway/jspecify/GenericsTests.java

@msridhar msridhar marked this pull request as draft April 1, 2026 02:14
@msridhar msridhar marked this pull request as ready for review April 1, 2026 14:57
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java`:
- Around line 68-80: The Javadoc for appendAnnotation in
GenericTypePrettyPrintingVisitor uses an invalid HTML tag <emph>; update the
Javadoc for the method appendAnnotation(Attribute.TypeCompound compound,
StringBuilder sb) to replace the <emph> tag with the proper <em> tag so the
comment reads "... <em>unless</em> the simple name...". Ensure only the tag is
changed and spacing/punctuation around the sentence remains the same.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dee133ca-519e-430d-8cf7-955f15eba012

📥 Commits

Reviewing files that changed from the base of the PR and between d4c0e01 and 22c4b8d.

📒 Files selected for processing (1)
  • nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java`:
- Around line 66-79: The helper appendNullableAnnotationIfPresent currently
drops all non-Nullable type-use annotations; update it to only suppress printing
for NonNull while preserving other annotations: iterate the
Type.getAnnotationMirrors() as before, derive the annotation simple name via
compound.type.accept(this, null) (same symbol already used), if the name equals
"Nullable" append "@Nullable " to sb, if it equals "NonNull" continue/ignore,
otherwise append "@" + annotName + " " so other qualifiers are preserved; keep
the method signature appendNullableAnnotationIfPresent(Type t, StringBuilder sb)
and its loop structure but adjust the conditional logic to implement this
filtering.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b617c3b0-bf4a-437d-a960-a9c341d6812f

📥 Commits

Reviewing files that changed from the base of the PR and between 22c4b8d and a63e522.

📒 Files selected for processing (1)
  • nullaway/src/main/java/com/uber/nullaway/generics/GenericTypePrettyPrintingVisitor.java

@msridhar msridhar changed the title Don't print @NonNull annotations in types in error messages Print only @Nullable type use annotations in types in error messages Apr 1, 2026
@msridhar msridhar changed the title Print only @Nullable type use annotations in types in error messages Print only @Nullable type use annotations in error messages Apr 1, 2026
Copy link
Copy Markdown
Collaborator

@lazaroclapp lazaroclapp left a comment

Choose a reason for hiding this comment

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

LGTM!

@msridhar msridhar merged commit f4dcfc9 into master Apr 3, 2026
12 checks passed
@msridhar msridhar deleted the dont-print-nonnull-in-types branch April 3, 2026 15:58
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