Add missing @NonNull null check for collection/map target properties - #4103

I ran this locally on d31052d against 211b2be (JDK 21.0.9, both javac and ecj). The diagnosis holds up and the fix is correct. Measurements below, plus a few things I think are worth changing before it lands.

Verification

Control first, to check that the test actually defends the fix: the four new test files applied to unmodified main, nothing else.

CollectionTargetNullCheckTest.nullSourceListShouldNotBePassedToNonNullSetter:38
Expecting value to be false but was true
Tests run: 2, Failures: 2, Errors: 0, Skipped: 0

Same test on the PR head: Tests run: 2, Failures: 0. Full processor module on the PR head: Tests run: 3645, Failures: 0, Errors: 0, Skipped: 0.

The root-cause attribution checks out as well. git log -S sourceJSpecifyNullability -- CollectionAssignmentBuilder.java returns exactly one commit, 35334e5 (#4056 / #4057), while requiresNullCheck entered PropertyMapping earlier in 9554065 (#1243).

Blast radius is wider than the description suggests

The description shows a single List setter. I compiled a probe through both processors (one built from main, one from this branch) and diffed the generated mappers:

Two rows deserve comment.

The @NullMarked row is the widest part of this change. getSetterNullability resolves an unannotated setter inside a @NullMarked scope to NON_NULL, so every collection/map property of a @NullMarked bean whose assignment is not DIRECT gains a wrapper, not just explicitly annotated ones. That matches what the scalar path already does, so I read it as intended, but it is worth saying in the description because it is much broader than "a @NonNull setter".

The last row still emits target.setConv( null ) from ExistingInstanceSetterWrapperForCollectionsAndMaps under the default SET_TO_NULL. I do not think this PR should touch it. I measured the scalar equivalent and it behaves the same way (target.setNum( null ) into a @NonNull scalar setter), so the two paths remain symmetric and this is the feature's existing boundary rather than something this change missed. Mentioning it only so it is not mistaken for a gap.

The new null check is silent

JSpecifyVerboseNoteTest describes its own purpose as making JSpecify null-check decisions "no longer silent", and the scalar path emits PROPERTYMAPPING_JSPECIFY_ADD_NULL_CHECK whenever it decides to add one. The new branch here returns jspecifyDecision but only notes the false case.

Compiling the probe with -Amapstruct.verbose=true on this branch: the five properties newly guarded above produce no JSpecify note at all. The only add-note in the entire run comes from the scalar path:

Note: -- MapStruct: JSpecify adding null check for property "arrConv": source=UNKNOWN, target=NON_NULL.

arrConv is the identical UNKNOWN → NON_NULL decision, just routed through PropertyMapping rather than CollectionAssignmentBuilder. Emitting PROPERTYMAPPING_JSPECIFY_ADD_NULL_CHECK( targetPropertyName, sourceJSpecifyNullability, targetJSpecifyNullability ) in the true branch would make the two report identically.

The new @param javadoc states the precedence backwards

 * @param targetJSpecifyNullability … used to force a null check when the target requires
 *                                  {@code @NonNull} regardless of the source's own nullability

requiresNullCheck tests the source first and returns FALSE for a @NonNull source before it ever looks at the target, so the source wins. Measured, with a @NonNull source getter feeding the same @NonNull setter:

tSetterConv.setConv( integerListToStringList( src.getConv() ) );

Note: -- MapStruct: JSpecify skipping null check for property "conv": source is @NonNull.

The inline comment you added inside the method body has it right ("with a source that is not itself @NonNull"); only the @param text is inverted.

Test conventions in that package

Across the 20 test classes under test/nullcheck/jspecify:

• 19 carry @IssueKey. CollectionTargetNullCheckTest is the only one without.
• 18 register GeneratedSource; 16 of those call addComparisonToFixtureFor(...). This test registers the extension and never touches it, so the field is currently dead. (JSpecifySafetyGuardTest is the one existing precedent.) Since what is being fixed here is the shape of the generated code, the fixture comparison the neighbours use would pin if ( list != null ) directly rather than inferring it from a flag on the bean.
• The test asserts one direction only. Nothing in it fails if the assignment were dropped entirely instead of guarded. I added the other direction locally and it passes on this branch (assertThat( target.getNumbers() ).containsExactly( "1", "2", "3" ), Tests run: 4, Failures: 0), so this is a missing guard in the test rather than a defect. JSpecifyCollectionPropertyTest next door asserts both the mapped values and the fixture.

One cross-PR interaction

#4078 changes NullabilityResolver.requiresNullCheck from two parameters to three (adding paramNullability) and updates its single call site. This PR adds a second call site for the two-parameter form. Whichever lands second needs a rebase, and at that point someone has to decide whether the collection path should also honour the reused-method-parameter rule. Raising it for ordering only, nothing to change here.