Skip to content

Commit fa3541d

Browse files
amis92Copilot
amis92
and
Copilot
authored
Add Roslyn source generators for Concrete.Extensions symbols (#297)
* Add Roslyn source generators for Concrete.Extensions symbols Introduce Concrete.Extensions.Generators project with: - [GenerateSymbol(SymbolKind.X)] generates Kind property + 3 Accept overloads - [Bound] on properties generates CheckReferencesCore - WHAM001 analyzer warns on GetBoundField without [Bound] SymbolKind refactoring (prerequisite for 1:1 Kind-to-Visit mapping): - Split Link into CatalogueReference, PublicationReference, EntryReferencePath - Rename Namespace to GamesystemNamespace, Container to ContainerEntryInstance - Fix RosterCostSymbol.Kind bug (was ResourceEntry, now RosterCost) Migration: 27 classes use [GenerateSymbol], 22 [Bound] annotations across 19 classes. ~250 lines of error-prone boilerplate eliminated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR review: remove unused usings, sort generator output - Remove unused using directives from AttributeSources.cs and GenerateSymbolGenerator.cs - Sort BoundGenerator output by type name and property name for deterministic generation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add WHAM002 analyzer: warn on non-static GetBoundField lambda Non-static lambdas in GetBoundField cause delegate allocation on every property access. The static modifier prevents accidental closure captures and keeps the hot path allocation-free. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Rename ContainerEntryInstance back to Container Revert SymbolKind.ContainerEntryInstance to SymbolKind.Container (shorter, original name). Rename VisitContainerEntryInstance to VisitContainer and IContainerEntryInstanceSymbol to IContainerSymbol to maintain 1:1 mapping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add unit tests for source generators and analyzers Add tests for GenerateSymbolGenerator, BoundGenerator, and BoundAnalyzer: - GenerateSymbolGenerator: Kind+Accept for Symbol subclasses, Accept-only for non-Symbol classes, sealed override for abstract bases, compilation - BoundGenerator: single/multiple [Bound] properties, alphabetical sort, attribute emission, compilation - BoundAnalyzer: WHAM001 (GetBoundField without [Bound]), WHAM002 (non-static lambda), no false positives outside properties 15 tests total, all passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Update AGENTS.md with generator test docs Add generator test command to Build & test section and add a new 'Modify generators/analyzers' task to Common tasks. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d5b489e commit fa3541d

63 files changed

Lines changed: 1253 additions & 346 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎AGENTS.md‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ git submodule update --init # first time (required)
2525
dotnet restore && dotnet build # build all
2626
dotnet test # all tests
2727
dotnet test tests/WarHub.ArmouryModel.RosterEngine.Tests/ # conformance only
28+
dotnet test tests/WarHub.ArmouryModel.Concrete.Extensions.Generators.Tests/ # generator/analyzer tests
2829
dotnet pack # NuGet packages (Release mode)
2930
```
3031

@@ -49,6 +50,7 @@ XML file → DTO (*Core) → SourceNode tree → SourceTree
4950
| Source Generator (`*Core` → `SourceNode`) | `src/WarHub.ArmouryModel.Source.CodeGeneration/` |
5051
| ISymbol public API, Binder, Diagnostics | `src/WarHub.ArmouryModel.Extensions/` |
5152
| Symbol implementations, lazy binding | `src/WarHub.ArmouryModel.Concrete.Extensions/` |
53+
| Symbol source generators + analyzer | `src/WarHub.ArmouryModel.Concrete.Extensions.Generators/` |
5254

5355
### Roster engine (protocol-based, conformance-tested)
5456

@@ -133,7 +135,7 @@ Path: `src/WarHub.ArmouryModel.EditorServices/`
133135
Source, Source.BattleScribe, ProjectModel, Workspaces.BattleScribe, Workspaces.Gitree, CliTool (`wham` dotnet tool)
134136

135137
**Internal** (IsPackable=false, relaxed analysis):
136-
Extensions, Concrete.Extensions, EditorServices, RosterEngine, RosterEngine.Spec, Phalanx.SampleDataset
138+
Extensions, Concrete.Extensions, Concrete.Extensions.Generators, EditorServices, RosterEngine, RosterEngine.Spec, Phalanx.SampleDataset
137139

138140
**External submodule**: BattleScribeSpec.TestKit (from `lib/battlescribe-spec`)
139141

@@ -161,6 +163,13 @@ Extensions, Concrete.Extensions, EditorServices, RosterEngine, RosterEngine.Spec
161163
- `.github/workflows/ci.yml` — main CI (build, test, pack)
162164
- `.github/workflows/publish.yml` — NuGet publishing
163165

166+
**Modify generators/analyzers (Concrete.Extensions.Generators):**
167+
1. Generator project is netstandard2.0 (Roslyn requirement) — no records, no
168+
`ImmutableArray` collection expressions, explicit `using` directives needed
169+
2. Make changes in `src/WarHub.ArmouryModel.Concrete.Extensions.Generators/`
170+
3. Run generator tests: `dotnet test tests/WarHub.ArmouryModel.Concrete.Extensions.Generators.Tests/`
171+
4. Run consuming project tests: `dotnet test tests/WarHub.ArmouryModel.Concrete.Extensions.Tests/`
172+
164173
## Known gotchas
165174

166175
- **Submodule required**: `git submodule update --init` before building
@@ -172,6 +181,12 @@ Extensions, Concrete.Extensions, EditorServices, RosterEngine, RosterEngine.Spec
172181
marked IsPackable=false to avoid failures
173182
- **Code generation**: `WarHub.ArmouryModel.Source` uses a C# Source Generator —
174183
changes to `*Core` types require regeneration
184+
- **Concrete.Extensions generators**: `Concrete.Extensions.Generators` provides:
185+
- `[GenerateSymbol(SymbolKind.X)]` — generates `Kind` property + 3 `Accept` overloads
186+
- `[Bound]` on properties — generates `CheckReferencesCore` accessing all bound properties
187+
- `WHAM001` analyzer — warns when `GetBoundField` is called without `[Bound]`
188+
- `WHAM002` analyzer — warns when `GetBoundField` lambda is not `static`
189+
- Symbol classes using these must be `partial`
175190
- **BattleScribe quirks**: some spec default expectations match BattleScribe bugs rather than
176191
"correct" behavior; wham uses engine-specific overrides for these (documented in
177192
`docs/adrs/0004-battlescribe-spec-conformance-testing.md`)

‎src/WarHub.ArmouryModel.Concrete.Extensions.Generators/AttributeSources.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
namespace WarHub.ArmouryModel.Concrete.Generators;
2+
3+
internal static class AttributeSources
4+
{
5+
public const string GenerateSymbolAttributeName = "WarHub.ArmouryModel.Concrete.GenerateSymbolAttribute";
6+
public const string BoundAttributeName = "WarHub.ArmouryModel.Concrete.BoundAttribute";
7+
8+
public const string GenerateSymbolAttributeSource = """
9+
// <auto-generated/>
10+
#nullable enable
11+
12+
namespace WarHub.ArmouryModel.Concrete
13+
{
14+
[global::System.AttributeUsage(global::System.AttributeTargets.Class, Inherited = false)]
15+
[global::System.Diagnostics.Conditional("CONCRETE_EXTENSIONS_GENERATORS")]
16+
internal sealed class GenerateSymbolAttribute : global::System.Attribute
17+
{
18+
public GenerateSymbolAttribute(global::WarHub.ArmouryModel.SymbolKind kind)
19+
{
20+
Kind = kind;
21+
}
22+
23+
public global::WarHub.ArmouryModel.SymbolKind Kind { get; }
24+
}
25+
}
26+
""";
27+
28+
public const string BoundAttributeSource = """
29+
// <auto-generated/>
30+
#nullable enable
31+
32+
namespace WarHub.ArmouryModel.Concrete
33+
{
34+
[global::System.AttributeUsage(global::System.AttributeTargets.Property, Inherited = false)]
35+
[global::System.Diagnostics.Conditional("CONCRETE_EXTENSIONS_GENERATORS")]
36+
internal sealed class BoundAttribute : global::System.Attribute
37+
{
38+
}
39+
}
40+
""";
41+
}

‎src/WarHub.ArmouryModel.Concrete.Extensions.Generators/BoundAnalyzer.cs‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
using System.Collections.Immutable;
2+
using Microsoft.CodeAnalysis;
3+
using Microsoft.CodeAnalysis.CSharp;
4+
using Microsoft.CodeAnalysis.CSharp.Syntax;
5+
using Microsoft.CodeAnalysis.Diagnostics;
6+
using Microsoft.CodeAnalysis.Operations;
7+
8+
namespace WarHub.ArmouryModel.Concrete.Generators;
9+
10+
[DiagnosticAnalyzer(LanguageNames.CSharp)]
11+
public sealed class BoundAnalyzer : DiagnosticAnalyzer
12+
{
13+
public static readonly DiagnosticDescriptor GetBoundFieldWithoutBoundAttribute = new(
14+
id: "WHAM001",
15+
title: "Property calls GetBoundField without [Bound] attribute",
16+
messageFormat: "Property '{0}' calls GetBoundField but is not annotated with [Bound]. Add [Bound] to ensure it is included in CheckReferencesCore.",
17+
category: "WarHub.ArmouryModel",
18+
defaultSeverity: DiagnosticSeverity.Warning,
19+
isEnabledByDefault: true);
20+
21+
public static readonly DiagnosticDescriptor GetBoundFieldNonStaticLambda = new(
22+
id: "WHAM002",
23+
title: "GetBoundField called with non-static lambda",
24+
messageFormat: "GetBoundField lambda should be static to avoid delegate allocation on every access. Add the 'static' modifier.",
25+
category: "WarHub.ArmouryModel",
26+
defaultSeverity: DiagnosticSeverity.Warning,
27+
isEnabledByDefault: true);
28+
29+
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
30+
ImmutableArray.Create(GetBoundFieldWithoutBoundAttribute, GetBoundFieldNonStaticLambda);
31+
32+
public override void Initialize(AnalysisContext context)
33+
{
34+
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
35+
context.EnableConcurrentExecution();
36+
37+
context.RegisterOperationAction(AnalyzeInvocation, OperationKind.Invocation);
38+
}
39+
40+
private static void AnalyzeInvocation(OperationAnalysisContext context)
41+
{
42+
var invocation = (IInvocationOperation)context.Operation;
43+
if (invocation.TargetMethod.Name != "GetBoundField")
44+
return;
45+
46+
CheckBoundAttribute(context, invocation);
47+
CheckStaticLambda(context, invocation);
48+
}
49+
50+
private static void CheckBoundAttribute(OperationAnalysisContext context, IInvocationOperation invocation)
51+
{
52+
// Walk up to find the containing property
53+
var containingSymbol = context.ContainingSymbol;
54+
if (containingSymbol is not IMethodSymbol { AssociatedSymbol: IPropertySymbol property })
55+
return;
56+
57+
// Check if the property has [Bound] attribute
58+
var hasBoundAttribute = false;
59+
foreach (var attr in property.GetAttributes())
60+
{
61+
if (attr.AttributeClass?.Name == "BoundAttribute"
62+
&& attr.AttributeClass.ContainingNamespace.ToDisplayString() == "WarHub.ArmouryModel.Concrete")
63+
{
64+
hasBoundAttribute = true;
65+
break;
66+
}
67+
}
68+
69+
if (!hasBoundAttribute)
70+
{
71+
var diagnostic = Diagnostic.Create(
72+
GetBoundFieldWithoutBoundAttribute,
73+
invocation.Syntax.GetLocation(),
74+
property.Name);
75+
context.ReportDiagnostic(diagnostic);
76+
}
77+
}
78+
79+
private static void CheckStaticLambda(OperationAnalysisContext context, IInvocationOperation invocation)
80+
{
81+
// The last argument to GetBoundField is the binding lambda
82+
var args = invocation.Arguments;
83+
if (args.Length < 3)
84+
return;
85+
86+
var lambdaArg = args[args.Length - 1];
87+
if (lambdaArg.Value is not IDelegateCreationOperation delegateCreation)
88+
return;
89+
90+
if (delegateCreation.Target is not IAnonymousFunctionOperation anonymousFunc)
91+
return;
92+
93+
// Check the syntax node for the 'static' modifier
94+
var syntax = anonymousFunc.Syntax;
95+
bool isStatic = false;
96+
if (syntax is LambdaExpressionSyntax lambda)
97+
{
98+
isStatic = lambda.Modifiers.Any(SyntaxKind.StaticKeyword);
99+
}
100+
else if (syntax is AnonymousMethodExpressionSyntax anonymousMethod)
101+
{
102+
isStatic = anonymousMethod.Modifiers.Any(SyntaxKind.StaticKeyword);
103+
}
104+
105+
if (!isStatic)
106+
{
107+
context.ReportDiagnostic(
108+
Diagnostic.Create(GetBoundFieldNonStaticLambda, syntax.GetLocation()));
109+
}
110+
}
111+
}

‎src/WarHub.ArmouryModel.Concrete.Extensions.Generators/BoundGenerator.cs‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Collections.Immutable;
4+
using System.Text;
5+
using System.Threading;
6+
using Microsoft.CodeAnalysis;
7+
using Microsoft.CodeAnalysis.CSharp.Syntax;
8+
9+
namespace WarHub.ArmouryModel.Concrete.Generators;
10+
11+
[Generator(LanguageNames.CSharp)]
12+
public sealed class BoundGenerator : IIncrementalGenerator
13+
{
14+
public void Initialize(IncrementalGeneratorInitializationContext context)
15+
{
16+
context.RegisterPostInitializationOutput(static ctx =>
17+
{
18+
ctx.AddSource("BoundAttribute.g.cs", AttributeSources.BoundAttributeSource);
19+
});
20+
21+
var boundProperties = context.SyntaxProvider.ForAttributeWithMetadataName(
22+
AttributeSources.BoundAttributeName,
23+
predicate: static (node, _) => node is PropertyDeclarationSyntax,
24+
transform: static (ctx, ct) => ExtractModel(ctx, ct))
25+
.Where(static x => x is not null)
26+
.Select(static (x, _) => x!.Value);
27+
28+
// Collect all bound properties, then group by containing type
29+
var grouped = boundProperties.Collect().Select(static (items, ct) =>
30+
{
31+
var groups = new Dictionary<string, List<string>>();
32+
var typeInfos = new Dictionary<string, BoundTypeInfo>();
33+
foreach (var item in items)
34+
{
35+
ct.ThrowIfCancellationRequested();
36+
var key = item.FullTypeName;
37+
if (!groups.TryGetValue(key, out var list))
38+
{
39+
list = new List<string>();
40+
groups[key] = list;
41+
typeInfos[key] = new BoundTypeInfo(item.Namespace, item.ClassName, item.IsSealed);
42+
}
43+
list.Add(item.PropertyName);
44+
}
45+
var sortedKeys = new List<string>(groups.Keys);
46+
sortedKeys.Sort(StringComparer.Ordinal);
47+
var result = ImmutableArray.CreateBuilder<BoundClassModel>(groups.Count);
48+
foreach (var key in sortedKeys)
49+
{
50+
ct.ThrowIfCancellationRequested();
51+
var info = typeInfos[key];
52+
var props = groups[key];
53+
props.Sort(StringComparer.Ordinal);
54+
result.Add(new BoundClassModel(
55+
info.Namespace,
56+
info.ClassName,
57+
props.ToImmutableArray(),
58+
info.IsSealed));
59+
}
60+
return result.ToImmutable();
61+
});
62+
63+
context.RegisterSourceOutput(grouped, static (spc, models) =>
64+
{
65+
foreach (var model in models)
66+
{
67+
Execute(spc, model);
68+
}
69+
});
70+
}
71+
72+
private static BoundPropertyModel? ExtractModel(GeneratorAttributeSyntaxContext ctx, CancellationToken ct)
73+
{
74+
if (ctx.TargetSymbol is not IPropertySymbol propSymbol)
75+
return null;
76+
77+
var containingType = propSymbol.ContainingType;
78+
if (containingType is null)
79+
return null;
80+
81+
var ns = containingType.ContainingNamespace;
82+
var nsName = ns.IsGlobalNamespace ? null : ns.ToDisplayString();
83+
84+
return new BoundPropertyModel(
85+
ns: nsName,
86+
className: containingType.Name,
87+
fullTypeName: containingType.ToDisplayString(),
88+
propertyName: propSymbol.Name,
89+
isSealed: containingType.IsSealed);
90+
}
91+
92+
private static void Execute(SourceProductionContext spc, BoundClassModel model)
93+
{
94+
var sb = new StringBuilder();
95+
sb.AppendLine("// <auto-generated/>");
96+
sb.AppendLine("#nullable enable");
97+
sb.AppendLine();
98+
99+
if (model.Namespace is not null)
100+
{
101+
sb.AppendLine($"namespace {model.Namespace};");
102+
sb.AppendLine();
103+
}
104+
105+
sb.AppendLine($"partial class {model.ClassName}");
106+
sb.AppendLine("{");
107+
108+
// Use sealed override for sealed classes, plain override otherwise
109+
var modifiers = model.IsSealed ? "protected sealed override" : "protected override";
110+
sb.AppendLine($" {modifiers} void CheckReferencesCore()");
111+
sb.AppendLine(" {");
112+
foreach (var prop in model.PropertyNames)
113+
{
114+
sb.AppendLine($" _ = {prop};");
115+
}
116+
sb.AppendLine(" }");
117+
sb.AppendLine("}");
118+
119+
spc.AddSource($"{model.ClassName}.Bound.g.cs", sb.ToString());
120+
}
121+
122+
private readonly struct BoundPropertyModel
123+
{
124+
public BoundPropertyModel(string? ns, string className, string fullTypeName, string propertyName, bool isSealed)
125+
{
126+
Namespace = ns;
127+
ClassName = className;
128+
FullTypeName = fullTypeName;
129+
PropertyName = propertyName;
130+
IsSealed = isSealed;
131+
}
132+
133+
public string? Namespace { get; }
134+
public string ClassName { get; }
135+
public string FullTypeName { get; }
136+
public string PropertyName { get; }
137+
public bool IsSealed { get; }
138+
}
139+
140+
private readonly struct BoundTypeInfo
141+
{
142+
public BoundTypeInfo(string? ns, string className, bool isSealed)
143+
{
144+
Namespace = ns;
145+
ClassName = className;
146+
IsSealed = isSealed;
147+
}
148+
149+
public string? Namespace { get; }
150+
public string ClassName { get; }
151+
public bool IsSealed { get; }
152+
}
153+
154+
private readonly struct BoundClassModel
155+
{
156+
public BoundClassModel(string? ns, string className, ImmutableArray<string> propertyNames, bool isSealed)
157+
{
158+
Namespace = ns;
159+
ClassName = className;
160+
PropertyNames = propertyNames;
161+
IsSealed = isSealed;
162+
}
163+
164+
public string? Namespace { get; }
165+
public string ClassName { get; }
166+
public ImmutableArray<string> PropertyNames { get; }
167+
public bool IsSealed { get; }
168+
}
169+
}

0 commit comments

Comments
 (0)