make ValidateSymbolResult void returning (#1567)

* make ValidateSymbolResult void returning, clean up file existence validators

* update API approvals

src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

@@ -535,5 +535,5 @@ System.CommandLine.Parsing
   public delegate ValidateSymbolResult<in T> : System.MulticastDelegate, System.ICloneable, System.Runtime.Serialization.ISerializable
     .ctor(System.Object object, System.IntPtr method)
     public System.IAsyncResult BeginInvoke(T symbolResult, System.AsyncCallback callback, System.Object object)
-    public System.String EndInvoke(System.IAsyncResult result)
-    public System.String Invoke(T symbolResult)
+    public System.Void EndInvoke(System.IAsyncResult result)
+    public System.Void Invoke(T symbolResult)

src/System.CommandLine.Tests/OptionTests.cs

@@ -304,13 +304,12 @@ public void Option_T_default_value_factory_can_be_set_after_instantiation()
         public void Option_T_default_value_is_validated()
         {
             var option = new Option<int>("-x", () => 123);
-            option.AddValidator( symbol =>
-                    symbol.Tokens
-                    .Select(t => t.Value)
-                    .Where(v => v == "123")
-                    .Select(x => "ERR")
-                    .FirstOrDefault());
-
+            option.AddValidator(symbol =>
+                                    symbol.ErrorMessage = symbol.Tokens
+                                                                .Select(t => t.Value)
+                                                                .Where(v => v == "123")
+                                                                .Select(_ => "ERR")
+                                                                .FirstOrDefault());
 
             option
                 .Parse("-x 123")

src/System.CommandLine.Tests/ParsingValidationTests.cs

@@ -242,10 +242,8 @@ public void A_custom_validator_can_be_added_to_a_command()
                 if (commandResult.Children.Any(sr => sr.Symbol is IdentifierSymbol id && id.HasAlias("--one")) &&
                     commandResult.Children.Any(sr => sr.Symbol is IdentifierSymbol id && id.HasAlias("--two")))
                 {
-                    return "Options '--one' and '--two' cannot be used together.";
+                    commandResult.ErrorMessage = "Options '--one' and '--two' cannot be used together.";
                 }
-
-                return null;
             });
 
             var result = command.Parse("the-command --one --two");
@@ -266,7 +264,7 @@ public void A_custom_validator_can_be_added_to_an_option()
             {
                 var value = r.GetValueOrDefault<int>();
 
-                return $"Option {r.Token.Value} cannot be set to {value}";
+                r.ErrorMessage = $"Option {r.Token.Value} cannot be set to {value}";
             });
 
             var command = new RootCommand { option };
@@ -291,7 +289,7 @@ public void A_custom_validator_can_be_added_to_an_argument()
             {
                 var value = r.GetValueOrDefault<int>();
 
-                return $"Argument {r.Argument.Name} cannot be set to {value}";
+                r.ErrorMessage = $"Argument {r.Argument.Name} cannot be set to {value}";
             });
 
             var command = new RootCommand { argument };
@@ -320,14 +318,12 @@ public void All_custom_validators_are_called(string commandLine)
             option.AddValidator(_ =>
             {
                 optionValidatorWasCalled = true;
-                return null;
             });
 
             var argument = new Argument<string>("the-arg");
             argument.AddValidator(_ =>
             {
                 argumentValidatorWasCalled = true;
-                return null;
             });
 
             var rootCommand = new RootCommand
@@ -338,7 +334,6 @@ public void All_custom_validators_are_called(string commandLine)
             rootCommand.AddValidator(_ =>
             {
                 commandValidatorWasCalled = true;
-                return null;
             });
 
             rootCommand.Invoke(commandLine);
@@ -356,7 +351,7 @@ public void Validators_on_global_options_are_executed_when_invoking_a_subcommand
             var option = new Option<FileInfo>("--file");
             option.AddValidator(r =>
             {
-                return "Invoked validator";
+                r.ErrorMessage = "Invoked validator";
             });
 
             var subCommand = new Command("subcommand");
