extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-05-30 06:39:33 +02:00
Code Issues Packages Projects Releases Wiki Activity

refactor(parser): use var_id for most constants in ResolvedImportPattern (#14920)

Browse Source 
# Description

This PR replaces most of the constants in `ResolvedImportPattern` from
values to VarIds, this has benefits of:
1. less duplicated variables in state
2. precise span of variable, for example when calling `goto def` on a
const imported by the `use` command, this allows it to find the original
definition, instead of where the `use` command is.

Note that the logic is different here for nested submodules, not all
values are flattened and propagated to the outmost record variable, but
I didn't find any differences in real world usage.

I noticed that it was changed from `VarId` to `Value` in #10049.
Maybe @kubouch can find some edge cases where this PR fails to work as
expected.

In my view, the record constants for `ResolvedImportPattern` should even
reduced to single entry, if not able to get rid of.

# User-Facing Changes

# Tests + Formatting

# After Submitting
This commit is contained in:
zc he 2025-01-26 21:43:34 +08:00 committed by GitHub
parent f88ed6ecd5
commit f46f8b286b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 58 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
crates/nu-lsp/src/ast.rs
View File

@ -296,10 +296,10 @@ fn try_find_id_in_use(
.find_module(name) .find_module(name)
.and_then(|module_id| (module_id == *module_id_ref).then_some(Id::Module(module_id))), .and_then(|module_id| (module_id == *module_id_ref).then_some(Id::Module(module_id))),
None => working_set None => working_set
.find_variable(name) .find_module(name)
.map(Id::Variable) .map(Id::Module)
.or(working_set.find_decl(name).map(Id::Declaration)) .or(working_set.find_decl(name).map(Id::Declaration))
.or(working_set.find_module(name).map(Id::Module)), .or(working_set.find_variable(name).map(Id::Variable)),
_ => None, _ => None,
}; };
let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos)); let check_location = |span: &Span| location.map_or(true, |pos| span.contains(*pos));

11
crates/nu-parser/src/parse_keywords.rs
View File

