Skip to content

Commit 0b57b09

Browse files
amis92amis92
authored
fix: SourceRewriter (#75)
* feature: optimize Core.With and .Update for same value * feature: optimize Node.Withs and .UpdateWith for same values * fix: SourceRewriter closes #71 * docs: changelog entry
1 parent 98738a4 commit 0b57b09

File tree

10 files changed

+258
-60
lines changed

10 files changed

+258
-60
lines changed

‎CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111
* Support for BattleScribe v2.03 data format ([#47])
12-
* "Latest" channel (folder) for Catalogue.xsd
12+
* "Latest" channel (folder) for `Catalogue.xsd`.
1313
* `wham --info` command that displays more detailed program info ([#64]).
1414

1515
### Changed
@@ -18,6 +18,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1818
much more defaults, use other Nodes as value providers, and add more methods ([#58]).
1919
* `INodeWithCore<TCore>` is now covariant on `TCore` parameter, updating it's
2020
signature to `interface INodeWithCore<out TCore>` ([#63]).
21+
* Cores and Nodes' With and Update methods now check for equality of old and new,
22+
and when they're equal, return current instance ([#75]).
23+
* `SourceRewriter` implementation fixed to actually work ([#75]).
2124

2225
## Removed
2326
* `SelectionEntryNode.CategoryEntryId` property was removed. It was a leftover from old format, pre-2.01 ([#59]).
@@ -34,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3437
[#59]: https://github.com/WarHub/wham/pull/59
3538
[#63]: https://github.com/WarHub/wham/pull/63
3639
[#64]: https://github.com/WarHub/wham/pull/64
40+
[#75]: https://github.com/WarHub/wham/pull/75
3741

3842
## [0.6.17] - 2019-08-16
3943

‎src/WarHub.ArmouryModel.Source.CodeGeneration/Generators/ListNodePartialGenerator.cs

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -184,27 +184,33 @@ private MemberDeclarationSyntax AcceptGenericMethod()
184184

185185
private MemberDeclarationSyntax CreateWithNodesMethod()
186186
{
187-
const string Nodes = "nodes";
187+
const string NodesVarName = "nodes";
188188
return
189189
MethodDeclaration(
190-
GenericName(Names.ListNode)
191-
.AddTypeArgumentListArguments(
192-
Descriptor.GetNodeTypeIdentifierName()),
193-
Names.WithNodes)
194-
.AddModifiers(
195-
SyntaxKind.PublicKeyword,
196-
SyntaxKind.OverrideKeyword)
197-
.AddParameterListParameters(
198-
Parameter(
199-
Identifier(Nodes))
200-
.WithType(
201-
Descriptor.GetNodeTypeIdentifierName().ToNodeListType()))
202-
.AddBodyStatements(
203-
ReturnStatement(
204-
IdentifierName(Nodes)
205-
.MemberAccess(
206-
IdentifierName(Names.ToListNode))
207-
.InvokeWithArguments()));
190+
GenericName(Names.ListNode)
191+
.AddTypeArgumentListArguments(
192+
Descriptor.GetNodeTypeIdentifierName()),
193+
Names.WithNodes)
194+
.AddModifiers(
195+
SyntaxKind.PublicKeyword,
196+
SyntaxKind.OverrideKeyword)
197+
.AddParameterListParameters(
198+
Parameter(
199+
Identifier(NodesVarName))
200+
.WithType(
201+
Descriptor.GetNodeTypeIdentifierName().ToNodeListType()))
202+
.AddBodyStatements(
203+
ReturnStatement(
204+
ConditionalExpression(
205+
BinaryExpression(
206+
SyntaxKind.EqualsExpression,
207+
ThisExpression().MemberAccess(IdentifierName(Names.NodeList)),
208+
IdentifierName(NodesVarName)),
209+
ThisExpression(),
210+
IdentifierName(NodesVarName)
211+
.MemberAccess(
212+
IdentifierName(Names.ToListNode))
213+
.InvokeWithArguments())));
208214
}
209215
}
210216
}

‎src/WarHub.ArmouryModel.Source.CodeGeneration/Generators/NodeGenerator.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,11 +238,17 @@ MethodDeclarationSyntax UpdateWithMethod()
238238
.AddModifiers(SyntaxKind.VirtualKeyword)
239239
.AddBodyStatements(
240240
ReturnStatement(
241-
ObjectCreationExpression(
242-
Descriptor.GetNodeTypeIdentifierName())
243-
.InvokeWithArguments(
244-
IdentifierName(CoreParameter),
245-
LiteralExpression(SyntaxKind.NullLiteralExpression))));
241+
ConditionalExpression(
242+
BinaryExpression(
243+
SyntaxKind.EqualsExpression,
244+
ThisExpression().MemberAccess(CorePropertyIdentifierName),
245+
IdentifierName(CoreParameter)),
246+
ThisExpression(),
247+
ObjectCreationExpression(
248+
Descriptor.GetNodeTypeIdentifierName())
249+
.InvokeWithArguments(
250+
IdentifierName(CoreParameter),
251+
LiteralExpression(SyntaxKind.NullLiteralExpression)))));
246252
}
247253
MethodDeclarationSyntax DerivedUpdateWithMethod()
248254
{

‎src/WarHub.ArmouryModel.Source.CodeGeneration/Generators/RecordCorePartialGenerator.cs

Lines changed: 54 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.CodeAnalysis;
66
using Microsoft.CodeAnalysis.CSharp;
77
using Microsoft.CodeAnalysis.CSharp.Syntax;
8+
using MoreLinq;
89
using static Microsoft.CodeAnalysis.CSharp.SyntaxFactory;
910

1011
namespace WarHub.ArmouryModel.Source.CodeGeneration
@@ -65,7 +66,6 @@ StatementSyntax CreateCtorAssignment(CoreDescriptor.Entry entry)
6566
private IEnumerable<MethodDeclarationSyntax> GenerateUpdateMethods()
6667
{
6768
var abstractBaseUpdateName = Names.Update + Descriptor.CoreTypeIdentifier.Text;
68-
var arguments = Descriptor.Entries.Select(x => x.IdentifierName).ToArray();
6969
var parameters = Descriptor.Entries
7070
.Select(CreateParameter)
7171
.ToImmutableArray();
@@ -77,12 +77,7 @@ private IEnumerable<MethodDeclarationSyntax> GenerateUpdateMethods()
7777
x => x.AddModifiers(SyntaxKind.NewKeyword))
7878
.AddParameterListParameters(parameters)
7979
.AddBodyStatements(
80-
ReturnStatement(
81-
IsAbstract ?
82-
IdentifierName(abstractBaseUpdateName)
83-
.InvokeWithArguments(arguments) :
84-
ObjectCreationExpression(Descriptor.CoreType)
85-
.WithArguments(arguments)));
80+
UpdateStatements());
8681
if (IsAbstract)
8782
{
8883
yield return
@@ -104,7 +99,46 @@ private IEnumerable<MethodDeclarationSyntax> GenerateUpdateMethods()
10499
.AddBodyStatements(
105100
ReturnStatement(
106101
IdentifierName(Names.Update)
107-
.InvokeWithArguments(arguments)));
102+
.InvokeWithArguments(
103+
Descriptor.Entries.Select(x => x.IdentifierName))));
104+
}
105+
IEnumerable<StatementSyntax> UpdateStatements()
106+
{
107+
if (IsAbstract)
108+
{
109+
yield return
110+
ReturnStatement(
111+
IdentifierName(abstractBaseUpdateName)
112+
.InvokeWithArguments(
113+
Descriptor.Entries.Select(x => x.IdentifierName)));
114+
yield break;
115+
}
116+
const string EqualVarName = "equal";
117+
yield return
118+
LocalDeclarationStatement(
119+
VariableDeclaration(
120+
IdentifierName("var"))
121+
.AddVariables(
122+
VariableDeclarator(EqualVarName)
123+
.WithInitializer(
124+
EqualsValueClause(
125+
EqualsInitializer()))));
126+
yield return
127+
ReturnStatement(
128+
ConditionalExpression(
129+
IdentifierName(EqualVarName),
130+
ThisExpression(),
131+
ObjectCreationExpression(Descriptor.CoreType)
132+
.WithArguments(
133+
Descriptor.Entries.Select(x => x.IdentifierName))));
134+
ExpressionSyntax EqualsInitializer() =>
135+
(from entry in Descriptor.Entries
136+
let idName = entry.IdentifierName
137+
let thisAccess =
138+
ThisExpression()
139+
.MemberAccess(idName)
140+
select BinaryExpression(SyntaxKind.EqualsExpression, thisAccess, idName))
141+
.Aggregate((x, y) => BinaryExpression(SyntaxKind.LogicalAndExpression, x, y));
108142
}
109143
}
110144

@@ -114,7 +148,7 @@ private IEnumerable<MemberDeclarationSyntax> GenerateMutators()
114148
MethodDeclarationSyntax CreateRecordMutator(CoreDescriptor.Entry entry)
115149
{
116150
var arguments = Descriptor.Entries.Select(x => x.IdentifierName);
117-
var mutator =
151+
return
118152
MethodDeclaration(
119153
Descriptor.CoreType,
120154
GetMutatorIdentifier())
@@ -127,14 +161,18 @@ MethodDeclarationSyntax CreateRecordMutator(CoreDescriptor.Entry entry)
127161
.WithType(entry.Type))
128162
.AddBodyStatements(
129163
ReturnStatement(
130-
IdentifierName(Names.Update)
131-
.InvokeWithArguments(arguments)));
132-
return mutator;
164+
ConditionalExpression(
165+
BinaryExpression(
166+
SyntaxKind.EqualsExpression,
167+
ThisExpression()
168+
.MemberAccess(entry.IdentifierName),
169+
entry.IdentifierName),
170+
ThisExpression(),
171+
IdentifierName(Names.Update)
172+
.InvokeWithArguments(arguments))));
133173

134-
SyntaxToken GetMutatorIdentifier()
135-
{
136-
return Identifier($"{Names.WithPrefix}{entry.Identifier.ValueText}");
137-
}
174+
SyntaxToken GetMutatorIdentifier() =>
175+
Identifier(Names.WithPrefix + entry.Identifier.ValueText);
138176
}
139177
}
140178

‎src/WarHub.ArmouryModel.Source/Foundation/SourceRewriter.cs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,35 @@ public virtual ListNode<TNode> VisitListNode<TNode>(ListNode<TNode> list)
1515
return list.WithNodes(VisitNodeList(list.NodeList));
1616
}
1717

18-
public virtual NodeList<TNode> VisitNodeList<TNode>(NodeList<TNode> list) where TNode : SourceNode
18+
public virtual NodeList<TNode> VisitNodeList<TNode>(NodeList<TNode> list)
19+
where TNode : SourceNode
1920
{
20-
ImmutableArray<TNode>.Builder alternate = null;
21+
ImmutableArray<TNode>.Builder builder = null;
2122
for (int i = 0, n = list.Count; i < n; i++)
2223
{
23-
var item = list[i];
24-
var visited = VisitListElement(item);
25-
if (alternate is null)
24+
var original = list[i];
25+
var returned = VisitListElement(original);
26+
var itemChanged = original != returned;
27+
var builderExists = builder is { };
28+
if (itemChanged || builderExists)
2629
{
27-
if (item != visited)
30+
if (itemChanged && !builderExists)
2831
{
29-
alternate = ImmutableArray.CreateBuilder<TNode>(n);
30-
alternate.AddRange(list.Take(i));
32+
builder = ImmutableArray.CreateBuilder<TNode>(n);
33+
builder.AddRange(list.Take(i));
34+
}
35+
if (returned is { })
36+
{
37+
builder.Add(returned);
3138
}
32-
}
33-
else if (visited?.IsKind(SourceKind.Unknown) == false)
34-
{
35-
alternate.Add(visited);
3639
}
3740
}
38-
return alternate?.MoveToImmutable().ToNodeList() ?? list;
41+
return
42+
builder is null
43+
? list
44+
: builder.Capacity == builder.Count
45+
? builder.MoveToImmutable().ToNodeList()
46+
: builder.ToImmutable().ToNodeList();
3947
}
4048

4149
public virtual TNode VisitListElement<TNode>(TNode node) where TNode : SourceNode

‎tests/WarHub.ArmouryModel.Source.CodeGeneration.Tests.GeneratedCode/ItemCore.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,5 @@ public partial class ItemCore
1111

1212
[XmlAttribute("name")]
1313
public string Name { get; }
14-
15-
//public static CharacteristicType EmptyInstance { get; } = new Builder
16-
//{
17-
// Id = Guid.Empty.ToString()
18-
//}.ToImmutable();
1914
}
2015
}

‎tests/WarHub.ArmouryModel.Source.CodeGeneration.Tests/GeneratedCoreTests.cs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,20 @@
11
using System.Collections.Immutable;
2+
using FluentAssertions;
23
using WarHub.ArmouryModel.Source.CodeGeneration.Tests.GeneratedCode;
34
using Xunit;
45

56
namespace WarHub.ArmouryModel.Source.CodeGeneration.Tests
67
{
78
public class GeneratedCoreTests
89
{
10+
[Fact]
11+
public void Update_with_current_values_returns_this_instance()
12+
{
13+
var subject = new ContainerCore("id", "name", ImmutableArray<ItemCore>.Empty);
14+
var result = subject.Update("id", "name", ImmutableArray<ItemCore>.Empty);
15+
result.Should().BeSameAs(subject);
16+
}
17+
918
[Fact]
1019
public void With_SimpleProperty_DoesNotModifyInstance()
1120
{
@@ -15,6 +24,14 @@ public void With_SimpleProperty_DoesNotModifyInstance()
1524
Assert.Null(subject.Name);
1625
}
1726

27+
[Fact]
28+
public void With_SimpleProperty_when_new_value_equals_old_returns_this_instance()
29+
{
30+
var subject = new ItemCore("1", "item");
31+
var result = subject.WithName("item");
32+
result.Should().BeSameAs(subject);
33+
}
34+
1835
[Fact]
1936
public void With_SimpleProperty_CreatesModifiedInstance()
2037
{
@@ -33,6 +50,15 @@ public void With_CollectionProperty_DoesNotModifyInstance()
3350
Assert.Empty(subject.Items);
3451
}
3552

53+
[Fact]
54+
public void With_CollectionProperty_when_new_value_equals_old_returns_this_instance()
55+
{
56+
var subject = new ContainerCore.Builder().ToImmutable()
57+
.WithItems(ImmutableArray.Create(new ItemCore("1", "item")));
58+
var result = subject.WithItems(subject.Items);
59+
result.Should().BeSameAs(subject);
60+
}
61+
3662
[Fact]
3763
public void With_CollectionProperty_CreatesModifiedInstance()
3864
{

‎tests/WarHub.ArmouryModel.Source.CodeGeneration.Tests/GeneratedNodeTests.cs

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections.Generic;
22
using System.Collections.Immutable;
33
using System.Linq;
4+
using FluentAssertions;
45
using MoreLinq;
56
using WarHub.ArmouryModel.Source.CodeGeneration.Tests.GeneratedCode;
67
using Xunit;
@@ -15,7 +16,7 @@ public class GeneratedNodeTests
1516
public const string ItemId = "item1";
1617
public const string ItemName = "item name";
1718

18-
public class OneItemContainerPackage
19+
public static class OneItemContainerPackage
1920
{
2021
public static ContainerNode CreateContainer()
2122
{
@@ -392,5 +393,26 @@ public void FirstAncestorOrSelf_OnRecursiveTreeItem_GivenContainerName_FindsCont
392393
var actual = item.FirstAncestorOrSelf<RecursiveContainerNode>(x => x.Name == Name2);
393394
Assert.Same(expected, actual);
394395
}
396+
397+
[Fact]
398+
public void With_when_value_equals_old_returns_this_instance()
399+
{
400+
var subject = NodeFactory.Container("id", "name", NodeFactory.ItemList(NodeFactory.Item("a", "b")));
401+
var result = subject
402+
.WithId("id")
403+
.WithName("name")
404+
.WithItems(subject.Items.NodeList);
405+
result.Should().BeSameAs(subject);
406+
}
407+
408+
[Fact]
409+
public void WithNodes_when_NodeList_equals_old_returns_this_instance()
410+
{
411+
var subject = NodeFactory.ItemList(
412+
NodeFactory.Item("1", "a1"),
413+
NodeFactory.Item("2", "a2"));
414+
var result = subject.WithNodes(subject.NodeList);
415+
result.Should().BeSameAs(subject);
416+
}
395417
}
396418
}

‎tests/WarHub.ArmouryModel.Source.CodeGeneration.Tests/WarHub.ArmouryModel.Source.CodeGeneration.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
</PropertyGroup>
77

88
<ItemGroup>
9+
<PackageReference Include="FluentAssertions" />
910
<PackageReference Include="Microsoft.NET.Test.Sdk" />
1011
<PackageReference Include="NSubstitute" />
1112
<PackageReference Include="xunit" />

0 commit comments

Comments
 (0)