[
https://issues.apache.org/jira/browse/GROOVY-12250?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12250:
-------------------------------
Description:
The {{NullChecker}} type-checking extension recognises a range of null-guard
patterns ({{if (x != null)}}, Groovy truth, {{instanceof}},
{{Objects.nonNull}}, {{assert}}, etc. — GROOVY-12208) but does not recognise
calls which _guarantee_ an argument is non-null when they complete normally. In
particular, {{Objects.requireNonNull(x)}} — the most idiomatic Java null
validator and the natural "cast to non-null" escape hatch — neither narrows
{{x}} afterwards nor counts as a guard, and test assertions like
{{assertNotNull(x)}} leave test code full of false positives.
This issue adds narrowing for:
* *Validator methods* which throw on null, so their first argument is non-null
afterwards: {{Objects.requireNonNull(x)}} (1- and 2-arg forms, including
statically imported) and Guava-style {{checkNotNull(x)}}
* *Test assertions*: {{assertNotNull(...)}} narrows its _actual_ argument(s),
with message parameters recognised in any position — JUnit 4's {{(message,
actual)}} and JUnit 5 / TestNG's {{(actual, message)}} both work, driven by
parameter types rather than position
* *Fluent assertion chains*: {{assertThat(x).isNotNull()}} narrows {{x}},
looking through intermediate chained calls such as {{describedAs(...)}}
(AssertJ, Truth, or similar)
Consistent with the checker's existing design (annotations are matched by
simple name from any package), validator and assertion methods are matched by
simple name, so any library following the common naming conventions is
recognised without configuration.
Example:
{code:groovy}
import groovy.transform.TypeChecked
@TypeChecked(extensions = 'groovy.typecheckers.NullChecker')
int labelWidth(@Nullable String label) {
Objects.requireNonNull(label, 'label must be supplied')
label.length() // ok: requireNonNull throws on
null
}
{code}
Previously this reported: {{[Static type checking] - Potential null
dereference: 'label' is @Nullable}}
The narrowing also applies in flow-sensitive mode ({{NullChecker(strict:
true)}}), clearing the nullable state of tracked unannotated variables, and
composes with the existing checks — e.g. {{foo(Objects.requireNonNull(x))}} no
longer reports passing a {{@Nullable}} value to a {{@NonNull}} parameter
(correct, since evaluation order guarantees the throw happens first).
Methods with similar names that are not null validators still behave as before
(an unrecognised {{verifyNotNull(x)}} does not narrow), and {{assertThat(x)}}
without a following {{isNotNull()}} does not narrow.
> NullChecker: narrow arguments of null-validator and assertion methods
> (requireNonNull, assertNotNull, assertThat(x).isNotNull())
> --------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12250
> URL: https://issues.apache.org/jira/browse/GROOVY-12250
> Project: Groovy
> Issue Type: Improvement
> Components: groovy-typecheckers
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> The {{NullChecker}} type-checking extension recognises a range of null-guard
> patterns ({{if (x != null)}}, Groovy truth, {{instanceof}},
> {{Objects.nonNull}}, {{assert}}, etc. — GROOVY-12208) but does not recognise
> calls which _guarantee_ an argument is non-null when they complete normally.
> In particular, {{Objects.requireNonNull(x)}} — the most idiomatic Java null
> validator and the natural "cast to non-null" escape hatch — neither narrows
> {{x}} afterwards nor counts as a guard, and test assertions like
> {{assertNotNull(x)}} leave test code full of false positives.
> This issue adds narrowing for:
> * *Validator methods* which throw on null, so their first argument is
> non-null afterwards: {{Objects.requireNonNull(x)}} (1- and 2-arg forms,
> including statically imported) and Guava-style {{checkNotNull(x)}}
> * *Test assertions*: {{assertNotNull(...)}} narrows its _actual_ argument(s),
> with message parameters recognised in any position — JUnit 4's {{(message,
> actual)}} and JUnit 5 / TestNG's {{(actual, message)}} both work, driven by
> parameter types rather than position
> * *Fluent assertion chains*: {{assertThat(x).isNotNull()}} narrows {{x}},
> looking through intermediate chained calls such as {{describedAs(...)}}
> (AssertJ, Truth, or similar)
> Consistent with the checker's existing design (annotations are matched by
> simple name from any package), validator and assertion methods are matched by
> simple name, so any library following the common naming conventions is
> recognised without configuration.
> Example:
> {code:groovy}
> import groovy.transform.TypeChecked
> @TypeChecked(extensions = 'groovy.typecheckers.NullChecker')
> int labelWidth(@Nullable String label) {
> Objects.requireNonNull(label, 'label must be supplied')
> label.length() // ok: requireNonNull throws
> on null
> }
> {code}
> Previously this reported: {{[Static type checking] - Potential null
> dereference: 'label' is @Nullable}}
> The narrowing also applies in flow-sensitive mode ({{NullChecker(strict:
> true)}}), clearing the nullable state of tracked unannotated variables, and
> composes with the existing checks — e.g. {{foo(Objects.requireNonNull(x))}}
> no longer reports passing a {{@Nullable}} value to a {{@NonNull}} parameter
> (correct, since evaluation order guarantees the throw happens first).
> Methods with similar names that are not null validators still behave as
> before (an unrecognised {{verifyNotNull(x)}} does not narrow), and
> {{assertThat(x)}} without a following {{isNotNull()}} does not narrow.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
