Fix URL for RequireExplicitNullMarking checker by msridhar · Pull Request #1369 · uber/NullAway

msridhar · 2025-12-05T19:00:39Z

Summary by CodeRabbit

New Features
- Added a documentation link to the RequireExplicitNullMarking checker so diagnostics include a guidance URL.
Tests
- Added test coverage confirming the checker emits the expected documentation link in diagnostics.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-05T19:04:09Z

Walkthrough

An override method public String linkUrl() was added to com.uber.nullaway.RequireExplicitNullMarking to return a documentation URL. A new test defaultURL() was added in RequireExplicitNullMarkingTest to assert that the diagnostic includes the expected URL. No other functional changes were made to control flow or logic. The change addresses a previously unreachable URL referenced in the checker’s diagnostic.

Possibly related PRs

Tune warning message for RequireExplicitNullMarking #1366: Modifies com.uber.nullaway.RequireExplicitNullMarking (updates @BugPattern summary), directly related to the checker's diagnostic metadata and message content.

Suggested reviewers

yuxincs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly describes the main change: fixing the URL for the RequireExplicitNullMarking checker, which aligns with the PR's objective of addressing the broken URL issue #1368.
Linked Issues check	✅ Passed	The PR adds a linkUrl() method returning a working GitHub wiki URL and includes a test verifying the URL, directly addressing the broken ErrorProne URL issue [#1368].
Out of Scope Changes check	✅ Passed	All changes (linkUrl() method override and corresponding test) are directly related to fixing the broken URL in RequireExplicitNullMarking error messages as specified in issue #1368.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix-url

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7bb824f and c9a9e44.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java (1 hunks)
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

📚 Learning: 2025-08-14T18:50:06.159Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java
nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java

📚 Learning: 2025-08-29T18:41:43.584Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

Applied to files:

nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java
nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java

🔇 Additional comments (2)

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java (1)

77-80: linkUrl override correctly fixes the documentation target

The override cleanly wires this checker to the GitHub wiki anchor, with no extra logic or visible trailing whitespace; this matches the PR intent and looks good as-is.

nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1)

38-48: defaultURL test provides appropriate regression coverage

The new test follows existing CompilationTestHelper patterns and reliably asserts that the diagnostic includes the expected GitHub wiki URL, giving good coverage for the linkUrl behavior.

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.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6165b8e and b0d85ca.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java (1 hunks)
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-08-29T18:41:43.584Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java

📚 Learning: 2025-08-14T18:50:06.159Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java

📚 Learning: 2025-10-29T23:56:18.236Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1316
File: jdk-javac-plugin/src/main/java/com/uber/nullaway/javacplugin/NullnessAnnotationSerializer.java:261-293
Timestamp: 2025-10-29T23:56:18.236Z
Learning: In NullAway's jdk-javac-plugin NullnessAnnotationSerializer, type variable bounds with annotations (e.g., `T extends Nullable Object`) are checked at their declaration sites by the typeParamHasAnnotation method for both class-level and method-level type parameters. The hasJSpecifyAnnotationDeep method is designed to check type uses (return types, parameters, etc.) and does not need a TYPEVAR case because type variable declaration bounds are already handled separately.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)

GitHub Check: Build and test on macos-latest
GitHub Check: Build and test on windows-latest
GitHub Check: Build spring-framework with snapshot
GitHub Check: Build and test on ubuntu-latest
GitHub Check: Build caffeine with snapshot

🔇 Additional comments (1)

nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1)

38-48: Good focused coverage for diagnostic URL

This test cleanly asserts that the diagnostic message includes the new documentation URL and mirrors the existing CompilationTestHelper style, so future changes to the link will be caught.

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0d85ca and 7bb824f.

📒 Files selected for processing (2)

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java (1 hunks)
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1 hunks)

🧰 Additional context used

🧠 Learnings (3)

📓 Common learnings

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

📚 Learning: 2025-08-29T18:41:43.584Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1259
File: jdk-recent-unit-tests/src/test/java/com/uber/nullaway/jdk17/SwitchTests.java:318-321
Timestamp: 2025-08-29T18:41:43.584Z
Learning: Classes annotated with NullMarked are analyzed by NullAway even if they are not in packages specified by the AnnotatedPackages configuration. The NullMarked annotation guarantees NullAway analysis.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java

