From 93e9d1e18b22a9622c4ee4899944a091ab5edb26 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Mon, 28 Feb 2011 16:56:48 +0100 Subject: [PATCH] Use implicit conversion operator to convert from Pattern to AST nodes. --- .../CSharp/AstStructureTests.cs | 15 ++++ ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs | 5 ++ .../CSharp/Ast/AstNodeCollection.cs | 18 ++--- .../CSharp/Ast/PatternMatching/Pattern.cs | 25 +++---- .../CSharp/Ast/PatternMatching/Placeholder.cs | 68 ++++++++++++++----- .../CSharp/OutputVisitor/OutputVisitor.cs | 17 +++-- 6 files changed, 98 insertions(+), 50 deletions(-) diff --git a/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs b/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs index 16534dd1d6..183efacdb2 100644 --- a/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs +++ b/ICSharpCode.NRefactory.Tests/CSharp/AstStructureTests.cs @@ -27,5 +27,20 @@ namespace ICSharpCode.NRefactory.CSharp } } } + + [Test] + public void AstNodesDoNotDeriveFromEachOther() + { + // Ast nodes should derive only from abstract classes; not from concrete types. + // For example, we want to avoid that an AST consumer doing "if (node is PropertyDeclaration)" + // unknowingly also handles IndexerDeclarations. + foreach (Type type in typeof(AstNode).Assembly.GetExportedTypes()) { + if (type == typeof(CSharpModifierToken)) // CSharpModifierToken is the exception (though I'm not too happy about that) + continue; + if (type.IsSubclassOf(typeof(AstNode))) { + Assert.IsTrue(type.BaseType.IsAbstract, type.FullName); + } + } + } } } diff --git a/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs b/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs index a5361ef4f9..2dab5a5ad9 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/AstNode.cs @@ -606,6 +606,11 @@ namespace ICSharpCode.NRefactory.CSharp } protected internal abstract bool DoMatch(AstNode other, Match match); + + internal virtual bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return DoMatch(pos, match); + } #endregion // the Root role must be available when creating the null nodes, so we can't put it in the Roles class diff --git a/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs b/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs index 033ad79131..c7f266d97b 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/AstNodeCollection.cs @@ -174,18 +174,12 @@ namespace ICSharpCode.NRefactory.CSharp if (cur1 == null) break; - Pattern pattern = cur1 as Pattern; - if (pattern == null && cur1.NodeType == NodeType.Placeholder) - pattern = cur1.GetChildByRole(TypePlaceholder.ChildRole) as Pattern; - if (pattern != null) { - Debug.Assert(stack.Count == patternStack.Count); - success = pattern.DoMatchCollection(role, cur2, match, stack); - Debug.Assert(stack.Count >= patternStack.Count); - while (stack.Count > patternStack.Count) - patternStack.Push(cur1.NextSibling); - } else { - success = cur1.DoMatch(cur2, match); - } + Debug.Assert(stack.Count == patternStack.Count); + success = cur1.DoMatchCollection(role, cur2, match, stack); + Debug.Assert(stack.Count >= patternStack.Count); + while (stack.Count > patternStack.Count) + patternStack.Push(cur1.NextSibling); + cur1 = cur1.NextSibling; if (cur2 != null) cur2 = cur2.NextSibling; diff --git a/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Pattern.cs b/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Pattern.cs index 073415f519..37c89a58e1 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Pattern.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Pattern.cs @@ -29,34 +29,29 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching } } - internal virtual bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + public static implicit operator AstType(Pattern p) { - return DoMatch(pos, match); + return p != null ? new TypePlaceholder(p) : null; } - public AstType ToType() + public static implicit operator Expression(Pattern p) { - return new TypePlaceholder(this); + return p != null ? new ExpressionPlaceholder(p) : null; } - public Expression ToExpression() + public static implicit operator Statement(Pattern p) { - return new ExpressionPlaceholder(this); + return p != null ? new StatementPlaceholder(p) : null; } - public Statement ToStatement() + public static implicit operator BlockStatement(Pattern p) { - return new StatementPlaceholder(this); + return p != null ? new BlockStatementPlaceholder(p) : null; } - public BlockStatement ToBlock() + public static implicit operator VariableInitializer(Pattern p) { - return new BlockStatementPlaceholder(this); - } - - public VariableInitializer ToVariable() - { - return new VariablePlaceholder(this); + return p != null ? new VariablePlaceholder(p) : null; } // Make debugging easier by giving Patterns a ToString() implementation diff --git a/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Placeholder.cs b/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Placeholder.cs index 2bac1c6e19..f68feb443d 100644 --- a/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Placeholder.cs +++ b/ICSharpCode.NRefactory/CSharp/Ast/PatternMatching/Placeholder.cs @@ -2,16 +2,19 @@ // This code is distributed under MIT X11 license (for details please see \doc\license.txt) using System; +using System.Collections.Generic; namespace ICSharpCode.NRefactory.CSharp.PatternMatching { + // Placeholders do not store their child in the AST tree; but keep it as a separate child. + // This allows reusing the child in multiple placeholders; thus enabling the sharing of AST subtrees. sealed class TypePlaceholder : AstType { - public static readonly Role ChildRole = new Role("Child", AstNode.Null); + readonly AstNode child; public TypePlaceholder(AstNode child) { - AddChild(child, TypePlaceholder.ChildRole); + this.child = child; } public override NodeType NodeType { @@ -20,20 +23,27 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching public override S AcceptVisitor(IAstVisitor visitor, T data) { - return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data); + return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, child, data); } protected internal override bool DoMatch(AstNode other, Match match) { - return GetChildByRole(TypePlaceholder.ChildRole).DoMatch(other, match); + return child.DoMatch(other, match); + } + + internal override bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return child.DoMatchCollection(role, pos, match, backtrackingStack); } } sealed class ExpressionPlaceholder : Expression { + readonly AstNode child; + public ExpressionPlaceholder(AstNode child) { - AddChild(child, TypePlaceholder.ChildRole); + this.child = child; } public override NodeType NodeType { @@ -42,20 +52,27 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching public override S AcceptVisitor(IAstVisitor visitor, T data) { - return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data); + return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, child, data); } protected internal override bool DoMatch(AstNode other, Match match) { - return GetChildByRole(TypePlaceholder.ChildRole).DoMatch(other, match); + return child.DoMatch(other, match); + } + + internal override bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return child.DoMatchCollection(role, pos, match, backtrackingStack); } } sealed class StatementPlaceholder : Statement { + readonly AstNode child; + public StatementPlaceholder(AstNode child) { - AddChild(child, TypePlaceholder.ChildRole); + this.child = child; } public override NodeType NodeType { @@ -64,20 +81,27 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching public override S AcceptVisitor(IAstVisitor visitor, T data) { - return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data); + return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, child, data); } protected internal override bool DoMatch(AstNode other, Match match) { - return GetChildByRole(TypePlaceholder.ChildRole).DoMatch(other, match); + return child.DoMatch(other, match); + } + + internal override bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return child.DoMatchCollection(role, pos, match, backtrackingStack); } } sealed class BlockStatementPlaceholder : BlockStatement { + readonly AstNode child; + public BlockStatementPlaceholder(AstNode child) { - AddChild(child, TypePlaceholder.ChildRole); + this.child = child; } public override NodeType NodeType { @@ -86,20 +110,27 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching public override S AcceptVisitor(IAstVisitor visitor, T data) { - return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data); + return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, child, data); } protected internal override bool DoMatch(AstNode other, Match match) { - return GetChildByRole(TypePlaceholder.ChildRole).DoMatch(other, match); + return child.DoMatch(other, match); + } + + internal override bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return child.DoMatchCollection(role, pos, match, backtrackingStack); } } sealed class VariablePlaceholder : VariableInitializer { + readonly AstNode child; + public VariablePlaceholder(AstNode child) { - AddChild(child, TypePlaceholder.ChildRole); + this.child = child; } public override NodeType NodeType { @@ -108,12 +139,17 @@ namespace ICSharpCode.NRefactory.CSharp.PatternMatching public override S AcceptVisitor(IAstVisitor visitor, T data) { - return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data); + return ((IPatternAstVisitor)visitor).VisitPlaceholder(this, child, data); } protected internal override bool DoMatch(AstNode other, Match match) { - return GetChildByRole(TypePlaceholder.ChildRole).DoMatch(other, match); + return child.DoMatch(other, match); + } + + internal override bool DoMatchCollection(Role role, AstNode pos, Match match, Stack backtrackingStack) + { + return child.DoMatchCollection(role, pos, match, backtrackingStack); } } } diff --git a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs index efab140af3..86fa1a9245 100644 --- a/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs +++ b/ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs @@ -22,7 +22,7 @@ namespace ICSharpCode.NRefactory.CSharp readonly IOutputFormatter formatter; readonly CSharpFormattingPolicy policy; - AstNode currentContainerNode; + readonly Stack containerStack = new Stack(); readonly Stack positionStack = new Stack(); /// @@ -65,21 +65,23 @@ namespace ICSharpCode.NRefactory.CSharp #region StartNode/EndNode void StartNode(AstNode node) { - Debug.Assert(currentContainerNode == null || node.Parent == currentContainerNode); + // Ensure that nodes are visited in the proper nested order. + // Jumps to different subtrees are allowed only for the child of a placeholder node. + Debug.Assert(containerStack.Count == 0 || node.Parent == containerStack.Peek() || containerStack.Peek().NodeType == NodeType.Placeholder); if (positionStack.Count > 0) WriteSpecialsUpToNode(node); - currentContainerNode = node; + containerStack.Push(node); positionStack.Push(node.FirstChild); formatter.StartNode(node); } object EndNode(AstNode node) { - Debug.Assert(node == currentContainerNode); + Debug.Assert(node == containerStack.Peek()); AstNode pos = positionStack.Pop(); Debug.Assert(pos == null || pos.Parent == node); WriteSpecials(pos, null); - currentContainerNode = node.Parent; + containerStack.Pop(); formatter.EndNode(node); return null; } @@ -207,7 +209,7 @@ namespace ICSharpCode.NRefactory.CSharp void WriteIdentifier(string identifier, Role identifierRole = null) { WriteSpecialsUpToRole(identifierRole ?? AstNode.Roles.Identifier); - if (IsKeyword(identifier, currentContainerNode)) { + if (IsKeyword(identifier, containerStack.Peek())) { if (lastWritten == LastWritten.KeywordOrIdentifier) Space(); // this space is not strictly required, so we call Space() formatter.WriteToken("@"); @@ -265,7 +267,8 @@ namespace ICSharpCode.NRefactory.CSharp /// void Semicolon() { - if (currentContainerNode.Role != ForStatement.InitializerRole && currentContainerNode.Role != ForStatement.IteratorRole && currentContainerNode.Role != UsingStatement.ResourceAcquisitionRole) { + Role role = containerStack.Peek().Role; // get the role of the current node + if (!(role == ForStatement.InitializerRole || role == ForStatement.IteratorRole || role == UsingStatement.ResourceAcquisitionRole)) { WriteToken(";", AstNode.Roles.Semicolon); NewLine(); }