From 2200240ef60fc49f32b89eb85c9f3a8ba796b08d Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Thu, 24 Feb 2011 02:03:58 +0100 Subject: [PATCH] InsertParenthesesVisitor: don't insert parentheses for "a && b && c" --- .../CSharp/InsertParenthesesVisitorTests.cs | 74 ++++++++++++++++++- .../OutputVisitor/InsertParenthesesVisitor.cs | 17 ++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs index fd044068fd..5fa0b54694 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs @@ -159,7 +159,7 @@ namespace ICSharpCode.NRefactory.CSharp public void MethodCallOnQueryExpression() { Expression expr = new QueryExpression { - Clauses = new QueryClause[] { + Clauses = { new QueryFromClause { Identifier = "a", Expression = new IdentifierExpression("b") @@ -178,7 +178,7 @@ namespace ICSharpCode.NRefactory.CSharp public void SumOfQueries() { QueryExpression query = new QueryExpression { - Clauses = new QueryClause[] { + Clauses = { new QueryFromClause { Identifier = "a", Expression = new IdentifierExpression("b") @@ -206,7 +206,7 @@ namespace ICSharpCode.NRefactory.CSharp public void QueryInTypeTest() { Expression expr = new QueryExpression { - Clauses = new QueryClause[] { + Clauses = { new QueryFromClause { Identifier = "a", Expression = new IdentifierExpression("b") @@ -252,5 +252,73 @@ namespace ICSharpCode.NRefactory.CSharp Assert.AreEqual("(++a)++", InsertRequired(expr)); Assert.AreEqual("(++a)++", InsertReadable(expr)); } + + [Test] + public void Logical1() + { + Expression expr = new BinaryOperatorExpression( + new BinaryOperatorExpression( + new IdentifierExpression("a"), + BinaryOperatorType.ConditionalAnd, + new IdentifierExpression("b") + ), + BinaryOperatorType.ConditionalAnd, + new IdentifierExpression("c") + ); + + Assert.AreEqual("a && b && c", InsertRequired(expr)); + Assert.AreEqual("a && b && c", InsertReadable(expr)); + } + + [Test] + public void Logical2() + { + Expression expr = new BinaryOperatorExpression( + new IdentifierExpression("a"), + BinaryOperatorType.ConditionalAnd, + new BinaryOperatorExpression( + new IdentifierExpression("b"), + BinaryOperatorType.ConditionalAnd, + new IdentifierExpression("c") + ) + ); + + Assert.AreEqual("a && (b && c)", InsertRequired(expr)); + Assert.AreEqual("a && (b && c)", InsertReadable(expr)); + } + + [Test] + public void Logical3() + { + Expression expr = new BinaryOperatorExpression( + new IdentifierExpression("a"), + BinaryOperatorType.ConditionalOr, + new BinaryOperatorExpression( + new IdentifierExpression("b"), + BinaryOperatorType.ConditionalAnd, + new IdentifierExpression("c") + ) + ); + + Assert.AreEqual("a || b && c", InsertRequired(expr)); + Assert.AreEqual("a || (b && c)", InsertReadable(expr)); + } + + [Test] + public void Logical4() + { + Expression expr = new BinaryOperatorExpression( + new IdentifierExpression("a"), + BinaryOperatorType.ConditionalAnd, + new BinaryOperatorExpression( + new IdentifierExpression("b"), + BinaryOperatorType.ConditionalOr, + new IdentifierExpression("c") + ) + ); + + Assert.AreEqual("a && (b || c)", InsertRequired(expr)); + Assert.AreEqual("a && (b || c)", InsertReadable(expr)); + } } } diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs index abdd06e630..cfa12e17a7 100644 --- a/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs @@ -186,7 +186,13 @@ namespace ICSharpCode.NRefactory.CSharp } } else { if (InsertParenthesesForReadability && precedence < Equality) { - ParenthesizeIfRequired(binaryOperatorExpression.Left, Equality); + // In readable mode, boost the priority of the left-hand side if the operator + // there isn't the same as the operator on this expression. + if (GetBinaryOperatorType(binaryOperatorExpression.Left) == binaryOperatorExpression.Operator) { + ParenthesizeIfRequired(binaryOperatorExpression.Left, precedence); + } else { + ParenthesizeIfRequired(binaryOperatorExpression.Left, Equality); + } ParenthesizeIfRequired(binaryOperatorExpression.Right, Equality); } else { // all other binary operators are left-associative @@ -197,6 +203,15 @@ namespace ICSharpCode.NRefactory.CSharp return base.VisitBinaryOperatorExpression(binaryOperatorExpression, data); } + BinaryOperatorType? GetBinaryOperatorType(Expression expr) + { + BinaryOperatorExpression boe = expr as BinaryOperatorExpression; + if (boe != null) + return boe.Operator; + else + return null; + } + public override object VisitIsExpression(IsExpression isExpression, object data) { if (InsertParenthesesForReadability) {