📚 Learning: 2025-08-14T18:50:06.159Z

Learnt from: msridhar
Repo: uber/NullAway PR: 1245
File: guava-recent-unit-tests/src/test/java/com/uber/nullaway/guava/NullAwayGuavaParametricNullnessTests.java:101-102
Timestamp: 2025-08-14T18:50:06.159Z
Learning: In NullAway JSpecify tests, when JDK version requirements exist due to bytecode annotation reading capabilities, prefer failing tests over skipping them on unsupported versions to ensure CI catches regressions and enforces proper JDK version usage for developers.

Applied to files:

nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java
nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java

🔇 Additional comments (1)

nullaway/src/test/java/com/uber/nullaway/RequireExplicitNullMarkingTest.java (1)

38-48: Test correctly validates URL in diagnostic.

The test properly verifies that the documentation URL appears in the diagnostic message, which validates the fix for issue #1368. The test structure follows the existing pattern in missingNullMarkedAnnotationOnTopLevelClass() and uses the appropriate compilation helper configuration.

Note: If the trailing space is removed from the linkUrl() method (as suggested in the review of the source file), line 44 will need to be updated to remove the trailing space from the expected diagnostic string.

codecov · 2025-12-05T19:13:38Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.48%. Comparing base (6165b8e) to head (c9a9e44).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1369   +/-   ##
=========================================
  Coverage     88.48%   88.48%           
- Complexity     2613     2614    +1     
=========================================
  Files            98       98           
  Lines          8750     8751    +1     
  Branches       1745     1745           
=========================================
+ Hits           7742     7743    +1     
  Misses          504      504           
  Partials        504      504

☔ 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.

github-actions · 2025-12-06T20:10:19Z

Main Branch:

Benchmark                          Mode  Cnt   Score   Error  Units
AutodisposeBenchmark.compile      thrpt   25  10.671 ± 0.054  ops/s
CaffeineBenchmark.compile         thrpt   25   2.020 ± 0.015  ops/s
DFlowMicroBenchmark.compile       thrpt   25  42.346 ± 0.510  ops/s
NullawayReleaseBenchmark.compile  thrpt   25   1.677 ± 0.010  ops/s

With This PR:

Benchmark                          Mode  Cnt   Score   Error  Units
AutodisposeBenchmark.compile      thrpt   25  10.736 ± 0.043  ops/s
CaffeineBenchmark.compile         thrpt   25   2.050 ± 0.012  ops/s
DFlowMicroBenchmark.compile       thrpt   25  42.410 ± 0.441  ops/s
NullawayReleaseBenchmark.compile  thrpt   25   1.698 ± 0.020  ops/s

Fix URL for RequireExplicitNullMarking checker

b0d85ca

msridhar requested a review from yuxincs December 5, 2025 19:00

coderabbitai Bot reviewed Dec 5, 2025

View reviewed changes

comment on trailing space

7bb824f

coderabbitai Bot reviewed Dec 5, 2025

View reviewed changes

Comment thread nullaway/src/main/java/com/uber/nullaway/RequireExplicitNullMarking.java

remove trailing space

c9a9e44

msridhar enabled auto-merge (squash) December 5, 2025 19:35

msridhar added the run-benchmarks label Dec 6, 2025

yuxincs approved these changes Dec 6, 2025

View reviewed changes

msridhar merged commit 6dd3976 into master Dec 6, 2025
12 checks passed

msridhar deleted the fix-url branch December 6, 2025 22:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix URL for RequireExplicitNullMarking checker#1369

Fix URL for RequireExplicitNullMarking checker#1369
msridhar merged 3 commits intomasterfrom
fix-url

msridhar commented Dec 5, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

github-actions Bot commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

msridhar commented Dec 5, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Possibly related PRs

Suggested reviewers

Pre-merge checks and finishing touches

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Dec 6, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

msridhar commented Dec 5, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Dec 5, 2025 •

edited

Loading

codecov Bot commented Dec 5, 2025 •

edited

Loading