WIP: CliError

Directory.Packages.props

@@ -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" />
@@ -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>

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;
@@ -36,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
@@ -75,8 +75,8 @@ 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>();
         }
 
 // TODO: check that constructing empty ParseResult directly is correct
@@ -102,7 +102,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

src/System.CommandLine/Parsing/ArgumentResult.cs

@@ -141,7 +141,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);
         }
 
@@ -171,17 +171,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)
             {
@@ -223,7 +219,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;

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

src/System.CommandLine/Parsing/CommandResult.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;
@@ -64,7 +64,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

src/System.CommandLine/Parsing/Location.cs

@@ -49,6 +49,7 @@ public Location(string text, string source, int start, Location? outerLocation,
         public bool IsImplicit
             => Source == Implicit;
 
+        /// <inheritdoc/>
         public override string ToString()
             => $"{(OuterLocation is null ? "" : OuterLocation.ToString() + "; ")}{Text} from {Source}[{Start}, {Length}, {Offset}]";
 

src/System.CommandLine/Parsing/SymbolResult.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 SymbolResult(SymbolResultTree symbolResultTree, SymbolResult?
         /// <summary>
         /// The parse errors associated with this symbol result.
         /// </summary>
-        public IEnumerable<ParseError> Errors
+        public IEnumerable<CliDiagnostic> Errors
         {
             get
             {
@@ -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>
@@ -100,7 +100,7 @@ public IEnumerable<ParseError> Errors
         /// </summary>
         /// <param name="name">The name of the symbol for which to find a result.</param>
         /// <returns>An argument result if the argument was matched by the parser or has a default value; otherwise, <c>null</c>.</returns>
-        public SymbolResult? GetResult(string name) => 
+        public SymbolResult? GetResult(string name) =>
             SymbolResultTree.GetResult(name);
 
         /// <inheritdoc cref="ParseResult.GetValue{T}(CliArgument{T})"/>

src/System.CommandLine/Parsing/SymbolResultTree.cs

@@ -1,15 +1,14 @@
-// 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
 {
     internal sealed class SymbolResultTree : Dictionary<CliSymbol, SymbolResult>
     {
         private readonly CliCommand _rootCommand;
-        internal List<ParseError>? Errors;
+        internal List<CliDiagnostic>? Errors;
 // TODO: unmatched tokens
 /*
         internal List<CliToken>? UnmatchedTokens;
@@ -27,11 +26,11 @@ internal SymbolResultTree(
 
             if (tokenizeErrors is not null)
             {
-                Errors = new List<ParseError>(tokenizeErrors.Count);
+                Errors = new List<CliDiagnostic>(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), []));
                 }
             }
         }
@@ -51,7 +50,6 @@ internal SymbolResultTree(
         internal DirectiveResult? GetResult(CliDirective directive)
             => TryGetValue(directive, out SymbolResult? result) ? (DirectiveResult)result : default;
 */
-        // TODO: Determine how this is used. It appears to be O^n in the size of the tree and so if it is called multiple times, we should reconsider to avoid O^(N*M)
         internal IEnumerable<SymbolResult> GetChildren(SymbolResult parent)
         {
             // Argument can't have children
@@ -71,7 +69,7 @@ internal Dictionary<CliSymbol, ValueResult> GetValueResultDictionary()
         {
             var dict = new Dictionary<CliSymbol, ValueResult>();
             foreach (KeyValuePair<CliSymbol, SymbolResult> pair in this)
-            {               
+            {
                 var result = pair.Value;
                 if (result is OptionResult optionResult)
                 {
@@ -87,8 +85,8 @@ internal Dictionary<CliSymbol, ValueResult> GetValueResultDictionary()
             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, CommandResult commandResult, CommandResult rootCommandResult)
         {
@@ -104,7 +102,7 @@ internal void AddUnmatchedToken(CliToken token, CommandResult commandResult, Com
                 }
 
 */
-            AddError(new ParseError(LocalizationResources.UnrecognizedCommandOrArgument(token.Value), commandResult));
+            AddError(new CliDiagnostic(new("", "", LocalizationResources.UnrecognizedCommandOrArgument(token.Value), CliDiagnosticSeverity.Warning, null), [], symbolResult: commandResult));
             //            }
         }
 
@@ -113,7 +111,6 @@ internal void AddUnmatchedToken(CliToken token, CommandResult commandResult, Com
             if (_symbolsByName is null)
             {
                 _symbolsByName = new();
-                // TODO: See if we can avoid populating the entire tree and just populate the portion/cone we need
                 PopulateSymbolsByName(_rootCommand);
             }
 
@@ -140,7 +137,6 @@ internal void AddUnmatchedToken(CliToken token, CommandResult commandResult, Com
 // 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?
-// Additional Note: Couldn't commands know their children, and thus this involves querying the active command, and possibly the parents
         private void PopulateSymbolsByName(CliCommand command)
         {
             if (command.HasArguments)