Remove Argument.AllowedValues, use Validator to implement the logic (#1959)

* delegate Option.AcceptOnlyFromAmong to Argument.AcceptOnlyFromAmong

* use Validator to implement AllowedValues

Author: adamsitnik

src/System.CommandLine.Tests/ParsingValidationTests.cs

@@ -119,6 +119,34 @@ public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_p
                   .Be(LocalizationResources.Instance.UnrecognizedArgument("not-key1", new[] { "key1", "key2" }));
         }
 
+        [Fact]
+        public void When_FromAmong_is_used_multiple_times_only_the_most_recently_provided_values_are_taken_into_account()
+        {
+            Argument<string> argument = new("key");
+            argument.AcceptOnlyFromAmong("key1");
+
+            var command = new Command("set")
+            {
+                argument
+            };
+
+            var result = command.Parse("set key2");
+
+            result.Errors
+              .Should()
+              .ContainSingle()
+              .Which
+              .Message
+              .Should()
+              .Be(LocalizationResources.Instance.UnrecognizedArgument("key2", new[] { "key1" }));
+
+            argument.AcceptOnlyFromAmong("key2");
+
+            result = command.Parse("set key2");
+
+            result.Errors.Should().BeEmpty();
+        }
+
         [Fact]
         public void When_FromAmong_is_used_for_multiple_arguments_and_invalid_input_is_provided_for_the_second_one_then_the_error_is_informative()
         {

src/System.CommandLine/Argument.cs

@@ -37,8 +37,6 @@ protected Argument(string? name = null, string? description = null)
             Description = description;
         }
 
-        internal HashSet<string>? AllowedValues { get; private set; }
-
         /// <summary>
         /// Gets or sets the arity of the argument.
         /// </summary>
@@ -127,16 +125,6 @@ private protected override string DefaultName
 
         internal virtual bool HasCustomParser => false;
 
-        internal void AddAllowedValues(IReadOnlyList<string> values)
-        {
-            if (AllowedValues is null)
-            {
-                AllowedValues = new HashSet<string>();
-            }
-
-            AllowedValues.UnionWith(values);
-        }
-
         /// <inheritdoc />
         public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
         {

src/System.CommandLine/Argument{T}.cs

@@ -1,9 +1,7 @@
 // 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.CommandLine.Binding;
-using System.CommandLine.Completions;
 using System.CommandLine.Parsing;
 using System.IO;
 
@@ -179,12 +177,31 @@ public void SetDefaultValueFactory(Func<ArgumentResult, T> defaultValueFactory)
         /// <returns>The configured argument.</returns>
         public Argument<T> AcceptOnlyFromAmong(params string[] values)
         {
-            AllowedValues?.Clear();
-            AddAllowedValues(values);
-            CompletionSources.Clear();
-            CompletionSources.Add(values);
+            if (values is not null && values.Length > 0)
+            {
+                Validators.Clear();
+                Validators.Add(UnrecognizedArgumentError);
+                CompletionSources.Clear();
+                CompletionSources.Add(values);
+            }
 
             return this;
+
+            void UnrecognizedArgumentError(ArgumentResult argumentResult)
+            {
+                for (var i = 0; i < argumentResult.Tokens.Count; i++)
+                {
+                    var token = argumentResult.Tokens[i];
+
+                    if (token.Symbol is null || token.Symbol == this)
+                    {
+                        if (Array.IndexOf(values, token.Value) < 0)
+                        {
+                            argumentResult.ErrorMessage = argumentResult.LocalizationResources.UnrecognizedArgument(token.Value, values);
+                        }
+                    }
+                }
+            }
         }
 
         /// <summary>

src/System.CommandLine/Parsing/ArgumentResult.cs

@@ -83,7 +83,7 @@ public void OnlyTake(int numberOfTokens)
 
                 if (!string.IsNullOrWhiteSpace(ErrorMessage))
                 {
-                    return new ParseError(ErrorMessage!, this);
+                    return new ParseError(ErrorMessage!, Parent is OptionResult option ? option : this);
                 }
             }
 

src/System.CommandLine/Parsing/ParseResultVisitor.cs

@@ -506,9 +506,7 @@ private void ValidateAndConvertArgumentResult(ArgumentResult argumentResult)
         {
             var argument = argumentResult.Argument;
 
-            var parseError =
-                argumentResult.Parent?.UnrecognizedArgumentError(argument) ??
-                argumentResult.CustomError(argument);
+            var parseError = argumentResult.CustomError(argument);
 
             if (parseError is { })
             {

src/System.CommandLine/Parsing/SymbolResult.cs

@@ -185,29 +185,5 @@ internal ArgumentResult GetOrCreateDefaultArgumentResult(Argument argument) =>
 
         /// <inheritdoc/>
         public override string ToString() => $"{GetType().Name}: {this.Token()} {string.Join(" ", Tokens.Select(t => t.Value))}";
-
-        internal ParseError? UnrecognizedArgumentError(Argument argument)
-        {
-            if (argument.AllowedValues?.Count > 0 &&
-                Tokens.Count > 0)
-            {
-                for (var i = 0; i < Tokens.Count; i++)
-                {
-                    var token = Tokens[i];
-
-                    if (token.Symbol is null || token.Symbol == argument)
-                    {
-                        if (!argument.AllowedValues.Contains(token.Value))
-                        {
-                            return new ParseError(
-                                LocalizationResources.UnrecognizedArgument(token.Value, argument.AllowedValues),
-                                this);
-                        }
-                    }
-                }
-            }
-
-            return null;
-        }
     }
 }