commit d770d2f81305452790b7afdcee71a008bd9fb057 Author: Cedric Vivier Date: Sun Jan 18 00:17:18 2009 +0800 BOO-1123: WARNING: Assignment/Comparison made to/with same expression. --- ast.model.boo | 107 +++++++++++++------- src/Boo.Lang.Compiler/Ast/AstUtil.cs | 16 +--- src/Boo.Lang.Compiler/CompilerWarningFactory.cs | 10 ++ .../Steps/StricterErrorChecking.cs | 42 ++++++++ src/Boo.Lang/Resources/strings.txt | 2 + tests/testcases/warnings/BCW0020-1.boo | 43 ++++++++ tests/testcases/warnings/BCW0021-1.boo | 65 ++++++++++++ 7 files changed, 233 insertions(+), 52 deletions(-) diff --git a/ast.model.boo b/ast.model.boo index 30ffd0a..39156a1 100755 --- a/ast.model.boo +++ b/ast.model.boo @@ -9,10 +9,11 @@ namespace Boo.Ast class CompileUnit(Node): Modules as ModuleCollection +[Flags] enum TypeMemberModifiers: None = 0 Private = 1 - Internal = 2 + Internal = 2 Protected = 4 Public = 8 Transient = 16 @@ -348,45 +349,73 @@ class MethodInvocationExpression(Expression, INodeWithArguments): Arguments as ExpressionCollection NamedArguments as ExpressionPairCollection + enum BinaryOperatorType: - None - Addition - Subtraction - Multiply - Division - Modulus - Exponentiation - LessThan - LessThanOrEqual - GreaterThan - GreaterThanOrEqual - Equality - Inequality - Match - NotMatch - Assign - InPlaceAddition - InPlaceSubtraction - InPlaceMultiply - InPlaceDivision - InPlaceModulus - InPlaceBitwiseAnd - InPlaceBitwiseOr - ReferenceEquality - ReferenceInequality - TypeTest - Member - NotMember - Or - And - BitwiseOr - BitwiseAnd - ExclusiveOr - InPlaceExclusiveOr - ShiftLeft - InPlaceShiftLeft - ShiftRight - InPlaceShiftRight + None = 0 + + //BinaryOperatorMask.Arithmetic = 0xF + Addition = 0x1 + Subtraction = 0x2 + Multiply = 0x3 + Division = 0x4 + Modulus = 0x5 + Exponentiation = 0x6 + + //BinaryOperatorMask.Comparison = 0xFF0 + LessThan = 0x010 + LessThanOrEqual = 0x020 + GreaterThan = 0x030 + GreaterThanOrEqual = 0x040 + Equality = 0x050 + Inequality = 0x060 + Match = 0x070 + NotMatch = 0x080 + //BinaryOperatorMask.TypeComparison = 0xF00 + ReferenceEquality = 0x100 + ReferenceInequality = 0x200 + TypeTest = 0x300 + Member = 0x400 + NotMember = 0x500 + + //BinaryOperatorMask.Assignment = 0xFF000 + Assign = 0x01000 + //BinaryOperatorMask.InPlaceAssignment = 0xF0000 + InPlaceAddition = 0x10000 + InPlaceSubtraction = 0x20000 + InPlaceMultiply = 0x30000 + InPlaceDivision = 0x40000 + InPlaceModulus = 0x50000 + InPlaceBitwiseAnd = 0x60000 + InPlaceBitwiseOr = 0x70000 + InPlaceExclusiveOr = 0x80000 + InPlaceShiftLeft = 0x90000 + InPlaceShiftRight = 0xA0000 + + //BinaryOperatorMask.Logical = 0x0F00000 + Or = 0x0100000 + And = 0x0200000 + + //BinaryOperatorMask.Bitwise = 0xF000000 + BitwiseOr = 0x1000000 + BitwiseAnd = 0x2000000 + ExclusiveOr = 0x3000000 + ShiftLeft = 0x4000000 + ShiftRight = 0x5000000 + + +enum BinaryOperatorMask: + None = 0 + + Arithmetic = 0xF + Comparison = 0xFF0 + TypeComparison = 0xF00 + + Assignment = 0xFF000 + InPlaceAssignment = 0xF0000 + + Logical = 0x0F00000 + Bitwise = 0xF000000 + enum UnaryOperatorType: None diff --git a/src/Boo.Lang.Compiler/Ast/AstUtil.cs b/src/Boo.Lang.Compiler/Ast/AstUtil.cs index e187eac..c00a06d 100755 --- a/src/Boo.Lang.Compiler/Ast/AstUtil.cs +++ b/src/Boo.Lang.Compiler/Ast/AstUtil.cs @@ -263,22 +263,12 @@ namespace Boo.Lang.Compiler.Ast } return false; } - + public static bool IsAssignmentOperator(BinaryOperatorType op) { - return BinaryOperatorType.Assign == op || - BinaryOperatorType.InPlaceAddition == op || - BinaryOperatorType.InPlaceSubtraction == op || - BinaryOperatorType.InPlaceMultiply == op || - BinaryOperatorType.InPlaceDivision == op || - BinaryOperatorType.InPlaceModulus == op || - BinaryOperatorType.InPlaceBitwiseAnd == op || - BinaryOperatorType.InPlaceBitwiseOr == op || - BinaryOperatorType.InPlaceExclusiveOr == op || - BinaryOperatorType.InPlaceShiftLeft == op || - BinaryOperatorType.InPlaceShiftRight == op; + return (((int)op & (int)BinaryOperatorMask.Assignment) != 0); } - + public static Constructor CreateConstructor(Node lexicalInfoProvider, TypeMemberModifiers modifiers) { Constructor constructor = new Constructor(lexicalInfoProvider.LexicalInfo); diff --git a/src/Boo.Lang.Compiler/CompilerWarningFactory.cs b/src/Boo.Lang.Compiler/CompilerWarningFactory.cs index 31649bf..532dc68 100644 --- a/src/Boo.Lang.Compiler/CompilerWarningFactory.cs +++ b/src/Boo.Lang.Compiler/CompilerWarningFactory.cs @@ -144,6 +144,16 @@ namespace Boo.Lang.Compiler return new CompilerWarning("BCW0019", node.LexicalInfo, node.Declaration.Name); } + public static CompilerWarning AssignmentToSameVariable(BinaryExpression node) + { + return new CompilerWarning("BCW0020", node.LexicalInfo); + } + + public static CompilerWarning ComparisonWithSameVariable(BinaryExpression node) + { + return new CompilerWarning("BCW0021", node.LexicalInfo); + } + private static string NodeTypeString(Node node) { diff --git a/src/Boo.Lang.Compiler/Steps/StricterErrorChecking.cs b/src/Boo.Lang.Compiler/Steps/StricterErrorChecking.cs index ed0c030..5ab0a47 100644 --- a/src/Boo.Lang.Compiler/Steps/StricterErrorChecking.cs +++ b/src/Boo.Lang.Compiler/Steps/StricterErrorChecking.cs @@ -187,6 +187,18 @@ namespace Boo.Lang.Compiler.Steps Warnings.Add( CompilerWarningFactory.IsInsteadOfIsa(node)); } + } + + //check that the assignment or comparison is meaningful + if (BinaryOperatorType.Assign == node.Operator + || ((int)node.Operator & (int)BinaryOperatorMask.Comparison) != 0) + { + if (AreEquivalentExpressions(node.Left, node.Right)) + Warnings.Add( + (BinaryOperatorType.Assign == node.Operator) + ? CompilerWarningFactory.AssignmentToSameVariable(node) + : CompilerWarningFactory.ComparisonWithSameVariable(node) + ); } } @@ -197,6 +209,36 @@ namespace Boo.Lang.Compiler.Steps Error(CompilerErrorFactory.ExplodeExpressionMustMatchVarArgCall(node)); } } + + bool AreEquivalentExpressions(Expression a, Expression b) + { + if (a.NodeType != b.NodeType) + return false; + + switch (a.NodeType) + { + case NodeType.ReferenceExpression: + ReferenceExpression are = (ReferenceExpression) a; + ReferenceExpression bre = (ReferenceExpression) b; + if (are.Name != bre.Name) + return false; + return true; + + case NodeType.MemberReferenceExpression: + MemberReferenceExpression amre = (MemberReferenceExpression) a; + MemberReferenceExpression bmre = (MemberReferenceExpression) b; + if (amre.Name != bmre.Name) + return false; + if (!AreEquivalentExpressions(amre.Target, bmre.Target)) + return false; + return true; + + case NodeType.SelfLiteralExpression: + case NodeType.SuperLiteralExpression: + return true; + } + return false; + } private bool IsLastArgumentOfVarArgInvocation(UnaryExpression node) { diff --git a/src/Boo.Lang/Resources/strings.txt b/src/Boo.Lang/Resources/strings.txt index cafbbbc..20ccacd 100644 --- a/src/Boo.Lang/Resources/strings.txt +++ b/src/Boo.Lang/Resources/strings.txt @@ -188,6 +188,8 @@ BCW0016=WARNING: Namespace '{0}' is never used. BCW0017=WARNING: New protected {0} '{1}' declared in sealed class '{2}'. BCW0018=WARNING: Overriding 'object.Finalize()' is bad practice. You should use destructor syntax instead. BCW0019=WARNING: 'except {0}:' is ambiguous. Did you mean 'except as {0}:'? +BCW0020=WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0021=WARNING: Comparison made with same expression. Did you mean to compare with something else? ; BooC Command Line Errors BCE0500=Response file '{0}' listed more than once. diff --git a/tests/testcases/warnings/BCW0020-1.boo b/tests/testcases/warnings/BCW0020-1.boo new file mode 100644 index 0000000..36026d4 --- /dev/null +++ b/tests/testcases/warnings/BCW0020-1.boo @@ -0,0 +1,43 @@ +""" +BCW0020-1.boo(21,11): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0020-1.boo(24,16): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0020-1.boo(27,11): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0020-1.boo(32,3): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0020-1.boo(34,3): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +BCW0020-1.boo(37,9): BCW0020: WARNING: Assignment made to same expression. Did you mean to assign to something else? +""" + +class Test: + public static sx = 0 + public static sy = 0 + + x = 0 + y = 2 + + def Good(x as int): + self.x = x + + def Ambiguous(x as int): + x = x # + + def Bad1(x as int): + self.x = self.x # + + def Bad2(y as int): + x = x #implicit field + +x = 1 +y = 2 +x = y +x = x # +y = x +y = y # +y = 0 + +Test.sx = Test.sx # +Test.sx = Test.sy + +x &= x +x ^= x +x |= x + diff --git a/tests/testcases/warnings/BCW0021-1.boo b/tests/testcases/warnings/BCW0021-1.boo new file mode 100644 index 0000000..ec05a7b --- /dev/null +++ b/tests/testcases/warnings/BCW0021-1.boo @@ -0,0 +1,65 @@ +""" +BCW0021-1.boo(29,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(33,22): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(37,30): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(45,9): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(48,9): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(53,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(55,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(57,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(59,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(61,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +BCW0021-1.boo(63,14): BCW0021: WARNING: Comparison made with same expression. Did you mean to compare with something else? +""" + +class Test: + public static T = Test() + + public static sx = 0 + public static sy = 0 + + x = 0 + y = 2 + + def Good(x as int): + if self.x == x: + return true + + def Ambiguous(x as int): + if x == x: # + return true + + def Bad1(x as int): + while self.x == self.x: # + pass + + def Bad2(y as int): + print "foo" unless x == x #implicit field + + +x = 1 +y = 2 + +print x == 1 +print y == 0 +print x == x # +print x == y +print y == x +print y == y # + +if Test.T.sx != Test.T.sy: + pass + +if Test.T.sx == Test.T.sx: # + pass +if Test.T.sx != Test.T.sx: # + pass +if Test.T.sx > Test.T.sx: # + pass +if Test.T.sx < Test.T.sx: # + pass +if Test.T.sx >= Test.T.sx: # + pass +if Test.T.sx <= Test.T.sx: # + pass +