Skip to content

Commit

Permalink
Rebasing on powderhouse
Browse files Browse the repository at this point in the history
  • Loading branch information
BenjaminMichaelis committed Oct 8, 2024
1 parent 05bd9eb commit 288554a
Show file tree
Hide file tree
Showing 17 changed files with 58 additions and 127 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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<ParseError> { error };
var error = new CliDiagnostic(new("", "", "a sweet error message", CliDiagnosticSeverity.Warning, null), []);
var errors = new List<CliDiagnostic> { error };
var errorSubsystem = new ErrorReportingSubsystem();
var consoleHack = new ConsoleHack().RedirectToBuffer(true);

Expand All @@ -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<ParseError> { 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<CliDiagnostic> { error, anotherError };
var errorSubsystem = new ErrorReportingSubsystem();
var consoleHack = new ConsoleHack().RedirectToBuffer(true);

Expand All @@ -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<ParseError> { };
var errors = new List<CliDiagnostic> { };
var errorSubsystem = new ErrorReportingSubsystem();
var consoleHack = new ConsoleHack().RedirectToBuffer(true);

Expand All @@ -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<bool>("-v")};
var rootCommand = new CliRootCommand { new CliOption<bool>("-v") };
var configuration = new CliConfiguration(rootCommand);
var errorSubsystem = new ErrorReportingSubsystem();
IReadOnlyList<string> args = [""];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public override void Execute(PipelineResult pipelineResult)
pipelineResult.SetSuccess();
}

public void Report(ConsoleHack consoleHack, IReadOnlyList<ParseError> errors)
public void Report(ConsoleHack consoleHack, IReadOnlyList<CliDiagnostic> errors)
{
ConsoleHelpers.ResetTerminalForegroundColor();
ConsoleHelpers.SetTerminalForegroundRed();
Expand Down
12 changes: 6 additions & 6 deletions src/System.CommandLine.Subsystems/PipelineResult.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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<ParseError> errors = [];
private ValueProvider valueProvider { get; }
private readonly List<CliDiagnostic> errors = [];
private ValueProvider valueProvider { get; }

public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeline, ConsoleHack? consoleHack = null)
{
Expand Down Expand Up @@ -44,18 +44,18 @@ public PipelineResult(ParseResult parseResult, string rawInput, Pipeline? pipeli
=> ParseResult.GetValueResult(valueSymbol);


public void AddErrors(IEnumerable<ParseError> errors)
public void AddErrors(IEnumerable<CliDiagnostic> errors)
{
if (errors is not null)
{
this.errors.AddRange(errors);
}
}

public void AddError(ParseError error)
public void AddError(CliDiagnostic error)
=> errors.Add(error);

public IEnumerable<ParseError> GetErrors(bool excludeParseErrors = false)
public IEnumerable<CliDiagnostic> GetErrors(bool excludeParseErrors = false)
=> excludeParseErrors || ParseResult is null
? errors
: ParseResult.Errors.Concat(errors);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {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.", severity: CliDiagnosticSeverity.Error, null), []));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.Name}", severity: CliDiagnosticSeverity.Error, null), [], cliSymbolResult: valueResult));
}
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.CommandLine.Parsing;
using System.CommandLine.ValueSources;
using static System.Runtime.InteropServices.JavaScript.JSType;

namespace System.CommandLine.Validation;

Expand All @@ -24,7 +23,7 @@ internal ValidationContext(PipelineResult pipelineResult, ValidationSubsystem va
/// Adds an error to the PipelineContext.
/// </summary>
/// <param name="error">The <see cref="ParseError"/> to add</param>
public void AddError(ParseError error)
public void AddError(CliDiagnostic error)
=> pipelineResult.AddError(error);

/// <summary>
Expand Down
6 changes: 3 additions & 3 deletions src/System.CommandLine.Subsystems/Validation/Validator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ internal Validator(string name, Type valueConditionType, params Type[] moreValue
/// <remarks>
/// This method needs to be evolved as we replace ParseError with CliError
/// </remarks>
protected static List<ParseError> AddValidationError(ref List<ParseError>? parseErrors, string message, IEnumerable<object?> errorValues)
protected static List<CliDiagnostic> AddValidationError(ref List<CliDiagnostic>? parseErrors, string message, IEnumerable<object?> 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<ParseError>();
parseErrors.Add(new ParseError(message));
parseErrors ??= new List<CliDiagnostic>();
parseErrors.Add(new CliDiagnostic(new("", "", message, severity: CliDiagnosticSeverity.Error, null), []));
return parseErrors;
}

Expand Down
9 changes: 5 additions & 4 deletions src/System.CommandLine.Subsystems/ValidationSubsystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down Expand Up @@ -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.Name} must have {condition.Name} validator.", severity: CliDiagnosticSeverity.Error, null), [], cliSymbolResult: valueResult));
}
return;
return;
}
validator.Validate(value, valueSymbol, valueResult, condition, validationContext);

