Files
nushell/crates/nu-engine/src/compile/call.rs
132ikl 8f9aa1a250 Change help commands to use name from scope instead of the name from the declaration (#14490)
# Description

Before this PR, `help commands` uses the name from a command's
declaration rather than the name in the scope. This is problematic when
trying to view the help page for the `main` command of a module. For
example, `std bench`:

```nushell
use std/bench
help bench
# => Error: nu::parser::not_found
# => 
# =>   × Not found.
# =>    ╭─[entry #10:1:6]
# =>  1 │ help bench
# =>    ·      ──┬──
# =>    ·        ╰── did not find anything under this name
# =>    ╰────
```

This can also cause confusion when importing specific commands from
modules. Furthermore, if there are multiple commands with the same name
from different modules, the help text for _both_ will appear when
querying their help text (this is especially problematic for `main`
commands, see #14033):

```nushell
use std/iter
help iter find
# => Error: nu::parser::not_found
# => 
# =>   × Not found.
# =>    ╭─[entry #3:1:6]
# =>  1│ help iter find
# =>    ·      ────┬────
# =>    ·          ╰── did not find anything under this name
# =>    ╰────
help find
# => Searches terms in the input.
# => 
# => Search terms: filter, regex, search, condition
# => 
# => Usage:
# =>   > find {flags} ...(rest) 
# [...]
# => Returns the first element of the list that matches the
# => closure predicate, `null` otherwise
# [...]
# (full text omitted for brevity)
```

This PR changes `help commands` to use the name as it is in scope, so
prefixing any command in scope with `help` will show the correct help
text.


```nushell
use std/bench
help bench
# [help text for std bench]
use std/iter
help iter find
# [help text for std iter find]

use std
help std bench
# [help text for std bench]
help std iter find
# [help text for std iter find]
```

Additionally, the IR code generation for commands called with the
`--help` text has been updated to reflect this change.

This does have one side effect: when a module has a `main` command
defined, running `help <name>` (which checks `help aliases`, then `help
commands`, then `help modules`) will show the help text for the `main`
command rather than the module. The help text for the module is still
accessible with `help modules <name>`.

Fixes #10499, #10311, #11609, #13470, #14033, and #14402.
Partially fixes #10707.
Does **not** fix #11447.

# User-Facing Changes

* Help text for commands can be obtained by running `help <command
name>`, where the command name is the same thing you would type in order
to execute the command. Previously, it was the name of the function as
written in the source file.
  * For example, for the following module `spam` with command `meow`:
    ```nushell
    module spam { 
        # help text
        export def meow [] {}
    }
    ```
    * Before this PR:
* Regardless of how `meow` is `use`d, the help text is viewable by
running `help meow`.
    * After this PR:
* When imported with `use spam`: The `meow` command is executed by
running `spam meow` and the `help` text is viewable by running `help
spam meow`.
* When imported with `use spam foo`: The `meow` command is executed by
running `meow` and the `help` text is viewable by running `meow`.
* When a module has a `main` command defined, `help <module name>` will
return help for the main command, rather than the module. To access the
help for the module, use `help modules <module name>`.

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`

# After Submitting
N/A
2024-12-10 09:27:30 -06:00

273 lines
9.3 KiB
Rust

use std::sync::Arc;
use nu_protocol::{
ast::{Argument, Call, Expression, ExternalArgument},
engine::StateWorkingSet,
ir::{Instruction, IrAstRef, Literal},
IntoSpanned, RegId, Span, Spanned,
};
use super::{compile_expression, keyword::*, BlockBuilder, CompileError, RedirectModes};
pub(crate) fn compile_call(
working_set: &StateWorkingSet,
builder: &mut BlockBuilder,
call: &Call,
redirect_modes: RedirectModes,
io_reg: RegId,
) -> Result<(), CompileError> {
let decl = working_set.get_decl(call.decl_id);
// Check if this call has --help - if so, just redirect to `help`
if call.named_iter().any(|(name, _, _)| name.item == "help") {
let name = working_set
.find_decl_name(call.decl_id) // check for name in scope
.and_then(|name| std::str::from_utf8(name).ok())
.unwrap_or(decl.name()); // fall back to decl's name
return compile_help(working_set, builder, name.into_spanned(call.head), io_reg);
}
// Try to figure out if this is a keyword call like `if`, and handle those specially
if decl.is_keyword() {
match decl.name() {
"if" => {
return compile_if(working_set, builder, call, redirect_modes, io_reg);
}
"match" => {
return compile_match(working_set, builder, call, redirect_modes, io_reg);
}
"const" => {
// This differs from the behavior of the const command, which adds the const value
// to the stack. Since `load-variable` also checks `engine_state` for the variable
// and will get a const value though, is it really necessary to do that?
return builder.load_empty(io_reg);
}
"alias" => {
// Alias does nothing
return builder.load_empty(io_reg);
}
"let" | "mut" => {
return compile_let(working_set, builder, call, redirect_modes, io_reg);
}
"try" => {
return compile_try(working_set, builder, call, redirect_modes, io_reg);
}
"loop" => {
return compile_loop(working_set, builder, call, redirect_modes, io_reg);
}
"while" => {
return compile_while(working_set, builder, call, redirect_modes, io_reg);
}
"for" => {
return compile_for(working_set, builder, call, redirect_modes, io_reg);
}
"break" => {
return compile_break(working_set, builder, call, redirect_modes, io_reg);
}
"continue" => {
return compile_continue(working_set, builder, call, redirect_modes, io_reg);
}
"return" => {
return compile_return(working_set, builder, call, redirect_modes, io_reg);
}
"def" | "export def" => {
return builder.load_empty(io_reg);
}
_ => (),
}
}
// Keep AST if the decl needs it.
let requires_ast = decl.requires_ast_for_arguments();
// It's important that we evaluate the args first before trying to set up the argument
// state for the call.
//
// We could technically compile anything that isn't another call safely without worrying about
// the argument state, but we'd have to check all of that first and it just isn't really worth
// it.
enum CompiledArg<'a> {
Positional(RegId, Span, Option<IrAstRef>),
Named(
&'a str,
Option<&'a str>,
Option<RegId>,
Span,
Option<IrAstRef>,
),
Spread(RegId, Span, Option<IrAstRef>),
}
let mut compiled_args = vec![];
for arg in &call.arguments {
let arg_reg = arg
.expr()
.map(|expr| {
let arg_reg = builder.next_register()?;
compile_expression(
working_set,
builder,
expr,
RedirectModes::value(arg.span()),
None,
arg_reg,
)?;
Ok(arg_reg)
})
.transpose()?;
let ast_ref = arg
.expr()
.filter(|_| requires_ast)
.map(|expr| IrAstRef(Arc::new(expr.clone())));
match arg {
Argument::Positional(_) | Argument::Unknown(_) => {
compiled_args.push(CompiledArg::Positional(
arg_reg.expect("expr() None in non-Named"),
arg.span(),
ast_ref,
))
}
Argument::Named((name, short, _)) => compiled_args.push(CompiledArg::Named(
&name.item,
short.as_ref().map(|spanned| spanned.item.as_str()),
arg_reg,
arg.span(),
ast_ref,
)),
Argument::Spread(_) => compiled_args.push(CompiledArg::Spread(
arg_reg.expect("expr() None in non-Named"),
arg.span(),
ast_ref,
)),
}
}
// Now that the args are all compiled, set up the call state (argument stack and redirections)
for arg in compiled_args {
match arg {
CompiledArg::Positional(reg, span, ast_ref) => {
builder.push(Instruction::PushPositional { src: reg }.into_spanned(span))?;
builder.set_last_ast(ast_ref);
}
CompiledArg::Named(name, short, Some(reg), span, ast_ref) => {
if !name.is_empty() {
let name = builder.data(name)?;
builder.push(Instruction::PushNamed { name, src: reg }.into_spanned(span))?;
} else {
let short = builder.data(short.unwrap_or(""))?;
builder
.push(Instruction::PushShortNamed { short, src: reg }.into_spanned(span))?;
}
builder.set_last_ast(ast_ref);
}
CompiledArg::Named(name, short, None, span, ast_ref) => {
if !name.is_empty() {
let name = builder.data(name)?;
builder.push(Instruction::PushFlag { name }.into_spanned(span))?;
} else {
let short = builder.data(short.unwrap_or(""))?;
builder.push(Instruction::PushShortFlag { short }.into_spanned(span))?;
}
builder.set_last_ast(ast_ref);
}
CompiledArg::Spread(reg, span, ast_ref) => {
builder.push(Instruction::AppendRest { src: reg }.into_spanned(span))?;
builder.set_last_ast(ast_ref);
}
}
}
// Add any parser info from the call
for (name, info) in &call.parser_info {
let name = builder.data(name)?;
let info = Box::new(info.clone());
builder.push(Instruction::PushParserInfo { name, info }.into_spanned(call.head))?;
}
if let Some(mode) = redirect_modes.out {
builder.push(mode.map(|mode| Instruction::RedirectOut { mode }))?;
}
if let Some(mode) = redirect_modes.err {
builder.push(mode.map(|mode| Instruction::RedirectErr { mode }))?;
}
// The state is set up, so we can do the call into io_reg
builder.push(
Instruction::Call {
decl_id: call.decl_id,
src_dst: io_reg,
}
.into_spanned(call.head),
)?;
Ok(())
}
pub(crate) fn compile_help(
working_set: &StateWorkingSet<'_>,
builder: &mut BlockBuilder,
decl_name: Spanned<&str>,
io_reg: RegId,
) -> Result<(), CompileError> {
let help_command_id =
working_set
.find_decl(b"help")
.ok_or_else(|| CompileError::MissingRequiredDeclaration {
decl_name: "help".into(),
span: decl_name.span,
})?;
let name_data = builder.data(decl_name.item)?;
let name_literal = builder.literal(decl_name.map(|_| Literal::String(name_data)))?;
builder.push(Instruction::PushPositional { src: name_literal }.into_spanned(decl_name.span))?;
builder.push(
Instruction::Call {
decl_id: help_command_id,
src_dst: io_reg,
}
.into_spanned(decl_name.span),
)?;
Ok(())
}
pub(crate) fn compile_external_call(
working_set: &StateWorkingSet,
builder: &mut BlockBuilder,
head: &Expression,
args: &[ExternalArgument],
redirect_modes: RedirectModes,
io_reg: RegId,
) -> Result<(), CompileError> {
// Pass everything to run-external
let run_external_id = working_set
.find_decl(b"run-external")
.ok_or(CompileError::RunExternalNotFound { span: head.span })?;
let mut call = Call::new(head.span);
call.decl_id = run_external_id;
call.arguments.push(Argument::Positional(head.clone()));
for arg in args {
match arg {
ExternalArgument::Regular(expr) => {
call.arguments.push(Argument::Positional(expr.clone()));
}
ExternalArgument::Spread(expr) => {
call.arguments.push(Argument::Spread(expr.clone()));
}
}
}
compile_call(working_set, builder, &call, redirect_modes, io_reg)
}