Skip to content

Commit be70945

Browse files
robertmclawsclaude
robertmclaws
and
claude
committed
Address PR #582 review feedback from natemcmaster
- Rename CommandInfo to CommandData for naming consistency with other POCOs - Extract ConstructorParameterData to its own file - Delete obsolete update_generator.ps1 patching script - Improve error messages in GenerateModelFactory to distinguish between missing IServiceProvider vs unregistered services - Add fallback to DefaultMetadataResolver for subcommand metadata providers - Use Task.FromResult for sync OnExecute methods to avoid async warnings - Add [DynamicallyAccessedMembers] annotation to SubcommandAttributeConvention for AOT compatibility Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent cc5c90c commit be70945

6 files changed

Lines changed: 71 additions & 174 deletions

File tree

src/CommandLineUtils.Generators/CommandInfo.cs renamed to src/CommandLineUtils.Generators/CommandData.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
namespace McMaster.Extensions.CommandLineUtils.Generators
88
{
99
/// <summary>
10-
/// Information about a command class.
10+
/// Data about a command class.
1111
/// </summary>
12-
internal sealed class CommandInfo
12+
internal sealed class CommandData
1313
{
1414
public INamedTypeSymbol TypeSymbol { get; set; } = null!;
1515
public string FullTypeName { get; set; } = "";

‎src/CommandLineUtils.Generators/CommandMetadataGenerator.cs‎

Lines changed: 42 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
5151
.ForAttributeWithMetadataName(
5252
CommandAttributeFullName,
5353
predicate: static (node, _) => node is ClassDeclarationSyntax,
54-
transform: static (ctx, ct) => GetCommandInfo(ctx, ct))
54+
transform: static (ctx, ct) => GetCommandData(ctx, ct))
5555
.Where(static m => m is not null)
5656
.Select(static (m, _) => m!);
5757

@@ -83,7 +83,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
8383
});
8484
}
8585

86-
private static CommandInfo? GetCommandInfo(GeneratorAttributeSyntaxContext context, CancellationToken cancellationToken)
86+
private static CommandData? GetCommandData(GeneratorAttributeSyntaxContext context, CancellationToken cancellationToken)
8787
{
8888
if (context.TargetSymbol is not INamedTypeSymbol typeSymbol)
8989
{
@@ -101,7 +101,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
101101
return null;
102102
}
103103

104-
var info = new CommandInfo
104+
var info = new CommandData
105105
{
106106
TypeSymbol = typeSymbol,
107107
FullTypeName = typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat.WithGlobalNamespaceStyle(SymbolDisplayGlobalNamespaceStyle.Omitted)),
@@ -154,7 +154,7 @@ public void Initialize(IncrementalGeneratorInitializationContext context)
154154
return info;
155155
}
156156