@ -2468,7 +2468,11 @@ pub fn parse_use(
let mut constants = vec![]; let mut constants = vec![];
for (name, const_val) in definitions.constants { for (name, const_vid) in definitions.constants {
constants.push((name, const_vid));
}
for (name, const_val) in definitions.constant_values {
let const_var_id = let const_var_id =
working_set.add_variable(name.clone(), name_span, const_val.get_type(), false); working_set.add_variable(name.clone(), name_span, const_val.get_type(), false);
working_set.set_variable_const_val(const_var_id, const_val); working_set.set_variable_const_val(const_var_id, const_val);
@ -2967,7 +2971,10 @@ pub fn parse_overlay_use(working_set: &mut StateWorkingSet, call: Box<Call>) ->
) )
} }
} else { } else {
(ResolvedImportPattern::new(vec![], vec![], vec![]), vec![]) (
ResolvedImportPattern::new(vec![], vec![], vec![], vec![]),
vec![],
)
}; };
if errors.is_empty() { if errors.is_empty() {

65
crates/nu-protocol/src/module.rs
View File

@ -9,19 +9,24 @@ use indexmap::IndexMap;
pub struct ResolvedImportPattern { pub struct ResolvedImportPattern {
pub decls: Vec<(Vec<u8>, DeclId)>, pub decls: Vec<(Vec<u8>, DeclId)>,
pub modules: Vec<(Vec<u8>, ModuleId)>, pub modules: Vec<(Vec<u8>, ModuleId)>,
pub constants: Vec<(Vec<u8>, Value)>, pub constants: Vec<(Vec<u8>, VarId)>,
/// TODO: for referencing module name as a record, e.g. `$module_name.const_name`
/// values got multiple duplicates in memory.
pub constant_values: Vec<(Vec<u8>, Value)>,
} }
impl ResolvedImportPattern { impl ResolvedImportPattern {
pub fn new( pub fn new(
decls: Vec<(Vec<u8>, DeclId)>, decls: Vec<(Vec<u8>, DeclId)>,
modules: Vec<(Vec<u8>, ModuleId)>, modules: Vec<(Vec<u8>, ModuleId)>,
constants: Vec<(Vec<u8>, Value)>, constants: Vec<(Vec<u8>, VarId)>,
constant_values: Vec<(Vec<u8>, Value)>,
) -> Self { ) -> Self {
ResolvedImportPattern { ResolvedImportPattern {
decls, decls,
modules, modules,
constants, constants,
constant_values,
} }
} }
} }
@ -123,6 +128,7 @@ impl Module {
} else { } else {
// Import pattern was just name without any members // Import pattern was just name without any members
let mut decls = vec![]; let mut decls = vec![];
let mut const_vids = vec![];
let mut const_rows = vec![]; let mut const_rows = vec![];
let mut errors = vec![]; let mut errors = vec![];
@ -148,14 +154,18 @@ impl Module {
decls.push((new_name, sub_decl_id)); decls.push((new_name, sub_decl_id));
} }
const_rows.extend(sub_results.constants); const_vids.extend(sub_results.constants);
const_rows.extend(sub_results.constant_values);
} }
decls.extend(self.decls_with_head(&final_name)); decls.extend(self.decls_with_head(&final_name));
for (name, var_id) in self.consts() { for (name, var_id) in self.consts() {
match working_set.get_constant(var_id) { match working_set.get_constant(var_id) {
Ok(const_val) => const_rows.push((name, const_val.clone())), Ok(const_val) => {
const_vids.push((name.clone(), var_id));
const_rows.push((name, const_val.clone()))
}
Err(err) => errors.push(err), Err(err) => errors.push(err),
} }
} }
@ -163,7 +173,7 @@ impl Module {
let span = self.span.unwrap_or(backup_span); let span = self.span.unwrap_or(backup_span);
// only needs to bring `$module` with a record value if it defines any constants. // only needs to bring `$module` with a record value if it defines any constants.
let constants = if const_rows.is_empty() { let constant_values = if const_rows.is_empty() {
vec![] vec![]
} else { } else {
vec![( vec![(
@ -179,7 +189,12 @@ impl Module {
}; };
return ( return (
ResolvedImportPattern::new(decls, vec![(final_name.clone(), self_id)], constants), ResolvedImportPattern::new(
decls,
vec![(final_name.clone(), self_id)],
const_vids,
constant_values,
),
errors, errors,
); );
}; };
@ -204,32 +219,39 @@ impl Module {
vec![(final_name, main_decl_id)], vec![(final_name, main_decl_id)],
vec![], vec![],
vec![], vec![],
vec![],
), ),
errors, errors,
) )
} else { } else {
( (
ResolvedImportPattern::new(vec![], vec![], vec![]), ResolvedImportPattern::new(vec![], vec![], vec![], vec![]),
vec![ParseError::ExportNotFound(*span)], vec![ParseError::ExportNotFound(*span)],
) )
} }
} else if let Some(decl_id) = self.decls.get(name) { } else if let Some(decl_id) = self.decls.get(name) {
( (
ResolvedImportPattern::new(vec![(name.clone(), *decl_id)], vec![], vec![]), ResolvedImportPattern::new(
vec![(name.clone(), *decl_id)],
vec![],
vec![],
vec![],
),
errors, errors,
) )
} else if let Some(var_id) = self.constants.get(name) { } else if let Some(var_id) = self.constants.get(name) {
match working_set.get_constant(*var_id) { match working_set.get_constant(*var_id) {
Ok(const_val) => ( Ok(_) => (
ResolvedImportPattern::new( ResolvedImportPattern::new(
vec![], vec![],
vec![], vec![],
vec![(name.clone(), const_val.clone())], vec![(name.clone(), *var_id)],
vec![],
), ),
errors, errors,
), ),
Err(err) => ( Err(err) => (
ResolvedImportPattern::new(vec![], vec![], vec![]), ResolvedImportPattern::new(vec![], vec![], vec![], vec![]),
vec![err], vec![err],
), ),
} }
@ -245,7 +267,7 @@ impl Module {
) )
} else { } else {
( (
ResolvedImportPattern::new(vec![], vec![], vec![]), ResolvedImportPattern::new(vec![], vec![], vec![], vec![]),
vec![ParseError::ExportNotFound(*span)], vec![ParseError::ExportNotFound(*span)],
) )
} }
@ -254,6 +276,7 @@ impl Module {
let mut decls = vec![]; let mut decls = vec![];
let mut submodules = vec![]; let mut submodules = vec![];
let mut constants = vec![]; let mut constants = vec![];
let mut constant_values = vec![];
let mut errors = vec![]; let mut errors = vec![];
for (_, id) in &self.submodules { for (_, id) in &self.submodules {
@ -270,23 +293,25 @@ impl Module {
submodules.extend(sub_results.modules); submodules.extend(sub_results.modules);
constants.extend(sub_results.constants); constants.extend(sub_results.constants);
constant_values.extend(sub_results.constant_values);
errors.extend(sub_errors); errors.extend(sub_errors);
} }
decls.extend(self.decls()); decls.extend(self.decls());
constants.extend(self.constants.iter().filter_map(|(name, var_id)| { for (name, var_id) in self.constants.iter() {
match working_set.get_constant(*var_id) { match working_set.get_constant(*var_id) {
Ok(const_val) => Some((name.clone(), const_val.clone())), Ok(_) => {
constants.push((name.clone(), *var_id));
}
Err(err) => { Err(err) => {
errors.push(err); errors.push(err);
None
} }
} }
})); }
submodules.extend(self.submodules()); submodules.extend(self.submodules());
( (
ResolvedImportPattern::new(decls, submodules, constants), ResolvedImportPattern::new(decls, submodules, constants, constant_values),
errors, errors,
) )
} }
@ -294,6 +319,7 @@ impl Module {
let mut decls = vec![]; let mut decls = vec![];
let mut modules = vec![]; let mut modules = vec![];
let mut constants = vec![]; let mut constants = vec![];
let mut constant_values = vec![];
let mut errors = vec![]; let mut errors = vec![];
for (name, span) in names { for (name, span) in names {
@ -307,7 +333,7 @@ impl Module {
decls.push((name.clone(), *decl_id)); decls.push((name.clone(), *decl_id));
} else if let Some(var_id) = self.constants.get(name) { } else if let Some(var_id) = self.constants.get(name) {
match working_set.get_constant(*var_id) { match working_set.get_constant(*var_id) {
Ok(const_val) => constants.push((name.clone(), const_val.clone())), Ok(_) => constants.push((name.clone(), *var_id)),
Err(err) => errors.push(err), Err(err) => errors.push(err),
} }
} else if let Some(submodule_id) = self.submodules.get(name) { } else if let Some(submodule_id) = self.submodules.get(name) {
@ -324,6 +350,7 @@ impl Module {
decls.extend(sub_results.decls); decls.extend(sub_results.decls);
modules.extend(sub_results.modules); modules.extend(sub_results.modules);
constants.extend(sub_results.constants); constants.extend(sub_results.constants);
constant_values.extend(sub_results.constant_values);
errors.extend(sub_errors); errors.extend(sub_errors);
} else { } else {
errors.push(ParseError::ExportNotFound(*span)); errors.push(ParseError::ExportNotFound(*span));
@ -331,7 +358,7 @@ impl Module {
} }
( (
ResolvedImportPattern::new(decls, modules, constants), ResolvedImportPattern::new(decls, modules, constants, constant_values),
errors, errors,
) )
} }