Improve AnnotateNullableMethods to avoid duplicate annotations and annotation placement issues

[stefanodallapalma]

Problem

Same issues as #843 (AnnotateNullableParameters), but for method return types:

1. Duplicate annotations when a different nullable annotation is already present

// Input: already annotated with @CheckForNull
@CheckForNull
public String getName() { return null; }

// After: ❌ duplicate
@CheckForNull
@Nullable
public String getName() { return null; }

Also fails to detect TYPE_USE annotations already on the return type (e.g. Outer.@Nullable Inner), causing re-annotation on subsequent Repeat.repeatUntilStable cycles:

// Input
public static Outer.@Nullable Inner test() { return null; }

// After: ❌ double-annotated
public static @Nullable Outer.@Nullable Inner test() { return null; }

2. Compilation error with non-TYPE_USE annotations on nested/array return types

// ❌ @CheckForNull not applicable in this type context
public Outer.@CheckForNull Inner bar() { return null; }

Solution

Mirrors the approach from PR #843:

• Simple name matching — skip methods already carrying any annotation named Nullable, CheckForNull, regardless of package.

• Non-TYPE_USE list — non-type use annotations (such as CheckForNull) stay as method-level annotations. All others default to TYPE_USE behavior.

Requested review

@timtebeek

NOTE: Test case typescriptCode() fails locally due to RPC process shut down early with exit code 1. I've no context on what that test is meant for. Worth checking it.

[timtebeek]

Thanks! Can we resolve the conflicts (revert the changes) to AnnotateNullableParameters?

[stefanodallapalma]

Good catch. Haven't noticed AnnotateNullableParameter slipped through

[timtebeek]

It seems a bit odd to have a set of one; do we see this grow in the future? Might there be some overfit for the test cases that uses the same?

[stefanodallapalma]

perhaps better use TYPE_USE_NULLABLE_ANNOTATIONS as in AnnotateNullableParameters

private static final Set<String> TYPE_USE_NULLABLE_ANNOTATIONS = new HashSet<>(Arrays.asList(
            "jakarta.annotation.Nullable",
            "org.checkerframework.checker.nullness.qual.Nullable",
            "org.eclipse.jdt.annotation.Nullable",
            "org.jspecify.annotations.Nullable"
    ));

[timtebeek]

Any reason we're not using the same contains("null") as we did for parameters? it's hard to be exhaustive, and we don't know if folks have their own "@NullAllowed" annotation for instance.

[stefanodallapalma]

no reason tbh, I've started updating the recipe yesterday alongside the AnnotateNullableParameters and haven't had time to align with all changes in AnnotateNullableParameter

[stefanodallapalma]

Hi @timtebeek, do you know when can I expect this and #843 to be released?

[timtebeek]

In the next hour or so! :)