InsertParenthesesVisitor: don't insert parentheses for "a && b && c"

icsharpcode · Feb 24, 2011 · 2200240 · 2200240
1 parent 5a34a9c
commit 2200240
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 4 deletions.
diff --git a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs
@@ -159,7 +159,7 @@ public void TypeTestInConditional()
 		public void MethodCallOnQueryExpression()
 		{
 			Expression expr = new QueryExpression {
-				Clauses = new QueryClause[] {
+				Clauses = {
 					new QueryFromClause {
 						Identifier = "a",
 						Expression = new IdentifierExpression("b")
@@ -178,7 +178,7 @@ public void MethodCallOnQueryExpression()
 		public void SumOfQueries()
 		{
 			QueryExpression query = new QueryExpression {
-				Clauses = new QueryClause[] {
+				Clauses = {
 					new QueryFromClause {
 						Identifier = "a",
 						Expression = new IdentifierExpression("b")
@@ -206,7 +206,7 @@ public void SumOfQueries()
 		public void QueryInTypeTest()
 		{
 			Expression expr = new QueryExpression {
-				Clauses = new QueryClause[] {
+				Clauses = {
 					new QueryFromClause {
 						Identifier = "a",
 						Expression = new IdentifierExpression("b")
@@ -252,5 +252,73 @@ public void PostPre()
 			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
@@ -186,7 +186,13 @@ public override object VisitBinaryOperatorExpression(BinaryOperatorExpression bi
 				}
 			} 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 @@ public override object VisitBinaryOperatorExpression(BinaryOperatorExpression bi
 			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) {