Preserve covariant return types when merging null annotations

[wenytang-ms]

Summary

Preserve the covariant return type already selected by Java method resolution when merging null annotations from override-equivalent inherited methods.

Addresses redhat-developer/vscode-java#4500. This is a follow-up to #5250, not a change to the language server's null-analysis settings.

Root cause

Scope.mostSpecificMethodBinding() can select a method whose return type is more specific than the first entry in moreSpecific. However, methodWithMergedNullAnnotations() initializes the merged return type from moreSpecific[0], rather than from the selected method.

Since strongerType() deliberately preserves its first argument's underlying Java type, merging annotations can replace the selected subtype with a supertype. In AWS SDK 2.42.26, this makes GetParametersByPathRequest.builder().build() appear to return SsmRequest when annotation-based null analysis is enabled.

The fix starts with current.returnType and merges annotations from every candidate onto that type. Parameter annotation merging is unchanged.

Regression history

Historical ECJ builds using the same AWS input and null-analysis settings show:

Revision	Result
27f2d7a, parent of #5237	Compiles
62bd2a3, #5237	Fails with an Object return type
618869e, #5250	Fails with an SsmRequest return type

The earlier follow-up fixed the explicit covariant override reported in #5249, but did not cover the separately inherited generic builder methods used by the AWS SDK.

Regression coverage

• testGH5249_inheritedBuilders: generic and covariant build() declarations in both interface orders, resolved from source and then from class files.
• testGH5249_inheritedNullAnnotations: preserve the covariant Java type while merging the strongest return nullness from three interfaces, in both orders.

The tests do not depend on the AWS SDK.

Validation

The Maven/Tycho compiler reactor succeeded, and NullTypeAnnotationTest reported 958 tests, 0 failures, 0 errors. The Maven-built ECJ also compiles both original AWS expressions with null analysis enabled.

mvn -B -ntp -pl org.eclipse.jdt.core.tests.compiler -am verify
  "-Dtest=NullTypeAnnotationTest#testGH5249*+testGH5070*"
  -DfailIfNoTests=false
  "-Dtycho.surefire.argLine=--add-modules ALL-SYSTEM -Dcompliance=1.8,21 -Djdt.performance.asserts=disabled"
  -DcompilerBaselineMode=disable -DcompilerBaselineReplace=none

The test provider ran the full NullTypeAnnotationTest suite for this invocation.

This draft fixes the compiler. The downstream VS Code issue will additionally require consuming the fixed JDT Core version through JDT LS.

[wenytang-ms]

Current

[wenytang-ms]

Fixed

[wenytang-ms]

@stephan-herrmann please help review this PR, thanks

[wenytang-ms]

@srikanth-sankaran please review this pr.

[wenytang-ms]

@srikanth-sankaran @stephan-herrmann please review

[srikanth-sankaran]

@srikanth-sankaran @stephan-herrmann please review

Sorry for the delay. @stephan-herrmann is Mr Null Analysis. But he is a volunteer. In case he is not able to wrap up the review by this week, I will certainly take a look and do some preparatory work for him.

[stephan-herrmann]

I'll take a look later today.

[stephan-herrmann]

Thanks for the PR, which is going in a good direction.

To further harden the solution I created the following variant of testGH5249_inheritedBuilders():

		import org.eclipse.jdt.annotation.*;
		public class Test {
			Builders.@NonNull Child build(Builders.ChildBuilder builder) {
				return builder.build();
			}
			Builders.Child buildReversed(Builders.ReversedChildBuilder builder) {
				return builder.build();
			}
		}

			import org.eclipse.jdt.annotation.*;
			public class Builders {
				public static class Parent {}
				public static class Child extends Parent {}
				public interface ParentBuilder {
					@NonNull Parent build();
				}
				public interface GenericBuilder<T> {
					T build();
				}
				public interface ChildBuilder extends ParentBuilder, GenericBuilder<Child> {}
				public interface ReversedChildBuilder extends GenericBuilder<Child>, ParentBuilder {}
			}

Before the fix this was affected by the same bug, but with the proposed fix it still reports

----------
1. WARNING in Test.java (at line 4)
	return builder.build();
	       ^^^^^^^^^^^^^^^
Null type safety (type annotations): The expression of type 'Builders.Child' needs unchecked conversion to conform to 'Builders.@NonNull Child'
----------

This indicates that strongerType() fails to preserve the @NonNull from the ParentBuilder's method.

I'll use the opportunity trying to improve strongerType() in an additional commit.

Additionally I checked the other caller of strongerType(): ReferenceBinding.getSingleAbstractMethod(). This caller is already OK: it always starts with the return type of the current candidate method, superimposing null info from every otherMethod as we go.

[stephan-herrmann]

Thanks @wenytang-ms !