Should `positionals` be opt-in when `strict: true`?

[bakkot]

Elsewhere I've mentioned I think the goal of strict: true should be that you can write a well-behaved script just by calling parseArgs with your options configured, with no further validation required (except validation of the values of value-taking options, which is out of scope). Here by "well-behaved" I mean it will surface an error to the user if they try to use the script in a way the author did not intend.

Right now, if I'm writing a script which does not expect positional arguments, and I do the obvious thing of

let { values } = util.parseArgs({ options });

but the user passes a positional argument, I'm never going to notice, and the user's input is going to be silently dropped. And it's very likely that I'll end up shipping this, because it's probably not going to occur to me to try passing a positional argument and so I will never notice this problem.

That seems contrary to the goal above. So I'd like to suggest having a positionals configuration option, which must be explicitly opted into (when strict: true). For people who want positionals, this ought to be a small hiccup: they'll notice right away that passing a positional argument doesn't work. And for people who don't, they'll get the right behavior. So neither group is at risk of shipping a script which is not well-behaved.

(Good errors help here: e.g., "this script has not been configured to take positional arguments" is clear to users that they're misusing the script, and to authors who want to take positional arguments that they need to configure it to take positional arguments.)

---

Another advantage of having this piece of information is that it lets you improve error messages. If the parser encounters an unknown flag --foo, and the script author has explicitly opted in to positionals, the error message can suggest that if the user intended to pass --foo as a positional argument they can do so by putting -- --foo at the end of the command invocation. But you wouldn't want to suggest this in a script which isn't using positionals.

[ljharb]

That seems pretty reasonable - maybe a “positionalArgs” option, that can be true, or an integer?

[bakkot]

Would the integer be a minimum, a maximum, or a precise number of positional arguments to expect? None of those are very satisfying answers, I think.

I suppose you could allow specifying both a minimum and a maximum, at the cost of even more config knobs.

[ljharb]

I’d expect a maximum, but yes, it could instead be true, or an object with min/max integer properties.

[shadowspawn]

Quick comment. Good description! However, I don't like adding configuration to use basic features of the parser, so still thinking about it.

[bakkot]

However, I don't like adding configuration to use basic features of the parser

Well, the other half of that is that absent this config options, scripts which don't want positionals but still want to be well-behaved will need to add extra logic to use the parser at all: const results = parseArgs({options}); if (result.positionals.length > 0) throw new Error('this script does not take positional arguments');.

So one of the two groups (expecting-positionals and not-expecting-positionals) is inevitably going to have to add something, to get the behavior they want and be well-behaved.

Not being able to configure this amounts to deciding that the not-expecting-positionals group is the one which will need to add something. Which is fine, in general, but my concern is that in this particular case that group is not even going to notice they needed to add something to be well-behaved, and consequently ship a poorly-behaved script, which is (I think) a bad outcome.

(And, separately, having this bit would allow better error messages, which I think is valuable.)

[shadowspawn]

Here by "well-behaved" I mean it will surface an error to the user if they try to use the script in a way the author did not intend.

This is a thinking-out-loud comment, not fully formed. I like the concept, but I am concerned it is a slippery slope of diminishing returns. At what point do we say no?

---

A deliberately over-stated description of "well-behaved", but where do we draw the line?

• We just need one more piece of configuration for situation x, then we could detect and display an error. And it has to be on by default or does not achieve "well-behaved" by default. So it isn't just a config knob, it is a required config knob for usage not covered by the default or not able to have a default.

• And we should block some standard functionality, because it allows us to detect more errors. (Like blocking --foo VALUE.)

The extreme end-point is a safe "well-behaved" subset of functionality that requires extensive configuration and does not behave like other programs. And tutorials will say "use strict:false to simplify setup"!

Ok, please resume normal levels of cynicism and scepticism.

[ljharb]

Why must there be a point where we say no? Zero config strict mode means that any time there’s something we discover is a footgun, we’d want to make the default behavior safe, even if that means common cases would require more configuration.

[shadowspawn]

Would the integer be a minimum, a maximum, or a precise number of positional arguments to expect? None of those are very satisfying answers, I think.

I suppose you could allow specifying both a minimum and a maximum, at the cost of even more config knobs.

Some common use cases are:

• no positionals. e.g. ls
• an exact number of positionals. e.g. copy source dest
• variadic, possibly with a minimum e.g. print *.js

[shadowspawn]

Zero config strict mode means that any time there’s something we discover is a footgun, we’d want to make the default behavior safe, even if that means common cases would require more configuration.

I don't think "zero config" belongs in that sentence. 😉

[shadowspawn]

Why must there be a point where we say no?

Ok, I'll bite.

Because there isn't agreement there is demand or a need for a bullet-proof argument parser built into node. That wasn't in the previous proposal, or the initial spec here, or the current spec.

Because each additional piece of safety configuration offers the potential to detect one more footgun, but makes the library harder to get started with, or less convenient for the end user. As the starting configuration gets more intimidating, the chances people start from a boiler-plate starting configuration which bypasses the protections increase. Or people use strict:false. Or people go elsewhere and the adoption of the library is lowered.

I care about the cost-benefit ratio. I think there is often a trade-off between safety and usability and some protections are not worth the penalties.

To be clear, I opened the issue about whether there is going to be a strict mode at all, and I would prefer to include a strict mode in the first implementation. But not bullet-proof.

[bakkot]

I am concerned it is a slippery slope of diminishing returns. At what point do we say no?

Yeah, I agree this slope is pretty slippery. I think the heuristic I would use for strict: true is something like:

• if authors in the most common scenario would reasonably expect the validation to happen (e.g. any type: 'string' option is given a value), that validation should definitely happen
• if authors in some common scenario are unlikely to notice they need to do extra validation to meet my definition of well-behaved, that validation should probably happen by default (this issue)
• other kinds of validation should not happen by default, or for that matter at all (e.g. validation that the values for particular options parse as numbers: because script authors must parse strings to numbers themselves, it should be obvious they need to verify that the format is as they expect)

The third thing, in particular, means that there are always going to be scripts which are not well-behaved by my definition without the author performing additional validation. That's OK, as long as it's reasonably obvious to authors that they are going to need to do that validation themselves.

I think having a boolean for positionals is the right balance on this issue. It gets rid of the sharp edge of "it didn't occur to me to check for and reject positionals" (which is a very easy mistake to make). And once you've explicitly opted in to getting an array of positional arguments, hopefully at that point you're going to think to check if you got too few or too many (if those concepts are relevant to your application). Some people still won't do those checks, but you can't save everyone.

I could see an argument [heh] for having minPositionals and maxPositionals, both defaulting to 0. I'm not advocating that, to be clear, although I'm fine with that as a resolution to this issue. But I personally can't think any other kinds of validation beyond that (other than those already planned) which would fall into either of my first two bullet points, which makes me feel better about stopping at exactly this point on the slippery slope, and no further.

---

This of course hinges on author expectations, and in particular how many people we think are going to run into a given sharp edge. I think it's going to be pretty common to write applications which aren't expecting any positional arguments, so I think that's a scenario we should weight pretty highly.

There's always going to be someone out there who is surprised by, say, the fact that --foo=false gives them the string false, but I think that's a more minority case - there's just not that many applications where this behavior would result in a bug and the author probably isn't going to think check for it. So I don't think that needs to be validated by default, even though there will be some number of scripts which end up being not-well-behaved for lack of validation here.

[shadowspawn]

Nice heuristic. Thanks for taking the time to lay it out!

I am up for an opt-in boolean for positionals.

There's always going to be someone out there who is surprised by, say, the fact that --foo=false gives them the string false,

If foo is type:boolean I think this will be an error in strict mode, which of particular note addresses the --explode=false concerns raised in previous proposal discussion.