diff --git a/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs b/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs index 30771d8..bfcc3e3 100644 --- a/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs +++ b/CliFx.Analyzers.Tests/CommandSchemaAnalyzerTests.cs @@ -662,6 +662,44 @@ public class MyCommand : ICommand [CommandOption('o', Validators = new[] {typeof(MyValidator)})] public string Option { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Option with a name that doesn't start with a letter character", + DiagnosticDescriptors.CliFx0048, + + // language=cs + @" +[Command] +public class MyCommand : ICommand +{ + [CommandOption(""0foo"")] + public string Option { get; set; } + + public ValueTask ExecuteAsync(IConsole console) => default; +}" + ) + }; + + yield return new object[] + { + new AnalyzerTestCase( + "Option with a short name that isn't a letter character", + DiagnosticDescriptors.CliFx0049, + + // language=cs + @" +[Command] +public class MyCommand : ICommand +{ + [CommandOption('0')] + public string Option { get; set; } + public ValueTask ExecuteAsync(IConsole console) => default; }" ) diff --git a/CliFx.Analyzers/CommandSchemaAnalyzer.cs b/CliFx.Analyzers/CommandSchemaAnalyzer.cs index 5c0a307..36aec8c 100644 --- a/CliFx.Analyzers/CommandSchemaAnalyzer.cs +++ b/CliFx.Analyzers/CommandSchemaAnalyzer.cs @@ -7,6 +7,7 @@ using Microsoft.CodeAnalysis.Diagnostics; namespace CliFx.Analyzers { + // TODO: split into multiple analyzers [DiagnosticAnalyzer(LanguageNames.CSharp)] public class CommandSchemaAnalyzer : DiagnosticAnalyzer { @@ -25,7 +26,9 @@ namespace CliFx.Analyzers DiagnosticDescriptors.CliFx0044, DiagnosticDescriptors.CliFx0045, DiagnosticDescriptors.CliFx0046, - DiagnosticDescriptors.CliFx0047 + DiagnosticDescriptors.CliFx0047, + DiagnosticDescriptors.CliFx0048, + DiagnosticDescriptors.CliFx0049 ); private static bool IsScalarType(ITypeSymbol typeSymbol) => @@ -307,7 +310,7 @@ namespace CliFx.Analyzers // Invalid validators var invalidValidatorsOptions = options - .Where(p => !p.Validators.All(v => v.AllInterfaces.Any(KnownSymbols.IsArgumentValueValidatorInterface))) + .Where(o => !o.Validators.All(v => v.AllInterfaces.Any(KnownSymbols.IsArgumentValueValidatorInterface))) .ToArray(); foreach (var option in invalidValidatorsOptions) @@ -316,6 +319,30 @@ namespace CliFx.Analyzers DiagnosticDescriptors.CliFx0047, option.Property.Locations.First() )); } + + // Non-letter first character in name + var nonLetterFirstCharacterInNameOptions = options + .Where(o => !string.IsNullOrWhiteSpace(o.Name) && !char.IsLetter(o.Name[0])) + .ToArray(); + + foreach (var option in nonLetterFirstCharacterInNameOptions) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0048, option.Property.Locations.First() + )); + } + + // Non-letter short name + var nonLetterShortNameOptions = options + .Where(o => o.ShortName != null && !char.IsLetter(o.ShortName.Value)) + .ToArray(); + + foreach (var option in nonLetterShortNameOptions) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.CliFx0049, option.Property.Locations.First() + )); + } } private static void CheckCommandType(SymbolAnalysisContext context) diff --git a/CliFx.Analyzers/DiagnosticDescriptors.cs b/CliFx.Analyzers/DiagnosticDescriptors.cs index 7da2dff..bec96a5 100644 --- a/CliFx.Analyzers/DiagnosticDescriptors.cs +++ b/CliFx.Analyzers/DiagnosticDescriptors.cs @@ -109,6 +109,20 @@ namespace CliFx.Analyzers "Usage", DiagnosticSeverity.Error, true ); + public static readonly DiagnosticDescriptor CliFx0048 = + new DiagnosticDescriptor(nameof(CliFx0048), + "Option name must begin with a letter character.", + "Option name must begin with a letter character.", + "Usage", DiagnosticSeverity.Error, true + ); + + public static readonly DiagnosticDescriptor CliFx0049 = + new DiagnosticDescriptor(nameof(CliFx0049), + "Option short name must be a letter character.", + "Option short name must be a letter character.", + "Usage", DiagnosticSeverity.Error, true + ); + public static readonly DiagnosticDescriptor CliFx0100 = new DiagnosticDescriptor(nameof(CliFx0100), "Use the provided IConsole abstraction instead of System.Console to ensure that the command can be tested in isolation", diff --git a/CliFx.Tests/ApplicationSpecs.cs b/CliFx.Tests/ApplicationSpecs.cs index 135a2cc..619dce1 100644 --- a/CliFx.Tests/ApplicationSpecs.cs +++ b/CliFx.Tests/ApplicationSpecs.cs @@ -451,5 +451,45 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + + [Fact] + public async Task Command_options_must_have_names_that_start_with_a_letter_character() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } + + [Fact] + public async Task Command_options_must_have_short_names_that_are_letter_characters() + { + var (console, _, stdErr) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(Array.Empty()); + + // Assert + exitCode.Should().NotBe(0); + stdErr.GetString().Should().NotBeNullOrWhiteSpace(); + + _output.WriteLine(stdErr.GetString()); + } } } \ No newline at end of file diff --git a/CliFx.Tests/ArgumentBindingSpecs.cs b/CliFx.Tests/ArgumentBindingSpecs.cs index 875195a..bd1012c 100644 --- a/CliFx.Tests/ArgumentBindingSpecs.cs +++ b/CliFx.Tests/ArgumentBindingSpecs.cs @@ -191,6 +191,34 @@ namespace CliFx.Tests _output.WriteLine(stdErr.GetString()); } + [Fact] + public async Task Argument_that_begins_with_a_dash_is_not_parsed_as_option_name_if_it_does_not_start_with_a_letter_character() + { + // Arrange + var (console, stdOut, _) = VirtualConsole.CreateBuffered(); + + var application = new CliApplicationBuilder() + .AddCommand() + .UseConsole(console) + .Build(); + + // Act + var exitCode = await application.RunAsync(new[] + { + "cmd", "--int", "-13" + }); + + var commandInstance = stdOut.GetString().DeserializeJson(); + + // Assert + exitCode.Should().Be(0); + + commandInstance.Should().BeEquivalentTo(new SupportedArgumentTypesCommand + { + Int = -13 + }); + } + [Fact] public async Task All_provided_option_arguments_must_be_bound_to_corresponding_properties() { diff --git a/CliFx.Tests/Commands/Invalid/NonLetterCharacterNameCommand.cs b/CliFx.Tests/Commands/Invalid/NonLetterCharacterNameCommand.cs new file mode 100644 index 0000000..d254fbc --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/NonLetterCharacterNameCommand.cs @@ -0,0 +1,11 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command("cmd")] + public class NonLetterCharacterNameCommand : SelfSerializeCommandBase + { + [CommandOption("0foo")] + public string? Apples { get; set; } + } +} \ No newline at end of file diff --git a/CliFx.Tests/Commands/Invalid/NonLetterCharacterShortNameCommand.cs b/CliFx.Tests/Commands/Invalid/NonLetterCharacterShortNameCommand.cs new file mode 100644 index 0000000..0367ad6 --- /dev/null +++ b/CliFx.Tests/Commands/Invalid/NonLetterCharacterShortNameCommand.cs @@ -0,0 +1,11 @@ +using CliFx.Attributes; + +namespace CliFx.Tests.Commands.Invalid +{ + [Command("cmd")] + public class NonLetterCharacterShortNameCommand : SelfSerializeCommandBase + { + [CommandOption('0')] + public string? Apples { get; set; } + } +} \ No newline at end of file diff --git a/CliFx/Domain/CommandInput.cs b/CliFx/Domain/CommandInput.cs index 21c3fd7..5f708c6 100644 --- a/CliFx/Domain/CommandInput.cs +++ b/CliFx/Domain/CommandInput.cs @@ -161,7 +161,9 @@ namespace CliFx.Domain var argument = commandLineArguments[index]; // Name - if (argument.StartsWith("--", StringComparison.Ordinal)) + if (argument.StartsWith("--", StringComparison.Ordinal) && + argument.Length > 2 && + char.IsLetter(argument[2])) { // Flush previous if (!string.IsNullOrWhiteSpace(currentOptionAlias)) @@ -171,7 +173,9 @@ namespace CliFx.Domain currentOptionValues = new List(); } // Short name - else if (argument.StartsWith('-')) + else if (argument.StartsWith('-') && + argument.Length > 1 && + char.IsLetter(argument[1])) { foreach (var alias in argument.Substring(1)) { diff --git a/CliFx/Domain/RootSchema.cs b/CliFx/Domain/RootSchema.cs index 84362ad..53b7ebf 100644 --- a/CliFx/Domain/RootSchema.cs +++ b/CliFx/Domain/RootSchema.cs @@ -232,6 +232,30 @@ namespace CliFx.Domain invalidValidatorOptions ); } + + var nonLetterFirstCharacterInNameOptions = command.Options + .Where(o => !string.IsNullOrWhiteSpace(o.Name) && !char.IsLetter(o.Name[0])) + .ToArray(); + + if (nonLetterFirstCharacterInNameOptions.Any()) + { + throw CliFxException.OptionsWithNonLetterCharacterName( + command, + nonLetterFirstCharacterInNameOptions + ); + } + + var nonLetterShortNameOptions = command.Options + .Where(o => o.ShortName != null && !char.IsLetter(o.ShortName.Value)) + .ToArray(); + + if (nonLetterShortNameOptions.Any()) + { + throw CliFxException.OptionsWithNonLetterCharacterShortName( + command, + nonLetterShortNameOptions + ); + } } private static void ValidateCommands(IReadOnlyList commands) diff --git a/CliFx/Exceptions/CliFxException.cs b/CliFx/Exceptions/CliFxException.cs index fb8c69f..3a3fed5 100644 --- a/CliFx/Exceptions/CliFxException.cs +++ b/CliFx/Exceptions/CliFxException.cs @@ -296,6 +296,32 @@ Specified validators must inherit from {typeof(ArgumentValueValidator<>).FullNam return new CliFxException(message.Trim()); } + + internal static CliFxException OptionsWithNonLetterCharacterName( + CommandSchema command, + IReadOnlyList invalidOptions) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains one or more options whose names don't start with a letter character: +{invalidOptions.JoinToString(Environment.NewLine)} + +Option names must start with a letter character (i.e. not a digit and not a special character)."; + + return new CliFxException(message.Trim()); + } + + internal static CliFxException OptionsWithNonLetterCharacterShortName( + CommandSchema command, + IReadOnlyList invalidOptions) + { + var message = $@" +Command '{command.Type.FullName}' is invalid because it contains one or more options whose short names are not letter characters: +{invalidOptions.JoinToString(Environment.NewLine)} + +Option short names must be letter characters (i.e. not digits and not special characters)."; + + return new CliFxException(message.Trim()); + } } // End-user-facing exceptions