157-
private static void ExtractSpecialProperties(INamedTypeSymbol typeSymbol, CommandInfo info)
157+
private static void ExtractSpecialProperties(INamedTypeSymbol typeSymbol, CommandData info)
158158
{
159159
foreach (var member in typeSymbol.GetMembers())
160160
{
@@ -195,7 +195,7 @@ private static void ExtractSpecialProperties(INamedTypeSymbol typeSymbol, Comman
195195
}
196196
}
197197

198-
private static void ExtractConstructors(INamedTypeSymbol typeSymbol, CommandInfo info)
198+
private static void ExtractConstructors(INamedTypeSymbol typeSymbol, CommandData info)
199199
{
200200
// Get all public instance constructors, ordered by parameter count descending
201201
var constructors = typeSymbol.InstanceConstructors
@@ -285,7 +285,7 @@ private static CommandAttributeData ExtractCommandAttributeData(AttributeData at
285285
return data;
286286
}
287287

288-
private static void ExtractPropertyMetadata(IPropertySymbol property, CommandInfo info)
288+
private static void ExtractPropertyMetadata(IPropertySymbol property, CommandData info)
289289
{
290290
foreach (var attr in property.GetAttributes())
291291
{
@@ -723,7 +723,7 @@ private static string FormatTypedConstant(TypedConstant constant)
723723
}
724724
}
725725

726-
private static void ExtractSubcommandMetadata(AttributeData attr, CommandInfo info)
726+
private static void ExtractSubcommandMetadata(AttributeData attr, CommandData info)
727727
{
728728
if (attr.ConstructorArguments.Length > 0)
729729
{
@@ -751,7 +751,7 @@ private static void ExtractSubcommandMetadata(AttributeData attr, CommandInfo in
751751
}
752752
}
753753

754-
private static void ExtractExecuteMethods(INamedTypeSymbol typeSymbol, CommandInfo info)
754+
private static void ExtractExecuteMethods(INamedTypeSymbol typeSymbol, CommandData info)
755755
{
756756
foreach (var member in typeSymbol.GetMembers())
757757
{
@@ -781,7 +781,7 @@ private static void ExtractExecuteMethods(INamedTypeSymbol typeSymbol, CommandIn
781781
}
782782
}
783783

784-
private static void AnalyzeExecuteMethod(IMethodSymbol method, CommandInfo info)
784+
private static void AnalyzeExecuteMethod(IMethodSymbol method, CommandData info)
785785
{
786786
// Check return type
787787
if (method.ReturnType.SpecialType == SpecialType.System_Int32)
@@ -813,7 +813,7 @@ private static void AnalyzeExecuteMethod(IMethodSymbol method, CommandInfo info)
813813
}
814814
}
815815

816-
private static string GenerateMetadataProvider(CommandInfo info)
816+
private static string GenerateMetadataProvider(CommandData info)
817817
{
818818
var sb = new StringBuilder();
819819

@@ -859,8 +859,8 @@ private static string GenerateMetadataProvider(CommandInfo info)
859859
// Subcommands
860860
GenerateSubcommandsProperty(sb, info, indent);
861861

862-
// CommandInfo
863-
GenerateCommandInfoProperty(sb, info, indent);
862+
// CommandData
863+
GenerateCommandDataProperty(sb, info, indent);
864864

865865
// ExecuteHandler
866866
GenerateExecuteHandler(sb, info, indent);
@@ -905,7 +905,7 @@ private static string GenerateMetadataProvider(CommandInfo info)
905905
return sb.ToString();
906906
}
907907

908-
private static void GenerateOptionsProperty(StringBuilder sb, CommandInfo info, string indent)
908+
private static void GenerateOptionsProperty(StringBuilder sb, CommandData info, string indent)
909909
{
910910
if (info.Options.Count == 0)
911911
{
@@ -953,7 +953,7 @@ private static void GenerateOptionsProperty(StringBuilder sb, CommandInfo info,
953953
sb.AppendLine();
954954
}
955955

956-
private static void GenerateArgumentsProperty(StringBuilder sb, CommandInfo info, string indent)
956+
private static void GenerateArgumentsProperty(StringBuilder sb, CommandData info, string indent)
957957
{
958958
if (info.Arguments.Count == 0)
959959
{
@@ -1016,7 +1016,7 @@ private static void GenerateValidatorsProperty(StringBuilder sb, List<Validation
10161016
sb.AppendLine($"{indent}}},");
10171017
}
10181018

1019-
private static void GenerateSubcommandsProperty(StringBuilder sb, CommandInfo info, string indent)
1019+
private static void GenerateSubcommandsProperty(StringBuilder sb, CommandData info, string indent)
10201020
{
10211021
if (info.Subcommands.Count == 0)
10221022
{
@@ -1030,7 +1030,7 @@ private static void GenerateSubcommandsProperty(StringBuilder sb, CommandInfo in
10301030
{
10311031
sb.AppendLine($"{indent} new SubcommandMetadata(typeof({sub.TypeName}))");
10321032
sb.AppendLine($"{indent} {{");
1033-
sb.AppendLine($"{indent} MetadataProviderFactory = () => CommandMetadataRegistry.TryGetProvider(typeof({sub.TypeName}), out var p) ? p : throw new InvalidOperationException(\"No metadata provider for \" + typeof({sub.TypeName}))");
1033+
sb.AppendLine($"{indent} MetadataProviderFactory = () => CommandMetadataRegistry.TryGetProvider(typeof({sub.TypeName}), out var p) ? p : DefaultMetadataResolver.Instance.GetProvider(typeof({sub.TypeName}))");
10341034
sb.AppendLine($"{indent} }},");
10351035
}
10361036
sb.AppendLine($"{indent} }};");
@@ -1040,10 +1040,10 @@ private static void GenerateSubcommandsProperty(StringBuilder sb, CommandInfo in
10401040
sb.AppendLine();
10411041
}
10421042

1043-
private static void GenerateCommandInfoProperty(StringBuilder sb, CommandInfo info, string indent)
1043+
private static void GenerateCommandDataProperty(StringBuilder sb, CommandData info, string indent)
10441044
{
10451045
var cmd = info.CommandAttribute;
1046-
sb.AppendLine($"{indent} public CommandMetadata? CommandInfo => new CommandMetadata");
1046+
sb.AppendLine($"{indent} public CommandMetadata? CommandData => new CommandMetadata");
10471047
sb.AppendLine($"{indent} {{");
10481048
// Use explicit name if set, otherwise use inferred name from class name
10491049
var commandName = cmd.Name ?? info.InferredName;
@@ -1074,7 +1074,7 @@ private static void GenerateCommandInfoProperty(StringBuilder sb, CommandInfo in
10741074
sb.AppendLine();
10751075
}
10761076

1077-
private static void GenerateExecuteHandler(StringBuilder sb, CommandInfo info, string indent)
1077+
private static void GenerateExecuteHandler(StringBuilder sb, CommandData info, string indent)
10781078
{
10791079
if (!info.HasOnExecute)
10801080
{
@@ -1090,9 +1090,6 @@ private static void GenerateExecuteHandler(StringBuilder sb, CommandInfo info, s
10901090
sb.AppendLine();
10911091
sb.AppendLine($"{indent} public bool IsAsync => {(info.OnExecuteIsAsync ? "true" : "false")};");
10921092
sb.AppendLine();
1093-
sb.AppendLine($"{indent} public async Task<int> InvokeAsync(object model, CommandLineApplication app, CancellationToken cancellationToken)");
1094-
sb.AppendLine($"{indent} {{");
1095-
sb.AppendLine($"{indent} var typedModel = ({info.ClassName})model;");
10961093

10971094
// Build the argument list based on what parameters the method has
10981095
var args = new List<string>();
@@ -1108,7 +1105,10 @@ private static void GenerateExecuteHandler(StringBuilder sb, CommandInfo info, s
11081105

11091106
if (info.OnExecuteIsAsync)
11101107
{
1111-
// OnExecuteAsync method
1108+
// OnExecuteAsync method - use async/await
1109+
sb.AppendLine($"{indent} public async Task<int> InvokeAsync(object model, CommandLineApplication app, CancellationToken cancellationToken)");
1110+
sb.AppendLine($"{indent} {{");
1111+
sb.AppendLine($"{indent} var typedModel = ({info.ClassName})model;");
11121112
if (info.OnExecuteReturnsInt)
11131113
{
11141114
sb.AppendLine($"{indent} return await typedModel.OnExecuteAsync({argsString});");
@@ -1121,15 +1121,18 @@ private static void GenerateExecuteHandler(StringBuilder sb, CommandInfo info, s
11211121
}
11221122
else
11231123
{
1124-
// OnExecute method
1124+
// OnExecute method - use Task.FromResult to avoid async warnings
1125+
sb.AppendLine($"{indent} public Task<int> InvokeAsync(object model, CommandLineApplication app, CancellationToken cancellationToken)");
1126+
sb.AppendLine($"{indent} {{");
1127+
sb.AppendLine($"{indent} var typedModel = ({info.ClassName})model;");
11251128
if (info.OnExecuteReturnsInt)
11261129
{
1127-
sb.AppendLine($"{indent} return typedModel.OnExecute({argsString});");
1130+
sb.AppendLine($"{indent} return Task.FromResult(typedModel.OnExecute({argsString}));");
11281131
}
11291132
else
11301133
{
11311134
sb.AppendLine($"{indent} typedModel.OnExecute({argsString});");
1132-
sb.AppendLine($"{indent} return 0;");
1135+
sb.AppendLine($"{indent} return Task.FromResult(0);");
11331136
}
11341137
}
11351138

@@ -1139,7 +1142,7 @@ private static void GenerateExecuteHandler(StringBuilder sb, CommandInfo info, s
11391142
sb.AppendLine();
11401143
}
11411144

1142-
private static void GenerateValidateHandler(StringBuilder sb, CommandInfo info, string indent)
1145+
private static void GenerateValidateHandler(StringBuilder sb, CommandData info, string indent)
11431146
{
11441147
if (!info.HasOnValidate)
11451148
{
@@ -1163,7 +1166,7 @@ private static void GenerateValidateHandler(StringBuilder sb, CommandInfo info,
11631166
sb.AppendLine();
11641167
}
11651168

1166-
private static void GenerateValidationErrorHandler(StringBuilder sb, CommandInfo info, string indent)
1169+
private static void GenerateValidationErrorHandler(StringBuilder sb, CommandData info, string indent)
11671170
{
11681171
if (!info.HasOnValidationError)
11691172
{
@@ -1187,7 +1190,7 @@ private static void GenerateValidationErrorHandler(StringBuilder sb, CommandInfo
11871190
sb.AppendLine();
11881191
}
11891192

1190-
private static void GenerateSpecialPropertiesProperty(StringBuilder sb, CommandInfo info, string indent)
1193+
private static void GenerateSpecialPropertiesProperty(StringBuilder sb, CommandData info, string indent)
11911194
{
11921195
var sp = info.SpecialProperties;
11931196
if (!sp.HasAny)
@@ -1230,7 +1233,7 @@ private static void GenerateSpecialPropertiesProperty(StringBuilder sb, CommandI
12301233
sb.AppendLine();
12311234
}
12321235

1233-
private static void GenerateHelpOptionProperty(StringBuilder sb, CommandInfo info, string indent)
1236+
private static void GenerateHelpOptionProperty(StringBuilder sb, CommandData info, string indent)
12341237
{
12351238
if (info.HelpOption == null)
12361239
{
@@ -1251,7 +1254,7 @@ private static void GenerateHelpOptionProperty(StringBuilder sb, CommandInfo inf
12511254
sb.AppendLine();
12521255
}
12531256

1254-
private static void GenerateVersionOptionProperty(StringBuilder sb, CommandInfo info, string indent)
1257+
private static void GenerateVersionOptionProperty(StringBuilder sb, CommandData info, string indent)
12551258
{
12561259
if (info.VersionOption == null)
12571260
{
@@ -1274,7 +1277,7 @@ private static void GenerateVersionOptionProperty(StringBuilder sb, CommandInfo
12741277
sb.AppendLine();
12751278
}
12761279

1277-
private static void GenerateModelFactory(StringBuilder sb, CommandInfo info, string indent)
1280+
private static void GenerateModelFactory(StringBuilder sb, CommandData info, string indent)
12781281
{
12791282
sb.AppendLine($"{indent} private sealed class GeneratedModelFactory : IModelFactory<{info.ClassName}>");
12801283
sb.AppendLine($"{indent} {{");
@@ -1331,8 +1334,12 @@ private static void GenerateModelFactory(StringBuilder sb, CommandInfo info, str
13311334
}
13321335
else if (info.Constructors.Count > 0)
13331336
{
1334-
// No default constructor - we need to throw if we get here
1335-
sb.AppendLine($"{indent} throw new InvalidOperationException(\"Unable to create instance of {info.ClassName}. No services registered for constructor parameters.\");");
1337+
// No default constructor - provide specific error messages
1338+
sb.AppendLine($"{indent} if (_services == null)");
1339+
sb.AppendLine($"{indent} {{");
1340+
sb.AppendLine($"{indent} throw new InvalidOperationException(\"Unable to create instance of {info.ClassName}. The type requires constructor parameters but no IServiceProvider was configured. Use CommandLineApplication.Execute<T>(args, services) to provide services.\");");
1341+
sb.AppendLine($"{indent} }}");
1342+
sb.AppendLine($"{indent} throw new InvalidOperationException(\"Unable to create instance of {info.ClassName}. Required service(s) not registered in the IServiceProvider.\");");
13361343
}
13371344
else
13381345
{
@@ -1346,7 +1353,7 @@ private static void GenerateModelFactory(StringBuilder sb, CommandInfo info, str
13461353
sb.AppendLine($"{indent} }}");
13471354
}
13481355

1349-
private static string GenerateModuleInitializer(ImmutableArray<CommandInfo> commands)
1356+
private static string GenerateModuleInitializer(ImmutableArray<CommandData> commands)
13501357
{
13511358
var sb = new StringBuilder();
13521359

‎src/CommandLineUtils.Generators/ConstructorData.cs‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,6 @@
55

66
namespace McMaster.Extensions.CommandLineUtils.Generators
77
{
8-
/// <summary>
9-
/// Data for a constructor parameter.
10-
/// </summary>
11-
internal sealed class ConstructorParameterData
12-
{
13-
/// <summary>
14-
/// The fully qualified type name of the parameter.
15-
/// </summary>
16-
public string TypeName { get; set; } = "";
17-
18-
/// <summary>
19-
/// The name of the parameter.
20-
/// </summary>
21-
public string Name { get; set; } = "";
22-
}
23-
248
/// <summary>
259
/// Data for a constructor.
2610
/// </summary>

‎src/CommandLineUtils.Generators/ConstructorParameterData.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) Nate McMaster.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
namespace McMaster.Extensions.CommandLineUtils.Generators
5+
{
6+
/// <summary>
7+
/// Data for a constructor parameter.
8+
/// </summary>
9+
internal sealed class ConstructorParameterData
10+
{
11+
/// <summary>
12+
/// The fully qualified type name of the parameter.
13+
/// </summary>
14+
public string TypeName { get; set; } = "";
15+
16+
/// <summary>
17+
/// The name of the parameter.
18+
/// </summary>
19+
public string Name { get; set; } = "";
20+
}
21+
}

0 commit comments

Comments
 (0)