Use implicit conversion operator to convert from Pattern to AST nodes.

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);
+				}
+			}
+		}
 	}
 }

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<Pattern.PossibleMatch> 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

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;

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<PossibleMatch> 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

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<AstNode> ChildRole = new Role<AstNode>("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<T, S>(IAstVisitor<T, S> visitor, T data)
 		{
-			return ((IPatternAstVisitor<T, S>)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data);
+			return ((IPatternAstVisitor<T, S>)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<Pattern.PossibleMatch> 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<T, S>(IAstVisitor<T, S> visitor, T data)
 		{
-			return ((IPatternAstVisitor<T, S>)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data);
+			return ((IPatternAstVisitor<T, S>)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<Pattern.PossibleMatch> 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<T, S>(IAstVisitor<T, S> visitor, T data)
 		{
-			return ((IPatternAstVisitor<T, S>)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data);
+			return ((IPatternAstVisitor<T, S>)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<Pattern.PossibleMatch> 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<T, S>(IAstVisitor<T, S> visitor, T data)
 		{
-			return ((IPatternAstVisitor<T, S>)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data);
+			return ((IPatternAstVisitor<T, S>)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<Pattern.PossibleMatch> 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<T, S>(IAstVisitor<T, S> visitor, T data)
 		{
-			return ((IPatternAstVisitor<T, S>)visitor).VisitPlaceholder(this, GetChildByRole(TypePlaceholder.ChildRole), data);
+			return ((IPatternAstVisitor<T, S>)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<Pattern.PossibleMatch> backtrackingStack)
+		{
+			return child.DoMatchCollection(role, pos, match, backtrackingStack);
 		}
 	}
 }

ICSharpCode.NRefactory/CSharp/OutputVisitor/OutputVisitor.cs

@@ -22,7 +22,7 @@ namespace ICSharpCode.NRefactory.CSharp
 		readonly IOutputFormatter formatter;
 		readonly CSharpFormattingPolicy policy;
 		
-		AstNode currentContainerNode;
+		readonly Stack<AstNode> containerStack = new Stack<AstNode>();
 		readonly Stack<AstNode> positionStack = new Stack<AstNode>();
 		
 		/// <summary>
@@ -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<Identifier> 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
 		/// </summary>
 		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();
 			}