mirror of
https://github.com/nushell/nushell.git
synced 2025-08-09 13:26:01 +02:00
Module: support defining const and use const variables inside of function (#9773)
<!-- if this PR closes one or more issues, you can automatically link the PR with them by using one of the [*linking keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword), e.g. - this PR should close #xxxx - fixes #xxxx you can also mention related issues, PRs or discussions! --> # Description <!-- Thank you for improving Nushell. Please, check our [contributing guide](../CONTRIBUTING.md) and talk to the core team before making major changes. Description of your pull request goes here. **Provide examples and/or screenshots** if your changes affect the user experience. --> Relative: #8248 After this pr, user can define const variable inside a module.  And user can export const variables, the following screenshot shows how it works (it follows https://github.com/nushell/nushell/issues/8248#issuecomment-1637442612):  ## About the change 1. To make module support const, we need to change `parse_module_block` to support `const` keyword. 2. To suport export `const`, we need to make module tracking variables, so we add `variables` attribute to `Module` 3. During eval, the const variable may not exists in `stack`, because we don't eval `const` when we define a module, so we need to find variables which are already registered in `engine_state` ## One more thing to note about the const value. Consider the following code ``` module foo { const b = 3; export def bar [] { $b } } use foo bar const b = 4; bar ``` The result will be 3 (which is defined in module) rather than 4. I think it's expected behavior. It's something like [dynamic binding](https://www.gnu.org/software/emacs/manual/html_node/elisp/Dynamic-Binding-Tips.html) vs [lexical binding](https://www.gnu.org/software/emacs/manual/html_node/elisp/Lexical-Binding.html) in lisp like language, and lexical binding should be right behavior which generates more predicable result, and it doesn't introduce really subtle bugs in nushell code. What if user want dynamic-binding?(For example: the example code returns `4`) There is no way to do this, user should consider passing the value as argument to custom command rather than const. ## TODO - [X] adding tests for the feature. - [X] support export const out of module to use. # User-Facing Changes <!-- List of all changes that impact the user experience here. This helps us keep track of breaking changes. --> # Tests + Formatting <!-- Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect -A clippy::result_large_err` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` to run the tests for the standard library > **Note** > from `nushell` you can also use the `toolkit` as follows > ```bash > use toolkit.nu # or use an `env_change` hook to activate it automatically > toolkit check pr > ``` --> # After Submitting <!-- If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date. -->
This commit is contained in:
@ -940,9 +940,8 @@ pub fn parse_export_in_block(
|
||||
let full_name = if lite_command.parts.len() > 1 {
|
||||
let sub = working_set.get_span_contents(lite_command.parts[1]);
|
||||
match sub {
|
||||
b"alias" | b"def" | b"def-env" | b"extern" | b"extern-wrapped" | b"use" | b"module" => {
|
||||
[b"export ", sub].concat()
|
||||
}
|
||||
b"alias" | b"def" | b"def-env" | b"extern" | b"extern-wrapped" | b"use" | b"module"
|
||||
| b"const" => [b"export ", sub].concat(),
|
||||
_ => b"export".to_vec(),
|
||||
}
|
||||
} else {
|
||||
@ -1000,6 +999,7 @@ pub fn parse_export_in_block(
|
||||
match full_name.as_slice() {
|
||||
b"export alias" => parse_alias(working_set, lite_command, None),
|
||||
b"export def" | b"export def-env" => parse_def(working_set, lite_command, None),
|
||||
b"export const" => parse_const(working_set, &lite_command.parts[1..]),
|
||||
b"export use" => {
|
||||
let (pipeline, _) = parse_use(working_set, &lite_command.parts);
|
||||
pipeline
|
||||
@ -1400,6 +1400,59 @@ pub fn parse_export_in_module(
|
||||
|
||||
result
|
||||
}
|
||||
b"const" => {
|
||||
let pipeline = parse_const(working_set, &spans[1..]);
|
||||
let export_def_decl_id = if let Some(id) = working_set.find_decl(b"export const") {
|
||||
id
|
||||
} else {
|
||||
working_set.error(ParseError::InternalError(
|
||||
"missing 'export const' command".into(),
|
||||
export_span,
|
||||
));
|
||||
return (garbage_pipeline(spans), vec![]);
|
||||
};
|
||||
|
||||
// Trying to warp the 'const' call into the 'export const' in a very clumsy way
|
||||
if let Some(PipelineElement::Expression(
|
||||
_,
|
||||
Expression {
|
||||
expr: Expr::Call(ref def_call),
|
||||
..
|
||||
},
|
||||
)) = pipeline.elements.get(0)
|
||||
{
|
||||
call = def_call.clone();
|
||||
|
||||
call.head = span(&spans[0..=1]);
|
||||
call.decl_id = export_def_decl_id;
|
||||
} else {
|
||||
working_set.error(ParseError::InternalError(
|
||||
"unexpected output from parsing a definition".into(),
|
||||
span(&spans[1..]),
|
||||
));
|
||||
};
|
||||
|
||||
let mut result = vec![];
|
||||
|
||||
if let Some(decl_name_span) = spans.get(2) {
|
||||
let decl_name = working_set.get_span_contents(*decl_name_span);
|
||||
let decl_name = trim_quotes(decl_name);
|
||||
|
||||
if let Some(decl_id) = working_set.find_variable(decl_name) {
|
||||
result.push(Exportable::VarDecl {
|
||||
name: decl_name.to_vec(),
|
||||
id: decl_id,
|
||||
});
|
||||
} else {
|
||||
working_set.error(ParseError::InternalError(
|
||||
"failed to find added variable".into(),
|
||||
span(&spans[1..]),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
result
|
||||
}
|
||||
_ => {
|
||||
working_set.error(ParseError::Expected(
|
||||
"def, def-env, alias, use, module, or extern keyword",
|
||||
@ -1589,6 +1642,9 @@ pub fn parse_module_block(
|
||||
None, // using commands named as the module locally is OK
|
||||
))
|
||||
}
|
||||
b"const" => block
|
||||
.pipelines
|
||||
.push(parse_const(working_set, &command.parts)),
|
||||
b"extern" | b"extern-wrapped" => {
|
||||
block
|
||||
.pipelines
|
||||
@ -1713,6 +1769,9 @@ pub fn parse_module_block(
|
||||
module.add_submodule(name, id);
|
||||
}
|
||||
}
|
||||
Exportable::VarDecl { name, id } => {
|
||||
module.add_variable(name, id);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1730,7 +1789,7 @@ pub fn parse_module_block(
|
||||
}
|
||||
_ => {
|
||||
working_set.error(ParseError::ExpectedKeyword(
|
||||
"def, def-env, extern, alias, use, module, export or export-env keyword".into(),
|
||||
"def, const, def-env, extern, alias, use, module, export or export-env keyword".into(),
|
||||
command.parts[0],
|
||||
));
|
||||
|
||||
@ -2208,7 +2267,7 @@ pub fn parse_use(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline
|
||||
return (garbage_pipeline(spans), vec![]);
|
||||
};
|
||||
|
||||
let (import_pattern, module, module_id) = if let Some(module_id) = import_pattern.head.id {
|
||||
let (mut import_pattern, module, module_id) = if let Some(module_id) = import_pattern.head.id {
|
||||
let module = working_set.get_module(module_id).clone();
|
||||
(
|
||||
ImportPattern {
|
||||
@ -2219,6 +2278,7 @@ pub fn parse_use(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline
|
||||
},
|
||||
members: import_pattern.members,
|
||||
hidden: HashSet::new(),
|
||||
module_name_var_id: None,
|
||||
},
|
||||
module,
|
||||
module_id,
|
||||
@ -2239,6 +2299,7 @@ pub fn parse_use(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline
|
||||
},
|
||||
members: import_pattern.members,
|
||||
hidden: HashSet::new(),
|
||||
module_name_var_id: None,
|
||||
},
|
||||
module,
|
||||
module_id,
|
||||
@ -2276,12 +2337,29 @@ pub fn parse_use(working_set: &mut StateWorkingSet, spans: &[Span]) -> (Pipeline
|
||||
id: *module_id,
|
||||
}),
|
||||
)
|
||||
.chain(
|
||||
definitions
|
||||
.variables
|
||||
.iter()
|
||||
.map(|(name, variable_id)| Exportable::VarDecl {
|
||||
name: name.clone(),
|
||||
id: *variable_id,
|
||||
}),
|
||||
)
|
||||
.collect();
|
||||
|
||||
// Extend the current scope with the module's exportables
|
||||
working_set.use_decls(definitions.decls);
|
||||
working_set.use_modules(definitions.modules);
|
||||
working_set.use_variables(definitions.variables);
|
||||
|
||||
let module_name_var_id = working_set.add_variable(
|
||||
module.name(),
|
||||
module.span.unwrap_or(Span::unknown()),
|
||||
Type::Any,
|
||||
false,
|
||||
);
|
||||
import_pattern.module_name_var_id = Some(module_name_var_id);
|
||||
// Create a new Use command call to pass the import pattern as parser info
|
||||
let import_pattern_expr = Expression {
|
||||
expr: Expr::ImportPattern(import_pattern),
|
||||
@ -2703,7 +2781,7 @@ pub fn parse_overlay_use(working_set: &mut StateWorkingSet, call: Box<Call>) ->
|
||||
)
|
||||
}
|
||||
} else {
|
||||
(ResolvedImportPattern::new(vec![], vec![]), vec![])
|
||||
(ResolvedImportPattern::new(vec![], vec![], vec![]), vec![])
|
||||
};
|
||||
|
||||
if errors.is_empty() {
|
||||
|
@ -2976,6 +2976,7 @@ pub fn parse_import_pattern(working_set: &mut StateWorkingSet, spans: &[Span]) -
|
||||
},
|
||||
members: vec![],
|
||||
hidden: HashSet::new(),
|
||||
module_name_var_id: None,
|
||||
};
|
||||
|
||||
if spans.len() > 1 {
|
||||
|
Reference in New Issue
Block a user