feat: issue optional warning using '==' for wrapper types

org.eclipse.jdt.apt.pluggable.tests/src/org/eclipse/jdt/apt/pluggable/tests/ProcessorTestStatus.java

@@ -20,6 +20,7 @@
  * Utility class to hold results of processor-based tests.
  * All methods are static.
  */
+@SuppressWarnings("reference-comparison")
 public final class ProcessorTestStatus {
 
 	/**

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java

@@ -2184,6 +2184,8 @@ public interface IProblem {
 	int UnlikelyCollectionMethodArgumentType = 1200;
 	/** @since 3.13 */
 	int UnlikelyEqualsArgumentType = 1201;
+	/** @since 3.38 */
+	int DubiousReferenceComparison = 1202;
 
 	/* Local-Variable Type Inference */
 	/** @since 3.14 */
@@ -2621,5 +2623,4 @@ public interface IProblem {
 	 * @noreference preview feature
 	 */
 	int DisallowedStatementInPrologue = PreviewRelated + 2023;
-
 }

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java

@@ -915,6 +915,25 @@ public TypeBinding resolveType(BlockScope scope) {
 				}
 			}
 		}
+
+		boolean leftTypeIsNumber = new String(leftType.readableName()).equals("java.lang.Number"); //$NON-NLS-1$
+		boolean rightTypeIsNumber = new String(rightType.readableName()).equals("java.lang.Number"); //$NON-NLS-1$
+
+		// Check for dubious comparisons of references, for both wrapper types or strings.
+		// Putting primitives (or calculations) in the mix results in unboxing the wrappers to their
+		// primitives, so this only warns when both sides are wrappers. e.g. (x1 == (x2 + 1)) results in
+		// (x2 + 1) becoming a primitive, and an unboxing of x1 as well, not necessitating a warning.
+		// When comparing a reference to null the assumption is that the developer is aware of comparing
+		// references, so the comparison is not unlikely
+		if((leftType != TypeBinding.NULL && rightType != TypeBinding.NULL) &&
+				((leftType.isBoxedPrimitiveType() && rightType.isBoxedPrimitiveType()) ||
+				(!leftType.isPrimitiveType() && (rightType.isBoxedPrimitiveType() || rightTypeIsNumber)) ||
+				((leftType.isBoxedPrimitiveType() || leftTypeIsNumber) && !rightType.isPrimitiveType()) ||
+				(leftType.id == rightType.id && (leftType.id == TypeIds.T_JavaLangString || leftType.isArrayType()))))
+		{
+			scope.problemReporter().dubiousComparison(this, operatorToString(), leftType, rightType);
+		}
+
 		// both base type
 		if (leftType.isBaseType() && rightType.isBaseType()) {
 			int leftTypeID = leftType.id;

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java

@@ -1302,7 +1302,7 @@
 					if (destinationPath == null) {
 						destinationPath = this.main.destinationPath;
 					}
 					if (destinationPath != null && destinationPath != NONE) {
 						if (File.separatorChar == '/') {
 							parameters.put(Logger.OUTPUT, destinationPath);
 						} else {
@@ -3973,6 +3973,9 @@
 					CompilerOptions.OPTION_ReportDeadCodeInTrivialIfStatement,
 					CompilerOptions.DISABLED);
 				return;
+			} else if (token.equals("dubiousReferenceComparison")) { //$NON-NLS-1$
+				setSeverity(CompilerOptions.OPTION_ReportDubiousReferenceComparison, severity, isEnabling);
+				return;
 			}
 			break;
 		case 'e' :
@@ -4649,11 +4652,11 @@
 			if (this.destinationPath == null) {
 				currentDestinationPath =
 					extractDestinationPathFromSourceFile(unitResult);
 			} else if (this.destinationPath != NONE) {
 				currentDestinationPath = this.destinationPath;
 				generateClasspathStructure = true;
 			} // else leave currentDestinationPath null
 		} else if (compilationUnit.destinationPath != NONE) {
 			currentDestinationPath = compilationUnit.destinationPath;
 			generateClasspathStructure = true;
 		} // else leave currentDestinationPath null

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties

