diff --git a/org.eclipse.jdt.apt.pluggable.tests/src/org/eclipse/jdt/apt/pluggable/tests/ProcessorTestStatus.java b/org.eclipse.jdt.apt.pluggable.tests/src/org/eclipse/jdt/apt/pluggable/tests/ProcessorTestStatus.java index 51b52eb7775..43cd1af1d18 100644 --- a/org.eclipse.jdt.apt.pluggable.tests/src/org/eclipse/jdt/apt/pluggable/tests/ProcessorTestStatus.java +++ b/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 { /** diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java index d7a0139da84..f3f962454e2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/core/compiler/IProblem.java @@ -2188,6 +2188,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 */ @@ -2625,5 +2627,4 @@ public interface IProblem { * @noreference preview feature */ int DisallowedStatementInPrologue = PreviewRelated + 2023; - } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java index 2231ddf745b..b857c0cbe49 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java +++ b/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; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java index cd1e843ec6b..c66ee7842a2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/Main.java @@ -3973,6 +3973,9 @@ private void handleErrorOrWarningToken(String token, boolean isEnabling, int sev CompilerOptions.OPTION_ReportDeadCodeInTrivialIfStatement, CompilerOptions.DISABLED); return; + } else if (token.equals("dubiousReferenceComparison")) { //$NON-NLS-1$ + setSeverity(CompilerOptions.OPTION_ReportDubiousReferenceComparison, severity, isEnabling); + return; } break; case 'e' : diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties index ca945c719a8..e1355065f6e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/batch/messages.properties +++ b/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\ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java index e0c2618ee24..9d87182c4bb 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/CompilerOptions.java +++ b/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$ @@ -379,6 +380,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 @@ -606,6 +608,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$ @@ -836,6 +839,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: @@ -987,6 +992,7 @@ public static String[] warningOptionNames() { OPTION_ReportDeprecationInDeprecatedCode, OPTION_ReportDeprecationWhenOverridingDeprecatedMethod, OPTION_ReportDiscouragedReference, + OPTION_ReportDubiousReferenceComparison, OPTION_ReportEmptyStatement, OPTION_ReportEnumIdentifier, OPTION_ReportFallthroughCase, @@ -1196,6 +1202,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: @@ -1268,6 +1276,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$ @@ -1414,6 +1424,7 @@ public Map 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)); @@ -2009,6 +2020,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) { @@ -2382,6 +2394,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$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/IrritantSet.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/IrritantSet.java index 5f22ad875bc..9950cc1f07d 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/impl/IrritantSet.java +++ b/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 @@ -198,6 +203,9 @@ public class IrritantSet { UNLIKELY_ARGUMENT_TYPE .set(CompilerOptions.UnlikelyEqualsArgumentType); + + DUBIOUS_REFERENCE_COMPARISON + .set(CompilerOptions.DubiousReferenceComparison); } // Internal state diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java index 76a37691cef..bc3a994056c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/ProblemReporter.java @@ -719,6 +719,8 @@ public static int getIrritant(int problemID) { return CompilerOptions.UnlikelyCollectionMethodArgumentType; case IProblem.UnlikelyEqualsArgumentType: return CompilerOptions.UnlikelyEqualsArgumentType; + case IProblem.DubiousReferenceComparison: + return CompilerOptions.DubiousReferenceComparison; case IProblem.NonPublicTypeInAPI: case IProblem.NotExportedTypeInAPI: @@ -794,6 +796,7 @@ public static int getProblemCategory(int severity, int problemID) { case CompilerOptions.NonNullTypeVariableFromLegacyInvocation : case CompilerOptions.UnlikelyCollectionMethodArgumentType : case CompilerOptions.UnlikelyEqualsArgumentType: + case CompilerOptions.DubiousReferenceComparison: case CompilerOptions.APILeak: case CompilerOptions.UnstableAutoModuleName: return CategorizedProblem.CAT_POTENTIAL_PROGRAMMING_PROBLEM; @@ -11667,6 +11670,15 @@ public void unlikelyArgumentType(Expression argument, MethodBinding method, Type 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()) }, diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties index 11ec07df00a..8b1f227d621 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/problem/messages.properties +++ b/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} @@ -1181,8 +1181,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}'') diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractBatchCompilerTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractBatchCompilerTest.java index 383c50b4111..4d4ccbd8565 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractBatchCompilerTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AbstractBatchCompilerTest.java @@ -658,14 +658,16 @@ protected void runTest(boolean shouldCompileOK, String[] testFiles, Object extra "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)); } } diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java index c1dd988f1b6..3cd01bda8b2 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java +++ b/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); diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AutoBoxingTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AutoBoxingTest.java index e4c6a02276a..785c933cb1c 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AutoBoxingTest.java +++ b/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; } diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java index d3583e905ac..19aec5e5c9f 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/BatchCompilerTest.java @@ -849,6 +849,9 @@ public void test012b(){ " 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" + @@ -1068,6 +1071,7 @@ public void test013() { "