### Eclipse Workspace Patch 1.0 #P org.eclipse.jdt.core.tests.compiler Index: src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java,v retrieving revision 1.26 diff -u -r1.26 AssignmentTest.java --- src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java 24 Jan 2006 10:50:06 -0000 1.26 +++ src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java 27 Jan 2006 12:52:28 -0000 @@ -188,7 +188,7 @@ "2. ERROR in X.java (at line 9)\n" + " if(a!=null)\n" + " ^\n" + - "The variable a can only be null; it was either set to null or checked for null when last used\n" + + "The variable a cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n" + "3. ERROR in X.java (at line 13)\n" + " System.out.println(a+b);\n" + Index: src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java,v retrieving revision 1.5 diff -u -r1.5 NullReferenceTest.java --- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 24 Jan 2006 10:50:06 -0000 1.5 +++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 27 Jan 2006 12:52:30 -0000 @@ -33,7 +33,7 @@ // -Dcompliance=1.4 (for example) to lower it if needed static { // TESTS_NAMES = new String[] { "test011" }; -// TESTS_NUMBERS = new int[] { 729 }; +// TESTS_NUMBERS = new int[] { 333 }; // TESTS_NUMBERS = new int[] { 2999 }; // TESTS_RANGE = new int[] { 2050, -1 }; // TESTS_RANGE = new int[] { 1, 2049 }; @@ -1223,11 +1223,6 @@ " if (o == null && o == null) {\n" + " ^\n" + "The variable o can only be null; it was either set to null or checked for null when last used\n" + - "----------\n" + - "2. ERROR in X.java (at line 6)\n" + - " if (o == null) { /* */ }\n" + - " ^\n" + - "The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n"); } @@ -1601,15 +1596,7 @@ } // null analysis - if/else -// PMT: exactly the case we talked about; what happens is that the first -// if shade doubts upon o; what we could do is to avoid marking in case -// of error? not sure this is appropriate though, because of inner code -// into the if itself; I believe I somewhat did that on purpose: the latest -// wins; completed with o.toString()... -// basically, the choice is about what we should do in case of error: -// neglect the effect of the error, or propagate this effect; the second -// tends to produce less repeated errors (I believe) than the first... -// PREMATURE could refine adding a null-dependent reachable mark... not urgent +// rationale: erroneous tests no more change null status public void test0312_if_else() { this.runNegativeTest( new String[] { @@ -1619,8 +1606,8 @@ " void foo() {\n" + " Object o = new Object();\n" + " if (o == null) { /* */ }\n" + // complain - " if (o != null) { /* */ }\n" + // quiet - " o.toString();\n" + // complain + " if (o != null) { /* */ }\n" + // complain + " o.toString();\n" + // quiet " }\n" + "}\n"}, "----------\n" + @@ -1629,10 +1616,10 @@ " ^\n" + "The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n" + - "2. ERROR in X.java (at line 7)\n" + - " o.toString();\n" + - " ^\n" + - "The variable o may be null\n" + + "2. ERROR in X.java (at line 6)\n" + + " if (o != null) { /* */ }\n" + + " ^\n" + + "The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n"); } @@ -2062,6 +2049,30 @@ "----------\n"); } +// null analysis - if/else +// avoid double diagnostic (Frédéric) +public void test0333_if_else() { + this.runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(Object[] o) {\n" + + " if (o != null) {\n" + + " int length = o == null ? 0 : o.length;\n" + // complain + " for (int i = 0; i < length; i++) {\n" + + " o[i].toString();\n" + // quiet, protected by if above + " }\n" + + " }\n" + + " }\n" + + "}"}, + "----------\n" + + "1. ERROR in X.java (at line 4)\n" + + " int length = o == null ? 0 : o.length;\n" + + " ^\n" + + "The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + + "----------\n"); +} + // null analysis -- while public void test0401_while() { this.runNegativeTest( @@ -2551,7 +2562,6 @@ } // null analysis -- while -// REVIEW we get one extraneous message that looks a bit strange public void test0423_while() { this.runNegativeTest( new String[] { @@ -2572,11 +2582,6 @@ " if (o == null) { /* */ }\n" + " ^\n" + "The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + - "----------\n" + - "2. ERROR in X.java (at line 8)\n" + - " o = null;\n" + - " ^\n" + - "The variable o can only be null; it was either set to null or checked for null when last used\n" + "----------\n"); } @@ -5230,6 +5235,9 @@ } // moved from AssignmentTest +// new policy (do not modify null info in case of problem in comparison) +// removes errors on 8 - which is better, and 3 - which might be +// questionable public void test1004() { this.runNegativeTest( new String[] { @@ -5261,20 +5269,10 @@ " ^\n" + "The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n" + - "3. ERROR in X.java (at line 6)\n" + - " x.foo(null); // 3\n" + - " ^\n" + - "The variable x can only be null; it was either set to null or checked for null when last used\n" + - "----------\n" + - "4. ERROR in X.java (at line 9)\n" + + "3. ERROR in X.java (at line 9)\n" + " } else if (x != null) { // 6\n" + " ^\n" + "The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + - "----------\n" + - "5. ERROR in X.java (at line 12)\n" + - " x.foo(null); // 8\n" + - " ^\n" + - "The variable x may be null\n" + "----------\n"); } @@ -5428,6 +5426,11 @@ " if (other != null) {\n" + " ^^^^^\n" + "The variable other cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + + "----------\n" + + "2. ERROR in X.java (at line 11)\n" + + " if (other == null) {\n" + + " ^^^^^\n" + + "The variable other cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n"); } @@ -5459,7 +5462,6 @@ ); } -// REVIEW here we do not catch the dead branch: x cannot equal this then null with no assignment in between public void test1013() { this.runNegativeTest( new String[] { @@ -5478,11 +5480,6 @@ " if (x == null) {\n" + " ^\n" + "The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + - "----------\n" + - "2. ERROR in X.java (at line 5)\n" + - " x.foo(this);\n" + - " ^\n" + - "The variable x can only be null; it was either set to null or checked for null when last used\n" + "----------\n"); } @@ -5988,7 +5985,7 @@ "2. ERROR in X.java (at line 8)\n" + " if(a!=null)\n" + " ^\n" + - "The variable a can only be null; it was either set to null or checked for null when last used\n" + + "The variable a cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n"); } @@ -6082,7 +6079,7 @@ " o60 = o0, o61 = o0, o62 = o0, o63 = o0, o64 = o0,\n" + " o65 = o0, o66 = o0, o67 = o0, o68 = o0, o69 = o0;\n" + " if (o65 == null) { /* */ }\n" + // complain - " if (o65 != null) { /* */ }\n" + // quiet (already reported) + " if (o65 != null) { /* */ }\n" + // complain " }\n" + "}\n"}, "----------\n" + @@ -6090,6 +6087,11 @@ " if (o65 == null) { /* */ }\n" + " ^^^\n" + "The variable o65 cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + + "----------\n" + + "2. ERROR in X.java (at line 19)\n" + + " if (o65 != null) { /* */ }\n" + + " ^^^\n" + + "The variable o65 cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + "----------\n"); } #P org.eclipse.jdt.core Index: compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java,v retrieving revision 1.60 diff -u -r1.60 EqualExpression.java --- compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java 24 Jan 2006 10:50:38 -0000 1.60 +++ compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java 27 Jan 2006 12:52:34 -0000 @@ -36,20 +36,23 @@ private void checkVariableComparison(BlockScope scope, FlowContext flowContext, FlowInfo flowInfo, FlowInfo initsWhenTrue, FlowInfo initsWhenFalse, LocalVariableBinding local, int nullStatus, Expression reference) { switch (nullStatus) { case FlowInfo.NULL : - flowContext.recordUsingNullReference(scope, local, reference, - FlowContext.CAN_ONLY_NULL_NON_NULL, flowInfo); - if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) { - initsWhenTrue.markAsComparedEqualToNull(local); // from thereon it is set - initsWhenFalse.markAsComparedEqualToNonNull(local); // from thereon it is set - } else { - initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set - initsWhenFalse.markAsComparedEqualToNull(local); // from thereon it is set + if (!flowContext.recordUsingNullReference(scope, local, reference, + FlowContext.CAN_ONLY_NULL_NON_NULL, flowInfo)) { + // no error reported (yet), mark downstream flow + // WORK consider inverting the logic - saves a not operation + if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) { + initsWhenTrue.markAsComparedEqualToNull(local); // from thereon it is set + initsWhenFalse.markAsComparedEqualToNonNull(local); // from thereon it is set + } else { + initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set + initsWhenFalse.markAsComparedEqualToNull(local); // from thereon it is set + } } break; case FlowInfo.NON_NULL : - flowContext.recordUsingNullReference(scope, local, reference, - FlowContext.CAN_ONLY_NULL, flowInfo); - if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) { + if (!flowContext.recordUsingNullReference(scope, local, reference, + FlowContext.CAN_ONLY_NULL, flowInfo) + && ((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) { initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set } break; Index: compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java,v retrieving revision 1.18 diff -u -r1.18 FinallyFlowContext.java --- compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java 24 Jan 2006 10:50:38 -0000 1.18 +++ compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java 27 Jan 2006 12:52:34 -0000 @@ -186,7 +186,7 @@ return true; } - public void recordUsingNullReference(Scope scope, LocalVariableBinding local, + public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local, Expression reference, int checkType, FlowInfo flowInfo) { if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0) { if (deferNullDiagnostic) { // within an enclosing loop, be conservative @@ -196,21 +196,22 @@ if (flowInfo.isProtectedNonNull(local)) { if (checkType == CAN_ONLY_NULL_NON_NULL) { scope.problemReporter().localVariableCannotBeNull(local, reference); + return true; } - return; + return false; } if (flowInfo.isProtectedNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } break; case MAY_NULL : if (flowInfo.isProtectedNonNull(local)) { - return; + return false; } if (flowInfo.isProtectedNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } break; default: @@ -222,25 +223,25 @@ case CAN_ONLY_NULL_NON_NULL : if (flowInfo.isDefinitelyNonNull(local)) { scope.problemReporter().localVariableCannotBeNull(local, reference); - return; + return true; } case CAN_ONLY_NULL: if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } break; case MAY_NULL : if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } if (flowInfo.isPotentiallyNull(local)) { scope.problemReporter().localVariableMayBeNull(local, reference); - return; + return true; } if (flowInfo.isDefinitelyNonNull(local)) { - return; // shortcut: cannot be null + return false; // shortcut: cannot be null } break; default: @@ -250,6 +251,7 @@ recordNullReference(local, reference, checkType); // prepare to re-check with try/catch flow info } + return false; } void removeFinalAssignmentIfAny(Reference reference) { Index: compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java,v retrieving revision 1.31 diff -u -r1.31 LoopingFlowContext.java --- compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java 24 Jan 2006 10:50:38 -0000 1.31 +++ compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java 27 Jan 2006 12:52:34 -0000 @@ -303,11 +303,11 @@ nullCheckTypes[nullCount++] = status; } -public void recordUsingNullReference(Scope scope, LocalVariableBinding local, +public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local, Expression reference, int checkType, FlowInfo flowInfo) { if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) != 0 || flowInfo.isDefinitelyUnknown(local)) { - return; + return false; } switch (checkType) { case CAN_ONLY_NULL_NON_NULL : @@ -315,35 +315,37 @@ if (flowInfo.isDefinitelyNonNull(local)) { if (checkType == CAN_ONLY_NULL_NON_NULL) { scope.problemReporter().localVariableCannotBeNull(local, reference); + return true; } - return; + return false; } if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } if (flowInfo.isPotentiallyUnknown(local)) { - return; + return false; } recordNullReference(local, reference, checkType); - return; + return false; case MAY_NULL : if (flowInfo.isDefinitelyNonNull(local)) { - return; + return false; } if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } if (flowInfo.isPotentiallyNull(local)) { scope.problemReporter().localVariableMayBeNull(local, reference); - return; + return true; } recordNullReference(local, reference, checkType); - return; + return false; default: // never happens } + return false; } void removeFinalAssignmentIfAny(Reference reference) { Index: compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java,v retrieving revision 1.47 diff -u -r1.47 FlowContext.java --- compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java 24 Jan 2006 10:50:38 -0000 1.47 +++ compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java 27 Jan 2006 12:52:34 -0000 @@ -472,48 +472,50 @@ * perform supplementary checks against flow info instances that cannot * be known at the time of calling this method (they are influenced by * code that follows the current point) + * @return true if a problem has been reported, false else */ -public void recordUsingNullReference(Scope scope, LocalVariableBinding local, +public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local, Expression reference, int checkType, FlowInfo flowInfo) { if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) != 0 || flowInfo.isDefinitelyUnknown(local)) { - return; + return false; } switch (checkType) { case CAN_ONLY_NULL_NON_NULL : if (flowInfo.isDefinitelyNonNull(local)) { scope.problemReporter().localVariableCannotBeNull(local, reference); - return; + return true; } else if (flowInfo.isPotentiallyUnknown(local)) { - return; + return false; } case CAN_ONLY_NULL: if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } else if (flowInfo.isPotentiallyUnknown(local)) { - return; + return false; } break; case MAY_NULL : if (flowInfo.isDefinitelyNull(local)) { scope.problemReporter().localVariableCanOnlyBeNull(local, reference); - return; + return true; } if (flowInfo.isPotentiallyNull(local)) { scope.problemReporter().localVariableMayBeNull(local, reference); - return; + return false; } break; default: // never happens } if (parent != null) { - parent.recordUsingNullReference(scope, local, reference, checkType, + return parent.recordUsingNullReference(scope, local, reference, checkType, flowInfo); } + return false; } void removeFinalAssignmentIfAny(Reference reference) {