ast: Require --help to parse more keywords as decorated statement (#10000)

This makes it so

```fish
if -e foo
    # do something
end
```

complains about `-e` not being a command instead of `end` being used
outside of an if-block.

That means both that `-e` could now be used as a command name (it
already can outside of `if`!) *and* that we get a better error!

The only way to get `if` to be a decorated statement now is to use `if
-h` or `if --help` specifically (with a literal option).

The same goes for switch, while and begin.

It would be possible, alternatively, to disallow `if -e` and point
towards using `test` instead, but the "unknown command" message should
already point towards using `test` more than pointing at the
"end" (that might be quite far away).
This commit is contained in:
Fabian Boehm 2023-09-19 17:34:13 +02:00 committed by GitHub
parent 6194899c6b
commit 4d2f7b0c0d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 31 additions and 8 deletions

View file

@ -3503,20 +3503,27 @@ impl<'s> Populator<'s> {
return got_error(self);
}
// The only block-like builtin that takes any parameters is 'function'. So go to decorated
// statements if the subsequent token looks like '--'. The logic here is subtle:
// In some cases a block starter is a decorated statement instead, mostly if invoked with "--help".
// The logic here is subtle:
//
// If we are 'begin', then we expect to be invoked with no arguments.
// If we are 'function', then we are a non-block if we are invoked with -h or --help
// If we are 'begin', it's only really a block if it has no arguments.
// If we are 'function' or another block starter, then we are a non-block if we are invoked with -h or --help
// If we are anything else, we require an argument, so do the same thing if the subsequent
// token is a statement terminator.
if self.peek_token(0).typ == ParseTokenType::string {
// If we are a function, then look for help arguments. Otherwise, if the next token
// If we are one of these, then look for specifically help arguments. Otherwise, if the next token
// looks like an option (starts with a dash), then parse it as a decorated statement.
if (self.peek_token(0).keyword == ParseKeyword::kw_function
let help_only_kws = [
ParseKeyword::kw_begin,
ParseKeyword::kw_function,
ParseKeyword::kw_if,
ParseKeyword::kw_switch,
ParseKeyword::kw_while,
];
if (help_only_kws.contains(&self.peek_token(0).keyword)
&& self.peek_token(1).is_help_argument)
|| (self.peek_token(0).keyword != ParseKeyword::kw_function
&& self.peek_token(1).has_dash_prefix)
|| (!help_only_kws.contains(&self.peek_token(0).keyword)
&& self.peek_token(1).is_dash_prefix_string())
{
return new_decorated_statement(self);
}

View file

@ -73,3 +73,19 @@ echo "bind -M" | $fish
# CHECKERR: ^
# CHECKERR: (Type 'help bind' for related documentation)
$fish -c 'if -e; end'
# CHECKERR: fish: Unknown command: -e
# CHECKERR: fish:
# CHECKERR: if -e; end
# CHECKERR: ^^
$fish -c 'begin --notanoption; end'
# CHECKERR: fish: Unknown command: --notanoption
# CHECKERR: fish:
# CHECKERR: begin --notanoption; end
# CHECKERR: ^~~~~~~~~~~~^
$fish -c 'begin --help'
# CHECKERR: fish: begin: missing man page
# CHECKERR: Documentation may not be installed.
# CHECKERR: `help begin` will show an online version