@@ -389,7 +384,7 @@ public async Task A_custom_validator_added_to_a_global_option_is_checked(string
             var handlerWasCalled = false;
 
             var globalOption = new Option<int>("--value");
-            globalOption.AddValidator(_ => "oops!");
+            globalOption.AddValidator(r => r.ErrorMessage = "oops!");
 
             var grandchildCommand = new Command("grandchild");
 
@@ -419,7 +414,7 @@ public void Custom_validator_error_messages_are_not_repeated()
         {
             var errorMessage = "that's not right...";
             var argument = new Argument<string>();
-            argument.AddValidator(o => errorMessage);
+            argument.AddValidator(r => r.ErrorMessage = errorMessage);
 
             var cmd = new Command("get")
             {

src/System.CommandLine/ArgumentExtensions.cs

@@ -4,7 +4,6 @@
 using System.Collections.Generic;
 using System.CommandLine.Parsing;
 using System.CommandLine.Completions;
-using System.Linq;
 using System.IO;
 
 namespace System.CommandLine
@@ -90,12 +89,7 @@ public static TArgument FromAmong<TArgument>(
         /// <returns>The configured argument.</returns>
         public static Argument<FileInfo> ExistingOnly(this Argument<FileInfo> argument)
         {
-            argument.AddValidator(symbol =>
-                                      symbol.Tokens
-                                            .Select(t => t.Value)
-                                            .Where(filePath => !File.Exists(filePath))
-                                            .Select(symbol.LocalizationResources.FileDoesNotExist)
-                                            .FirstOrDefault());
+            argument.AddValidator(Validate.FileExists);
             return argument;
         }
 
@@ -106,12 +100,7 @@ public static Argument<FileInfo> ExistingOnly(this Argument<FileInfo> argument)
         /// <returns>The configured argument.</returns>
         public static Argument<DirectoryInfo> ExistingOnly(this Argument<DirectoryInfo> argument)
         {
-            argument.AddValidator(symbol =>
-                                      symbol.Tokens
-                                            .Select(t => t.Value)
-                                            .Where(filePath => !Directory.Exists(filePath))
-                                            .Select(symbol.LocalizationResources.DirectoryDoesNotExist)
-                                            .FirstOrDefault());
+            argument.AddValidator(Validate.DirectoryExists);
             return argument;
         }
 
@@ -122,12 +111,7 @@ public static Argument<DirectoryInfo> ExistingOnly(this Argument<DirectoryInfo>
         /// <returns>The configured argument.</returns>
         public static Argument<FileSystemInfo> ExistingOnly(this Argument<FileSystemInfo> argument)
         {
-            argument.AddValidator(symbol =>
-                                      symbol.Tokens
-                                            .Select(t => t.Value)
-                                            .Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
-                                            .Select(symbol.LocalizationResources.FileOrDirectoryDoesNotExist)
-                                            .FirstOrDefault());
+            argument.AddValidator(Validate.FileOrDirectoryExists);
             return argument;
         }
 
@@ -141,30 +125,15 @@ public static Argument<T> ExistingOnly<T>(this Argument<T> argument)
         {
             if (typeof(IEnumerable<FileInfo>).IsAssignableFrom(typeof(T)))
             {
-                argument.AddValidator(
-                    a => a.Tokens
-                          .Select(t => t.Value)
-                          .Where(filePath => !File.Exists(filePath))
-                          .Select(a.LocalizationResources.FileDoesNotExist)
-                          .FirstOrDefault());
+                argument.AddValidator(Validate.FileExists);
             }
             else if (typeof(IEnumerable<DirectoryInfo>).IsAssignableFrom(typeof(T)))
             {
-                argument.AddValidator(
-                    a => a.Tokens
-                          .Select(t => t.Value)
-                          .Where(filePath => !Directory.Exists(filePath))
-                          .Select(a.LocalizationResources.DirectoryDoesNotExist)
-                          .FirstOrDefault());
+                argument.AddValidator(Validate.DirectoryExists);
             }
             else
             {
-                argument.AddValidator(
-                    a => a.Tokens
-                          .Select(t => t.Value)
-                          .Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
-                          .Select(a.LocalizationResources.FileOrDirectoryDoesNotExist)
-                          .FirstOrDefault());
+                argument.AddValidator(Validate.FileOrDirectoryExists);
             }
 
             return argument;
@@ -181,23 +150,21 @@ public static TArgument LegalFilePathsOnly<TArgument>(
         {
             var invalidPathChars = Path.GetInvalidPathChars();
 
-            argument.AddValidator(symbol =>
+            argument.AddValidator(result =>
             {
-                for (var i = 0; i < symbol.Tokens.Count; i++)
+                for (var i = 0; i < result.Tokens.Count; i++)
                 {
-                    var token = symbol.Tokens[i];
+                    var token = result.Tokens[i];
 
                     // File class no longer check invalid character
                     // https://blogs.msdn.microsoft.com/jeremykuhne/2018/03/09/custom-directory-enumeration-in-net-core-2-1/
                     var invalidCharactersIndex = token.Value.IndexOfAny(invalidPathChars);
 
                     if (invalidCharactersIndex >= 0)
                     {
-                        return symbol.LocalizationResources.InvalidCharactersInPath(token.Value[invalidCharactersIndex]);
+                        result.ErrorMessage = result.LocalizationResources.InvalidCharactersInPath(token.Value[invalidCharactersIndex]);
                     }
                 }
-
-                return null;
             });
 
             return argument;
@@ -215,20 +182,18 @@ public static TArgument LegalFileNamesOnly<TArgument>(
         {
             var invalidFileNameChars = Path.GetInvalidFileNameChars();
 
-            argument.AddValidator(symbol =>
+            argument.AddValidator(result =>
             {
-                for (var i = 0; i < symbol.Tokens.Count; i++)
+                for (var i = 0; i < result.Tokens.Count; i++)
                 {
-                    var token = symbol.Tokens[i];
+                    var token = result.Tokens[i];
                     var invalidCharactersIndex = token.Value.IndexOfAny(invalidFileNameChars);
 
                     if (invalidCharactersIndex >= 0)
                     {
-                        return symbol.LocalizationResources.InvalidCharactersInFileName(token.Value[invalidCharactersIndex]);
+                        result.ErrorMessage =  result.LocalizationResources.InvalidCharactersInFileName(token.Value[invalidCharactersIndex]);
                     }
                 }
-
-                return null;
             });
 
             return argument;

src/System.CommandLine/Help/VersionOption.cs

@@ -38,10 +38,8 @@ private void AddValidators()
                     parent.Children.Where(r => r.Symbol is not VersionOption)
                           .Any(IsNotImplicit))
                 {
-                    return result.LocalizationResources.VersionOptionCannotBeCombinedWithOtherArguments(result.Token.Value ?? result.Symbol.Name);
+                    result.ErrorMessage =  result.LocalizationResources.VersionOptionCannotBeCombinedWithOtherArguments(result.Token.Value ?? result.Symbol.Name);
                 }
-
-                return null;
             });
         }
 

src/System.CommandLine/Option.cs

@@ -298,6 +298,7 @@ private string GetLongestAlias()
             return max.RemovePrefix();
         }
 
+        /// <inheritdoc />
         public override IEnumerable<CompletionItem> GetCompletions(CompletionContext context)
         {
             if (_argument is null)

src/System.CommandLine/OptionExtensions.cs

@@ -90,14 +90,7 @@ public static TOption AddCompletions<TOption>(
         /// <returns>The option being extended.</returns>
         public static Option<FileInfo> ExistingOnly(this Option<FileInfo> option)
         {
-            option.Argument.AddValidator(
-                a =>
-                    a.Tokens
-                     .Select(t => t.Value)
-                     .Where(filePath => !File.Exists(filePath))
-                     .Select(a.LocalizationResources.FileDoesNotExist)
-                     .FirstOrDefault());
-
+            option.Argument.AddValidator(Validate.FileExists);
             return option;
         }
 
@@ -108,14 +101,7 @@ public static Option<FileInfo> ExistingOnly(this Option<FileInfo> option)
         /// <returns>The option being extended.</returns>
         public static Option<DirectoryInfo> ExistingOnly(this Option<DirectoryInfo> option)
         {
-            option.Argument.AddValidator(
-                a =>
-                    a.Tokens
-                     .Select(t => t.Value)
-                     .Where(filePath => !Directory.Exists(filePath))
-                     .Select(a.LocalizationResources.DirectoryDoesNotExist)
-                     .FirstOrDefault());
-
+            option.Argument.AddValidator(Validate.DirectoryExists);
             return option;
         }
 
@@ -126,14 +112,7 @@ public static Option<DirectoryInfo> ExistingOnly(this Option<DirectoryInfo> opti
         /// <returns>The option being extended.</returns>
         public static Option<FileSystemInfo> ExistingOnly(this Option<FileSystemInfo> option)
         {
-            option.Argument.AddValidator(
-                a =>
-                    a.Tokens
-                     .Select(t => t.Value)
-                     .Where(filePath => !Directory.Exists(filePath) && !File.Exists(filePath))
-                     .Select(a.LocalizationResources.FileOrDirectoryDoesNotExist)
-                     .FirstOrDefault());
-
+            option.Argument.AddValidator(Validate.FileOrDirectoryExists);
             return option;
         }
 

src/System.CommandLine/Parsing/ArgumentResult.cs

@@ -90,12 +90,12 @@ public void OnlyTake(int numberOfTokens)
 
             for (var i = 0; i < argument.Validators.Count; i++)
             {
-                var symbolValidator = argument.Validators[i];
-                var errorMessage = symbolValidator(this);
+                var validateSymbolResult = argument.Validators[i];
+                validateSymbolResult(this);
 
-                if (!string.IsNullOrWhiteSpace(errorMessage))
+                if (!string.IsNullOrWhiteSpace(ErrorMessage))
                 {
-                    return new ParseError(errorMessage!, this);
+                    return new ParseError(ErrorMessage!, this);
                 }
             }
 

src/System.CommandLine/Parsing/ParseResultVisitor.cs

@@ -325,12 +325,12 @@ private void ValidateCommandResult()
 
             for (var i = 0; i < command.Validators.Count; i++)
             {
-                var validator = command.Validators[i];
-                var errorMessage = validator(_innermostCommandResult);
+                var validateSymbolResult = command.Validators[i];
+                validateSymbolResult(_innermostCommandResult);
 
-                if (!string.IsNullOrWhiteSpace(errorMessage))
+                if (!string.IsNullOrWhiteSpace(_innermostCommandResult.ErrorMessage))
                 {
-                    AddErrorToResult(_innermostCommandResult, new ParseError(errorMessage!, _innermostCommandResult));
+                    AddErrorToResult(_innermostCommandResult, new ParseError(_innermostCommandResult.ErrorMessage!, _innermostCommandResult));
                 }
             }
 
@@ -440,11 +440,11 @@ private void ValidateAndConvertOptionResult(OptionResult optionResult)
                 for (var i = 0; i < optionResult.Option.Validators.Count; i++)
                 {
                     var validate = optionResult.Option.Validators[i];
-                    var message = validate(optionResult);
+                    validate(optionResult);
 
-                    if (!string.IsNullOrWhiteSpace(message))
+                    if (!string.IsNullOrWhiteSpace(optionResult.ErrorMessage))
                     {
-                        AddErrorToResult(optionResult, new ParseError(message!, optionResult));
+                        AddErrorToResult(optionResult, new ParseError(optionResult.ErrorMessage!, optionResult));
                     }
                 }
             }

src/System.CommandLine/Parsing/ValidateSymbolResult.cs

@@ -1,14 +1,13 @@
 // 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
-{
-    /// <summary>
-    /// A delegate used to validate symbol results during parsing.
-    /// </summary>
-    /// <typeparam name="T">The type of the <see cref="SymbolResult"/>.</typeparam>
-    /// <param name="symbolResult">The symbol result</param>
-    /// <returns>An error message to indicate a validation error; otherwise, <c>null</c>.</returns>
-    public delegate string? ValidateSymbolResult<in T>(T symbolResult) 
-        where T : SymbolResult;
-}
+namespace System.CommandLine.Parsing;
+
+/// <summary>
+/// A delegate used to validate symbol results during parsing.
+/// </summary>
+/// <typeparam name="T">The type of the <see cref="SymbolResult"/>.</typeparam>
+/// <param name="symbolResult">The symbol result</param>
+/// <remarks>To display an error, set <see cref="SymbolResult.ErrorMessage"/>.</remarks>
+public delegate void ValidateSymbolResult<in T>(T symbolResult) 
+    where T : SymbolResult;