Consider where arity checks should occur

[KathleenDollard]

In the case of a specific upper bound arity other than 1 or a large number, parsing is affected by the upper bound arity. It is also affected by a zero only arity. Because of this, arity needs to be available for CliOption and CliArgument during parsing.

Question 1: Should CLI authors be able to define arity in raw parser scenarios?

If we do not allow this, it would be settable from pipelines only.

Pros of allowing definition:

• We have to have arity associated with the option or argument, why not let raw usage folks access it?

Cons of allowing definition:

• It sets up a non-standard way to add a validation in pipeline scenarios

Question 2: Should we create non-critical arity issues in raw parser scenarios?

Pros of creating:

• We are in a position to do the work, so could. We need to do it if we decide to report errors in raw parser usage scenarios

Cons of creating:

• If there are critical errors in parsing, early termination subsystems do not run. We would want them to run if the parser can succeed. Thus, if we create non-critical parser issues, we need a way to distinguish critical and non-critical parser diagnostics
  • This is not an issue today as the early termination scenarios are very simple. We should consider the fantasies we have about rich help in determining this behavior

Question 2: Should we report non-critical arity issues in raw parser scenarios?

We need to get this decision right. Changing it would be a breaking change in CLIs.

Pros of reporting:

• The user gave us the data, we know the answer, why not share?
• If we do not report them, the end user gets some, but not all arity issues
• It seems mean to know about an issue and not tell the user

Cons of reporting:

• This sets up a different source for errors, which is likely to result in them being less rich, less with less flexibility in text, etc.

Proposal

One way to work around the issues above is the following.

• Allow arity to be defined directly on CliOption and CliArgument. Assume this will be confusing and ensure validation treats them identically. When added as validation, the subsystem will update arity
• Add a Kind or Source to CliDiagnosticDescriptor. This would be at least parser/not parser or critical/not critical to indicate whether proceeding is OK
• Keep a separate diagnostic list for ParseResult and the PipelineResult. PipelineResult would selectively use ParseResult errors, but all of ParseResult, including diagnostics, would be immutable
• Non-critical errors in ParseResult would need to be transferred, often with mutations, to the PipelineResult. PipelineResult.Errors would include critical errors, but may not copy/transfer them and would probably be unable to mutate them
• Attempt to simplify validation diagnostics to a single diagnostic like "Unexpected number of arguments. A count of {0} was expected and {1} was found." This removes some logic from the parser regarding errors for scenarios that people are getting no other validation errors and simplifies the work of the validation subsystem.

The result is that ParseResult is the sole source of critical errors, and PipelineResult is the best source for validation errors.

This is not perfect because the same condition could (and perhaps generally would) result in a different error in ParseResult and PipelineResult. While this is weird, it allows raw parser usage to receive errors we know about while the pipeline validation reporting remains self consistent and in the control of the current validation subsystem. 

I wrote this proposal because my head was screaming every time I saw validation in the parser.