From 58fd1d1d2cadec6fb2cc6b85df61727db56a2ad2 Mon Sep 17 00:00:00 2001 From: Adam Welc Date: Thu, 8 Aug 2024 02:00:29 +0100 Subject: [PATCH] [move-compler] Added parser resilience for use declarations (#18879) ## Description This PR adds parsing resilience when parsing use declarations. The idea is to recognize partially parsed statements (examples below and also in the new test) to enable auto-completion in the IDE. ``` use a::m2:: use a::m2::{foo use a::m2::{foo, bar use a::{m2::{foo, bar use a::{m2::{foo, bar}, m3 ``` ## Test plan New and all tests must pass --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [x] CLI: Additional compiler errors for incomplete name access chains (such as `use some_pkg::some_module::`) might appear in the compiler output. - [ ] Rust SDK: - [ ] REST API: --- .../move/crates/move-analyzer/src/symbols.rs | 2 + .../move-compiler/src/expansion/translate.rs | 9 + .../crates/move-compiler/src/parser/ast.rs | 35 ++++ .../crates/move-compiler/src/parser/syntax.rs | 167 +++++++++++++----- .../move_2024/ide_mode/use_incomplete.exp | 77 ++++++++ .../move_2024/ide_mode/use_incomplete.move | 49 +++++ ...ule_member_invalid_missing_close_brace.exp | 8 +- ...le_member_invalid_missing_close_brace.move | 8 +- ...odule_member_invalid_missing_semicolon.exp | 12 +- ...dule_member_invalid_missing_semicolon.move | 8 +- 10 files changed, 312 insertions(+), 63 deletions(-) create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.exp create mode 100644 external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.move diff --git a/external-crates/move/crates/move-analyzer/src/symbols.rs b/external-crates/move/crates/move-analyzer/src/symbols.rs index 9cb720c35b248..fdeebc948436a 100644 --- a/external-crates/move/crates/move-analyzer/src/symbols.rs +++ b/external-crates/move/crates/move-analyzer/src/symbols.rs @@ -2889,6 +2889,7 @@ impl<'a> ParsingSymbolicator<'a> { self.chain_symbols(function); self.chain_symbols(ty); } + P::Use::Partial { .. } => (), } } @@ -2928,6 +2929,7 @@ impl<'a> ParsingSymbolicator<'a> { self.use_decl_member_symbols(mod_ident_str.clone(), name, alias_opt); } } + P::ModuleUse::Partial { .. } => (), } } diff --git a/external-crates/move/crates/move-compiler/src/expansion/translate.rs b/external-crates/move/crates/move-compiler/src/expansion/translate.rs index 7d4230b6cf5cc..093a28e8f9af0 100644 --- a/external-crates/move/crates/move-compiler/src/expansion/translate.rs +++ b/external-crates/move/crates/move-compiler/src/expansion/translate.rs @@ -1574,6 +1574,7 @@ fn use_( }; use_funs.explicit.push(explicit); } + P::Use::Partial { .. } => (), // no actual module to process } } @@ -1703,6 +1704,14 @@ fn module_use( } } } + P::ModuleUse::Partial { .. } => { + let mident = module_ident(&mut context.defn_context, in_mident); + if !context.defn_context.module_members.contains_key(&mident) { + context.env().add_diag(unbound_module(&mident)); + return; + }; + add_module_alias!(mident, mident.value.module.0) + } } } diff --git a/external-crates/move/crates/move-compiler/src/parser/ast.rs b/external-crates/move/crates/move-compiler/src/parser/ast.rs index a4378ed18a950..edfddcb304f2b 100644 --- a/external-crates/move/crates/move-compiler/src/parser/ast.rs +++ b/external-crates/move/crates/move-compiler/src/parser/ast.rs @@ -104,12 +104,31 @@ pub enum Use { ty: Box, method: Name, }, + // used for one of the three cases when `LeadingNameAccess` represents `some_pkg` + // - `some_pkg` + // - `some_pkg::` + // - `some_pkg::{` + // where first location represents `::` and the second one represents `{` + Partial { + package: LeadingNameAccess, + colon_colon: Option, + opening_brace: Option, + }, } #[derive(Debug, PartialEq, Clone, Eq)] pub enum ModuleUse { Module(Option), Members(Vec<(Name, Option)>), + // used for one of the three cases when defining `ModuleUse` for `some_mod`: + // - `... some_mod` + // - `... some_mod::` + // - `... some_mod::{` + // where first location represents `::` and the second one represents `{` + Partial { + colon_colon: Option, + opening_brace: Option, + }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -1489,6 +1508,13 @@ impl AstDebug for ModuleUse { alias.map(|alias| w.write(&format!("as {}", alias.value))); }) }), + ModuleUse::Partial { + colon_colon, + opening_brace, + } => { + colon_colon.map(|_| w.write("::")); + opening_brace.map(|_| w.write("{")); + } } } } @@ -1523,6 +1549,15 @@ impl AstDebug for Use { ty.ast_debug(w); w.write(format!(".{method}")); } + Use::Partial { + package, + colon_colon, + opening_brace, + } => { + w.write(package.to_string()); + colon_colon.map(|_| w.write("::")); + opening_brace.map(|_| w.write("{")); + } } w.write(";") } diff --git a/external-crates/move/crates/move-compiler/src/parser/syntax.rs b/external-crates/move/crates/move-compiler/src/parser/syntax.rs index 74f3e75c80422..e31d1bb8c6637 100644 --- a/external-crates/move/crates/move-compiler/src/parser/syntax.rs +++ b/external-crates/move/crates/move-compiler/src/parser/syntax.rs @@ -1499,12 +1499,8 @@ fn parse_sequence(context: &mut Context) -> Result> { let mut uses = vec![]; while context.tokens.peek() == Tok::Use { let start_loc = context.tokens.start_loc(); - uses.push(parse_use_decl( - vec![], - start_loc, - Modifiers::empty(), - context, - )?); + let tmp = parse_use_decl(vec![], start_loc, Modifiers::empty(), context)?; + uses.push(tmp); } let mut seq: Vec = vec![]; @@ -4132,45 +4128,88 @@ fn parse_use_decl( } let address_start_loc = context.tokens.start_loc(); let address = parse_leading_name_access(context)?; - consume_token_( + let colon_colon_loc = context.tokens.current_token_loc(); + if let Err(diag) = consume_token_( context.tokens, Tok::ColonColon, start_loc, " after an address in a use declaration", - )?; - match context.tokens.peek() { - Tok::LBrace => { - let parse_inner = |ctxt: &mut Context<'_, '_, '_>| { - let (name, _, use_) = parse_use_module(ctxt)?; - Ok((name, use_)) - }; - let use_decls = parse_comma_list( - context, - Tok::LBrace, - Tok::RBrace, - &TokenSet::from([Tok::Identifier]), - parse_inner, - "a module use clause", - ); - - Use::NestedModuleUses(address, use_decls) + ) { + context.add_diag(*diag); + Use::Partial { + package: address, + colon_colon: None, + opening_brace: None, } - _ => { - let (name, end_loc, use_) = parse_use_module(context)?; - let loc = make_loc(context.tokens.file_hash(), address_start_loc, end_loc); - let module_ident = sp( - loc, - ModuleIdent_ { - address, - module: name, - }, - ); - Use::ModuleUse(module_ident, use_) + } else { + // add `;` to stop set to limit number of eaten tokens if the list is parsed + // incorrectly + context.stop_set.add(Tok::Semicolon); + match context.tokens.peek() { + Tok::LBrace => { + let lbrace_loc = context.tokens.current_token_loc(); + let parse_inner = |ctxt: &mut Context<'_, '_, '_>| { + parse_use_module(ctxt).map(|(name, _, use_)| (name, use_)) + }; + let use_decls = parse_comma_list( + context, + Tok::LBrace, + Tok::RBrace, + &TokenSet::from([Tok::Identifier]), + parse_inner, + "a module use clause", + ); + let use_ = if use_decls.is_empty() { + // empty list does not make much sense as it contains no alias + // information and it actually helps IDE to treat this case as a partial + // use + Use::Partial { + package: address, + colon_colon: Some(colon_colon_loc), + opening_brace: Some(lbrace_loc), + } + } else { + Use::NestedModuleUses(address, use_decls) + }; + context.stop_set.remove(Tok::Semicolon); + use_ + } + _ => { + let use_ = match parse_use_module(context) { + Ok((name, end_loc, use_)) => { + let loc = make_loc( + context.tokens.file_hash(), + address_start_loc, + end_loc, + ); + let module_ident = sp( + loc, + ModuleIdent_ { + address, + module: name, + }, + ); + Use::ModuleUse(module_ident, use_) + } + Err(diag) => { + context.add_diag(*diag); + Use::Partial { + package: address, + colon_colon: Some(colon_colon_loc), + opening_brace: None, + } + } + }; + context.stop_set.remove(Tok::Semicolon); + use_ + } } } } }; - consume_token(context.tokens, Tok::Semicolon)?; + if let Err(diag) = consume_token(context.tokens, Tok::Semicolon) { + context.add_diag(*diag); + } let end_loc = context.tokens.previous_end_loc(); let loc = make_loc(context.tokens.file_hash(), start_loc, end_loc); Ok(UseDecl { @@ -4193,19 +4232,49 @@ fn parse_use_module( let alias_opt = parse_use_alias(context)?; let module_use = match (&alias_opt, context.tokens.peek()) { (None, Tok::ColonColon) => { - consume_token(context.tokens, Tok::ColonColon)?; - let sub_uses = match context.tokens.peek() { - Tok::LBrace => parse_comma_list( - context, - Tok::LBrace, - Tok::RBrace, - &TokenSet::from([Tok::Identifier]), - parse_use_member, - "a module member alias", - ), - _ => vec![parse_use_member(context)?], - }; - ModuleUse::Members(sub_uses) + let colon_colon_loc = context.tokens.current_token_loc(); + if let Err(diag) = consume_token(context.tokens, Tok::ColonColon) { + context.add_diag(*diag); + ModuleUse::Partial { + colon_colon: None, + opening_brace: None, + } + } else { + match context.tokens.peek() { + Tok::LBrace => { + let lbrace_loc = context.tokens.current_token_loc(); + let sub_uses = parse_comma_list( + context, + Tok::LBrace, + Tok::RBrace, + &TokenSet::from([Tok::Identifier]), + parse_use_member, + "a module member alias", + ); + if sub_uses.is_empty() { + // empty list does not make much sense as it contains no alias + // information and it actually helps IDE to treat this case as a partial + // module use + ModuleUse::Partial { + colon_colon: Some(colon_colon_loc), + opening_brace: Some(lbrace_loc), + } + } else { + ModuleUse::Members(sub_uses) + } + } + _ => match parse_use_member(context) { + Ok(m) => ModuleUse::Members(vec![m]), + Err(diag) => { + context.add_diag(*diag); + ModuleUse::Partial { + colon_colon: Some(colon_colon_loc), + opening_brace: None, + } + } + }, + } + } } _ => ModuleUse::Module(alias_opt.map(ModuleName)), }; diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.exp b/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.exp new file mode 100644 index 0000000000000..0e34cb2cfccff --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.exp @@ -0,0 +1,77 @@ +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:5:9 + │ +5 │ let _tmp = 42; // reset parser to see if the next line compiles + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected an identifier + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:11:9 + │ +11 │ let _tmp = 42; // reset parser to see if the next line compiles + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected ',' or '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:11:22 + │ +10 │ use a::m2::{foo + │ - To match this '{' +11 │ let _tmp = 42; // reset parser to see if the next line compiles + │ ^ Expected '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:17:9 + │ +17 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected ',' or '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:17:22 + │ +16 │ use a::m2::{foo, bar + │ - To match this '{' +17 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^ Expected '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:24:9 + │ +24 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected ',' or '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:24:22 + │ +23 │ use a::{m2::{foo, bar + │ - To match this '{' +24 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^ Expected '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:32:9 + │ +32 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^^^ + │ │ + │ Unexpected 'let' + │ Expected ',' or '}' + +error[E01002]: unexpected token + ┌─ tests/move_2024/ide_mode/use_incomplete.move:32:22 + │ +31 │ use a::{m2::{foo, bar}, m3 + │ - To match this '{' +32 │ let _tmp = 42; // reset parser to see if the next lines compile + │ ^ Expected '}' + diff --git a/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.move b/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.move new file mode 100644 index 0000000000000..e0c8e30aafe0d --- /dev/null +++ b/external-crates/move/crates/move-compiler/tests/move_2024/ide_mode/use_incomplete.move @@ -0,0 +1,49 @@ +module a::m { + + public fun test1() { + use a::m2:: + let _tmp = 42; // reset parser to see if the next line compiles + m2::foo(); + } + + public fun test2() { + use a::m2::{foo + let _tmp = 42; // reset parser to see if the next line compiles + foo(); + } + + public fun test3() { + use a::m2::{foo, bar + let _tmp = 42; // reset parser to see if the next lines compile + foo(); + bar(); + } + + public fun test4() { + use a::{m2::{foo, bar + let _tmp = 42; // reset parser to see if the next lines compile + foo(); + bar(); + + } + + public fun test5() { + use a::{m2::{foo, bar}, m3 + let _tmp = 42; // reset parser to see if the next lines compile + m3::baz(); + foo(); + bar(); + } +} + +module a::m2 { + + public fun foo() {} + + public fun bar() {} +} + +module a::m3 { + + public fun baz() {} +} diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.exp index 2c8419ace14a8..a3c5d6ba63037 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.exp @@ -1,16 +1,16 @@ error[E01002]: unexpected token ┌─ tests/move_check/parser/use_module_member_invalid_missing_close_brace.move:6:5 │ -4 │ use 0x1::X::{S as XS - │ - To match this '{' +4 │ use 0x42::M::{S as XS + │ - To match this '{' 5 │ -6 │ fun foo() { +6 │ fun foo() {} │ ^ Expected '}' error[E01002]: unexpected token ┌─ tests/move_check/parser/use_module_member_invalid_missing_close_brace.move:6:5 │ -6 │ fun foo() { +6 │ fun foo() {} │ ^^^ │ │ │ Unexpected 'fun' diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.move index ed776d516fbd6..022ba96b63c1f 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.move +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_close_brace.move @@ -1,9 +1,11 @@ module 0x42::M { - use 0x1::X::{S as XS + use 0x42::M::{S as XS - fun foo() { + fun foo() {} - } + struct S has drop {} + + fun bar(_p: XS) {} } diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.exp b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.exp index b7adf60971e58..7cfa235226b66 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.exp +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.exp @@ -1,9 +1,9 @@ error[E01002]: unexpected token - ┌─ tests/move_check/parser/use_module_member_invalid_missing_semicolon.move:5:1 + ┌─ tests/move_check/parser/use_module_member_invalid_missing_semicolon.move:6:5 │ -5 │ } - │ ^ - │ │ - │ Unexpected '}' - │ Expected ';' +6 │ fun foo() {} + │ ^^^ + │ │ + │ Unexpected 'fun' + │ Expected ';' diff --git a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.move b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.move index 645abec93ca36..c1280790461fe 100644 --- a/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.move +++ b/external-crates/move/crates/move-compiler/tests/move_check/parser/use_module_member_invalid_missing_semicolon.move @@ -1,5 +1,11 @@ module 0x42::M { - use 0x1::X::{S as XS,} + use 0x42::M::{S as XS,} + + fun foo() {} + + struct S has drop {} + + fun bar(_p: XS) {} }