@@ -399,6 +399,9 @@ misc.usage.warn = {1} {2}\n\
 \      dep-ann              missing @Deprecated annotation\n\
 \      deprecation        + deprecation outside deprecated code\n\
 \      discouraged        + use of types matching a discouraged access rule\n\
+\      dubiousReferenceComparison\n\
+\                         + dubious reference comparison of primitive wrappers\n\
+\                           or strings\n\
 \      emptyBlock           undocumented empty block\n\
 \      enumIdentifier       ''enum'' used as identifier\n\
 \      enumSwitch           incomplete enum switch\n\

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java

@@ -206,6 +206,7 @@ public class CompilerOptions {
 	public static final String OPTION_ReportUnlikelyCollectionMethodArgumentType = "org.eclipse.jdt.core.compiler.problem.unlikelyCollectionMethodArgumentType"; //$NON-NLS-1$
 	public static final String OPTION_ReportUnlikelyCollectionMethodArgumentTypeStrict = "org.eclipse.jdt.core.compiler.problem.unlikelyCollectionMethodArgumentTypeStrict"; //$NON-NLS-1$
 	public static final String OPTION_ReportUnlikelyEqualsArgumentType = "org.eclipse.jdt.core.compiler.problem.unlikelyEqualsArgumentType"; //$NON-NLS-1$
+	public static final String OPTION_ReportDubiousReferenceComparison =  "org.eclipse.jdt.core.compiler.problem.dubiousReferenceComparison"; //$NON-NLS-1$
 
 	public static final String OPTION_ReportAPILeak = "org.eclipse.jdt.core.compiler.problem.APILeak"; //$NON-NLS-1$
 	public static final String OPTION_ReportUnstableAutoModuleName = "org.eclipse.jdt.core.compiler.problem.unstableAutoModuleName";   //$NON-NLS-1$
@@ -377,6 +378,7 @@ public class CompilerOptions {
 	// group 3
 	public static final int InsufficientResourceManagement = IrritantSet.GROUP3 | ASTNode.Bit1;
 	public static final int IncompatibleOwningContract = IrritantSet.GROUP3 | ASTNode.Bit2;
+	public static final int DubiousReferenceComparison = IrritantSet.GROUP3 | ASTNode.Bit3;
 
 
 	// Severity level for handlers
@@ -601,6 +603,7 @@ public class CompilerOptions {
 		"nls", //$NON-NLS-1$
 		"null", //$NON-NLS-1$
 		"rawtypes", //$NON-NLS-1$
+		"reference-comparison", //$NON-NLS-1$
 		"removal", //$NON-NLS-1$
 		"resource", //$NON-NLS-1$
 		"restriction", //$NON-NLS-1$
@@ -831,6 +834,8 @@ public static String optionKeyFromIrritant(int irritant) {
 				return OPTION_ReportUnlikelyCollectionMethodArgumentType;
 			case UnlikelyEqualsArgumentType:
 				return OPTION_ReportUnlikelyEqualsArgumentType;
+			case DubiousReferenceComparison :
+				return OPTION_ReportDubiousReferenceComparison;
 			case APILeak:
 				return OPTION_ReportAPILeak;
 			case UnstableAutoModuleName:
@@ -982,6 +987,7 @@ public static String[] warningOptionNames() {
 			OPTION_ReportDeprecationInDeprecatedCode,
 			OPTION_ReportDeprecationWhenOverridingDeprecatedMethod,
 			OPTION_ReportDiscouragedReference,
+			OPTION_ReportDubiousReferenceComparison,
 			OPTION_ReportEmptyStatement,
 			OPTION_ReportEnumIdentifier,
 			OPTION_ReportFallthroughCase,
@@ -1191,6 +1197,8 @@ public static String warningTokenFromIrritant(int irritant) {
 			case UnlikelyEqualsArgumentType:
 			case UnlikelyCollectionMethodArgumentType:
 				return "unlikely-arg-type"; //$NON-NLS-1$
+			case DubiousReferenceComparison:
+				return "reference-comparison"; //$NON-NLS-1$
 			case APILeak:
 				return "exports"; //$NON-NLS-1$
 			case UnstableAutoModuleName:
@@ -1263,6 +1271,8 @@ public static IrritantSet warningTokenToIrritants(String warningToken) {
 			case 'r' :
 				if ("rawtypes".equals(warningToken)) //$NON-NLS-1$
 					return IrritantSet.RAW;
+				if ("reference-comparison".equals(warningToken)) //$NON-NLS-1$
+					return IrritantSet.DUBIOUS_REFERENCE_COMPARISON;
 				if ("resource".equals(warningToken)) //$NON-NLS-1$
 					return IrritantSet.RESOURCE;
 				if ("restriction".equals(warningToken)) //$NON-NLS-1$
@@ -1409,6 +1419,7 @@ public Map<String, String> getMap() {
 		optionsMap.put(OPTION_EmulateJavacBug8031744, this.emulateJavacBug8031744 ? ENABLED : DISABLED);
 		optionsMap.put(OPTION_ReportRedundantSuperinterface, getSeverityString(RedundantSuperinterface));
 		optionsMap.put(OPTION_ReportComparingIdentical, getSeverityString(ComparingIdentical));
+		optionsMap.put(OPTION_ReportDubiousReferenceComparison, getSeverityString(DubiousReferenceComparison));
 		optionsMap.put(OPTION_ReportMissingSynchronizedOnInheritedMethod, getSeverityString(MissingSynchronizedModifierInInheritedMethod));
 		optionsMap.put(OPTION_ReportMissingHashCodeMethod, getSeverityString(ShouldImplementHashcode));
 		optionsMap.put(OPTION_ReportDeadCode, getSeverityString(DeadCode));
@@ -2002,6 +2013,7 @@ && getSeverity(ExplicitlyClosedAutoCloseable) == ProblemSeverities.Ignore) {
 			this.reportUnlikelyCollectionMethodArgumentTypeStrict = ENABLED.equals(optionValue);
 		}
 		if ((optionValue = optionsMap.get(OPTION_ReportUnlikelyEqualsArgumentType)) != null) updateSeverity(UnlikelyEqualsArgumentType, optionValue);
+		if ((optionValue = optionsMap.get(OPTION_ReportDubiousReferenceComparison)) != null) updateSeverity(DubiousReferenceComparison, optionValue);
 		if ((optionValue = optionsMap.get(OPTION_ReportAPILeak)) != null) updateSeverity(APILeak, optionValue);
 		if ((optionValue = optionsMap.get(OPTION_ReportUnstableAutoModuleName)) != null) updateSeverity(UnstableAutoModuleName, optionValue);
 		if ((optionValue = optionsMap.get(OPTION_AnnotationBasedNullAnalysis)) != null) {
@@ -2367,6 +2379,7 @@ public String toString() {
 		buf.append("\n\t- unlikely argument type for collection methods: ").append(getSeverityString(UnlikelyCollectionMethodArgumentType)); //$NON-NLS-1$
 		buf.append("\n\t- unlikely argument type for collection methods, strict check against expected type: ").append(this.reportUnlikelyCollectionMethodArgumentTypeStrict ? ENABLED : DISABLED); //$NON-NLS-1$
 		buf.append("\n\t- unlikely argument types for equals(): ").append(getSeverityString(UnlikelyEqualsArgumentType)); //$NON-NLS-1$
+		buf.append("\n\t- dubious operand type for reference comparison: ").append(getSeverityString(DubiousReferenceComparison)); //$NON-NLS-1$
 		buf.append("\n\t- API leak: ").append(getSeverityString(APILeak)); //$NON-NLS-1$
 		buf.append("\n\t- unstable auto module name: ").append(getSeverityString(UnstableAutoModuleName)); //$NON-NLS-1$
 		buf.append("\n\t- SuppressWarnings not fully analysed: ").append(getSeverityString(SuppressWarningsNotAnalysed)); //$NON-NLS-1$

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/IrritantSet.java

@@ -73,6 +73,7 @@ public class IrritantSet {
 	public static final IrritantSet UNQUALIFIED_FIELD_ACCESS = new IrritantSet(CompilerOptions.UnqualifiedFieldAccess);
 	public static final IrritantSet RESOURCE = new IrritantSet(CompilerOptions.UnclosedCloseable);
 	public static final IrritantSet UNLIKELY_ARGUMENT_TYPE = new IrritantSet(CompilerOptions.UnlikelyCollectionMethodArgumentType);
+	public static final IrritantSet DUBIOUS_REFERENCE_COMPARISON = new IrritantSet(CompilerOptions.DubiousReferenceComparison);
 	public static final IrritantSet API_LEAK = new IrritantSet(CompilerOptions.APILeak);
 	public static final IrritantSet MODULE = new IrritantSet(CompilerOptions.UnstableAutoModuleName);
 
@@ -87,7 +88,10 @@ public class IrritantSet {
 		.set(
 			CompilerOptions.UnlikelyEqualsArgumentType
 			| CompilerOptions.SuppressWarningsNotAnalysed
-			| CompilerOptions.AnnotatedTypeArgumentToUnannotated);
+			| CompilerOptions.AnnotatedTypeArgumentToUnannotated)
+		// group-3 infos enabled by default
+		.set(CompilerOptions.DubiousReferenceComparison)
+		;
 
 		COMPILER_DEFAULT_WARNINGS
 			// group-0 warnings enabled by default
@@ -140,7 +144,8 @@ public class IrritantSet {
 				|CompilerOptions.UnstableAutoModuleName
 				|CompilerOptions.PreviewFeatureUsed)
 			.set(CompilerOptions.InsufficientResourceManagement
-				|CompilerOptions.IncompatibleOwningContract);
+				|CompilerOptions.IncompatibleOwningContract)
+			;
 		// default errors IF AnnotationBasedNullAnalysis is enabled:
 		COMPILER_DEFAULT_ERRORS.set(
 				CompilerOptions.NullSpecViolation
@@ -196,6 +201,9 @@ public class IrritantSet {
 
 		UNLIKELY_ARGUMENT_TYPE
 			.set(CompilerOptions.UnlikelyEqualsArgumentType);
+
+		DUBIOUS_REFERENCE_COMPARISON
+			.set(CompilerOptions.DubiousReferenceComparison);
 	}
 	// Internal state
 

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java

@@ -718,6 +718,8 @@
 			return CompilerOptions.UnlikelyCollectionMethodArgumentType;
 		case IProblem.UnlikelyEqualsArgumentType:
 			return CompilerOptions.UnlikelyEqualsArgumentType;
+		case IProblem.DubiousReferenceComparison:
+			return CompilerOptions.DubiousReferenceComparison;
 
 		case IProblem.NonPublicTypeInAPI:
 		case IProblem.NotExportedTypeInAPI:
@@ -793,6 +795,7 @@
 			case CompilerOptions.NonNullTypeVariableFromLegacyInvocation :
 			case CompilerOptions.UnlikelyCollectionMethodArgumentType :
 			case CompilerOptions.UnlikelyEqualsArgumentType:
+			case CompilerOptions.DubiousReferenceComparison:
 			case CompilerOptions.APILeak:
 			case CompilerOptions.UnstableAutoModuleName:
 				return CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM;
@@ -4047,7 +4050,7 @@
 		case ProblemReasons.TypeParameterArityMismatch :
 			problemConstructor = (ProblemMethodBinding) targetConstructor;
 			shownConstructor = problemConstructor.closestMatch;
 			if (shownConstructor.typeVariables == Binding.NO_TYPE_VARIABLES) {
 				this.handle(
 					IProblem.NonGenericConstructor,
 					new String[] {
@@ -4627,7 +4630,7 @@
 		case ProblemReasons.TypeParameterArityMismatch :
 			problemMethod = (ProblemMethodBinding) method;
 			shownMethod = problemMethod.closestMatch;
 			if (shownMethod.typeVariables == Binding.NO_TYPE_VARIABLES) {
 				this.handle(
 					IProblem.NonGenericMethod ,
 					new String[] {
@@ -5293,13 +5296,13 @@
 }
 
 private boolean isRecoveredName(char[] simpleName) {
 	return simpleName == RecoveryScanner.FAKE_IDENTIFIER;
 }
 
 private boolean isRecoveredName(char[][] qualifiedName) {
 	if(qualifiedName == null) return false;
 	for (char[] segment : qualifiedName) {
 		if(segment == RecoveryScanner.FAKE_IDENTIFIER) return true;
 	}
 	return false;
 }
@@ -5533,7 +5536,7 @@
 		case ProblemReasons.TypeParameterArityMismatch :
 			problemConstructor = (ProblemMethodBinding) targetConstructor;
 			shownConstructor = problemConstructor.closestMatch;
 			boolean noTypeVariables = shownConstructor.typeVariables == Binding.NO_TYPE_VARIABLES;
 			severity = computeSeverity(noTypeVariables ? IProblem.JavadocNonGenericConstructor : IProblem.JavadocIncorrectArityForParameterizedConstructor);
 			if (severity == ProblemSeverities.Ignore) return;
 			if (noTypeVariables) {
@@ -5774,7 +5777,7 @@
 		case ProblemReasons.TypeParameterArityMismatch :
 			problemMethod = (ProblemMethodBinding) method;
 			shownMethod = problemMethod.closestMatch;
 			boolean noTypeVariables = shownMethod.typeVariables == Binding.NO_TYPE_VARIABLES;
 			severity = computeSeverity(noTypeVariables ? IProblem.JavadocNonGenericMethod : IProblem.JavadocIncorrectArityForParameterizedMethod);
 			if (severity == ProblemSeverities.Ignore) return;
 			if (noTypeVariables) {
@@ -11644,6 +11647,15 @@
 			argument.sourceEnd);
 }
 
+public void dubiousComparison(EqualExpression comparison, String operator, TypeBinding leftType, TypeBinding rightType) {
+	this.handle(
+			IProblem.DubiousReferenceComparison,
+			new String[] {operator, new String(leftType.shortReadableName()), new String(rightType.shortReadableName())},
+			new String[] {operator, new String(leftType.readableName()), new String(rightType.readableName())},
+			comparison.sourceStart,
+			comparison.sourceEnd);
+}
+
 public void nonPublicTypeInAPI(TypeBinding type, int sourceStart, int sourceEnd) {
 	handle(IProblem.NonPublicTypeInAPI,
 			new String[] { new String(type.readableName()) },
@@ -12237,7 +12249,7 @@
 public void sealedSuperTypeInDifferentPackage(int problem, SourceTypeBinding type, TypeReference curType, TypeBinding superTypeBinding, PackageBinding superPackageBinding) {
 	String typeName = new String(type.sourceName);
 	String name = new String(superTypeBinding.sourceName());
 	String packageName = superPackageBinding.compoundName == CharOperation.NO_CHAR_CHAR ? "default" : //$NON-NLS-1$
 		CharOperation.toString(superPackageBinding.compoundName);
 	String[] arguments = new String[] {typeName, packageName, name};
 	this.handle(problem,
@@ -12343,7 +12355,7 @@
 public void sealedPermittedTypeOutsideOfPackage(ReferenceBinding permType, SourceTypeBinding type, ASTNode node, PackageBinding packageBinding) {
 	String permTypeName = new String(permType.sourceName);
 	String name = new String(type.sourceName());
 	String packageName = packageBinding.compoundName == CharOperation.NO_CHAR_CHAR ? "default" : //$NON-NLS-1$
 		CharOperation.toString(packageBinding.compoundName);
 	String[] arguments = new String[] {permTypeName, packageName, name};
 	this.handle(IProblem.SealedPermittedTypeOutsideOfPackage,

org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties

@@ -247,7 +247,6 @@
 210 = Syntax error on keyword "{0}", no accurate correction available
 211 = Comparing identical expressions
 212 = Type {0} cannot be safely cast to {1}
-
 220 = Unmatched bracket
 221 = The primitive type {0} of {1} does not have a field {2}
 222 = Invalid expression as statement
@@ -908,6 +907,7 @@
 # more programming problems:
 1200 = Unlikely argument type {0} for {1} on a {2}
 1201 = Unlikely argument type for equals(): {0} seems to be unrelated to {2}
+1202 = Dubious operand types for '{0}': {1} and {2} are references
 
 ### Autoclosable try
 1251 = Duplicate resource reference {0}
@@ -1179,8 +1179,7 @@
 # Statements before Super
 2022 = Cannot use {0} in a pre-construction context
 2023 = {0} statement not allowed in prologue
-
-
+
 ### ELABORATIONS
 ## Access restrictions
 78592 = The type ''{1}'' is not API (restriction on classpath entry ''{0}'')

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractBatchCompilerTest.java

@@ -44,7 +44,7 @@
 	 */
 	public static boolean isJREVersionEqualTo(String compilerVersion) {
 		String specVersion = System.getProperty("java.specification.version");
 		return specVersion != null && Integer.valueOf(specVersion) == Integer.valueOf(compilerVersion);
 	}
 	/**
 	 * Abstract normalizer for output comparison. This class merely embodies a
@@ -658,14 +658,16 @@
 					"Unexpected standard output for invocation with arguments ["
 						+ extraArguments + "]",
 					expectedOutOutputString,
-					outOutputString);
+					outputDirNormalizer
+						.normalized(outOutputString));
 		}
 		if (!errCompareOK) {
 			assertEquals(
 					"Unexpected error output for invocation with arguments ["
 						+ extraArguments + "]",
 					expectedErrOutputString,
-					errOutputString);
+					outputDirNormalizer
+						.normalized(errOutputString));
 		}
 	}
 
@@ -855,8 +857,8 @@
 				expected = expectedClasspathEntries[j++];
 				if (expected == null) {
 					assertNull(result.destinationPath);
 				} else if (expected == Main.NONE &&
 						result.destinationPath != Main.NONE) {
 					fail("expected 'none' output directory");
 				} else if (! expected.equals(result.destinationPath)) {
 					System.out.println("\"" + result.destinationPath + "\"");

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java

@@ -34,6 +34,7 @@ public AssignmentTest(String name) {
 protected Map getCompilerOptions() {
 	Map options = super.getCompilerOptions();
 	options.put(CompilerOptions.OPTION_ReportDeadCode, CompilerOptions.IGNORE);
+	options.put(CompilerOptions.OPTION_ReportDubiousReferenceComparison, CompilerOptions.IGNORE);
 	options.put(CompilerOptions.OPTION_ReportNullReference, CompilerOptions.ERROR);
 	options.put(CompilerOptions.OPTION_ReportPotentialNullReference, CompilerOptions.ERROR);
 	options.put(CompilerOptions.OPTION_ReportRedundantNullCheck, CompilerOptions.ERROR);

org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AutoBoxingTest.java

@@ -35,6 +35,7 @@ public AutoBoxingTest(String name) {
 	protected Map getCompilerOptions() {
 		Map defaultOptions = super.getCompilerOptions();
 		defaultOptions.put(CompilerOptions.OPTION_ReportAutoboxing, CompilerOptions.WARNING);
+		defaultOptions.put(CompilerOptions.OPTION_ReportDubiousReferenceComparison, CompilerOptions.IGNORE);
 		return defaultOptions;
 	}