Skip to content

Commit

Permalink
WIP on CliError
Browse files Browse the repository at this point in the history
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
  • Loading branch information
BenjaminMichaelis committed Oct 8, 2024
1 parent d25d2da commit 05bd9eb
Show file tree
Hide file tree
Showing 10 changed files with 198 additions and 82 deletions.
9 changes: 3 additions & 6 deletions Directory.Packages.props
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
<Project>

<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>false</CentralPackageTransitivePinningEnabled>
<!-- Using multiple feeds isn't supported by Maestro: https://github.com/dotnet/arcade/issues/14155. -->
<NoWarn>$(NoWarn);NU1507</NoWarn>
</PropertyGroup>

<ItemGroup>
<!-- Roslyn dependencies -->
<PackageVersion Include="Microsoft.CodeAnalysis" Version="4.0.1" />
Expand All @@ -24,14 +22,13 @@
<PackageVersion Include="Microsoft.CSharp" Version="4.7.0" />
<PackageVersion Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
<PackageVersion Include="Newtonsoft.Json" Version="13.0.2" />
<PackageVersion Include="System.Memory" Version="4.5.4" />
<PackageVersion Include="System.Collections.Immutable" Version="8.0.0" />
<PackageVersion Include="System.Memory" Version="4.5.5" />
<PackageVersion Include="system.reactive.core" Version="5.0.0" />
</ItemGroup>

<ItemGroup Condition="'$(DisableArcade)' == '1'">
<!-- The xunit version should be kept in sync with the one that Arcade promotes -->
<PackageVersion Include="xunit" Version="2.4.2" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.4.3" />
</ItemGroup>

</Project>
</Project>
4 changes: 2 additions & 2 deletions src/System.CommandLine/ParseResult.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.Collections.Generic;
Expand Down Expand Up @@ -121,7 +121,7 @@ internal ParseResult(
/// <summary>
/// Gets the parse errors found while parsing command line input.
/// </summary>
public IReadOnlyList<ParseError> Errors { get; }
public IReadOnlyList<CliDiagnostic> Errors { get; }

/*
// TODO: don't expose tokens
Expand Down
8 changes: 2 additions & 6 deletions src/System.CommandLine/Parsing/CliArgumentResultInternal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void OnlyTake(int numberOfTokens)
/// <inheritdoc/>
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);
}

Expand Down Expand Up @@ -170,17 +170,13 @@ private ArgumentConversionResult ValidateAndConvert(bool useValidators)
}
}
*/

// TODO: defaults
/*
if (Parent!.UseDefaultValueFor(this))
{
var defaultValue = Argument.GetDefaultValue(this);

// 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)
{
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/System.CommandLine/Parsing/CliCommandResultInternal.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.Collections.Generic;
Expand Down Expand Up @@ -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
Expand Down
111 changes: 111 additions & 0 deletions src/System.CommandLine/Parsing/CliDiagnostic.cs
Original file line number Diff line number Diff line change
@@ -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
}

/// <summary>
/// Describes an error that occurs while parsing command line input.
/// </summary>
public sealed class CliDiagnostic
{
// TODO: reevaluate whether we should be exposing a SymbolResult here
// TODO: Rename to CliError

/// <summary>
/// Initializes a new instance of the <see cref="CliDiagnostic"/> class.
/// </summary>
/// <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="location">The location of the error.</param>
public CliDiagnostic(
CliDiagnosticDescriptor descriptor,
object?[]? messageArgs,
ImmutableDictionary<string, object>? properties = null,
SymbolResult? symbolResult = null,
Location? location = null)
{
Descriptor = descriptor;
MessageArgs = messageArgs;
Properties = properties;
SymbolResult = symbolResult;
}

/// <summary>
/// Gets a message to explain the error to a user.
/// </summary>
public string Message
{
get
{
if (MessageArgs is not null)
{
return string.Format(Descriptor.MessageFormat, MessageArgs);
}
return Descriptor.MessageFormat;
}
}

public ImmutableDictionary<string, object>? Properties { get; }

public CliDiagnosticDescriptor Descriptor { get; }

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; }

/// <inheritdoc />
public override string ToString() => Message;
}
6 changes: 3 additions & 3 deletions src/System.CommandLine/Parsing/CliSymbolResultInternal.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.Collections.Generic;
Expand All @@ -24,7 +24,7 @@ private protected CliSymbolResultInternal(SymbolResultTree symbolResultTree, Cli
/// <summary>
/// The parse errors associated with this symbol result.
/// </summary>
public IEnumerable<ParseError> Errors
public IEnumerable<CliDiagnostic> Errors
{
get
{
Expand Down Expand Up @@ -64,7 +64,7 @@ public IEnumerable<ParseError> 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 ParseError(errorMessage, this));
internal virtual void AddError(string errorMessage) => SymbolResultTree.AddError(new CliDiagnostic(new("", "", errorMessage, severity: CliDiagnosticSeverity.Error, null), [], symbolResult: this));
/// <summary>
/// Finds a result for the specific argument anywhere in the parse tree, including parent and child symbol results.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions src/System.CommandLine/Parsing/Location.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public Location(string text, string source, int index, Location? outerLocation,
public bool IsImplicit
=> Source == Implicit;

/// <inheritdoc/>
public override string ToString()
=> $"{(OuterLocation is null ? "" : OuterLocation.ToString() + "; ")}{Text} from {Source}[{Index}, {Length}, {Offset}]";

Expand Down
54 changes: 0 additions & 54 deletions src/System.CommandLine/Parsing/ParseError.cs

This file was deleted.

Loading

0 comments on commit 05bd9eb

Please sign in to comment.