Expand Down Expand Up @@ -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.Command.Name} must have {condition.Name} validator.", severity: CliDiagnosticSeverity.Error, null), []));
}
return;
}
Expand Down
8 changes: 4 additions & 4 deletions src/System.CommandLine.Subsystems/ValueConditions/Range.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ internal Range(ValueSource<T>? lowerBound, ValueSource<T>? upperBound, RangeBoun
LowerBound = lowerBound;
UpperBound = upperBound;
RangeBound = rangeBound;
}
}

/// <inheritdoc/>
public void Validate(object? value,
CliValueSymbol valueSymbol,
Expand All @@ -59,15 +59,15 @@ 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.Name}' is below the lower bound of {LowerBound}", severity: CliDiagnosticSeverity.Error, null), [], cliSymbolResult: valueResult));
}


if (UpperBound is not null
&& 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.Name}' is above the upper bound of {UpperBound}", severity: CliDiagnosticSeverity.Error, null), [], cliSymbolResult: valueResult));
}
}

Expand Down
8 changes: 2 additions & 6 deletions src/System.CommandLine/ParseResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -39,7 +36,7 @@ internal ParseResult(
*/
// TODO: unmatched tokens
// List<CliToken>? unmatchedTokens,
List<ParseError>? errors,
List<CliDiagnostic>? errors,
// TODO: commandLineText should be string array
string? commandLineText = null //,
// TODO: invocation
Expand Down Expand Up @@ -82,12 +79,11 @@ internal ParseResult(
// TODO: unmatched tokens
// _unmatchedTokens = unmatchedTokens is null ? Array.Empty<CliToken>() : unmatchedTokens;

Errors = errors is not null ? errors : Array.Empty<ParseError>();
Errors = errors is not null ? errors : Array.Empty<CliDiagnostic>();
}

public CliSymbol? GetSymbolByName(string name, bool valuesOnly = false)
{

symbolLookupByName ??= new SymbolLookupByName(this);
return symbolLookupByName.TryGetSymbol(name, out var symbol, valuesOnly: valuesOnly)
? symbol
Expand Down
6 changes: 3 additions & 3 deletions src/System.CommandLine/Parsing/CliArgumentResultInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ public void OnlyTake(int numberOfTokens)
public override string ToString() => $"{nameof(CliArgumentResultInternal)} {Argument.Name}: {string.Join(" ", Tokens.Select(t => $"<{t.Value}>"))}";

/// <inheritdoc/>
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);
}

Expand Down Expand Up @@ -218,7 +218,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;
Expand Down
2 changes: 1 addition & 1 deletion src/System.CommandLine/Parsing/CliCommandResultInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 8 additions & 9 deletions src/System.CommandLine/Parsing/CliDiagnostic.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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
{
Expand All @@ -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;
Expand All @@ -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; }
Expand Down Expand Up @@ -65,19 +68,18 @@ public sealed class CliDiagnostic
/// <param name="descriptor">Contains information about the error.</param>
/// <param name="messageArgs">The arguments to be passed to the <see cref="CliDiagnosticDescriptor.MessageFormat"/> in the <paramref name="descriptor"/>.</param>
/// <param name="properties">Properties to be associated with the diagnostic.</param>
/// <param name="symbolResult">The symbol result detailing the symbol that failed to parse and the tokens involved.</param>
/// <param name="cliSymbolResult">Contains information about a single value entered.</param>
/// <param name="location">The location of the error.</param>
public CliDiagnostic(
CliDiagnosticDescriptor descriptor,
object?[]? messageArgs,
ImmutableDictionary<string, object>? properties = null,
SymbolResult? symbolResult = null,
CliSymbolResult? cliSymbolResult = null,
Location? location = null)
{
Descriptor = descriptor;
MessageArgs = messageArgs;
Properties = properties;
SymbolResult = symbolResult;
}

/// <summary>
Expand All @@ -101,10 +103,7 @@ public string Message

public object?[]? MessageArgs { get; }

/// <summary>
/// Gets the symbol result detailing the symbol that failed to parse and the tokens involved.
/// </summary>
public SymbolResult? SymbolResult { get; }
public CliSymbolResult? CliSymbolResult { get; }

/// <inheritdoc />
public override string ToString() => Message;
Expand Down
3 changes: 2 additions & 1 deletion src/System.CommandLine/Parsing/CliSymbolResultInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ public IEnumerable<CliDiagnostic> Errors
/// Adds an error message for this symbol result to it's parse tree.
/// </summary>
/// <remarks>Setting an error will cause the parser to indicate an error for the user and prevent invocation of the command line.</remarks>
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));
/// <summary>
/// Finds a result for the specific argument anywhere in the parse tree, including parent and child symbol results.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion src/System.CommandLine/Parsing/ParseOperation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ internal ParseResult Parse()
{
if (_symbolResultTree.ErrorCount > 0)
{
_primaryAction = new ParseErrorAction();
_primaryAction = new CliDiagnosticAction();
}
}
*/
Expand Down
Loading

0 comments on commit 288554a

Please sign in to comment.