From 73129820f8d7bc92a1065488c75bba893d1004fd Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sat, 12 Feb 2011 15:29:00 +0100 Subject: [PATCH] Bugfixes for InsertParenthesesVisitor and OutputVisitor. --- .../CSharp/InsertParenthesesVisitorTests.cs | 77 +++++++++++++++++++ .../CSharp/Ast/Expressions/QueryExpression.cs | 2 +- .../OutputVisitor/InsertParenthesesVisitor.cs | 38 +++++++-- .../CSharp/OutputVisitor/OutputVisitor.cs | 7 +- 4 files changed, 116 insertions(+), 8 deletions(-) diff --git a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs index 2649d51ffb..604ec79bcf 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs @@ -133,5 +133,82 @@ namespace ICSharpCode.NRefactory.CSharp Assert.AreEqual("a is int? ?b:c", InsertRequired(expr)); Assert.AreEqual("(a is int?)?b:c", InsertReadable(expr)); } + + [Test] + public void MethodCallOnQueryExpression() + { + Expression expr = new QueryExpression { + Clauses = new QueryClause[] { + new QueryFromClause { + Identifier = "a", + Expression = new IdentifierExpression("b") + }, + new QuerySelectClause { + Expression = new IdentifierExpression("a").Invoke("c") + } + } + }.Invoke("ToArray"); + + Assert.AreEqual("(from a in b" + Environment.NewLine + "select a.c ()).ToArray ()", InsertRequired(expr)); + Assert.AreEqual("(from a in b" + Environment.NewLine + "select a.c ()).ToArray ()", InsertReadable(expr)); + } + + [Test] + public void SumOfQueries() + { + QueryExpression query = new QueryExpression { + Clauses = new QueryClause[] { + new QueryFromClause { + Identifier = "a", + Expression = new IdentifierExpression("b") + }, + new QuerySelectClause { + Expression = new IdentifierExpression("a") + } + } + }; + Expression expr = new BinaryOperatorExpression( + query, + BinaryOperatorType.Add, + query.Clone() + ); + + Assert.AreEqual("(from a in b" + Environment.NewLine + + "select a) + from a in b" + Environment.NewLine + + "select a", InsertRequired(expr)); + Assert.AreEqual("(from a in b" + Environment.NewLine + + "select a) + (from a in b" + Environment.NewLine + + "select a)", InsertReadable(expr)); + } + + [Test] + public void PrePost() + { + Expression expr = new UnaryOperatorExpression( + UnaryOperatorType.Increment, + new UnaryOperatorExpression( + UnaryOperatorType.PostIncrement, + new IdentifierExpression("a") + ) + ); + + Assert.AreEqual("++a++", InsertRequired(expr)); + Assert.AreEqual("++(a++)", InsertReadable(expr)); + } + + [Test] + public void PostPre() + { + Expression expr = new UnaryOperatorExpression( + UnaryOperatorType.PostIncrement, + new UnaryOperatorExpression( + UnaryOperatorType.Increment, + new IdentifierExpression("a") + ) + ); + + Assert.AreEqual("(++a)++", InsertRequired(expr)); + Assert.AreEqual("(++a)++", InsertReadable(expr)); + } } } diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/QueryExpression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/QueryExpression.cs index d54e1417c4..eb4e77fa48 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/QueryExpression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/QueryExpression.cs @@ -35,7 +35,7 @@ namespace ICSharpCode.NRefactory.CSharp public override S AcceptVisitor(AstVisitor visitor, T data) { - throw new NotImplementedException(); + return visitor.VisitQueryExpression (this, data); } } diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs index ff52114382..76d6580a03 100644 --- a/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs @@ -20,12 +20,13 @@ namespace ICSharpCode.NRefactory.CSharp /// public bool InsertParenthesesForReadability { get; set; } - const int Primary = 15; + const int Primary = 16; + const int QueryOrLambda = 15; const int Unary = 14; const int RelationalAndTypeTesting = 10; const int Equality = 9; const int Conditional = 2; - const int AssignmentAndLambda = 1; + const int Assignment = 1; /// /// Gets the row number in the C# 4.0 spec operator precedence table. @@ -33,6 +34,11 @@ namespace ICSharpCode.NRefactory.CSharp static int GetPrecedence(Expression expr) { // Note: the operator precedence table on MSDN is incorrect + if (expr is QueryExpression) { + // Not part of the table in the C# spec, but we need to ensure that queries within + // primary expressions get parenthesized. + return QueryOrLambda; + } UnaryOperatorExpression uoe = expr as UnaryOperatorExpression; if (uoe != null) { if (uoe.Operator == UnaryOperatorType.PostDecrement || uoe.Operator == UnaryOperatorType.PostIncrement) @@ -84,7 +90,7 @@ namespace ICSharpCode.NRefactory.CSharp if (expr is ConditionalExpression) return Conditional; if (expr is AssignmentExpression || expr is LambdaExpression) - return AssignmentAndLambda; + return Assignment; // anything else: primary expression return Primary; } @@ -132,7 +138,10 @@ namespace ICSharpCode.NRefactory.CSharp // Unary expressions public override object VisitUnaryOperatorExpression(UnaryOperatorExpression unaryOperatorExpression, object data) { - ParenthesizeIfRequired(unaryOperatorExpression.Expression, InsertParenthesesForReadability ? Primary : Unary); + ParenthesizeIfRequired(unaryOperatorExpression.Expression, GetPrecedence(unaryOperatorExpression)); + UnaryOperatorExpression child = unaryOperatorExpression.Expression as UnaryOperatorExpression; + if (child != null && InsertParenthesesForReadability) + Parenthesize(child); return base.VisitUnaryOperatorExpression(unaryOperatorExpression, data); } @@ -233,15 +242,32 @@ namespace ICSharpCode.NRefactory.CSharp public override object VisitAssignmentExpression(AssignmentExpression assignmentExpression, object data) { // assignment is right-associative - ParenthesizeIfRequired(assignmentExpression.Left, AssignmentAndLambda + 1); + ParenthesizeIfRequired(assignmentExpression.Left, Assignment + 1); if (InsertParenthesesForReadability) { ParenthesizeIfRequired(assignmentExpression.Right, RelationalAndTypeTesting + 1); } else { - ParenthesizeIfRequired(assignmentExpression.Right, AssignmentAndLambda); + ParenthesizeIfRequired(assignmentExpression.Right, Assignment); } return base.VisitAssignmentExpression(assignmentExpression, data); } // don't need to handle lambdas, they have lowest precedence and unambiguous associativity + + public override object VisitQueryExpression(QueryExpression queryExpression, object data) + { + // Query expressions are strange beasts: + // "var a = -from b in c select d;" is valid, so queries bind stricter than unary expressions. + // However, the end of the query is greedy. So their start sort of have a high precedence, + // while their end has a very low precedence. We handle this by checking whether a query is used + // as left part of a binary operator, and parenthesize it if required. + if (queryExpression.Role == BinaryOperatorExpression.LeftRole) + Parenthesize(queryExpression); + if (InsertParenthesesForReadability) { + // when readability is desired, always parenthesize query expressions within unary or binary operators + if (queryExpression.Parent is UnaryOperatorExpression || queryExpression.Parent is BinaryOperatorExpression) + Parenthesize(queryExpression); + } + return base.VisitQueryExpression(queryExpression, data); + } } } diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs index 51f75d75f1..b0ef7b0d0f 100644 --- a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs @@ -896,8 +896,13 @@ namespace ICSharpCode.NRefactory.CSharp public object VisitUnaryOperatorExpression(UnaryOperatorExpression unaryOperatorExpression, object data) { StartNode(unaryOperatorExpression); - WriteToken(UnaryOperatorExpression.GetOperatorSymbol(unaryOperatorExpression.Operator), UnaryOperatorExpression.OperatorRole); + UnaryOperatorType opType = unaryOperatorExpression.Operator; + string opSymbol = UnaryOperatorExpression.GetOperatorSymbol(opType); + if (!(opType == UnaryOperatorType.PostIncrement || opType == UnaryOperatorType.PostDecrement)) + WriteToken(opSymbol, UnaryOperatorExpression.OperatorRole); unaryOperatorExpression.Expression.AcceptVisitor(this, data); + if (opType == UnaryOperatorType.PostIncrement || opType == UnaryOperatorType.PostDecrement) + WriteToken(opSymbol, UnaryOperatorExpression.OperatorRole); return EndNode(unaryOperatorExpression); }