From 05bd9ebbe212be0b2595d07eb528711eafbe388d Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Tue, 6 Aug 2024 11:45:10 -0700 Subject: [PATCH 1/3] WIP on CliError Get rid of compile error and clean up some docs File scoped namespace Reverting whitespace changes Adding link Removing out-dated TODO Additional review feedback Adding additional reference link --- Directory.Packages.props | 9 +- src/System.CommandLine/ParseResult.cs | 4 +- .../Parsing/CliArgumentResultInternal.cs | 8 +- .../Parsing/CliCommandResultInternal.cs | 4 +- .../Parsing/CliDiagnostic.cs | 111 ++++++++++++++++++ .../Parsing/CliSymbolResultInternal.cs | 6 +- src/System.CommandLine/Parsing/Location.cs | 1 + src/System.CommandLine/Parsing/ParseError.cs | 54 --------- .../Parsing/SymbolResultTree.cs | 78 ++++++++++-- .../System.CommandLine.csproj | 5 +- 10 files changed, 198 insertions(+), 82 deletions(-) create mode 100644 src/System.CommandLine/Parsing/CliDiagnostic.cs delete mode 100644 src/System.CommandLine/Parsing/ParseError.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index f46c178e87..1233827813 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -1,12 +1,10 @@ - true false $(NoWarn);NU1507 - @@ -24,14 +22,13 @@ - + + - - - + \ No newline at end of file diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 07d068f747..5696154816 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; @@ -121,7 +121,7 @@ internal ParseResult( /// /// Gets the parse errors found while parsing command line input. /// - public IReadOnlyList Errors { get; } + public IReadOnlyList Errors { get; } /* // TODO: don't expose tokens diff --git a/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs b/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs index a6479af553..8c59a3edb9 100644 --- a/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs @@ -140,7 +140,7 @@ public void OnlyTake(int numberOfTokens) /// internal override void AddError(string errorMessage) { - SymbolResultTree.AddError(new ParseError(errorMessage, AppliesToPublicSymbolResult)); + SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, CliDiagnosticSeverity.Warning, null), [], symbolResult: AppliesToPublicSymbolResult)); _conversionResult = ArgumentConversionResult.Failure(this, errorMessage, ArgumentConversionResultType.Failed); } @@ -170,9 +170,6 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators) } } */ - - // TODO: defaults - /* if (Parent!.UseDefaultValueFor(this)) { var defaultValue = Argument.GetDefaultValue(this); @@ -180,7 +177,6 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators) // default value factory provided by the user might report an error, which sets _conversionResult return _conversionResult ?? ArgumentConversionResult.Success(this, defaultValue); } - */ if (Argument.ConvertArguments is null) { @@ -222,7 +218,7 @@ ArgumentConversionResult ReportErrorIfNeeded(ArgumentConversionResult result) { if (result.Result >= ArgumentConversionResultType.Failed) { - SymbolResultTree.AddError(new ParseError(result.ErrorMessage!, AppliesToPublicSymbolResult)); + SymbolResultTree.AddError(new CliDiagnostic(new("ArgumentConversionResultTypeFailed", "Type Conversion Failed", result.ErrorMessage!, CliDiagnosticSeverity.Warning, null), [], symbolResult: AppliesToPublicSymbolResult)); } return result; diff --git a/src/System.CommandLine/Parsing/CliCommandResultInternal.cs b/src/System.CommandLine/Parsing/CliCommandResultInternal.cs index 764ecf824b..36413e3c23 100644 --- a/src/System.CommandLine/Parsing/CliCommandResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliCommandResultInternal.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; @@ -84,7 +84,7 @@ internal void Validate(bool completeValidation) if (Command.HasSubcommands) { SymbolResultTree.InsertFirstError( - new ParseError(LocalizationResources.RequiredCommandWasNotProvided(), this)); + new CliDiagnostic(new("validateSubCommandError", "Validation Error", LocalizationResources.RequiredCommandWasNotProvided(), CliDiagnosticSeverity.Warning, null), [], symbolResult: this)); } // TODO: validators diff --git a/src/System.CommandLine/Parsing/CliDiagnostic.cs b/src/System.CommandLine/Parsing/CliDiagnostic.cs new file mode 100644 index 0000000000..8328683f7b --- /dev/null +++ b/src/System.CommandLine/Parsing/CliDiagnostic.cs @@ -0,0 +1,111 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Collections.Immutable; + +namespace System.CommandLine.Parsing; + +/* + * Pattern based on: + * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnostic.cs + * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnosticDescriptor.cs + * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/DiagnosticDescriptor.cs + * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs + */ +internal static class ParseDiagnostics +{ + public const string DirectiveIsNotDefinedId = "CMD0001"; + public static readonly CliDiagnosticDescriptor DirectiveIsNotDefined = + new( + DirectiveIsNotDefinedId, + //TODO: use localized strings + "Directive is not defined", + "The directive '{0}' is not defined.", + CliDiagnosticSeverity.Error, + null); +} + +public sealed class CliDiagnosticDescriptor +{ + public CliDiagnosticDescriptor(string id, string title, string messageFormat, CliDiagnosticSeverity severity, string? helpUri) + { + Id = id; + Title = title; + MessageFormat = messageFormat; + Severity = severity; + HelpUri = helpUri; + } + + public string Id { get; } + public string Title { get; } + public string MessageFormat { get; } + public CliDiagnosticSeverity Severity { get; } + public string? HelpUri { get; } +} + +public enum CliDiagnosticSeverity +{ + Hidden = 0, + Info, + Warning, + Error +} + +/// +/// Describes an error that occurs while parsing command line input. +/// +public sealed class CliDiagnostic +{ + // TODO: reevaluate whether we should be exposing a SymbolResult here + // TODO: Rename to CliError + + /// + /// Initializes a new instance of the class. + /// + /// Contains information about the error. + /// The arguments to be passed to the in the . + /// Properties to be associated with the diagnostic. + /// The symbol result detailing the symbol that failed to parse and the tokens involved. + /// The location of the error. + public CliDiagnostic( + CliDiagnosticDescriptor descriptor, + object?[]? messageArgs, + ImmutableDictionary? properties = null, + SymbolResult? symbolResult = null, + Location? location = null) + { + Descriptor = descriptor; + MessageArgs = messageArgs; + Properties = properties; + SymbolResult = symbolResult; + } + + /// + /// Gets a message to explain the error to a user. + /// + public string Message + { + get + { + if (MessageArgs is not null) + { + return string.Format(Descriptor.MessageFormat, MessageArgs); + } + return Descriptor.MessageFormat; + } + } + + public ImmutableDictionary? Properties { get; } + + public CliDiagnosticDescriptor Descriptor { get; } + + public object?[]? MessageArgs { get; } + + /// + /// Gets the symbol result detailing the symbol that failed to parse and the tokens involved. + /// + public SymbolResult? SymbolResult { get; } + + /// + public override string ToString() => Message; +} diff --git a/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs b/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs index bf09249159..68e134a817 100644 --- a/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; @@ -24,7 +24,7 @@ private protected CliSymbolResultInternal(SymbolResultTree symbolResultTree, Cli /// /// The parse errors associated with this symbol result. /// - public IEnumerable Errors + public IEnumerable Errors { get { @@ -64,7 +64,7 @@ public IEnumerable Errors /// Adds an error message for this symbol result to it's parse tree. /// /// Setting an error will cause the parser to indicate an error for the user and prevent invocation of the command line. - internal virtual void AddError(string errorMessage) => SymbolResultTree.AddError(new ParseError(errorMessage, this)); + internal virtual void AddError(string errorMessage) => SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, severity: CliDiagnosticSeverity.Error, null), [], symbolResult: this)); /// /// Finds a result for the specific argument anywhere in the parse tree, including parent and child symbol results. /// diff --git a/src/System.CommandLine/Parsing/Location.cs b/src/System.CommandLine/Parsing/Location.cs index 30d3a5285e..dea0ccaa30 100644 --- a/src/System.CommandLine/Parsing/Location.cs +++ b/src/System.CommandLine/Parsing/Location.cs @@ -49,6 +49,7 @@ public Location(string text, string source, int index, Location? outerLocation, public bool IsImplicit => Source == Implicit; + /// public override string ToString() => $"{(OuterLocation is null ? "" : OuterLocation.ToString() + "; ")}{Text} from {Source}[{Index}, {Length}, {Offset}]"; diff --git a/src/System.CommandLine/Parsing/ParseError.cs b/src/System.CommandLine/Parsing/ParseError.cs deleted file mode 100644 index 9be6314ef3..0000000000 --- a/src/System.CommandLine/Parsing/ParseError.cs +++ /dev/null @@ -1,54 +0,0 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -namespace System.CommandLine.Parsing -{ - /// - /// Describes an error that occurs while parsing command line input. - /// - public sealed class ParseError - { - // TODO: add position - // TODO: reevaluate whether we should be exposing a CliSymbolResultInternal here - internal ParseError( - string message, - CliSymbolResultInternal? symbolResult = null) - { - if (string.IsNullOrWhiteSpace(message)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(message)); - } - - Message = message; - /* - CliSymbolResultInternal = symbolResult; - */ - } - - public ParseError( - string message) - { - if (string.IsNullOrWhiteSpace(message)) - { - throw new ArgumentException("Value cannot be null or whitespace.", nameof(message)); - } - - Message = message; - } - - /// - /// A message to explain the error to a user. - /// - public string Message { get; } - - /* Consider how results are attached to errors now that we have ValueResult and CommandValueResult. Should there be a common base? - /// - /// The symbol result detailing the symbol that failed to parse and the tokens involved. - /// - public CliSymbolResultInternal? CliSymbolResultInternal { get; } - */ - - /// - public override string ToString() => Message; - } -} diff --git a/src/System.CommandLine/Parsing/SymbolResultTree.cs b/src/System.CommandLine/Parsing/SymbolResultTree.cs index 571403bc27..d053c1c61e 100644 --- a/src/System.CommandLine/Parsing/SymbolResultTree.cs +++ b/src/System.CommandLine/Parsing/SymbolResultTree.cs @@ -1,8 +1,7 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Linq; namespace System.CommandLine.Parsing { @@ -26,11 +25,11 @@ internal SymbolResultTree( if (tokenizeErrors is not null) { - Errors = new List(tokenizeErrors.Count); + Errors = new List(tokenizeErrors.Count); for (var i = 0; i < tokenizeErrors.Count; i++) { - Errors.Add(new ParseError(tokenizeErrors[i])); + Errors.Add(new CliDiagnostic(new("", "", tokenizeErrors[i], CliDiagnosticSeverity.Warning, null), [])); } } } @@ -82,8 +81,8 @@ internal IReadOnlyDictionary BuildValueResultDictiona return dict; } - internal void AddError(ParseError parseError) => (Errors ??= new()).Add(parseError); - internal void InsertFirstError(ParseError parseError) => (Errors ??= new()).Insert(0, parseError); + internal void AddError(CliDiagnostic parseError) => (Errors ??= new()).Add(parseError); + internal void InsertFirstError(CliDiagnostic parseError) => (Errors ??= new()).Insert(0, parseError); internal void AddUnmatchedToken(CliToken token, CliCommandResultInternal commandResult, CliCommandResultInternal rootCommandResult) { @@ -99,10 +98,75 @@ internal void AddUnmatchedToken(CliToken token, CliCommandResultInternal command } */ - AddError(new ParseError(LocalizationResources.UnrecognizedCommandOrArgument(token.Value), commandResult)); + AddError(new CliDiagnostic(new("", "", LocalizationResources.UnrecognizedCommandOrArgument(token.Value), CliDiagnosticSeverity.Warning, null), [], symbolResult: commandResult)); /* + } + + public SymbolResult? GetResult(string name) + { + if (_symbolsByName is null) + { + _symbolsByName = new(); + PopulateSymbolsByName(_rootCommand); } */ } + +// TODO: symbolsbyname - this is inefficient +// results for some values may not be queried at all, dependent on other options +// so we could avoid using their value factories and adding them to the dictionary +// could we sort by name allowing us to do a binary search instead of allocating a dictionary? +// could we add codepaths that query for specific kinds of symbols so they don't have to search all symbols? + private void PopulateSymbolsByName(CliCommand command) + { + if (command.HasArguments) + { + for (var i = 0; i < command.Arguments.Count; i++) + { + AddToSymbolsByName(command.Arguments[i]); + } + } + + if (command.HasOptions) + { + for (var i = 0; i < command.Options.Count; i++) + { + AddToSymbolsByName(command.Options[i]); + } + } + + if (command.HasSubcommands) + { + for (var i = 0; i < command.Subcommands.Count; i++) + { + var childCommand = command.Subcommands[i]; + AddToSymbolsByName(childCommand); + PopulateSymbolsByName(childCommand); + } + } + + // TODO: Explore removing closure here + void AddToSymbolsByName(CliSymbol symbol) + { + if (_symbolsByName!.TryGetValue(symbol.Name, out var node)) + { + if (symbol.Name == node.Symbol.Name && + symbol.FirstParent?.Symbol is { } parent && + parent == node.Symbol.FirstParent?.Symbol) + { + throw new InvalidOperationException($"Command {parent.Name} has more than one child named \"{symbol.Name}\"."); + } + + _symbolsByName[symbol.Name] = new(symbol) + { + Next = node + }; + } + else + { + _symbolsByName[symbol.Name] = new(symbol); + } + } + } } } diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj index bee122292b..5c73ca87b2 100644 --- a/src/System.CommandLine/System.CommandLine.csproj +++ b/src/System.CommandLine/System.CommandLine.csproj @@ -1,4 +1,4 @@ - + true @@ -58,7 +58,7 @@ - + @@ -76,6 +76,7 @@ + From 5f0b87d15c40b10022400722351b1efde64ec9d4 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Tue, 8 Oct 2024 10:15:13 -0700 Subject: [PATCH 2/3] Rebasing on powderhouse --- .../ErrorReportingFunctionalTests.cs | 4 +- .../ErrorReportingSubsystemTests.cs | 16 ++-- .../ErrorReportingSubsystem.cs | 2 +- .../PipelineResult.cs | 12 +-- .../Validation/InclusiveGroupValidator.cs | 6 +- .../Validation/RangeValidator.cs | 4 +- .../Validation/ValidationContext.cs | 3 +- .../Validation/Validator.cs | 6 +- .../ValidationSubsystem.cs | 9 ++- .../ValueConditions/Range.cs | 8 +- src/System.CommandLine/ParseResult.cs | 8 +- .../Parsing/CliArgumentResultInternal.cs | 9 ++- .../Parsing/CliCommandResultInternal.cs | 2 +- .../Parsing/CliDiagnostic.cs | 17 ++-- .../Parsing/CliSymbolResultInternal.cs | 3 +- .../Parsing/ParseOperation.cs | 2 +- .../Parsing/SymbolResultTree.cs | 77 ++----------------- 17 files changed, 61 insertions(+), 127 deletions(-) diff --git a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs index 47e4808c18..b9deabd181 100644 --- a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs @@ -56,9 +56,9 @@ public void Help_display_can_be_disabled() var result = rootCommand.Parse("oops", config); - if (result.Action is ParseErrorAction parseError) + if (result.Action is CliDiagnosticAction CliDiagnostic) { - parseError.ShowHelp = false; + CliDiagnostic.ShowHelp = false; } result.Invoke(); diff --git a/src/System.CommandLine.Subsystems.Tests/ErrorReportingSubsystemTests.cs b/src/System.CommandLine.Subsystems.Tests/ErrorReportingSubsystemTests.cs index 1ce30a9422..b8284a71b5 100644 --- a/src/System.CommandLine.Subsystems.Tests/ErrorReportingSubsystemTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/ErrorReportingSubsystemTests.cs @@ -1,9 +1,9 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System.CommandLine.Parsing; using FluentAssertions; using Xunit; -using System.CommandLine.Parsing; namespace System.CommandLine.Subsystems.Tests; @@ -12,8 +12,8 @@ public class ErrorReportingSubsystemTests [Fact] public void Report_when_single_error_writes_to_console_hack() { - var error = new ParseError("a sweet error message"); - var errors = new List { error }; + var error = new CliDiagnostic(new("", "", "a sweet error message", CliDiagnosticSeverity.Warning, null), []); + var errors = new List { error }; var errorSubsystem = new ErrorReportingSubsystem(); var consoleHack = new ConsoleHack().RedirectToBuffer(true); @@ -25,9 +25,9 @@ public void Report_when_single_error_writes_to_console_hack() [Fact] public void Report_when_multiple_error_writes_to_console_hack() { - var error = new ParseError("a sweet error message"); - var anotherError = new ParseError("another sweet error message"); - var errors = new List { error, anotherError }; + var error = new CliDiagnostic(new("", "", "a sweet error message", CliDiagnosticSeverity.Warning, null), []); + var anotherError = new CliDiagnostic(new("", "", "another sweet error message", CliDiagnosticSeverity.Warning, null), []); + var errors = new List { error, anotherError }; var errorSubsystem = new ErrorReportingSubsystem(); var consoleHack = new ConsoleHack().RedirectToBuffer(true); @@ -39,7 +39,7 @@ public void Report_when_multiple_error_writes_to_console_hack() [Fact] public void Report_when_no_errors_writes_nothing_to_console_hack() { - var errors = new List { }; + var errors = new List { }; var errorSubsystem = new ErrorReportingSubsystem(); var consoleHack = new ConsoleHack().RedirectToBuffer(true); @@ -53,7 +53,7 @@ public void Report_when_no_errors_writes_nothing_to_console_hack() [InlineData("-non_existant_option")] public void GetIsActivated_GivenInvalidInput_SubsystemIsActive(string input) { - var rootCommand = new CliRootCommand {new CliOption("-v")}; + var rootCommand = new CliRootCommand { new CliOption("-v") }; var configuration = new CliConfiguration(rootCommand); var errorSubsystem = new ErrorReportingSubsystem(); IReadOnlyList args = [""]; diff --git a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs index 27f43ab9f2..039e700217 100644 --- a/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ErrorReportingSubsystem.cs @@ -33,7 +33,7 @@ public override void Execute(PipelineResult pipelineResult) pipelineResult.SetSuccess(); } - public void Report(ConsoleHack consoleHack, IReadOnlyList errors) + public void Report(ConsoleHack consoleHack, IReadOnlyList errors) { ConsoleHelpers.ResetTerminalForegroundColor(); ConsoleHelpers.SetTerminalForegroundRed(); diff --git a/src/System.CommandLine.Subsystems/PipelineResult.cs b/src/System.CommandLine.Subsystems/PipelineResult.cs index cb1763c3d1..8af9b55eda 100644 --- a/src/System.CommandLine.Subsystems/PipelineResult.cs +++ b/src/System.CommandLine.Subsystems/PipelineResult.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.CommandLine.Parsing; @@ -9,8 +9,8 @@ namespace System.CommandLine; public class PipelineResult { // TODO: Try to build workflow so it is illegal to create this without a ParseResult - private readonly List errors = []; - private ValueProvider valueProvider { get; } + private readonly List errors = []; + private ValueProvider valueProvider { get; } public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null) { @@ -44,7 +44,7 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli => ParseResult.GetValueResult(valueSymbol); - public void AddErrors(IEnumerable errors) + public void AddErrors(IEnumerable errors) { if (errors is not null) { @@ -52,10 +52,10 @@ public void AddErrors(IEnumerable errors) } } - public void AddError(ParseError error) + public void AddError(CliDiagnostic error) => errors.Add(error); - public IEnumerable GetErrors(bool excludeParseErrors = false) + public IEnumerable GetErrors(bool excludeParseErrors = false) => excludeParseErrors || ParseResult is null ? errors : ParseResult.Errors.Concat(errors); diff --git a/src/System.CommandLine.Subsystems/Validation/InclusiveGroupValidator.cs b/src/System.CommandLine.Subsystems/Validation/InclusiveGroupValidator.cs index 6c8a1c28e5..961b4538ab 100644 --- a/src/System.CommandLine.Subsystems/Validation/InclusiveGroupValidator.cs +++ b/src/System.CommandLine.Subsystems/Validation/InclusiveGroupValidator.cs @@ -44,9 +44,9 @@ public override void Validate(CliCommandResult commandResult, // TODO: Rework to allow localization var pluralToBe = "are"; var singularToBe = "is"; - validationContext.AddError(new ParseError( $"The members {string.Join(", ", groupMembers.Select(m => m.Name))} " + - $"must all be used if one is used. {string.Join(", ", missingMembers.Select(m => m.Name))} " + - $"{(missingMembers.Skip(1).Any() ? pluralToBe : singularToBe)} missing.")); + validationContext.AddError(new CliDiagnostic(new("", "", "The members {groupMembers} " + + "must all be used if one is used. {missingMembers} " + + "{toBe} missing.", severity: CliDiagnosticSeverity.Error, null), [string.Join(", ", groupMembers.Select(m => m.Name)), string.Join(", ", missingMembers.Select(m => m.Name)), (missingMembers.Skip(1).Any() ? pluralToBe : singularToBe)])); } } } diff --git a/src/System.CommandLine.Subsystems/Validation/RangeValidator.cs b/src/System.CommandLine.Subsystems/Validation/RangeValidator.cs index cfab72a183..5f0e682a45 100644 --- a/src/System.CommandLine.Subsystems/Validation/RangeValidator.cs +++ b/src/System.CommandLine.Subsystems/Validation/RangeValidator.cs @@ -24,9 +24,7 @@ public override void Validate(object? value, CliValueSymbol valueSymbol, } if (valueCondition.MustHaveValidator) { - validationContext.AddError(new ParseError($"Range validator missing for {valueSymbol.Name}")); + validationContext.AddError(new CliDiagnostic(new("", "", "Range validator missing for {valueSymbol}", severity: CliDiagnosticSeverity.Error, null), [valueSymbol.Name], cliSymbolResult: valueResult)); } } - - } diff --git a/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs b/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs index 3f22a12280..93c8bfe22d 100644 --- a/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs +++ b/src/System.CommandLine.Subsystems/Validation/ValidationContext.cs @@ -3,7 +3,6 @@ using System.CommandLine.Parsing; using System.CommandLine.ValueSources; -using static System.Runtime.InteropServices.JavaScript.JSType; namespace System.CommandLine.Validation; @@ -24,7 +23,7 @@ internal ValidationContext(PipelineResult pipelineResult, ValidationSubsystem va /// Adds an error to the PipelineContext. /// /// The to add - public void AddError(ParseError error) + public void AddError(CliDiagnostic error) => pipelineResult.AddError(error); /// diff --git a/src/System.CommandLine.Subsystems/Validation/Validator.cs b/src/System.CommandLine.Subsystems/Validation/Validator.cs index e33f732c11..e8d64e4a2e 100644 --- a/src/System.CommandLine.Subsystems/Validation/Validator.cs +++ b/src/System.CommandLine.Subsystems/Validation/Validator.cs @@ -31,11 +31,11 @@ internal Validator(string name, Type valueConditionType, params Type[] moreValue /// /// This method needs to be evolved as we replace ParseError with CliError /// - protected static List AddValidationError(ref List? parseErrors, string message, IEnumerable errorValues) + protected static List AddValidationError(ref List? parseErrors, string message, IEnumerable errorValues) { // TODO: Review the handling of errors. They are currently transient and returned by the Validate method, and to avoid allocating in the case of no errors (the common case) this method is used. This adds complexity to creating a new validator. - parseErrors ??= new List(); - parseErrors.Add(new ParseError(message)); + parseErrors ??= new List(); + parseErrors.Add(new CliDiagnostic(new("", "", message, severity: CliDiagnosticSeverity.Error, null), [])); return parseErrors; } diff --git a/src/System.CommandLine.Subsystems/ValidationSubsystem.cs b/src/System.CommandLine.Subsystems/ValidationSubsystem.cs index 00e44e9971..1ae38a131a 100644 --- a/src/System.CommandLine.Subsystems/ValidationSubsystem.cs +++ b/src/System.CommandLine.Subsystems/ValidationSubsystem.cs @@ -81,7 +81,8 @@ private void ValidateValue(CliValueSymbol valueSymbol, ValidationContext validat var value = validationContext.GetValue(valueSymbol); var valueResult = validationContext.GetValueResult(valueSymbol); - do { + do + { ValidateValueCondition(value, valueSymbol, valueResult, enumerator.Current, validationContext); } while (enumerator.MoveNext()); } @@ -123,9 +124,9 @@ private void ValidateValueCondition(object? value, CliValueSymbol valueSymbol, C { if (condition.MustHaveValidator) { - validationContext.AddError(new ParseError($"{valueSymbol.Name} must have {condition.Name} validator.")); + validationContext.AddError(new CliDiagnostic(new("", "", "{valueSymbol} must have {condition} validator.", severity: CliDiagnosticSeverity.Error, null), [valueSymbol.Name, condition.Name], cliSymbolResult: valueResult)); } - return; + return; } validator.Validate(value, valueSymbol, valueResult, condition, validationContext); @@ -169,7 +170,7 @@ private void ValidateCommandCondition(CliCommandResult commandResult, CommandCon { if (condition.MustHaveValidator) { - validationContext.AddError(new ParseError($"{commandResult.Command.Name} must have {condition.Name} validator.")); + validationContext.AddError(new CliDiagnostic(new("", "", "{commandResult} must have {condition} validator.", severity: CliDiagnosticSeverity.Error, null), [commandResult.Command.Name, condition.Name])); } return; } diff --git a/src/System.CommandLine.Subsystems/ValueConditions/Range.cs b/src/System.CommandLine.Subsystems/ValueConditions/Range.cs index 2d029e5e8e..70bb2c07f9 100644 --- a/src/System.CommandLine.Subsystems/ValueConditions/Range.cs +++ b/src/System.CommandLine.Subsystems/ValueConditions/Range.cs @@ -40,8 +40,8 @@ internal Range(ValueSource? lowerBound, ValueSource? upperBound, RangeBoun LowerBound = lowerBound; UpperBound = upperBound; RangeBound = rangeBound; - } - + } + /// public void Validate(object? value, CliValueSymbol valueSymbol, @@ -59,7 +59,7 @@ public void Validate(object? value, && validationContext.TryGetTypedValue(LowerBound, out var lowerValue) && comparableValue.CompareTo(lowerValue) < 0) { - validationContext.AddError(new ParseError($"The value for '{valueSymbol.Name}' is below the lower bound of {LowerBound}")); + validationContext.AddError(new CliDiagnostic(new("", "", "The value for '{valueSymbol}' is below the lower bound of {lowerBound}", severity: CliDiagnosticSeverity.Error, null), [valueSymbol.Name, LowerBound], cliSymbolResult: valueResult)); } @@ -67,7 +67,7 @@ public void Validate(object? value, && validationContext.TryGetTypedValue(UpperBound, out var upperValue) && comparableValue.CompareTo(upperValue) > 0) { - validationContext.AddError(new ParseError($"The value for '{valueSymbol.Name}' is above the upper bound of {UpperBound}")); + validationContext.AddError(new CliDiagnostic(new("", "", "The value for '{valueSymbol}' is above the upper bound of {upperBound}", severity: CliDiagnosticSeverity.Error, null), [valueSymbol.Name, UpperBound], cliSymbolResult: valueResult)); } } diff --git a/src/System.CommandLine/ParseResult.cs b/src/System.CommandLine/ParseResult.cs index 5696154816..3e91bf1ba0 100644 --- a/src/System.CommandLine/ParseResult.cs +++ b/src/System.CommandLine/ParseResult.cs @@ -3,9 +3,6 @@ using System.Collections.Generic; using System.CommandLine.Parsing; -using System.Linq; -using System.Threading.Tasks; -using System.Threading; namespace System.CommandLine { @@ -39,7 +36,7 @@ internal ParseResult( */ // TODO: unmatched tokens // List? unmatchedTokens, - List? errors, + List? errors, // TODO: commandLineText should be string array string? commandLineText = null //, // TODO: invocation @@ -82,12 +79,11 @@ internal ParseResult( // TODO: unmatched tokens // _unmatchedTokens = unmatchedTokens is null ? Array.Empty() : unmatchedTokens; - Errors = errors is not null ? errors : Array.Empty(); + Errors = errors is not null ? errors : Array.Empty(); } public CliSymbol? GetSymbolByName(string name, bool valuesOnly = false) { - symbolLookupByName ??= new SymbolLookupByName(this); return symbolLookupByName.TryGetSymbol(name, out var symbol, valuesOnly: valuesOnly) ? symbol diff --git a/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs b/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs index 8c59a3edb9..11e0e47efe 100644 --- a/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliArgumentResultInternal.cs @@ -138,9 +138,9 @@ public void OnlyTake(int numberOfTokens) public override string ToString() => $"{nameof(CliArgumentResultInternal)} {Argument.Name}: {string.Join(" ", Tokens.Select(t => $"<{t.Value}>"))}"; /// - internal override void AddError(string errorMessage) + internal override void AddError(string errorMessage, CliValueResult valueResult) { - SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, CliDiagnosticSeverity.Warning, null), [], symbolResult: AppliesToPublicSymbolResult)); + SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, CliDiagnosticSeverity.Warning, null), [], cliSymbolResult: valueResult)); _conversionResult = ArgumentConversionResult.Failure(this, errorMessage, ArgumentConversionResultType.Failed); } @@ -170,6 +170,8 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators) } } */ + // TODO: defaults + /* if (Parent!.UseDefaultValueFor(this)) { var defaultValue = Argument.GetDefaultValue(this); @@ -177,6 +179,7 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators) // default value factory provided by the user might report an error, which sets _conversionResult return _conversionResult ?? ArgumentConversionResult.Success(this, defaultValue); } + */ if (Argument.ConvertArguments is null) { @@ -218,7 +221,7 @@ ArgumentConversionResult ReportErrorIfNeeded(ArgumentConversionResult result) { if (result.Result >= ArgumentConversionResultType.Failed) { - SymbolResultTree.AddError(new CliDiagnostic(new("ArgumentConversionResultTypeFailed", "Type Conversion Failed", result.ErrorMessage!, CliDiagnosticSeverity.Warning, null), [], symbolResult: AppliesToPublicSymbolResult)); + SymbolResultTree.AddError(new CliDiagnostic(new("ArgumentConversionResultTypeFailed", "Type Conversion Failed", result.ErrorMessage!, CliDiagnosticSeverity.Warning, null), [], cliSymbolResult: ValueResult)); } return result; diff --git a/src/System.CommandLine/Parsing/CliCommandResultInternal.cs b/src/System.CommandLine/Parsing/CliCommandResultInternal.cs index 36413e3c23..4861943178 100644 --- a/src/System.CommandLine/Parsing/CliCommandResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliCommandResultInternal.cs @@ -84,7 +84,7 @@ internal void Validate(bool completeValidation) if (Command.HasSubcommands) { SymbolResultTree.InsertFirstError( - new CliDiagnostic(new("validateSubCommandError", "Validation Error", LocalizationResources.RequiredCommandWasNotProvided(), CliDiagnosticSeverity.Warning, null), [], symbolResult: this)); + new CliDiagnostic(new("validateSubCommandError", "Validation Error", LocalizationResources.RequiredCommandWasNotProvided(), CliDiagnosticSeverity.Warning, null), [], valueResult: )); } // TODO: validators diff --git a/src/System.CommandLine/Parsing/CliDiagnostic.cs b/src/System.CommandLine/Parsing/CliDiagnostic.cs index 8328683f7b..f8605ee78a 100644 --- a/src/System.CommandLine/Parsing/CliDiagnostic.cs +++ b/src/System.CommandLine/Parsing/CliDiagnostic.cs @@ -1,7 +1,8 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; namespace System.CommandLine.Parsing; @@ -11,6 +12,7 @@ namespace System.CommandLine.Parsing; * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnosticDescriptor.cs * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/DiagnosticDescriptor.cs * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs + * https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141791086 */ internal static class ParseDiagnostics { @@ -27,7 +29,7 @@ internal static class ParseDiagnostics public sealed class CliDiagnosticDescriptor { - public CliDiagnosticDescriptor(string id, string title, string messageFormat, CliDiagnosticSeverity severity, string? helpUri) + public CliDiagnosticDescriptor(string id, string title, [StringSyntax(StringSyntaxAttribute.CompositeFormat)] string messageFormat, CliDiagnosticSeverity severity, string? helpUri) { Id = id; Title = title; @@ -38,6 +40,7 @@ public CliDiagnosticDescriptor(string id, string title, string messageFormat, Cl public string Id { get; } public string Title { get; } + [StringSyntax(StringSyntaxAttribute.CompositeFormat)] public string MessageFormat { get; } public CliDiagnosticSeverity Severity { get; } public string? HelpUri { get; } @@ -65,19 +68,18 @@ public sealed class CliDiagnostic /// Contains information about the error. /// The arguments to be passed to the in the . /// Properties to be associated with the diagnostic. - /// The symbol result detailing the symbol that failed to parse and the tokens involved. + /// Contains information about a single value entered. /// The location of the error. public CliDiagnostic( CliDiagnosticDescriptor descriptor, object?[]? messageArgs, ImmutableDictionary? properties = null, - SymbolResult? symbolResult = null, + CliSymbolResult? cliSymbolResult = null, Location? location = null) { Descriptor = descriptor; MessageArgs = messageArgs; Properties = properties; - SymbolResult = symbolResult; } /// @@ -101,10 +103,7 @@ public string Message public object?[]? MessageArgs { get; } - /// - /// Gets the symbol result detailing the symbol that failed to parse and the tokens involved. - /// - public SymbolResult? SymbolResult { get; } + public CliSymbolResult? CliSymbolResult { get; } /// public override string ToString() => Message; diff --git a/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs b/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs index 68e134a817..4a679578ce 100644 --- a/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs +++ b/src/System.CommandLine/Parsing/CliSymbolResultInternal.cs @@ -64,7 +64,8 @@ public IEnumerable Errors /// Adds an error message for this symbol result to it's parse tree. /// /// Setting an error will cause the parser to indicate an error for the user and prevent invocation of the command line. - internal virtual void AddError(string errorMessage) => SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, severity: CliDiagnosticSeverity.Error, null), [], symbolResult: this)); + internal virtual void AddError(string errorMessage, CliValueResult valueResult) => + SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, severity: CliDiagnosticSeverity.Error, null), [], cliSymbolResult: valueResult)); /// /// Finds a result for the specific argument anywhere in the parse tree, including parent and child symbol results. /// diff --git a/src/System.CommandLine/Parsing/ParseOperation.cs b/src/System.CommandLine/Parsing/ParseOperation.cs index 3c941ecf97..50185de4a6 100644 --- a/src/System.CommandLine/Parsing/ParseOperation.cs +++ b/src/System.CommandLine/Parsing/ParseOperation.cs @@ -76,7 +76,7 @@ internal ParseResult Parse() { if (_symbolResultTree.ErrorCount > 0) { - _primaryAction = new ParseErrorAction(); + _primaryAction = new CliDiagnosticAction(); } } */ diff --git a/src/System.CommandLine/Parsing/SymbolResultTree.cs b/src/System.CommandLine/Parsing/SymbolResultTree.cs index d053c1c61e..1a6b3aeb9a 100644 --- a/src/System.CommandLine/Parsing/SymbolResultTree.cs +++ b/src/System.CommandLine/Parsing/SymbolResultTree.cs @@ -8,7 +8,7 @@ namespace System.CommandLine.Parsing internal sealed class SymbolResultTree : Dictionary { private readonly CliCommand _rootCommand; - internal List? Errors; + internal List? Errors; // TODO: unmatched tokens /* internal List? UnmatchedTokens; @@ -29,7 +29,9 @@ internal SymbolResultTree( for (var i = 0; i < tokenizeErrors.Count; i++) { - Errors.Add(new CliDiagnostic(new("", "", tokenizeErrors[i], CliDiagnosticSeverity.Warning, null), [])); + Errors.Add(new CliDiagnostic(new("", "", + tokenizeErrors[i], CliDiagnosticSeverity.Warning, null), [], + cliSymbolResult: null)); } } } @@ -81,8 +83,8 @@ internal IReadOnlyDictionary BuildValueResultDictiona return dict; } - internal void AddError(CliDiagnostic parseError) => (Errors ??= new()).Add(parseError); - internal void InsertFirstError(CliDiagnostic parseError) => (Errors ??= new()).Insert(0, parseError); + internal void AddError(CliDiagnostic CliDiagnostic) => (Errors ??= new()).Add(CliDiagnostic); + internal void InsertFirstError(CliDiagnostic CliDiagnostic) => (Errors ??= new()).Insert(0, CliDiagnostic); internal void AddUnmatchedToken(CliToken token, CliCommandResultInternal commandResult, CliCommandResultInternal rootCommandResult) { @@ -98,75 +100,10 @@ internal void AddUnmatchedToken(CliToken token, CliCommandResultInternal command } */ - AddError(new CliDiagnostic(new("", "", LocalizationResources.UnrecognizedCommandOrArgument(token.Value), CliDiagnosticSeverity.Warning, null), [], symbolResult: commandResult)); + AddError(new CliDiagnostic(new("", "", LocalizationResources.UnrecognizedCommandOrArgument(token.Value), CliDiagnosticSeverity.Warning, null), [])); /* - } - - public SymbolResult? GetResult(string name) - { - if (_symbolsByName is null) - { - _symbolsByName = new(); - PopulateSymbolsByName(_rootCommand); } */ } - -// TODO: symbolsbyname - this is inefficient -// results for some values may not be queried at all, dependent on other options -// so we could avoid using their value factories and adding them to the dictionary -// could we sort by name allowing us to do a binary search instead of allocating a dictionary? -// could we add codepaths that query for specific kinds of symbols so they don't have to search all symbols? - private void PopulateSymbolsByName(CliCommand command) - { - if (command.HasArguments) - { - for (var i = 0; i < command.Arguments.Count; i++) - { - AddToSymbolsByName(command.Arguments[i]); - } - } - - if (command.HasOptions) - { - for (var i = 0; i < command.Options.Count; i++) - { - AddToSymbolsByName(command.Options[i]); - } - } - - if (command.HasSubcommands) - { - for (var i = 0; i < command.Subcommands.Count; i++) - { - var childCommand = command.Subcommands[i]; - AddToSymbolsByName(childCommand); - PopulateSymbolsByName(childCommand); - } - } - - // TODO: Explore removing closure here - void AddToSymbolsByName(CliSymbol symbol) - { - if (_symbolsByName!.TryGetValue(symbol.Name, out var node)) - { - if (symbol.Name == node.Symbol.Name && - symbol.FirstParent?.Symbol is { } parent && - parent == node.Symbol.FirstParent?.Symbol) - { - throw new InvalidOperationException($"Command {parent.Name} has more than one child named \"{symbol.Name}\"."); - } - - _symbolsByName[symbol.Name] = new(symbol) - { - Next = node - }; - } - else - { - _symbolsByName[symbol.Name] = new(symbol); - } - } - } } } From acbd60f1b770493bcdb29aa2a36f6d383644dc82 Mon Sep 17 00:00:00 2001 From: Benjamin Michaelis Date: Tue, 15 Oct 2024 14:36:16 -0700 Subject: [PATCH 3/3] cleanup --- .../ErrorReportingFunctionalTests.cs | 6 +- .../Parsing/CliDiagnostic.cs | 64 ++++--------------- .../Parsing/CliDiagnosticDescriptor.cs | 47 ++++++++++++++ .../Parsing/CliDiagnosticSeverity.cs | 31 +++++++++ .../Parsing/ParseDiagnostics.cs | 17 +++++ .../System.CommandLine.csproj | 5 +- 6 files changed, 114 insertions(+), 56 deletions(-) create mode 100644 src/System.CommandLine/Parsing/CliDiagnosticDescriptor.cs create mode 100644 src/System.CommandLine/Parsing/CliDiagnosticSeverity.cs create mode 100644 src/System.CommandLine/Parsing/ParseDiagnostics.cs diff --git a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs index b9deabd181..f3e43ea5a0 100644 --- a/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs +++ b/src/System.CommandLine.Subsystems.Tests/ErrorReportingFunctionalTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. /* @@ -56,9 +56,9 @@ public void Help_display_can_be_disabled() var result = rootCommand.Parse("oops", config); - if (result.Action is CliDiagnosticAction CliDiagnostic) + if (result.Action is CliDiagnosticAction cliDiagnostic) { - CliDiagnostic.ShowHelp = false; + cliDiagnostic.ShowHelp = false; } result.Invoke(); diff --git a/src/System.CommandLine/Parsing/CliDiagnostic.cs b/src/System.CommandLine/Parsing/CliDiagnostic.cs index f8605ee78a..d291eae12a 100644 --- a/src/System.CommandLine/Parsing/CliDiagnostic.cs +++ b/src/System.CommandLine/Parsing/CliDiagnostic.cs @@ -2,66 +2,14 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Immutable; -using System.Diagnostics.CodeAnalysis; namespace System.CommandLine.Parsing; -/* - * Pattern based on: - * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnostic.cs - * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnosticDescriptor.cs - * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/DiagnosticDescriptor.cs - * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs - * https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141791086 - */ -internal static class ParseDiagnostics -{ - public const string DirectiveIsNotDefinedId = "CMD0001"; - public static readonly CliDiagnosticDescriptor DirectiveIsNotDefined = - new( - DirectiveIsNotDefinedId, - //TODO: use localized strings - "Directive is not defined", - "The directive '{0}' is not defined.", - CliDiagnosticSeverity.Error, - null); -} - -public sealed class CliDiagnosticDescriptor -{ - public CliDiagnosticDescriptor(string id, string title, [StringSyntax(StringSyntaxAttribute.CompositeFormat)] string messageFormat, CliDiagnosticSeverity severity, string? helpUri) - { - Id = id; - Title = title; - MessageFormat = messageFormat; - Severity = severity; - HelpUri = helpUri; - } - - public string Id { get; } - public string Title { get; } - [StringSyntax(StringSyntaxAttribute.CompositeFormat)] - public string MessageFormat { get; } - public CliDiagnosticSeverity Severity { get; } - public string? HelpUri { get; } -} - -public enum CliDiagnosticSeverity -{ - Hidden = 0, - Info, - Warning, - Error -} - /// /// Describes an error that occurs while parsing command line input. /// public sealed class CliDiagnostic { - // TODO: reevaluate whether we should be exposing a SymbolResult here - // TODO: Rename to CliError - /// /// Initializes a new instance of the class. /// @@ -70,6 +18,14 @@ public sealed class CliDiagnostic /// Properties to be associated with the diagnostic. /// Contains information about a single value entered. /// The location of the error. + /* + * Pattern based on: + * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnostic.cs + * https://github.com/mhutch/MonoDevelop.MSBuildEditor/blob/main/MonoDevelop.MSBuild/Analysis/MSBuildDiagnosticDescriptor.cs + * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/DiagnosticDescriptor.cs + * https://github.com/dotnet/roslyn/blob/main/src/Compilers/Core/Portable/Diagnostic/Diagnostic.cs + * https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141791086 + */ public CliDiagnostic( CliDiagnosticDescriptor descriptor, object?[]? messageArgs, @@ -80,6 +36,8 @@ public CliDiagnostic( Descriptor = descriptor; MessageArgs = messageArgs; Properties = properties; + CliSymbolResult = cliSymbolResult; + Location = location; } /// @@ -105,6 +63,8 @@ public string Message public CliSymbolResult? CliSymbolResult { get; } + public Location? Location { get; } + /// public override string ToString() => Message; } diff --git a/src/System.CommandLine/Parsing/CliDiagnosticDescriptor.cs b/src/System.CommandLine/Parsing/CliDiagnosticDescriptor.cs new file mode 100644 index 0000000000..2e3243ac8f --- /dev/null +++ b/src/System.CommandLine/Parsing/CliDiagnosticDescriptor.cs @@ -0,0 +1,47 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Diagnostics.CodeAnalysis; + +namespace System.CommandLine.Parsing; + +/// +/// Provides a description of a . +/// +public sealed class CliDiagnosticDescriptor +{ + public CliDiagnosticDescriptor(string id, string title, [StringSyntax(StringSyntaxAttribute.CompositeFormat)] string messageFormat, CliDiagnosticSeverity severity, string? helpUri) + { + Id = id; + Title = title; + MessageFormat = messageFormat; + Severity = severity; + HelpUri = helpUri; + } + + /// + /// A unique identifier for the diagnostic. + /// + public string Id { get; } + + /// + /// A short title describing the diagnostic. + /// + public string Title { get; } + + /// + /// A composite format string, which can be passed to to create a message. + /// + [StringSyntax(StringSyntaxAttribute.CompositeFormat)] + public string MessageFormat { get; } + + /// + /// The severity of the diagnostic. + /// + public CliDiagnosticSeverity Severity { get; } + + /// + /// An optional hyperlink that provides more information about the diagnostic. + /// + public string? HelpUri { get; } +} diff --git a/src/System.CommandLine/Parsing/CliDiagnosticSeverity.cs b/src/System.CommandLine/Parsing/CliDiagnosticSeverity.cs new file mode 100644 index 0000000000..956892fb97 --- /dev/null +++ b/src/System.CommandLine/Parsing/CliDiagnosticSeverity.cs @@ -0,0 +1,31 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace System.CommandLine.Parsing; + +/// +/// Describes how severe a is."/> +/// +// Pattern based on: https://github.com/dotnet/roslyn/blob/1cca63b5d8ea170f8d8e88e1574aa3ebe354c23b/src/Compilers/Core/Portable/Diagnostic/DiagnosticSeverity.cs. +public enum CliDiagnosticSeverity +{ + /// + /// Something that is not surfaced through normal means. + /// + Hidden = 0, + + /// + /// Information that does not indicate a problem (i.e. not prescriptive). + /// + Info, + + /// + /// Something suspicious but allowed. + /// + Warning, + + /// + /// Something that is definitely wrong and needs fixing. + /// + Error +} diff --git a/src/System.CommandLine/Parsing/ParseDiagnostics.cs b/src/System.CommandLine/Parsing/ParseDiagnostics.cs new file mode 100644 index 0000000000..b814c441e6 --- /dev/null +++ b/src/System.CommandLine/Parsing/ParseDiagnostics.cs @@ -0,0 +1,17 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace System.CommandLine.Parsing; + +internal static class ParseDiagnostics +{ + public const string DirectiveIsNotDefinedId = "CMD0001"; + public static readonly CliDiagnosticDescriptor DirectiveIsNotDefined = + new( + DirectiveIsNotDefinedId, + //TODO: use localized strings + "Directive is not defined", + "The directive '{0}' is not defined.", + CliDiagnosticSeverity.Error, + null); +} diff --git a/src/System.CommandLine/System.CommandLine.csproj b/src/System.CommandLine/System.CommandLine.csproj index 5c73ca87b2..ea33049a7b 100644 --- a/src/System.CommandLine/System.CommandLine.csproj +++ b/src/System.CommandLine/System.CommandLine.csproj @@ -1,4 +1,4 @@ - + true @@ -29,7 +29,10 @@ + + +