make module name parsing more strict

# User-Facing Changes

- Module names must now be valid identifiers, with an exception for
  hyphens:

```nushell
module foo/bar {} # × expected valid module name
module foo-bar {} # 
```
- Variable access in modules named with hyphens no longer causes
  "expected valid variable name" errors:

```nushell
module foo-bar { export const baz = 1 }
use foo-bar
$foo-bar.baz
```
This commit is contained in:
Solomon Victorino 2024-11-15 10:52:22 -07:00
parent a84d410f11
commit ee2f246216
5 changed files with 44 additions and 15 deletions

View File

@ -191,7 +191,7 @@ fn parse_module(
let new_span = working_set.get_span_for_file(file_id);
let starting_error_count = working_set.parse_errors.len();
parse_module_block(working_set, new_span, filename.as_bytes());
parse_module_block(working_set, new_span, &filename.into_spanned(call_head));
check_parse(
starting_error_count,

View File

@ -1,7 +1,7 @@
use crate::{
exportable::Exportable,
parse_block,
parser::{parse_redirection, redirecting_builtin_error},
parser::{is_module_name, parse_redirection, redirecting_builtin_error},
type_check::{check_block_input_output, type_compatible},
};
use itertools::Itertools;
@ -15,8 +15,8 @@ use nu_protocol::{
engine::{StateWorkingSet, DEFAULT_OVERLAY_NAME},
eval_const::eval_constant,
parser_path::ParserPath,
Alias, BlockId, DeclId, Module, ModuleId, ParseError, PositionalArg, ResolvedImportPattern,
Span, Spanned, SyntaxShape, Type, Value, VarId,
Alias, BlockId, DeclId, IntoSpanned, Module, ModuleId, ParseError, PositionalArg,
ResolvedImportPattern, Span, Spanned, SyntaxShape, Type, Value, VarId,
};
use std::{
collections::{HashMap, HashSet},
@ -1715,7 +1715,7 @@ fn collect_first_comments(tokens: &[Token]) -> Vec<Span> {
pub fn parse_module_block(
working_set: &mut StateWorkingSet,
span: Span,
module_name: &[u8],
module_name: &Spanned<String>,
) -> (Block, Module, Vec<Span>) {
working_set.enter_scope();
@ -1739,6 +1739,11 @@ pub fn parse_module_block(
}
}
if !is_module_name(module_name.item.as_bytes()) {
working_set.error(ParseError::Expected("valid module name", module_name.span));
}
let module_name = module_name.item.as_bytes();
let mut module = Module::from_span(module_name.to_vec(), span);
let mut block = Block::new_with_capacity(output.block.len());
@ -1948,13 +1953,13 @@ fn parse_module_file(
working_set: &mut StateWorkingSet,
path: ParserPath,
path_span: Span,
name_override: Option<String>,
name_override: Option<Spanned<String>>,
) -> Option<ModuleId> {
// Infer the module name from the stem of the file, unless overridden.
let module_name = if let Some(name) = name_override {
name
} else if let Some(stem) = path.file_stem() {
stem.to_string_lossy().to_string()
stem.to_string_lossy().to_string().into_spanned(path_span)
} else {
working_set.error(ParseError::ModuleNotFound(
path_span,
@ -1992,14 +1997,14 @@ fn parse_module_file(
// Parse the module
let (block, mut module, module_comments) =
parse_module_block(working_set, new_span, module_name.as_bytes());
parse_module_block(working_set, new_span, &module_name);
// Remove the file from the stack of files being processed.
working_set.files.pop();
let _ = working_set.add_block(Arc::new(block));
module.file = Some((path, file_id));
let module_id = working_set.add_module(&module_name, module, module_comments);
let module_id = working_set.add_module(&module_name.item, module, module_comments);
Some(module_id)
}
@ -2008,7 +2013,7 @@ pub fn parse_module_file_or_dir(
working_set: &mut StateWorkingSet,
path: &[u8],
path_span: Span,
name_override: Option<String>,
name_override: Option<Spanned<String>>,
) -> Option<ModuleId> {
let (module_path_str, err) = unescape_unquote_string(path, path_span);
if let Some(err) = err {
@ -2037,7 +2042,7 @@ pub fn parse_module_file_or_dir(
};
let module_name = if let Some(stem) = module_path.file_stem() {
stem.to_string_lossy().to_string()
stem.to_string_lossy().to_string().into_spanned(path_span)
} else {
working_set.error(ParseError::ModuleNotFound(
path_span,
@ -2250,8 +2255,11 @@ pub fn parse_module(
let block_span = Span::new(start, end);
let (block, module, inner_comments) =
parse_module_block(working_set, block_span, module_name.as_bytes());
let (block, module, inner_comments) = parse_module_block(
working_set,
block_span,
&module_name.clone().into_spanned(module_name_or_path_span),
);
let block_id = working_set.add_block(Arc::new(block));
@ -2893,7 +2901,7 @@ pub fn parse_overlay_use(working_set: &mut StateWorkingSet, call: Box<Call>) ->
working_set,
overlay_name.as_bytes(),
overlay_name_span,
new_name.as_ref().map(|spanned| spanned.item.clone()),
new_name.clone(),
) {
// try file or directory
let new_module = working_set.get_module(module_id).clone();

View File

@ -126,6 +126,12 @@ pub fn is_variable(bytes: &[u8]) -> bool {
}
}
pub fn is_module_name(bytes: &[u8]) -> bool {
bytes
.iter()
.all(|x| is_identifier_byte(*x) && *x != b'\\' || *x == b'-')
}
pub fn trim_quotes(bytes: &[u8]) -> &[u8] {
if (bytes.starts_with(b"\"") && bytes.ends_with(b"\"") && bytes.len() > 1)
|| (bytes.starts_with(b"\'") && bytes.ends_with(b"\'") && bytes.len() > 1)
@ -5559,7 +5565,7 @@ pub fn parse_expression(working_set: &mut StateWorkingSet, spans: &[Span]) -> Ex
pub fn parse_variable(working_set: &mut StateWorkingSet, span: Span) -> Option<VarId> {
let bytes = working_set.get_span_contents(span);
if is_variable(bytes) {
if is_variable(bytes) || is_module_name(bytes) {
working_set.find_variable(bytes)
} else {
working_set.error(ParseError::Expected("valid variable name", span));

View File

@ -145,6 +145,14 @@ fn export_module_which_defined_const() -> TestResult {
)
}
#[test]
fn export_module_with_hyphen_in_name() -> TestResult {
run_test(
r#"module spam-mod { export const b = 3; }; use spam-mod; $spam-mod.b"#,
"3",
)
}
#[test]
fn cannot_export_private_const() -> TestResult {
fail_test(

View File

@ -186,6 +186,13 @@ fn bad_var_name3() -> TestResult {
fail_test(r#"=foo=4 true"#, "Command `=foo=4` not found")
}
#[test]
fn bad_module_name() -> TestResult {
fail_test(r#"module foo/bar {}"#, "valid module name")?;
fail_test(r#"module foo\bar {}"#, "valid module name")?;
fail_test(r#"module foo=bar {}"#, "valid module name")
}
#[test]
fn assignment_with_no_var() -> TestResult {
let cases = [