GROOVY-11894: Provide a NullChecker for groovy-typecheckers

[paulk-asert]

No description provided.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 71.97802% with 51 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.3401%. Comparing base (2a7d1e9) to head (b3e2308).

Files with missing lines	Patch %	Lines
...che/groovy/typecheckers/NullCheckingVisitor.groovy	71.6667%	8 Missing and 43 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2426        +/-   ##
==================================================
+ Coverage     66.3339%   66.3401%   +0.0062%     
- Complexity      29941      30074       +133     
==================================================
  Files            1396       1399         +3     
  Lines          117085     117267       +182     
  Branches        20730      20791        +61     
==================================================
+ Hits            77667      77795       +128     
- Misses          33037      33047        +10     
- Partials         6381       6425        +44     

Files with missing lines	Coverage Δ
...main/groovy/groovy/typecheckers/NullChecker.groovy	100.0000% <100.0000%> (ø)
...roovy/groovy/typecheckers/StrictNullChecker.groovy	100.0000% <100.0000%> (ø)
...che/groovy/typecheckers/NullCheckingVisitor.groovy	71.6667% <71.6667%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Adds new Groovy type-checking extensions to perform compile-time null-safety analysis, plus tests and user documentation.

Changes:

• Introduces NullChecker (annotation-based) and StrictNullChecker (annotation + flow-sensitive) type checking extensions.
• Implements shared analysis logic in NullCheckingVisitor.
• Adds unit/spec tests and Asciidoc documentation with include-tagged examples.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
subprojects/groovy-typecheckers/src/main/groovy/org/apache/groovy/typecheckers/NullCheckingVisitor.groovy	Core visitor implementing nullability checks and (optional) flow-sensitive tracking
subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/NullChecker.groovy	Registers the annotation-based null checker as a TypeChecked extension
subprojects/groovy-typecheckers/src/main/groovy/groovy/typecheckers/StrictNullChecker.groovy	Registers the flow-sensitive null checker as a TypeChecked extension
subprojects/groovy-typecheckers/src/test/groovy/groovy/typecheckers/NullCheckerTest.groovy	JUnit tests for both checkers using custom GroovyShell configurations
subprojects/groovy-typecheckers/src/spec/test/NullCheckerTest.groovy	Spec tests with tagged snippets used by the docs
subprojects/groovy-typecheckers/src/spec/doc/typecheckers.adoc	Documentation section describing the new null checkers and examples

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In flow-sensitive mode, nullability does not propagate through assignments like y = x when x is known nullable (either annotated @Nullable or tracked in nullableVars). Because the else branch unconditionally removes the target from nullableVars unless RHS is a literal null (or a nullable-returning method via trackNullableReturn), StrictNullChecker can miss y being nullable. Consider treating isKnownNullable(expression.rightExpression) as nullable and keeping/adding the target in nullableVars (and similarly avoid marking @MonotonicNonNull as initialized when the assigned value may be null).

Suggested change

	if (fieldNonNull && isNullExpr(expression.rightExpression)) {
	reportError("Cannot assign null to @NonNull variable '${expression.leftExpression.name}'", expression)
	}
	// @MonotonicNonNull: once initialized with non-null, cannot assign null again
	if (target instanceof AnnotatedNode && hasMonotonicAnno(target)) {
	if (!isNullExpr(expression.rightExpression)) {
	monotonicInitialized.add(target)
	} else if (monotonicInitialized.contains(target)) {
	reportError("Cannot assign null to @MonotonicNonNull variable '${expression.leftExpression.name}' after non-null assignment", expression)
	}
	}
	if (isNullExpr(expression.rightExpression)) {
	nullableVars.add(target)
	} else {
	nullableVars.remove(target)
	if (flowSensitive) {
	trackNullableReturn(target, expression.rightExpression)
	Expression rhs = expression.rightExpression
	boolean rhsIsNull = isNullExpr(rhs)
	boolean rhsKnownNullable = isKnownNullable(rhs)
	if (fieldNonNull && rhsIsNull) {
	reportError("Cannot assign null to @NonNull variable '${expression.leftExpression.name}'", expression)
	}
	// @MonotonicNonNull: once initialized with non-null, cannot assign null again
	if (target instanceof AnnotatedNode && hasMonotonicAnno(target)) {
	if (!rhsIsNull && !rhsKnownNullable) {
	monotonicInitialized.add(target)
	} else if (rhsIsNull && monotonicInitialized.contains(target)) {
	reportError("Cannot assign null to @MonotonicNonNull variable '${expression.leftExpression.name}' after non-null assignment", expression)
	}
	}
	if (rhsIsNull || rhsKnownNullable) {
	nullableVars.add(target)
	} else {
	nullableVars.remove(target)
	if (flowSensitive) {
	trackNullableReturn(target, rhs)

[Copilot]

The flow-sensitive assignment logic has several edge cases that aren't currently exercised by tests: (1) casted-null assignments (e.g. def x = (String) null) should be treated as nullable in StrictNullChecker, (2) propagation of nullability through variable-to-variable assignment (def y = x) when x is nullable, and (3) @MonotonicNonNull should not be considered initialized when assigned from a value that may be null. Adding focused tests for these cases would prevent regressions and validate the intended flow-sensitive behavior.

[glaforge]

Maybe we could add checks (in the tests) for the JSpecify annotations?

[paulk-asert]

Maybe we could add checks (in the tests) for the JSpecify annotations?

Added