From e882ef4514b35ba4d73a59d01b1e74c8a803df0a Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 12 Jul 2015 09:31:19 -0700 Subject: [PATCH 1/8] Remove public modifiers from ProjectTree --- .../TestSources/ProjectTree.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs index 3320e1c..b690cb0 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs @@ -2,13 +2,13 @@ { using System.Collections.Immutable; - public interface IRule { } + interface IRule { } - public interface IProjectPropertiesContext { } + interface IProjectPropertiesContext { } - public interface IPropertySheet { } + interface IPropertySheet { } - public class ProjectPropertiesContext : IProjectPropertiesContext + class ProjectPropertiesContext : IProjectPropertiesContext { } From a1ced9023d10d4d4df1974932afcfdc55d9e5d7d Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 12 Jul 2015 09:33:12 -0700 Subject: [PATCH 2/8] Use image monikers in ProjectTree This is consistent with the Dev14 API for project trees. --- .../TestSources/ProjectTree.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs index b690cb0..883e6fd 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs @@ -18,8 +18,8 @@ partial class ProjectTree [Required] readonly string caption; readonly string filePath; - readonly System.Drawing.Image icon; - readonly System.Drawing.Image expandedIcon; + readonly string iconMoniker; + readonly string expandedIconMoniker; readonly bool visible; readonly IRule browseObjectProperties; readonly ImmutableHashSet capabilities; From a9332f9a47d569f0df80e83c519ee6ce638245c9 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 12 Jul 2015 12:11:52 -0700 Subject: [PATCH 3/8] Ensure when syncing children we throw regardless of collection size ImmutableList`1.RemoveRange does not enumerate its argument when the list is empty. ImmutableSortedSet`1.Except *does* enumerate its argument even when the collection is empty. This difference in behavior caused a test failure when preparing ProjectTree to change to use ordered Children instead of sorted children. This change makes sure that every element in the argument is "realized" from the enumerable, and thus exceptions are reliably thrown. --- .../CodeGen+CollectionHelpersGen.cs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen+CollectionHelpersGen.cs b/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen+CollectionHelpersGen.cs index 4850754..a751766 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen+CollectionHelpersGen.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen+CollectionHelpersGen.cs @@ -182,10 +182,13 @@ private MethodDeclarationSyntax CreateParamsElementArrayMethod(MetaField field, var lambdaParameter = SyntaxFactory.Parameter(SyntaxFactory.Identifier("v")); var argument = passThroughChildSync ? (ExpressionSyntax)Syntax.EnumerableExtension( - SyntaxFactory.IdentifierName(nameof(Enumerable.Select)), - ValuesParameterName, - SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList( - SyntaxFactory.Argument(Syntax.ThisDot(SyncImmediateChildToCurrentVersionMethodName))))) + SyntaxFactory.IdentifierName(nameof(Enumerable.ToList)), + Syntax.EnumerableExtension( + SyntaxFactory.IdentifierName(nameof(Enumerable.Select)), + ValuesParameterName, + SyntaxFactory.ArgumentList(SyntaxFactory.SingletonSeparatedList( + SyntaxFactory.Argument(Syntax.ThisDot(SyncImmediateChildToCurrentVersionMethodName))))), + SyntaxFactory.ArgumentList()) : ValuesParameterName; paramsArrayMethod = this.AddMethodBody( From ffd9a02e192cc571ae1e6545c0da6de4b7253c6b Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 12 Jul 2015 12:12:27 -0700 Subject: [PATCH 4/8] Switch ProjectTree data structure to ImmutableList. --- .../TestSources/ProjectTree.cs | 2 +- .../TestSources/ProjectTreeNodeTest.cs | 46 +++++++++++++++---- .../TestSources/ProjectTreeNodeTestBase.cs | 4 +- .../TestSources/ProjectTreePartial.cs | 2 +- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs index 883e6fd..52ad4ff 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTree.cs @@ -23,7 +23,7 @@ partial class ProjectTree readonly bool visible; readonly IRule browseObjectProperties; readonly ImmutableHashSet capabilities; - readonly ImmutableSortedSet children; + readonly ImmutableList children; } [GenerateImmutable(DefineInterface = true, GenerateBuilder = true, DefineWithMethodsPerProperty = true, DefineRootedStruct = true, Delta = true)] diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTest.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTest.cs index 1988236..92d63e0 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTest.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTest.cs @@ -441,11 +441,17 @@ public void MovingNodeAroundHierarchy() var moved = root.RemoveDescendent(aa).AddDescendent(aa, ab); var history = moved.ChangesSince(root); - Assert.Equal(1, history.Count); + Assert.Equal(2, history.Count); + Assert.Equal(ChangeKind.Replaced, history[0].Kind); Assert.Same(aa, history[0].Before); Assert.Same(aa, history[0].After); Assert.Equal(ProjectTreeChangedProperties.Parent, history[0].Changes); + + Assert.Equal(ChangeKind.Replaced, history[1].Kind); + Assert.Same(ab, history[1].Before); + Assert.Same(moved.Children[0], history[1].After); + Assert.Equal(ProjectTreeChangedProperties.PositionUnderParent, history[1].Changes); } [Fact] @@ -460,12 +466,19 @@ public void MovingNodeAroundHierarchyWithOtherChanges() var moved = root.RemoveDescendent(aa).AddDescendent(aaModified, ab); var history = moved.ChangesSince(root); - Assert.Equal(1, history.Count); + Assert.Equal(2, history.Count); + Assert.Equal(ChangeKind.Replaced, history[0].Kind); Assert.Equal(ProjectTreeChangedProperties.Parent | ProjectTreeChangedProperties.Visible, history[0].Changes); Assert.Same(aa, history[0].Before); Assert.Same(aaModified, history[0].After); Assert.Equal(aa.Identity, history[0].Identity); + + Assert.Equal(ChangeKind.Replaced, history[1].Kind); + Assert.Equal(ProjectTreeChangedProperties.PositionUnderParent, history[1].Changes); + Assert.Same(ab, history[1].Before); + Assert.Same(moved.Children[0], history[1].After); + Assert.Equal(ab.Identity, history[1].Identity); } [Fact] @@ -480,17 +493,24 @@ public void MovingNodeAroundHierarchyWithChildAdds() var moved = root.RemoveDescendent(aa).AddDescendent(aaModified, ab); var history = moved.ChangesSince(root); - Assert.Equal(2, history.Count); + Assert.Equal(3, history.Count); + Assert.Equal(ChangeKind.Replaced, history[0].Kind); Assert.Equal(ProjectTreeChangedProperties.Parent, history[0].Changes); Assert.Same(aa, history[0].Before); Assert.Same(aaModified, history[0].After); Assert.Equal(aa.Identity, history[0].Identity); - Assert.Equal(ChangeKind.Added, history[1].Kind); - Assert.Same(aaModified.Children[0], history[1].After); - Assert.Null(history[1].Before); - Assert.Equal(aaModified.Children[0].Identity, history[1].Identity); + Assert.Equal(ChangeKind.Replaced, history[1].Kind); + Assert.Equal(ProjectTreeChangedProperties.PositionUnderParent, history[1].Changes); + Assert.Same(ab, history[1].Before); + Assert.Same(moved.Children[0], history[1].After); + Assert.Equal(ab.Identity, history[1].Identity); + + Assert.Equal(ChangeKind.Added, history[2].Kind); + Assert.Same(aaModified.Children[0], history[2].After); + Assert.Null(history[2].Before); + Assert.Equal(aaModified.Children[0].Identity, history[2].Identity); } [Fact] @@ -506,7 +526,8 @@ public void MovingNodeAroundHierarchyWithChildRemoves() var moved = root.RemoveDescendent(aa).AddDescendent(aaModified, ab); var history = moved.ChangesSince(root); - Assert.Equal(2, history.Count); + Assert.Equal(3, history.Count); + Assert.Equal(ChangeKind.Removed, history[0].Kind); Assert.Same(aa.Children[0], history[0].Before); Assert.Null(history[0].After); @@ -517,6 +538,12 @@ public void MovingNodeAroundHierarchyWithChildRemoves() Assert.Same(aa, history[1].Before); Assert.Same(aaModified, history[1].After); Assert.Equal(aa.Identity, history[1].Identity); + + Assert.Equal(ChangeKind.Replaced, history[2].Kind); + Assert.Equal(ProjectTreeChangedProperties.PositionUnderParent, history[2].Changes); + Assert.Same(ab, history[2].Before); + Assert.Same(moved.Children[0], history[2].After); + Assert.Equal(ab.Identity, history[2].Identity); } [Fact] @@ -527,7 +554,8 @@ public void RepositioningNodeWithinParentsChildren() aa = ProjectTree.Create("AA"), ab = ProjectTree.Create("AB")); var ac = aa.WithCaption("AC"); - var modified = root.ReplaceDescendent(aa, ac); + // TODO: we should generate a method for ordered collections for reordering. + var modified = root.WithChildren(root.Children.Remove(aa).Insert(1, ac)); var history = modified.ChangesSince(root); Assert.Equal(1, history.Count); diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTestBase.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTestBase.cs index 2fdab3e..50e7bc6 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTestBase.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreeNodeTestBase.cs @@ -27,12 +27,12 @@ public abstract class ProjectTreeNodeTestBase : IDisposable private int nodeCounter; - internal ImmutableSortedSet Children { get; set; } + internal ImmutableList Children { get; set; } public ProjectTreeNodeTestBase() { this.nodeCounter = 0; - this.Children = ImmutableSortedSet.Create(ProjectTreeSort.Default); + this.Children = ImmutableList.Create(); } public void Dispose() diff --git a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreePartial.cs b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreePartial.cs index e38a97b..ca55f78 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreePartial.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Tests/TestSources/ProjectTreePartial.cs @@ -19,7 +19,7 @@ public static IReadOnlyList GetDelta(ProjectTree before, ProjectTree a static partial void CreateDefaultTemplate(ref ProjectTree.Template template) { - template.Children = ImmutableSortedSet.Create(ProjectTreeSort.Default); + template.Children = ImmutableList.Create(); template.Capabilities = ImmutableHashSet.Create(StringComparer.OrdinalIgnoreCase); template.Visible = true; } From 972192b40f2144f2103eb5cc273b41221c68dbe1 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Sun, 12 Jul 2015 20:50:28 -0700 Subject: [PATCH 5/8] Make collection analysis properties available to all fields --- .../CodeGen.cs | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen.cs b/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen.cs index 840c730..0201aed 100644 --- a/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen.cs +++ b/src/ImmutableObjectGraph.CodeGeneration.Roslyn/CodeGen.cs @@ -1059,24 +1059,9 @@ public MetaType RootAncestorOrThisType } } - public bool ChildrenAreSorted - { - get - { - // Not very precise, but it does the job for now. - return this.RecursiveParent.RecursiveField.Type.Name == nameof(ImmutableSortedSet); - } - } + public bool ChildrenAreSorted => this.RecursiveParent.RecursiveField.IsCollectionSorted; - public bool ChildrenAreOrdered - { - get - { - // Not very precise, but it does the job for now. - var namedType = this.RecursiveParent.RecursiveField.Type as INamedTypeSymbol; - return namedType != null && namedType.AllInterfaces.Any(iface => iface.Name == nameof(IReadOnlyList)); - } - } + public bool ChildrenAreOrdered => this.RecursiveParent.RecursiveField.IsCollectionOrdered; public IEnumerable GetFieldsBeyond(MetaType ancestor) { @@ -1222,6 +1207,30 @@ public bool IsCollection get { return IsCollectionType(this.Symbol.Type); } } + public bool IsCollectionOrdered + { + get + { + if (this.IsCollection) + { + // Not very precise, but it does the job for now. + var namedType = this.Type as INamedTypeSymbol; + return namedType?.AllInterfaces.Any(iface => iface.Name == nameof(IReadOnlyList)) ?? false; + } + + return false; + } + } + + public bool IsCollectionSorted + { + get + { + // Not very precise, but it does the job for now. + return this.Type.Name == nameof(ImmutableSortedSet); + } + } + public MetaType DeclaringType { get { return new MetaType(this.metaType.Generator, this.Symbol.ContainingType); } From c7218cc105edd200d79e8ddea0c5e4f1f6485c12 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 22 Dec 2015 08:24:58 -0800 Subject: [PATCH 6/8] Add xml doc comment --- src/ImmutableObjectGraph.Generation/CodeGen.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ImmutableObjectGraph.Generation/CodeGen.cs b/src/ImmutableObjectGraph.Generation/CodeGen.cs index 1d8bf0e..c66c8db 100644 --- a/src/ImmutableObjectGraph.Generation/CodeGen.cs +++ b/src/ImmutableObjectGraph.Generation/CodeGen.cs @@ -1387,6 +1387,11 @@ public MetaType TypeAsGeneratedImmutable public bool IsDictionary => IsDictionaryType(this.Symbol.Type); + /// + /// Gets a value indicating whether the elements in the collection have a defined order. + /// This may be because it is sorted, or because it is documented to retain the order + /// in which the elements were inserted (e.g. a list). + /// public bool IsCollectionOrdered { get From 78d1288a9f13fd26697fc057ba378eb8709b0de2 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Tue, 22 Dec 2015 17:51:33 -0800 Subject: [PATCH 7/8] Add project tree reordering test (that fails) --- .../TestSources/ProjectTreeNodeTest.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs b/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs index 499c2a5..1509112 100644 --- a/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs +++ b/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs @@ -564,5 +564,32 @@ public void RepositioningNodeWithinParentsChildren() Assert.Same(aa, history[0].Before); Assert.Same(ac, history[0].After); } + + [Fact] + public void ReorderChildrenInOrderedList() + { + ProjectTree aa, ab, ac, ad, ae; + var original = ProjectTree.Create("A").WithChildren( + aa = ProjectTree.Create("AA"), + ab = ProjectTree.Create("AB"), + ac = ProjectTree.Create("AC"), + ad = ProjectTree.Create("AD"), + ae = ProjectTree.Create("AE")); + var reordered = original.RemoveChild(ac).AddChild(ac); // move AC to bottom. + Assert.Equal(new string[] { "AA", "AB", "AD", "AE", "AC" }, reordered.Children.Select(c => c.Caption)); + + var changes = reordered.ChangesSince(original); + Assert.NotEmpty(changes); + + // Now move it back to its original position. + var restored = reordered.WithChildren( + reordered.Children.Remove(ac).Insert(2, ac)); + Assert.Equal(new string[] { "AA", "AB", "AC", "AD", "AE" }, restored.Children.Select(c => c.Caption)); + + changes = restored.ChangesSince(reordered); + Assert.NotEmpty(changes); + + Assert.Empty(restored.ChangesSince(original)); + } } } From 6eb190f09aa39f0dd3290d137efe04b981e60619 Mon Sep 17 00:00:00 2001 From: Andrew Arnott Date: Wed, 23 Dec 2015 11:07:38 -0800 Subject: [PATCH 8/8] Skip failing test --- .../TestSources/ProjectTreeNodeTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs b/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs index 1509112..c8395f6 100644 --- a/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs +++ b/src/ImmutableObjectGraph.Generation.Tests/TestSources/ProjectTreeNodeTest.cs @@ -565,7 +565,7 @@ public void RepositioningNodeWithinParentsChildren() Assert.Same(ac, history[0].After); } - [Fact] + [Fact(Skip = "Not yet passing")] public void ReorderChildrenInOrderedList() { ProjectTree aa, ab, ac, ad, ae;