Fix icsharpcode/NRefactory#28: Identity of lambda parameters

Revert change regarding TypeDeclaration.ClassType; using derived nested classes / factory methods is inconsistent API with all other AST nodes.
If we want to save that bit of memory, we could store ClassType in the flags instead (there's about 20 bits free), although I think it won't matter since TypeDeclaration is a relatively rare node.

ICSharpCode.NRefactory.CSharp/Ast/CompilationUnit.cs

@@ -58,6 +58,10 @@ namespace ICSharpCode.NRefactory.CSharp
 			}
 		}
 		
+		public AstNodeCollection<AstNode> Members {
+			get { return GetChildrenByRole(MemberRole); }
+		}
+		
 		List<Error> errors = new List<Error> ();
 		
 		public List<Error> Errors {

ICSharpCode.NRefactory.CSharp/Ast/Expressions/PrimitiveExpression.cs

@@ -24,6 +24,8 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.
 
+using System;
+
 namespace ICSharpCode.NRefactory.CSharp
 {
 	/// <summary>
@@ -60,6 +62,8 @@ namespace ICSharpCode.NRefactory.CSharp
 		public string LiteralValue {
 			get { return literalValue; }
 			set {
+				if (value == null)
+					throw new ArgumentNullException();
 				ThrowIfFrozen();
 				literalValue = value;
 			}
@@ -68,6 +72,7 @@ namespace ICSharpCode.NRefactory.CSharp
 		public PrimitiveExpression (object value)
 		{
 			this.Value = value;
+			this.literalValue = "";
 		}
 		
 		public PrimitiveExpression (object value, string literalValue)

ICSharpCode.NRefactory.CSharp/Ast/GeneralScope/TypeDeclaration.cs

@@ -41,7 +41,7 @@ namespace ICSharpCode.NRefactory.CSharp
 	/// <summary>
 	/// class Name&lt;TypeParameters&gt; : BaseTypes where Constraints;
 	/// </summary>
-	public abstract class TypeDeclaration : EntityDeclaration
+	public class TypeDeclaration : EntityDeclaration
 	{
 		public override NodeType NodeType {
 			get { return NodeType.TypeDeclaration; }
@@ -51,13 +51,31 @@ namespace ICSharpCode.NRefactory.CSharp
 			get { return EntityType.TypeDefinition; }
 		}
 		
+		ClassType classType;
 
-		public abstract CSharpTokenNode TypeKeyword {
-			get;
+		public CSharpTokenNode TypeKeyword {
+			get {
+				switch (classType) {
+					case ClassType.Class:
+						return GetChildByRole(Roles.ClassKeyword);
+					case ClassType.Struct:
+						return GetChildByRole(Roles.StructKeyword);
+					case ClassType.Interface:
+						return GetChildByRole(Roles.InterfaceKeyword);
+					case ClassType.Enum:
+						return GetChildByRole(Roles.EnumKeyword);
+					default:
+						return CSharpTokenNode.Null;
+				}
+			}
 		}
-
-		public abstract ClassType ClassType {
-			get;
+		
+		public ClassType ClassType {
+			get { return classType; }
+			set {
+				ThrowIfFrozen();
+				classType = value;
+			}
 		}
 
 		public AstNodeCollection<TypeParameterDeclaration> TypeParameters {
@@ -88,7 +106,7 @@ namespace ICSharpCode.NRefactory.CSharp
 		{
 			visitor.VisitTypeDeclaration (this);
 		}
-			
+		
 		public override T AcceptVisitor<T> (IAstVisitor<T> visitor)
 		{
 			return visitor.VisitTypeDeclaration (this);
@@ -107,87 +125,5 @@ namespace ICSharpCode.NRefactory.CSharp
 				&& this.BaseTypes.DoMatch(o.BaseTypes, match) && this.Constraints.DoMatch(o.Constraints, match)
 				&& this.Members.DoMatch(o.Members, match);
 		}
-
-		protected TypeDeclaration ()
-		{
-		}
-
-		public static TypeDeclaration Create(ClassType type)
-		{
-			switch (type) {
-				case ICSharpCode.NRefactory.CSharp.ClassType.Class:
-					return new Class ();
-				case ICSharpCode.NRefactory.CSharp.ClassType.Struct:
-					return new Struct ();
-				case ICSharpCode.NRefactory.CSharp.ClassType.Interface:
-					return new Interface ();
-				case ICSharpCode.NRefactory.CSharp.ClassType.Enum:
-					return new Enum ();
-				default:
-					throw new System.ArgumentOutOfRangeException();
-			}
-		}
-
-		#region Concrete Types
-		public class Class : TypeDeclaration
-		{
-			public override ClassType ClassType {
-				get {
-					return ClassType.Class;
-				}
-			}
-
-			public override CSharpTokenNode TypeKeyword {
-				get {
-					return GetChildByRole(Roles.ClassKeyword);
-				}
-			}
-		}
-
-		public class Struct : TypeDeclaration
-		{
-			public override ClassType ClassType {
-				get {
-					return ClassType.Struct;
-				}
-			}
-
-			public override CSharpTokenNode TypeKeyword {
-				get {
-					return GetChildByRole(Roles.StructKeyword);
-				}
-			}
-		}
-
-		public class Interface : TypeDeclaration
-		{
-			public override ClassType ClassType {
-				get {
-					return ClassType.Interface;
-				}
-			}
-
-			public override CSharpTokenNode TypeKeyword {
-				get {
-					return GetChildByRole(Roles.InterfaceKeyword);
-				}
-			}
-		}
-
-		public class Enum : TypeDeclaration
-		{
-			public override ClassType ClassType {
-				get {
-					return ClassType.Enum;
-				}
-			}
-
-			public override CSharpTokenNode TypeKeyword {
-				get {
-					return GetChildByRole(Roles.EnumKeyword);
-				}
-			}
-		}
-		#endregion
 	}
 }

ICSharpCode.NRefactory.CSharp/Parser/CSharpParser.cs

@@ -1,4 +1,4 @@
-// 
+// 
 // CSharpParser.cs
 //
 // Author:
@@ -483,7 +483,8 @@ namespace ICSharpCode.NRefactory.CSharp
 			
 			public override void Visit(Class c)
 			{
-				var newType = new TypeDeclaration.Class ();
+				var newType = new TypeDeclaration ();
+				newType.ClassType = ClassType.Class;
 				AddAttributeSection(newType, c);
 				
 				var location = LocationsBag.GetMemberLocation(c);
@@ -533,7 +534,8 @@ namespace ICSharpCode.NRefactory.CSharp
 			
 			public override void Visit(Struct s)
 			{
-				var newType = new TypeDeclaration.Struct();
+				var newType = new TypeDeclaration();
+				newType.ClassType = ClassType.Struct;
 				AddAttributeSection(newType, s);
 				var location = LocationsBag.GetMemberLocation(s);
 				AddModifiers(newType, location);
@@ -577,7 +579,8 @@ namespace ICSharpCode.NRefactory.CSharp
 			
 			public override void Visit(Interface i)
 			{
-				var newType = new TypeDeclaration.Interface ();
+				var newType = new TypeDeclaration();
+				newType.ClassType = ClassType.Interface;
 				AddAttributeSection(newType, i);
 				var location = LocationsBag.GetMemberLocation(i);
 				AddModifiers(newType, location);
@@ -667,7 +670,8 @@ namespace ICSharpCode.NRefactory.CSharp
 			
 			public override void Visit(Mono.CSharp.Enum e)
 			{
-				var newType = new TypeDeclaration.Enum ();
+				var newType = new TypeDeclaration();
+				newType.ClassType = ClassType.Enum;
 				AddAttributeSection(newType, e);
 				var location = LocationsBag.GetMemberLocation(e);
 				

ICSharpCode.NRefactory.CSharp/Refactoring/TypeSystemAstBuilder.cs

@@ -476,7 +476,8 @@ namespace ICSharpCode.NRefactory.CSharp.Refactoring
 					break;
 			}
 			
-			var decl = TypeDeclaration.Create(classType);
+			var decl = new TypeDeclaration();
+			decl.ClassType = classType;
 			decl.Modifiers = modifiers;
 			decl.Name = typeDefinition.Name;
 			

ICSharpCode.NRefactory.CSharp/Resolver/ResolveVisitor.cs

@@ -248,7 +248,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 			#if DEBUG
 			CSharpResolver oldResolver;
 			if (resolverBeforeDict.TryGetValue(node, out oldResolver)) {
-				Debug.Assert(oldResolver.LocalVariables.Count() == resolver.LocalVariables.Count());
+				Debug.Assert(oldResolver.LocalVariables.SequenceEqual(resolver.LocalVariables));
 			}
 			#endif
 			
@@ -1891,6 +1891,26 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 		#endregion
 		
 		#region Implicitly typed
+		// Implicitly-typed lambdas are really complex, as the lambda depends on the target type (the delegate to which
+		// the lambda is converted), but figuring out the target type might involve overload resolution (for method
+		// calls in which the lambda is used as argument), which requires knowledge about the lamdba.
+		// 
+		// The implementation in NRefactory works like this:
+		// 1. The lambda resolves to a ImplicitlyTypedLambda (derived from LambdaResolveResult).
+		//     The lambda body is not resolved yet (one of the few places where ResolveVisitor
+		//     deviates from the usual depth-first AST traversal).
+		// 2. The parent statement is resolved as usual. This might require analyzing the lambda in detail (for example
+		//    as part of overload resolution). Such analysis happens using LambdaResolveResult.IsValid, where the caller
+		//    (i.e. the overload resolution algorithm) supplies the parameter types to the lambda body. For every IsValid()
+		//    call, a nested LambdaTypeHypothesis is constructed for analyzing the lambda using the supplied type assignment.
+		//    Multiple IsValid() calls may use several LambdaTypeHypothesis instances, one for each set of parameter types.
+		// 3. When the resolver reports the conversions that occurred as part of the parent statement (as with any
+		//    conversions), the results from the LambdaTypeHypothesis corresponding to the actually chosen
+		//    conversion are merged into the main resolver.
+		// 4. LambdaResolveResult.Body is set to the main resolve result from the chosen nested resolver. I think this
+		//    is the only place where NRefactory is mutating a ResolveResult (normally all resolve results are immutable).
+		//    As this step is guaranteed to occur before the resolver returns the LamdbaResolveResult to user code, the
+		//    mutation shouldn't cause any problems.
 		sealed class ImplicitlyTypedLambda : LambdaBase
 		{
 			readonly LambdaExpression lambda;
@@ -1899,7 +1919,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 			readonly CSharpResolver storedContext;
 			readonly CSharpParsedFile parsedFile;
 			readonly List<LambdaTypeHypothesis> hypotheses = new List<LambdaTypeHypothesis>();
-			readonly List<IParameter> parameters = new List<IParameter>();
+			internal IList<IParameter> parameters = new List<IParameter>();
 			
 			internal LambdaTypeHypothesis winningHypothesis;
 			internal ResolveResult bodyResult;
@@ -1953,7 +1973,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				: this(parentVisitor)
 			{
 				this.selectClause = selectClause;
-				this.parameters.AddRange(parameters);
+				foreach (IParameter p in parameters)
+					this.parameters.Add(p);
 				
 				RegisterUndecidedLambda();
 			}
@@ -2078,7 +2099,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 		sealed class LambdaTypeHypothesis : IResolveVisitorNavigator
 		{
 			readonly ImplicitlyTypedLambda lambda;
-			internal readonly IParameter[] lambdaParameters;
+			readonly IParameter[] lambdaParameters;
 			internal readonly IType[] parameterTypes;
 			readonly ResolveVisitor visitor;
 			
@@ -2172,6 +2193,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 					throw new InvalidOperationException("Trying to merge conflicting hypotheses");
 				
 				lambda.winningHypothesis = this;
+				lambda.parameters = lambdaParameters; // replace untyped parameters with typed parameters
 				if (lambda.BodyExpression is Expression && returnValues.Count == 1) {
 					lambda.bodyResult = returnValues[0];
 				}
@@ -3365,9 +3387,9 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 				}
 				
 				implicitlyTypedLambda.EnforceMerge(this);
-				if (implicitlyTypedLambda.winningHypothesis.parameterTypes.Length == 2) {
+				if (implicitlyTypedLambda.Parameters.Count == 2) {
 					StoreCurrentState(queryJoinClause.IntoIdentifierToken);
-					groupVariable = implicitlyTypedLambda.winningHypothesis.lambdaParameters[1];
+					groupVariable = implicitlyTypedLambda.Parameters[1];
 				} else {
 					groupVariable = null;
 				}

ICSharpCode.NRefactory.Tests/CSharp/CSharpOutputVisitorTests.cs

@@ -53,7 +53,8 @@ namespace ICSharpCode.NRefactory.CSharp
 		[Test]
 		public void EnumDeclarationWithInitializers ()
 		{
-			TypeDeclaration type = new TypeDeclaration.Enum {
+			TypeDeclaration type = new TypeDeclaration {
+				ClassType = ClassType.Enum,
 				Name = "DisplayFlags",
 				Members = {
 					new EnumMemberDeclaration {

ICSharpCode.NRefactory.Tests/CSharp/Parser/GeneralScope/AttributeSectionTests.cs

@@ -97,7 +97,8 @@ public class Form1 {
 		{
 			ParseUtilCSharp.AssertGlobal(
 				@"[A, B] class Test {}",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Name = "Test",
 					Attributes = {
 						new AttributeSection {
@@ -113,7 +114,8 @@ public class Form1 {
 		{
 			ParseUtilCSharp.AssertGlobal(
 				"class Test<[A,B]C> {}",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Name = "Test",
 					TypeParameters = {
 						new TypeParameterDeclaration {
@@ -172,7 +174,8 @@ public class Form1 {
 		{
 			ParseUtilCSharp.AssertTypeMember (
 				@"[A(0, a:1, b=2)] class Test {}",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Name = "Test",
 					Attributes = {
 						new AttributeSection {

ICSharpCode.NRefactory.Tests/CSharp/Parser/GeneralScope/TypeDeclarationTests.cs

@@ -89,7 +89,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.GeneralScope
 		{
 			ParseUtilCSharp.AssertGlobal(
 				"public class G<T> {}",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Modifiers = Modifiers.Public,
 					Name = "G",
 					TypeParameters = { new TypeParameterDeclaration { Name = "T" } }
@@ -101,7 +102,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.GeneralScope
 		{
 			ParseUtilCSharp.AssertGlobal(
 				@"public class Test<T> where T : IMyInterface { }",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Modifiers = Modifiers.Public,
 					Name = "Test",
 					TypeParameters = { new TypeParameterDeclaration { Name = "T" } },
@@ -118,7 +120,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.GeneralScope
 		{
 			ParseUtilCSharp.AssertGlobal(
 				"public interface Generic<in T, out S> : System.IComparable where S : G<T[]>, new() where  T : MyNamespace.IMyInterface {}",
-				new TypeDeclaration.Interface {
+				new TypeDeclaration {
+					ClassType = ClassType.Interface,
 					Modifiers = Modifiers.Public,
 					Name = "Generic",
 					TypeParameters = {
@@ -164,7 +167,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.GeneralScope
 public abstract class MyClass : MyBase, Interface1, My.Test.Interface2
 {
 }",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Attributes = {
 						new AttributeSection {
 							Attributes = {
@@ -219,7 +223,8 @@ public abstract class MyClass : MyBase, Interface1, My.Test.Interface2
 		{
 			ParseUtilCSharp.AssertGlobal(
 				"partial class partial<[partial: where] where> where where : partial<where> { }",
-				new TypeDeclaration.Class {
+				new TypeDeclaration {
+					ClassType = ClassType.Class,
 					Modifiers = Modifiers.Partial,
 					Name = "partial",
 					TypeParameters = {
@@ -304,7 +309,8 @@ public abstract class MyClass : MyBase, Interface1, My.Test.Interface2
 		{
 			ParseUtilCSharp.AssertGlobal (
 				"enum DisplayFlags { D = 4\r\r\n}",
-				new TypeDeclaration.Enum {
+				new TypeDeclaration {
+					ClassType = ClassType.Enum,
 					Name = "DisplayFlags",
 					Members = {
 						new EnumMemberDeclaration {

ICSharpCode.NRefactory.Tests/CSharp/Parser/TypeMembers/MethodDeclarationTests.cs

@@ -165,7 +165,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.TypeMembers
 	T MyMethod<T>(T a) where T : ISomeInterface;
 }
 ",
-				new TypeDeclaration.Interface {
+				new TypeDeclaration {
+					ClassType = ClassType.Interface,
 					Name = "MyInterface",
 					Members = {
 						new MethodDeclaration {
@@ -190,7 +191,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.TypeMembers
 	void MyMethod<T>(T a) where T : ISomeInterface;
 }
 ",
-				new TypeDeclaration.Interface {
+				new TypeDeclaration {
+					ClassType = ClassType.Interface,
 					Name = "MyInterface",
 					Members = {
 						new MethodDeclaration {
@@ -215,7 +217,8 @@ namespace ICSharpCode.NRefactory.CSharp.Parser.TypeMembers
 	new void Dispose();
 }
 ",
-				new TypeDeclaration.Interface {
+				new TypeDeclaration {
+					ClassType = ClassType.Interface,
 					Name = "MyInterface",
 					BaseTypes = { new SimpleType("IDisposable") },
 					Members = {

ICSharpCode.NRefactory.Tests/CSharp/Resolver/LambdaTests.cs

@@ -17,7 +17,10 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
+using System.IO;
 using System.Linq;
+
+using ICSharpCode.NRefactory.CSharp.TypeSystem;
 using ICSharpCode.NRefactory.Semantics;
 using ICSharpCode.NRefactory.TypeSystem;
 using ICSharpCode.NRefactory.TypeSystem.Implementation;
@@ -560,5 +563,29 @@ class Test {
 			Assert.IsFalse(rr.IsError);
 			Assert.AreEqual("System.Threading.Tasks.Task`1[[System.Int32]]", rr.Type.ReflectionName);
 		}
+		
+		[Test]
+		public void LambdaParameterIdentity()
+		{
+			string code = @"using System;
+class TestClass {
+	void F() {
+		Func<int, int> f = $i => i + 1$;
+	}
+}";
+			
+			var prep = PrepareResolver(code);
+			var lambda = (LambdaExpression)prep.Item2;
+			var identifierInLambdaBody = ((BinaryOperatorExpression)lambda.Body).Left;
+			var resolver = prep.Item1;
+			
+			var resolvedParameter = ((LocalResolveResult)resolver.Resolve(lambda.Parameters.Single())).Variable;
+			var parameterInResolveResult = ((LambdaResolveResult)resolver.Resolve(lambda)).Parameters[0];
+			var referencedParameter = ((LocalResolveResult)resolver.Resolve(identifierInLambdaBody)).Variable;
+			
+			Assert.AreEqual("System.Int32" ,resolvedParameter.Type.ReflectionName);
+			Assert.AreSame(resolvedParameter, parameterInResolveResult);
+			Assert.AreSame(resolvedParameter, referencedParameter);
+		}
 	}
 }

ICSharpCode.NRefactory.Tests/CSharp/Resolver/ResolverTestBase.cs

@@ -143,7 +143,7 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 			}
 		}
 		
-		IEnumerable<TextLocation> FindDollarSigns(string code)
+		protected IEnumerable<TextLocation> FindDollarSigns(string code)
 		{
 			int line = 1;
 			int col = 1;
@@ -172,12 +172,8 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 			project = project.UpdateProjectContent(null, parsedFile);
 			compilation = project.CreateCompilation();
 			
-			FindNodeVisitor fnv = new FindNodeVisitor(dollars[0], dollars[1]);
-			cu.AcceptVisitor(fnv);
-			Assert.IsNotNull(fnv.ResultNode, "Did not find DOM node at the specified location");
-			
 			CSharpAstResolver resolver = new CSharpAstResolver(compilation, cu, parsedFile);
-			return Tuple.Create(resolver, fnv.ResultNode);
+			return Tuple.Create(resolver, FindNode(cu, dollars[0], dollars[1]));
 		}
 		
 		protected ResolveResult Resolve(string code)
@@ -216,6 +212,14 @@ namespace ICSharpCode.NRefactory.CSharp.Resolver
 			return (T)rr;
 		}
 		
+		protected AstNode FindNode(CompilationUnit cu, TextLocation start, TextLocation end)
+		{
+			FindNodeVisitor fnv = new FindNodeVisitor(start, end);
+			cu.AcceptVisitor(fnv);
+			Assert.IsNotNull(fnv.ResultNode, "Did not find DOM node at the specified location");
+			return fnv.ResultNode;
+		}
+		
 		sealed class FindNodeVisitor : DepthFirstAstVisitor
 		{
 			readonly TextLocation start;