commandline
commandline copied to clipboard
commandlineparser
Add support for validation and custom types
Add some sort of hook or attribute to plug validation into the parser pipeline
similar some similar frameworks use attributes or factory registration to allow an entrypoint for custom validation and type mapping (DI support would be awesome as well ;) )
a typical and widely used usage scenario for this would be filesystem path validation and mapping to IO classes such as FileInfo / DirectoryInfo an seemingly easy way to to add this type of validation functionality would be to allow validation attributes that inherit from a validation base class or interface and run custom or pre-written code
some examples of validation provided in similar frameworks are
[FileExists]
[DirectoryExists]
[FileNotExists]
[DirectoryNotExists]
[RegExValidation]
Powerargs provides a special class called a Reviver for custom type mapping as well that the framework looks for to try and map values to a custom or complex type ie path string => FileInfo
It is very hard to find a framework that does all 3 of these well currently and make this library a go to framework for myself and many other developers i am sure
as CLP 2.x is still in beta and major rewrite phase i think now would be a good time to look at feature expansion as well to increase the power of this framework
IMHO this would be overkill and it destroys the single-responsibility-principle aspect of this library.
@stanislavromanov what if it was set up to allow you to plug in an existing validation library (e.g. FluentValidation) that's run after all parsing or after each argument is parsed? The validations mentioned in the OP would either be built in (but optional) or compiled to a separate nuget and you could always design your own.
@stanislavromanov, i agree and disagree with you on this, without some sort of validation mechanism or custom mapping mechanism it really limits what can be used as an argument ie : FileInfo DirectoryInfo mapping, DateTime , TimeSpan, DateTimeOffset , are a few examples for mapping , being able to validate path exists is a good example of a custom validation , or an argument is in specified range .. I don't think allowing business rules in your parsing pipeline violates SRP and is a quite common requirement and a feature of many libraries,
+1 from me.
If I have to do my own validation, not only do I need to write code for annoying things like parsing and regex matching, I also don't get the benefits of automatic help text display on parsing errors (including the text that explains exactly what went wrong in the parsing).
Fighting this battle right now as well validating if a file actually exists on disk. I don't see an easy way to add a custom error to the help text and display the help text after successful parsing is done.
Is this possible?
So realistically, this is a parsing library and all the validation is around successful parsing of strings to numbers etc.
[FileExists], [DirectoryExists], [NetworkServerReachable], [AmazonServiceHasItem] and similar, would definitely be out-of-scope for a library like this.
For these things, your best bet is probably after successful parsing, run through your use-case validations, (ie check file specified exists, or the amazon service has the item, etc) and invoke the Help to display usage, and display your validation failure message afterwards in red or different color... "File specified 'xxx.txt' doesn't exist". And, by convention, exit with a non-zero exit code.
That would work. Do you know what snippet I would need to run in order to pull the current help usage text? Right now, I have this:
private static Options parseArgs(string[] args)
{
ParserResult<Options> result = Parser.Default.ParseArguments<Options>(args);
if (result.Tag == ParserResultType.NotParsed)
{
return null;
}
else
{
var parsedResult = result as Parsed<Options>;
Options o = parsedResult.Value;
if (!File.Exists(o.Solution))
{
Console.WriteLine($"Solution file does not exist: {o.Solution}");
return null;
}
return parsedResult.Value;
}
}
I would love to be able to pull the "usage" text from the parsedResult somehow and include that in the error message.
You almost need to return ParserResult<Options> from that instead, so you won't have a null return that you'll have to guard against.
Next, I'd have a ValidateOptions method that would then roll through the various specified options and check them for validity, and return a ValidationResults type of structure,
Which would then be processed by a ParseAndValidate method that would be responsible for analyzing the ParserResult<Options> from parseArgs and deciding if it needs to display usage and/or exit with a non-zero error code. Realistically a non-existing file wouldn't generate a usage banner, but your ParseAndValidate method could do that, and calling the GetHelp routines to output the usage banner.
Sorry i'm a little vague on the GetHelp routines, mainly because I picked up the project trying to push the v2 changes into a release state after the original author disappeared into contract work (I'm assuming)
On Tue, Mar 13, 2018 at 1:24 PM, Eric Johnson [email protected] wrote:
That would work. Do you know what snippet I would need to run in order to pull the current help usage text? Right now, I have this:
private static Options parseArgs(string[] args) { ParserResult<Options> result = Parser.Default.ParseArguments<Options>(args);
if (result.Tag == ParserResultType.NotParsed) { return null; } else { var parsedResult = result as Parsed<Options>; Options o = parsedResult.Value; if (!File.Exists(o.Solution)) { Console.WriteLine($"Solution file does not exist: {o.Solution}"); return null; } return parsedResult.Value; } }I would love to be able to pull the "usage" text from the parsedResult somehow and include that in the error message.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/commandlineparser/commandline/issues/146#issuecomment-372748635, or mute the thread https://github.com/notifications/unsubscribe-auth/ACG_zOoZud1x5vqP9DeTMGzTqBdPNl0Yks5teADZgaJpZM4QSHQ7 .
--
E.Newton Senior Software Craftsman Cell: 407-497-1429 EMail: [email protected]
The GetHelp routines are a bit difficult to use since most of it is marked internal, but you are in luck - with ParserResult<Options> result it's quite easy to generate the help text:
var help = HelpText.AutoBuild(result);
Console.WriteLine(help.ToString());
Unfortunately, AutoBuild<T>() probably won't help. We're probably only interested in running a validator over input that successfully parsed. The parser already catches syntax snafus for us. The role of the validator is to catch semantic errors. In which case, our abstract ParserResult<T> is instantiated as a Parsed<T> object. The first thing AutoBuild<T>() does is to throw an ArgumentException if its parserResult argument isn't a NotParsed<T> object.
You run into a similar issue if you try to use DefaultParsingErrorHandler<T> to roll your own version of AutoBuild<T>(). Meanwhile, NotParsed<T> has no public constructors (all internal), so we can't build our own failure to give to AutoBuild<T>().
Oh, and even if we were running a validator on a NotParsed<T> result, there's no way to add errors to an existing NotParsed<T> object, so AutoBuild<T> doesn't really help there, either.
All that said, I think it's debatable whether invoking CommandLineParser's help methods are even appropriate for this kind of problem. Why print a usage error if the specified file doesn't exist? That isn't a usage problem.
On the other hand, I can see some cases where it might make sense. For instance, if my program is something like: dosomething --repeat -3. This parses because -3 is a valid integer token. But it's a nonsensical response, and my HelpText for the repeat options is probably going to tell the user that only positive values are allowed.
This example blurs the line between syntax and semantics. If the parser were more sophisticated, we could specify that only positive integer tokens are allowed in the context of a repeat option, but we just don't have that capability, so it falls through to semantic checking.
@tmillican I am of the same mind on this topic. There's definitely a difference between specifying an invalid file and having a filename parsed into the wrong variable. Syntax vs semantics.
@ericnewton76:
Speaking to the original issue, I feel like this might be a case where inversion of control is more headache than its worth. Rather than concocting some way for CommandLineParser to call a plug-in validator, it might be a lot easier and cleaner to just provide more/better opportunities for the recipient of parser results to call back into the parser's output logic.
I'm even less familiar with HelpText and friends, but as @nemec points out, most of the help logic is marked internal, so about the only control we have over output is the onError delegate argument of AutoBuild<T>(). That's fine except that HelpText doesn't expose enough to inject error messages (without getting really hacky) and, of course, it only gets invoked in the context of errors.
If we had more building blocks like BuildHeaderText(), BuildOptionsText(), etc. then you could suppress and inject output pretty much any way you'd like, whether or not the parsing succeeded.
Because there are cases of actual syntax checking that are simply too complicated or specialized to fit into a general command line parser. For instance, a company might have a standard format for project numbers. That's a syntax issue, but I think it's fair to say that it's beyond the scope of CommandLineParser's niche. So it would be good to be able to augment it with a more sophisticated validator somehow.
If you can provide a custom validator instance, used for specific custom validation, it should handle minimum modifications in the CommandLine lib, and respect SRP. I think to something like this :
parsed.MapResult(
(Verb1Options opts, Validator validator) =>
{
if(opts.ValueToCheck != "thegoodvalue")
validator.ValidationError("The value should be like thegoodvalue");
return 0;
},
(Verb2Options opts, Validator validator) =>
{
....
return 0;
},
errs => 1);
If a single validator.ValidationError() call happens, then we should call the errs => 1 delegate, and WriteOut the custom message on the console.
Is there still interest to add validation capabilities to the framework?
I am thinking about adding to Option an optional Func<Error> which contains user-provided validation logic and runs after parsing is done.
The approach listed by @rducom would also be good IMHO.
In any case, I think it's an useful addition to the library.
I did a first "draft" thinking about custom validations in 3a689998bdad1dc959f3c5dbff526064c57b1925. Maybe somebody will have a look before providing more validations. A test case shows the general usage.
Is there any progress on this? If not, at the current state of things, what would be the best place to perform such validation as checking if the provided path is valid?
I would like an abstract base class or interface like:
public class MyOptions : IValidatedOptions {
[Option('s', "something", Required=true, HelpText="Some option")]
public string Something {get;set;}
// From Interface
public bool Validate(){
if(!Util.CheckSomething(Something))
return false; // this automatically tells CommandLineParser to fail the parsing and print the help.
}
}
Alternatively instead of bool it could return some kind of new ValidationError("Reason for failing").
If the parser sees options implement that interface - runs Validate() - if not, works as it works now.
I of course could easily write that in, but let's hear from maintainers first if they are considering any type of validation strategies.
@p-kaczynski
considering any type of validation strategies.
The library support validation in property setter as described here:
It integrate the validation Exception Messages with ERROR(s) in help.
@moh-hassan
This is fairly limited, as we don’t know the order in which the properties are set, and if an option might or might not be valid depending on another parameter, there isn’t a way to do it.
@p-kaczynski
The limitation of Property Setter validation can be enhanced by submitting a new PR.
I find your approach of IValidatedOptions is promising and it can be executed at the final stage of Parsing.
Also,an alternative, to minimize the overhead of parser validation, why not validation is executed as a first step in WithParsed method specially this validation is the business logic responsibility.
Python's command line parser uses the type= option to do this. https://docs.python.org/2/library/argparse.html#type
@donnyv Yes, Python validation is similar to the property setter validation in commandLineParser. What about if validation is applied per property and one property is depending on another one?
The way I do this in the meantime is by adding a Validate method to my options class:
[Verb("open", HelpText = "Open the foo file")]
public class FooOptions
{
[Option('f', "fooPath", Required = true, HelpText = "Foo")]
public string FooPath { get; }
protected FooOptions(string fooPath)
{
FooPath = fooPath;
}
public void Validate()
{
if (!File.Exists(FooPath))
{
throw new ArgumentValidationException($"Foo path does not exist!");
}
}
}
The exception I'm throwing is just a custom type:
class ArgumentValidationException : Exception
{
public ArgumentValidationException(string message) : base(message) { }
}
And in my parsing code I do something like this:
static int Main(string[] args)
{
try
{
return Parser.Default.ParseArguments<FooOptions>(args)
.MapResult(
(FooOptions opts) =>
{
opts.Validate();
//do something with opts.FooPath
return 0;
},
errors => 1);
}
catch (ArgumentValidationException e)
{
Console.WriteLine("Invalid arguments detected: {0}", e);
return 1;
}
}
@donnyv presumably a custom type can be initialized by primitives. In the case of MongoDB ObjectId it looks like it can be created from a string. So the options class could look like this:
[Verb("get", HelpText = "get a Mongo object")]
public class MongoOptions
{
private readonly string _objectIdStr;
[Option('o', "objectId", Required = true, HelpText = "MongoDB Object ID")]
public string ObjectIdStr =>
throw new InvalidOperationException($"You should use the {nameof(ObjectId)} property");
private ObjectId _objectId;
public ObjectId ObjectId => _objectId;
public MongoOptions(string objectIdStr)
{
_objectIdStr = objectIdStr;
}
public void Init()
{
try
{
_objectId = new ObjectId(_objectIdStr);
}
catch (Exception e)
{
throw new ArgumentException("Error initializing MongoDB object ID", e);
}
Validate();
}
public void Validate()
{
if (_objectId.CreationTime < DateTime.Now.AddYears(-3))
{
throw new ArgumentValidationException($"Object too old!");
}
}
}
And you would call Init instead of Validate in your parsing logic (e.g. Program.Main).
Hi,
Any news about this subject ? I need to do a custom validation, and when this custom validation fails, I would like to use the error flow (RunErrors) and not the normal flow (Run) :
var parserResult = parser.ParseArguments(args, optionsClasses);
parserResult.MapResult(Run, errors => RunErrors(errors, parserResult, args));
I found the idea of @p-kaczynski very good : If we have an interface like he describe IValidatedOptions, it's exactly my need...
Regards Sybaris
