diff --git a/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTest.cs b/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs similarity index 96% rename from ICSharpCode.NRefactory.Tests/CSharp/AstStructureTest.cs rename to ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs index 94e707f9dc..16534dd1d6 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTest.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs @@ -8,7 +8,7 @@ using NUnit.Framework; namespace ICSharpCode.NRefactory.CSharp { [TestFixture] - public class AstStructureTest + public class AstStructureTests { [Test] public void RolesAreStaticReadOnly() diff --git a/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs new file mode 100644 index 0000000000..6cafb6a5a2 --- /dev/null +++ b/ICSharpCode.NRefactory.Tests/CSharp/InsertParenthesesVisitorTests.cs @@ -0,0 +1,91 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt) +// This code is distributed under MIT X11 license (for details please see \doc\license.txt) + +using System; +using System.IO; +using NUnit.Framework; + +namespace ICSharpCode.NRefactory.CSharp +{ + [TestFixture] + public class InsertParenthesesVisitorTests + { + string InsertReadable(Expression expr) + { + expr = expr.Clone(); + expr.AcceptVisitor(new InsertParenthesesVisitor { InsertParenthesesForReadability = true }, null); + StringWriter w = new StringWriter(); + expr.AcceptVisitor(new OutputVisitor(w, new CSharpFormattingPolicy()), null); + return w.ToString(); + } + + string InsertRequired(Expression expr) + { + expr = expr.Clone(); + expr.AcceptVisitor(new InsertParenthesesVisitor { InsertParenthesesForReadability = false }, null); + StringWriter w = new StringWriter(); + expr.AcceptVisitor(new OutputVisitor(w, new CSharpFormattingPolicy()), null); + return w.ToString(); + } + + [Test] + public void EqualityInAssignment() + { + Expression expr = new AssignmentExpression( + new IdentifierExpression("cond"), + new BinaryOperatorExpression( + new IdentifierExpression("a"), + BinaryOperatorType.Equality, + new IdentifierExpression("b") + ) + ); + + Assert.AreEqual("cond = a == b", InsertRequired(expr)); + Assert.AreEqual("cond = (a == b)", InsertReadable(expr)); + } + + [Test] + public void TrickyCast1() + { + Expression expr = new UnaryOperatorExpression( + UnaryOperatorType.Minus, new IdentifierExpression("a") + ).CastTo(new PrimitiveType("int")); + + Assert.AreEqual("(int)-a", InsertRequired(expr)); + Assert.AreEqual("(int)(-a)", InsertReadable(expr)); + } + + [Test] + public void TrickyCast2() + { + Expression expr = new UnaryOperatorExpression( + UnaryOperatorType.Minus, new IdentifierExpression("a") + ).CastTo(new SimpleType("MyType")); + + Assert.AreEqual("(MyType)(-a)", InsertRequired(expr)); + Assert.AreEqual("(MyType)(-a)", InsertReadable(expr)); + } + + [Test] + public void TrickyCast3() + { + Expression expr = new UnaryOperatorExpression( + UnaryOperatorType.Not, new IdentifierExpression("a") + ).CastTo(new SimpleType("MyType")); + + Assert.AreEqual("(MyType)!a", InsertRequired(expr)); + Assert.AreEqual("(MyType)(!a)", InsertReadable(expr)); + } + + [Test] + public void CastAndInvoke() + { + Expression expr = new IdentifierExpression("a") + .CastTo(new PrimitiveType("string")) + .Member("Length"); + + Assert.AreEqual("((string)a).Length", InsertRequired(expr)); + Assert.AreEqual("((string)a).Length", InsertReadable(expr)); + } + } +} diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/BinaryOperatorExpressionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/BinaryOperatorExpressionTests.cs index e770064056..9e133b7637 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/BinaryOperatorExpressionTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/BinaryOperatorExpressionTests.cs @@ -69,9 +69,9 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.Expression OperatorPrecedenceTest("==", BinaryOperatorType.Equality, "&", BinaryOperatorType.BitwiseAnd, false); OperatorPrecedenceTest("&", BinaryOperatorType.BitwiseAnd, "^", BinaryOperatorType.ExclusiveOr, false); OperatorPrecedenceTest("^", BinaryOperatorType.ExclusiveOr, "|", BinaryOperatorType.BitwiseOr, false); - OperatorPrecedenceTest("|", BinaryOperatorType.BitwiseOr, "&&", BinaryOperatorType.LogicalAnd, false); - OperatorPrecedenceTest("&&", BinaryOperatorType.LogicalAnd, "||", BinaryOperatorType.LogicalOr, false); - OperatorPrecedenceTest("||", BinaryOperatorType.LogicalOr, "??", BinaryOperatorType.NullCoalescing, false); + OperatorPrecedenceTest("|", BinaryOperatorType.BitwiseOr, "&&", BinaryOperatorType.ConditionalAnd, false); + OperatorPrecedenceTest("&&", BinaryOperatorType.ConditionalAnd, "||", BinaryOperatorType.ConditionalOr, false); + OperatorPrecedenceTest("||", BinaryOperatorType.ConditionalOr, "??", BinaryOperatorType.NullCoalescing, false); } #endregion @@ -116,13 +116,13 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.Expression [Test] public void LogicalAndTest() { - TestBinaryOperatorExpressionTest("a && b", BinaryOperatorType.LogicalAnd); + TestBinaryOperatorExpressionTest("a && b", BinaryOperatorType.ConditionalAnd); } [Test] public void LogicalOrTest() { - TestBinaryOperatorExpressionTest("a || b", BinaryOperatorType.LogicalOr); + TestBinaryOperatorExpressionTest("a || b", BinaryOperatorType.ConditionalOr); } [Test] @@ -221,7 +221,7 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.Expression { const string expr = "i1 < 0 || i1 > (Count - 1)"; BinaryOperatorExpression boe = ParseUtilCSharp.ParseExpression(expr); - Assert.AreEqual(BinaryOperatorType.LogicalOr, boe.Operator); + Assert.AreEqual(BinaryOperatorType.ConditionalOr, boe.Operator); } } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/CastExpressionTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/CastExpressionTests.cs index a55d27c3af..57604d083b 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/CastExpressionTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Parser/Expression/CastExpressionTests.cs @@ -133,7 +133,7 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.Expression public void IntMaxValueToBigInt() { CastExpression ce = ParseUtilCSharp.ParseExpression("(BigInt)int.MaxValue"); - Assert.AreEqual("BigInt", ce.CastTo.ToString()); + Assert.AreEqual("BigInt", ce.Type.ToString()); Assert.IsTrue(ce.Expression is MemberReferenceExpression); } } diff --git a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/BinaryOperatorTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/BinaryOperatorTests.cs index 8e058940c0..1b141ecb01 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/Resolver/BinaryOperatorTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/Resolver/BinaryOperatorTests.cs @@ -345,26 +345,26 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public void LogicalAnd() { AssertConstant(true, resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalAnd, MakeConstant(true), MakeConstant(true))); + BinaryOperatorType.ConditionalAnd, MakeConstant(true), MakeConstant(true))); AssertConstant(false, resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalAnd, MakeConstant(false), MakeConstant(true))); + BinaryOperatorType.ConditionalAnd, MakeConstant(false), MakeConstant(true))); AssertError(typeof(bool), resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalAnd, MakeConstant(false), MakeResult(typeof(bool?)))); + BinaryOperatorType.ConditionalAnd, MakeConstant(false), MakeResult(typeof(bool?)))); } [Test] public void LogicalOr() { AssertConstant(true, resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalOr, MakeConstant(false), MakeConstant(true))); + BinaryOperatorType.ConditionalOr, MakeConstant(false), MakeConstant(true))); AssertConstant(false, resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalOr, MakeConstant(false), MakeConstant(false))); + BinaryOperatorType.ConditionalOr, MakeConstant(false), MakeConstant(false))); AssertError(typeof(bool), resolver.ResolveBinaryOperator( - BinaryOperatorType.LogicalOr, MakeConstant(false), MakeResult(typeof(bool?)))); + BinaryOperatorType.ConditionalOr, MakeConstant(false), MakeResult(typeof(bool?)))); } [Test] diff --git a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj index b4c71a86f4..f08560c21d 100644 --- a/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj +++ b/ICSharpCode.NRefactory.Tests/ICSharpCode.NRefactory.Tests.csproj @@ -58,7 +58,8 @@ - + + diff --git a/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs b/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs index db6718caaf..232d907c0a 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs @@ -63,7 +63,7 @@ namespace ICSharpCode.NRefactory.CSharp AstNode nextSibling; AstNode firstChild; AstNode lastChild; - Role role = Roles.Root; + Role role = RootRole; public abstract NodeType NodeType { get; @@ -159,10 +159,10 @@ namespace ICSharpCode.NRefactory.CSharp protected void SetChildByRole(Role role, T newChild) where T : AstNode { AstNode oldChild = GetChildByRole(role); - if (oldChild != null) - oldChild.ReplaceWith(newChild); - else + if (oldChild.IsNull) AddChild(newChild, role); + else + oldChild.ReplaceWith(newChild); } protected void SetChildrenByRole(Role role, IEnumerable newChildren) where T : AstNode @@ -229,7 +229,12 @@ namespace ICSharpCode.NRefactory.CSharp throw new ArgumentException ("NextSibling is not a child of this node.", "nextSibling"); // No need to test for "Cannot add children to null nodes", // as there isn't any valid nextSibling in null nodes. - + InsertChildBeforeUnsafe(nextSibling, child, role); + } + + + void InsertChildBeforeUnsafe(AstNode nextSibling, AstNode child, Role role) + { child.parent = this; child.role = role; child.nextSibling = nextSibling; @@ -286,6 +291,9 @@ namespace ICSharpCode.NRefactory.CSharp Remove(); return; } + if (this.parent == null) { + throw new InvalidOperationException(this.IsNull ? "Cannot replace the null nodes" : "Cannot replace the root node"); + } if (newNode.parent != null) { // TODO: what if newNode is used within *this* tree? // e.g. "parenthesizedExpr.ReplaceWith(parenthesizedExpr.Expression);" @@ -323,6 +331,32 @@ namespace ICSharpCode.NRefactory.CSharp } } + public AstNode ReplaceWith(Func replaceFunction) + { + if (replaceFunction == null) + throw new ArgumentNullException("replaceFunction"); + AstNode oldParent = parent; + AstNode oldSuccessor = nextSibling; + Role oldRole = role; + Remove(); + AstNode replacement = replaceFunction(this); + if (oldSuccessor != null && oldSuccessor.parent != oldParent) + throw new InvalidOperationException("replace function changed nextSibling of node being replaced?"); + if (!(replacement == null || replacement.IsNull)) { + if (replacement.parent != null) + throw new InvalidOperationException("replace function must return the root of a tree"); + if (!oldRole.IsValid(replacement)) { + throw new InvalidOperationException (string.Format("The new node '{0}' is not valid in the role {1}", replacement.GetType().Name, oldRole.ToString())); + } + + if (oldSuccessor != null) + oldParent.InsertChildBeforeUnsafe(oldSuccessor, replacement, oldRole); + else + oldParent.AddChildUnsafe(replacement, oldRole); + } + return replacement; + } + /// /// Clones the whole subtree starting at this AST node. /// @@ -526,12 +560,15 @@ namespace ICSharpCode.NRefactory.CSharp public abstract S AcceptVisitor (AstVisitor visitor, T data); + // the Root role must be available when creating the null nodes, so we can't put it in the Roles class + static readonly Role RootRole = new Role("Root"); + public static class Roles { /// /// Root of an abstract syntax tree. /// - public static readonly Role Root = new Role("Root", CSharp.AstNode.Null); + public static readonly Role Root = RootRole; // some pre defined constants for common roles public static readonly Role Identifier = new Role("Identifier", CSharp.Identifier.Null); diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/AssignmentExpression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/AssignmentExpression.cs index 7999380847..20dda792b9 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/AssignmentExpression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/AssignmentExpression.cs @@ -38,6 +38,16 @@ namespace ICSharpCode.NRefactory.CSharp public readonly static Role OperatorRole = BinaryOperatorExpression.OperatorRole; public readonly static Role RightRole = BinaryOperatorExpression.RightRole; + public AssignmentExpression() + { + } + + public AssignmentExpression(Expression left, Expression right) + { + this.Left = left; + this.Right = right; + } + public AssignmentOperatorType Operator { get; set; diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/BinaryOperatorExpression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/BinaryOperatorExpression.cs index 6fc70dddb5..98a7a033fb 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/BinaryOperatorExpression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/BinaryOperatorExpression.cs @@ -37,6 +37,17 @@ namespace ICSharpCode.NRefactory.CSharp public readonly static Role OperatorRole = new Role("Operator", CSharpTokenNode.Null); public readonly static Role RightRole = new Role("Right", Expression.Null); + public BinaryOperatorExpression() + { + } + + public BinaryOperatorExpression(Expression left, BinaryOperatorType op, Expression right) + { + this.Left = left; + this.Operator = op; + this.Right = right; + } + public BinaryOperatorType Operator { get; set; @@ -68,9 +79,9 @@ namespace ICSharpCode.NRefactory.CSharp return "&"; case BinaryOperatorType.BitwiseOr: return "|"; - case BinaryOperatorType.LogicalAnd: + case BinaryOperatorType.ConditionalAnd: return "&&"; - case BinaryOperatorType.LogicalOr: + case BinaryOperatorType.ConditionalOr: return "||"; case BinaryOperatorType.ExclusiveOr: return "^"; @@ -110,14 +121,19 @@ namespace ICSharpCode.NRefactory.CSharp public enum BinaryOperatorType { + // We avoid 'logical or' on purpose, because it's not clear if that refers to the bitwise + // or to the short-circuiting (conditional) operator: + // MCS and old NRefactory used bitwise='|', logical='||' + // but the C# spec uses logical='|', conditional='||' + /// left & right BitwiseAnd, /// left | right BitwiseOr, /// left && right - LogicalAnd, + ConditionalAnd, /// left || right - LogicalOr, + ConditionalOr, /// left ^ right ExclusiveOr, diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/CastExpression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/CastExpression.cs index 67093d44ed..1cf506bb52 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/CastExpression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/CastExpression.cs @@ -35,7 +35,7 @@ namespace ICSharpCode.NRefactory.CSharp get { return GetChildByRole (Roles.LPar); } } - public AstType CastTo { + public AstType Type { get { return GetChildByRole (Roles.Type); } set { SetChildByRole (Roles.Type, value); } } diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/Expression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/Expression.cs index ff4f62f927..3aa04402b5 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/Expression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/Expression.cs @@ -45,6 +45,13 @@ namespace ICSharpCode.NRefactory.CSharp return (Expression)base.Clone(); } + public Expression ReplaceWith(Func replaceFunction) + { + if (replaceFunction == null) + throw new ArgumentNullException("replaceFunction"); + return (Expression)base.ReplaceWith(node => replaceFunction((Expression)node)); + } + #region Builder methods /// /// Builds an member reference expression using this expression as target. @@ -100,6 +107,16 @@ namespace ICSharpCode.NRefactory.CSharp Arguments = arguments }; } + + public CastExpression CastTo(AstType type) + { + return new CastExpression { Type = type, Expression = this }; + } + + public AsExpression CastAs(AstType type) + { + return new AsExpression { Type = type, Expression = this }; + } #endregion } } diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/UnaryOperatorExpression.cs b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/UnaryOperatorExpression.cs index d563e63493..ae5a12338a 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Expressions/UnaryOperatorExpression.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Expressions/UnaryOperatorExpression.cs @@ -35,6 +35,16 @@ namespace ICSharpCode.NRefactory.CSharp { public readonly static Role OperatorRole = BinaryOperatorExpression.OperatorRole; + public UnaryOperatorExpression() + { + } + + public UnaryOperatorExpression(UnaryOperatorType op, Expression expression) + { + this.Operator = op; + this.Expression = expression; + } + public UnaryOperatorType Operator { get; set; diff --git a/ICSharpCode.NRefactory/CSharp/Ast/PrimitiveType.cs b/ICSharpCode.NRefactory/CSharp/Ast/PrimitiveType.cs index 5706f9b973..19f2e595f1 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/PrimitiveType.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/PrimitiveType.cs @@ -34,6 +34,15 @@ namespace ICSharpCode.NRefactory.CSharp public string Keyword { get; set; } public AstLocation Location { get; set; } + public PrimitiveType() + { + } + + public PrimitiveType(string keyword) + { + this.Keyword = keyword; + } + public override AstLocation StartLocation { get { return Location; diff --git a/ICSharpCode.NRefactory/CSharp/Ast/SimpleType.cs b/ICSharpCode.NRefactory/CSharp/Ast/SimpleType.cs index 7f69868bb9..2111ea533b 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/SimpleType.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/SimpleType.cs @@ -33,6 +33,15 @@ namespace ICSharpCode.NRefactory.CSharp { public class SimpleType : AstType { + public SimpleType() + { + } + + public SimpleType(string identifier) + { + this.Identifier = identifier; + } + public string Identifier { get { return GetChildByRole (Roles.Identifier).Name; diff --git a/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs b/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs index d38048a8dd..ac6bb7c7f5 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/Statements/Statement.cs @@ -37,6 +37,13 @@ namespace ICSharpCode.NRefactory.CSharp return (Statement)base.Clone(); } + public Statement ReplaceWith(Func replaceFunction) + { + if (replaceFunction == null) + throw new ArgumentNullException("replaceFunction"); + return (Statement)base.ReplaceWith(node => replaceFunction((Statement)node)); + } + public override NodeType NodeType { get { return NodeType.Statement; } } diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs new file mode 100644 index 0000000000..0b93f72cc2 --- /dev/null +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/InsertParenthesesVisitor.cs @@ -0,0 +1,252 @@ +// Copyright (c) AlphaSierraPapa for the SharpDevelop Team (for details please see \doc\copyright.txt) +// This code is distributed under MIT X11 license (for details please see \doc\license.txt) + +using System; + +namespace ICSharpCode.NRefactory.CSharp +{ + /// + /// Inserts the parentheses into the AST that are needed to ensure the AST can be printed correctly. + /// For example, if the AST contains + /// BinaryOperatorExpresson(2, Mul, BinaryOperatorExpression(1, Add, 1))); printing that AST + /// would incorrectly result in "2 * 1 + 1". By running InsertParenthesesVisitor, the necessary + /// parentheses are inserted: "2 * (1 + 1)". + /// + public class InsertParenthesesVisitor : DepthFirstAstVisitor + { + /// + /// Gets/Sets whether the visitor should insert parentheses to make the code better looking. + /// If this property is false, it will insert parentheses only where strictly required by the language spec. + /// + public bool InsertParenthesesForReadability { get; set; } + + const int Primary = 15; + const int Unary = 14; + const int RelationalAndTypeTesting = 10; + const int Equality = 9; + const int Conditional = 2; + const int AssignmentAndLambda = 1; + + /// + /// Gets the row number in the C# 4.0 spec operator precedence table. + /// + static int GetPrecedence(Expression expr) + { + // Note: the operator precedence table on MSDN is incorrect + UnaryOperatorExpression uoe = expr as UnaryOperatorExpression; + if (uoe != null) { + if (uoe.Operator == UnaryOperatorType.PostDecrement || uoe.Operator == UnaryOperatorType.PostIncrement) + return Primary; + else + return Unary; + } + if (expr is CastExpression) + return Unary; + BinaryOperatorExpression boe = expr as BinaryOperatorExpression; + if (boe != null) { + switch (boe.Operator) { + case BinaryOperatorType.Multiply: + case BinaryOperatorType.Divide: + case BinaryOperatorType.Modulus: + return 13; // multiplicative + case BinaryOperatorType.Add: + case BinaryOperatorType.Subtract: + return 12; // additive + case BinaryOperatorType.ShiftLeft: + case BinaryOperatorType.ShiftRight: + return 11; + case BinaryOperatorType.GreaterThan: + case BinaryOperatorType.GreaterThanOrEqual: + case BinaryOperatorType.LessThan: + case BinaryOperatorType.LessThanOrEqual: + return RelationalAndTypeTesting; + case BinaryOperatorType.Equality: + case BinaryOperatorType.InEquality: + return Equality; + case BinaryOperatorType.BitwiseAnd: + return 8; + case BinaryOperatorType.ExclusiveOr: + return 7; + case BinaryOperatorType.BitwiseOr: + return 6; + case BinaryOperatorType.ConditionalAnd: + return 5; + case BinaryOperatorType.ConditionalOr: + return 4; + case BinaryOperatorType.NullCoalescing: + return 3; + default: + throw new NotSupportedException("Invalid value for BinaryOperatorType"); + } + } + if (expr is IsExpression || expr is AsExpression) + return RelationalAndTypeTesting; + if (expr is ConditionalExpression) + return Conditional; + if (expr is AssignmentExpression || expr is LambdaExpression) + return AssignmentAndLambda; + // anything else: primary expression + return Primary; + } + + /// + /// Parenthesizes the expression if it does not have the minimum required precedence. + /// + static void ParenthesizeIfRequired(Expression expr, int minimumPrecedence) + { + if (GetPrecedence(expr) < minimumPrecedence) { + Parenthesize(expr); + } + } + + static void Parenthesize(Expression expr) + { + expr.ReplaceWith(e => new ParenthesizedExpression { Expression = e }); + } + + // Primary expressions + public override object VisitMemberReferenceExpression(MemberReferenceExpression memberReferenceExpression, object data) + { + ParenthesizeIfRequired(memberReferenceExpression.Target, Primary); + return base.VisitMemberReferenceExpression(memberReferenceExpression, data); + } + + public override object VisitPointerReferenceExpression(PointerReferenceExpression pointerReferenceExpression, object data) + { + ParenthesizeIfRequired(pointerReferenceExpression.Target, Primary); + return base.VisitPointerReferenceExpression(pointerReferenceExpression, data); + } + + public override object VisitInvocationExpression(InvocationExpression invocationExpression, object data) + { + ParenthesizeIfRequired(invocationExpression.Target, Primary); + return base.VisitInvocationExpression(invocationExpression, data); + } + + public override object VisitIndexerExpression(IndexerExpression indexerExpression, object data) + { + ParenthesizeIfRequired(indexerExpression.Target, Primary); + return base.VisitIndexerExpression(indexerExpression, data); + } + + // Unary expressions + public override object VisitUnaryOperatorExpression(UnaryOperatorExpression unaryOperatorExpression, object data) + { + ParenthesizeIfRequired(unaryOperatorExpression.Expression, Unary); + return base.VisitUnaryOperatorExpression(unaryOperatorExpression, data); + } + + public override object VisitCastExpression(CastExpression castExpression, object data) + { + ParenthesizeIfRequired(castExpression.Expression, Unary); + // There's a nasty issue in the C# grammar: cast expressions including certain operators are ambiguous in some cases + // "(int)-1" is fine, but "(A)-b" is not a cast. + UnaryOperatorExpression uoe = castExpression.Expression as UnaryOperatorExpression; + if (InsertParenthesesForReadability) { + if (uoe != null) + Parenthesize(castExpression.Expression); + } else { + if (uoe != null && !(uoe.Operator == UnaryOperatorType.BitNot || uoe.Operator == UnaryOperatorType.Not)) { + if (TypeCanBeMisinterpretedAsExpression(castExpression.Type)) { + Parenthesize(castExpression.Expression); + } + } + } + return base.VisitCastExpression(castExpression, data); + } + + static bool TypeCanBeMisinterpretedAsExpression(AstType type) + { + // SimpleTypes can always be misinterpreted as IdentifierExpressions + // MemberTypes can be misinterpreted as MemberReferenceExpressions if they don't use double colon + // PrimitiveTypes or ComposedTypes can never be misinterpreted as expressions. + MemberType mt = type as MemberType; + if (mt != null) + return !mt.IsDoubleColon; + else + return type is SimpleType; + } + + // Binary Operators + public override object VisitBinaryOperatorExpression(BinaryOperatorExpression binaryOperatorExpression, object data) + { + int precedence = GetPrecedence(binaryOperatorExpression); + if (binaryOperatorExpression.Operator == BinaryOperatorType.NullCoalescing) { + if (InsertParenthesesForReadability) { + ParenthesizeIfRequired(binaryOperatorExpression.Left, Primary); + ParenthesizeIfRequired(binaryOperatorExpression.Right, Primary); + } else { + // ?? is right-associate + ParenthesizeIfRequired(binaryOperatorExpression.Left, precedence + 1); + ParenthesizeIfRequired(binaryOperatorExpression.Right, precedence); + } + } else { + if (InsertParenthesesForReadability && precedence < Equality) { + ParenthesizeIfRequired(binaryOperatorExpression.Left, Equality); + ParenthesizeIfRequired(binaryOperatorExpression.Right, Equality); + } else { + // all other binary operators are left-associative + ParenthesizeIfRequired(binaryOperatorExpression.Left, precedence); + ParenthesizeIfRequired(binaryOperatorExpression.Right, precedence + 1); + } + } + return base.VisitBinaryOperatorExpression(binaryOperatorExpression, data); + } + + public override object VisitIsExpression(IsExpression isExpression, object data) + { + if (InsertParenthesesForReadability) { + // few people know the precedence of 'is', so always put parentheses in nice-looking mode. + ParenthesizeIfRequired(isExpression.Expression, Primary); + } else { + ParenthesizeIfRequired(isExpression.Expression, RelationalAndTypeTesting); + } + return base.VisitIsExpression(isExpression, data); + } + + public override object VisitAsExpression(AsExpression asExpression, object data) + { + if (InsertParenthesesForReadability) { + // few people know the precedence of 'as', so always put parentheses in nice-looking mode. + ParenthesizeIfRequired(asExpression.Expression, Primary); + } else { + ParenthesizeIfRequired(asExpression.Expression, RelationalAndTypeTesting); + } + return base.VisitAsExpression(asExpression, data); + } + + // Conditional operator + public override object VisitConditionalExpression(ConditionalExpression conditionalExpression, object data) + { + // Associativity here is a bit tricky: + // (a ? b : c ? d : e) == (a ? b : (c ? d : e)) + // (a ? b ? c : d : e) == (a ? (b ? c : d) : e) + // Only ((a ? b : c) ? d : e) strictly needs the additional parentheses + if (InsertParenthesesForReadability) { + // Precedence of ?: can be confusing; so always put parentheses in nice-looking mode. + ParenthesizeIfRequired(conditionalExpression.Condition, Primary); + ParenthesizeIfRequired(conditionalExpression.TrueExpression, Primary); + ParenthesizeIfRequired(conditionalExpression.FalseExpression, Primary); + } else { + ParenthesizeIfRequired(conditionalExpression.Condition, Conditional + 1); + ParenthesizeIfRequired(conditionalExpression.TrueExpression, Conditional); + ParenthesizeIfRequired(conditionalExpression.FalseExpression, Conditional); + } + return base.VisitConditionalExpression(conditionalExpression, data); + } + + public override object VisitAssignmentExpression(AssignmentExpression assignmentExpression, object data) + { + // assignment is right-associative + ParenthesizeIfRequired(assignmentExpression.Left, AssignmentAndLambda + 1); + if (InsertParenthesesForReadability) { + ParenthesizeIfRequired(assignmentExpression.Right, RelationalAndTypeTesting + 1); + } else { + ParenthesizeIfRequired(assignmentExpression.Right, AssignmentAndLambda); + } + return base.VisitAssignmentExpression(assignmentExpression, data); + } + + // don't need to handle lambdas, they have lowest precedence and unambiguous associativity + } +} diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs index 3c07a2c88c..73fd1b9deb 100644 --- a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs @@ -393,7 +393,9 @@ namespace ICSharpCode.NRefactory.CSharp { StartNode(assignmentExpression); assignmentExpression.Left.AcceptVisitor(this, data); + Space(policy.AroundAssignmentParentheses); WriteToken(AssignmentExpression.GetOperatorSymbol(assignmentExpression.Operator), AssignmentExpression.OperatorRole); + Space(policy.AroundAssignmentParentheses); assignmentExpression.Right.AcceptVisitor(this, data); return EndNode(assignmentExpression); } @@ -409,7 +411,49 @@ namespace ICSharpCode.NRefactory.CSharp { StartNode(binaryOperatorExpression); binaryOperatorExpression.Left.AcceptVisitor(this, data); + bool spacePolicy; + switch (binaryOperatorExpression.Operator) { + case BinaryOperatorType.BitwiseAnd: + case BinaryOperatorType.BitwiseOr: + case BinaryOperatorType.ExclusiveOr: + spacePolicy = policy.AroundBitwiseOperatorParentheses; + break; + case BinaryOperatorType.ConditionalAnd: + case BinaryOperatorType.ConditionalOr: + spacePolicy = policy.AroundLogicalOperatorParentheses; + break; + case BinaryOperatorType.GreaterThan: + case BinaryOperatorType.GreaterThanOrEqual: + case BinaryOperatorType.LessThanOrEqual: + case BinaryOperatorType.LessThan: + spacePolicy = policy.AroundRelationalOperatorParentheses; + break; + case BinaryOperatorType.Equality: + case BinaryOperatorType.InEquality: + spacePolicy = policy.AroundEqualityOperatorParentheses; + break; + case BinaryOperatorType.Add: + case BinaryOperatorType.Subtract: + spacePolicy = policy.AroundAdditiveOperatorParentheses; + break; + case BinaryOperatorType.Multiply: + case BinaryOperatorType.Divide: + case BinaryOperatorType.Modulus: + spacePolicy = policy.AroundMultiplicativeOperatorParentheses; + break; + case BinaryOperatorType.ShiftLeft: + case BinaryOperatorType.ShiftRight: + spacePolicy = policy.AroundShiftOperatorParentheses; + break; + case BinaryOperatorType.NullCoalescing: + spacePolicy = true; + break; + default: + throw new NotSupportedException("Invalid value for BinaryOperatorType"); + } + Space(spacePolicy); WriteToken(BinaryOperatorExpression.GetOperatorSymbol(binaryOperatorExpression.Operator), BinaryOperatorExpression.OperatorRole); + Space(spacePolicy); binaryOperatorExpression.Right.AcceptVisitor(this, data); return EndNode(binaryOperatorExpression); } @@ -419,7 +463,7 @@ namespace ICSharpCode.NRefactory.CSharp StartNode(castExpression); LPar(); Space(policy.WithinCastParentheses); - castExpression.CastTo.AcceptVisitor(this, data); + castExpression.Type.AcceptVisitor(this, data); Space(policy.WithinCastParentheses); RPar(); Space(policy.SpacesAfterTypecast); diff --git a/ICSharpCode.NRefactory/CSharp/Parser/CSharpParser.cs b/ICSharpCode.NRefactory/CSharp/Parser/CSharpParser.cs index f72bad8f22..ee69f7164e 100644 --- a/ICSharpCode.NRefactory/CSharp/Parser/CSharpParser.cs +++ b/ICSharpCode.NRefactory/CSharp/Parser/CSharpParser.cs @@ -1693,11 +1693,11 @@ namespace ICSharpCode.NRefactory.CSharp result.Operator = BinaryOperatorType.BitwiseOr; break; case Binary.Operator.LogicalAnd: - result.Operator = BinaryOperatorType.LogicalAnd; + result.Operator = BinaryOperatorType.ConditionalAnd; opLength = 2; break; case Binary.Operator.LogicalOr: - result.Operator = BinaryOperatorType.LogicalOr; + result.Operator = BinaryOperatorType.ConditionalOr; opLength = 2; break; } diff --git a/ICSharpCode.NRefactory/CSharp/Resolver/CSharpResolver.cs b/ICSharpCode.NRefactory/CSharp/Resolver/CSharpResolver.cs index a47a5acb91..3dbf53a42a 100644 --- a/ICSharpCode.NRefactory/CSharp/Resolver/CSharpResolver.cs +++ b/ICSharpCode.NRefactory/CSharp/Resolver/CSharpResolver.cs @@ -602,9 +602,9 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver // - If the user overloads a bitwise operator, that implicitly creates the corresponding logical operator. // - If both inputs are compile-time constants, it doesn't matter that we don't short-circuit. // - If inputs aren't compile-time constants, we don't evaluate anything, so again it doesn't matter that we don't short-circuit - if (op == BinaryOperatorType.LogicalAnd) { + if (op == BinaryOperatorType.ConditionalAnd) { overloadableOperatorName = GetOverloadableOperatorName(BinaryOperatorType.BitwiseAnd); - } else if (op == BinaryOperatorType.LogicalOr) { + } else if (op == BinaryOperatorType.ConditionalOr) { overloadableOperatorName = GetOverloadableOperatorName(BinaryOperatorType.BitwiseOr); } else if (op == BinaryOperatorType.NullCoalescing) { // null coalescing operator is not overloadable and needs to be handled separately @@ -780,10 +780,10 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver } } break; - case BinaryOperatorType.LogicalAnd: + case BinaryOperatorType.ConditionalAnd: methodGroup = logicalAndOperator; break; - case BinaryOperatorType.LogicalOr: + case BinaryOperatorType.ConditionalOr: methodGroup = logicalOrOperator; break; default: diff --git a/ICSharpCode.NRefactory/CSharp/Resolver/ResolveVisitor.cs b/ICSharpCode.NRefactory/CSharp/Resolver/ResolveVisitor.cs index 0e1c50fcfb..f950a474ee 100644 --- a/ICSharpCode.NRefactory/CSharp/Resolver/ResolveVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/Resolver/ResolveVisitor.cs @@ -541,7 +541,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver public override ResolveResult VisitCastExpression(CastExpression castExpression, object data) { if (resolverEnabled) { - return resolver.ResolveCast(ResolveType(castExpression.CastTo), Resolve(castExpression.Expression)); + return resolver.ResolveCast(ResolveType(castExpression.Type), Resolve(castExpression.Expression)); } else { ScanChildren(castExpression); return null; diff --git a/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj b/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj index c957d1ed0e..5bba339ae4 100644 --- a/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj +++ b/ICSharpCode.NRefactory/ICSharpCode.NRefactory.csproj @@ -146,6 +146,7 @@ +