Fix #79 - remove ParseResult.ValidationResult

This property had side-effects that changed the order in which validation was called. The recommended replacement is ParseResult.SelectedCommand.GetValidationResult(), and it should be called after OnParsingComplete handlers are finished.
natemcmaster · Apr 11, 2018 · f174cf3 · f174cf3
1 parent a69501f
commit f174cf3
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 17 deletions.
diff --git a/src/CommandLineUtils/Abstractions/ParseResult.cs b/src/CommandLineUtils/Abstractions/ParseResult.cs
@@ -1,8 +1,6 @@
 // Copyright (c) Nate McMaster.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System.ComponentModel.DataAnnotations;
-
 namespace McMaster.Extensions.CommandLineUtils.Abstractions
 {
     /// <summary>
@@ -14,10 +12,5 @@ public class ParseResult
         /// The application or subcommand that matches the command line arguments.
         /// </summary>
         public CommandLineApplication SelectedCommand { get; set; }
-
-        /// <summary>
-        /// The result of executing all valiation on selected options and arguments.
-        /// </summary>
-        public ValidationResult ValidationResult { get; set; }
     }
 }
diff --git a/src/CommandLineUtils/CommandLineApplication.cs b/src/CommandLineUtils/CommandLineApplication.cs
@@ -602,9 +602,10 @@ public int Execute(params string[] args)
                 return HelpExitCode;
             }
 
-            if (parseResult.ValidationResult != ValidationResult.Success)
+            var validationResult = command.GetValidationResult();
+            if (validationResult != ValidationResult.Success)
             {
-                return command.ValidationErrorHandler(parseResult.ValidationResult);
+                return command.ValidationErrorHandler(validationResult);
             }
 
             return command.Invoke();

diff --git a/src/CommandLineUtils/Internal/CommandLineProcessor.cs b/src/CommandLineUtils/Internal/CommandLineProcessor.cs
@@ -44,7 +44,6 @@ public ParseResult Process()
 
             finished:
             parseResult.SelectedCommand = _currentCommand;
-            parseResult.ValidationResult = _currentCommand.GetValidationResult();
             return parseResult;
         }
 

diff --git a/test/CommandLineUtils.Tests/CustomValidationAttributeTest.cs b/test/CommandLineUtils.Tests/CustomValidationAttributeTest.cs
@@ -20,7 +20,7 @@ public void CustomValidationAttributePasses(params string[] args)
             var app = new CommandLineApplication<RedBlueProgram>();
             app.Conventions.UseDefaultConventions();
             var result = app.Parse(args ?? new string[0]);
-            Assert.Equal(ValidationResult.Success, result.ValidationResult);
+            Assert.Equal(ValidationResult.Success, result.SelectedCommand.GetValidationResult());
             var program = Assert.IsType<CommandLineApplication<RedBlueProgram>>(result.SelectedCommand);
             Assert.Same(app, program);
             if (args != null)
@@ -38,14 +38,15 @@ public void CustomValidationAttributeFails(params string[] args)
             var app = new CommandLineApplication<RedBlueProgram>();
             app.Conventions.UseAttributes();
             var result = app.Parse(args);
-            Assert.NotEqual(ValidationResult.Success, result.ValidationResult);
+            var validationResult = result.SelectedCommand.GetValidationResult();
+            Assert.NotEqual(ValidationResult.Success, validationResult);
             var program = Assert.IsType<CommandLineApplication<RedBlueProgram>>(result.SelectedCommand);
             Assert.Same(app, program);
             if (args != null)
             {
                 Assert.Equal(args[1], app.Model.Color);
             }
-            Assert.Equal("The value for --color must be 'red' or 'blue'", result.ValidationResult.ErrorMessage);
+            Assert.Equal("The value for --color must be 'red' or 'blue'", validationResult.ErrorMessage);
         }
 
         private class RedBlueProgram

diff --git a/test/CommandLineUtils.Tests/ValidateMethodConventionTests.cs b/test/CommandLineUtils.Tests/ValidateMethodConventionTests.cs
@@ -57,7 +57,7 @@ private ValidationResult OnValidate(ValidationContext context, CommandLineContex
                 }
 
                 Assert.Equal(typeof(CommandLineApplication<MainValidate>), context.ObjectInstance.GetType());
-               
+
                 return ValidationResult.Success;
             }
         }
@@ -110,7 +110,7 @@ public void ValidatorShouldGetDeserailizedModelInSubcommands(string args, string
             // this tests that the model is actually given values before it passed to command validation
             var parseResult = app.Parse(args.Split(' '));
 
-            var result = parseResult.ValidationResult;
+            var result = parseResult.SelectedCommand.GetValidationResult();
 
             if (error == null)
             {

diff --git a/test/CommandLineUtils.Tests/ValidationTests.cs b/test/CommandLineUtils.Tests/ValidationTests.cs
@@ -339,11 +339,11 @@ public void ValidatesRange(int input, bool isValid)
             var result = app.Parse(input.ToString());
             if (isValid)
             {
-                Assert.Equal(ValidationResult.Success, result.ValidationResult);
+                Assert.Equal(ValidationResult.Success, result.SelectedCommand.GetValidationResult());
             }
             else
             {
-                Assert.NotEqual(ValidationResult.Success, result.ValidationResult);
+                Assert.NotEqual(ValidationResult.Success, result.SelectedCommand.GetValidationResult());
             }
         }
     }