From 9c7df5c24cd10ba18f1967f84e2c143438ccc0d1 Mon Sep 17 00:00:00 2001 From: Chandra Penke Date: Tue, 23 May 2023 23:29:21 -0700 Subject: [PATCH] Support combining actors and returning imports --- rust/candid_parser/src/typing.rs | 114 ++++++++++++++++-- .../tests/assets/combine_actors.did | 18 +++ rust/candid_parser/tests/assets/import/a.did | 5 + .../tests/assets/ok/combine_actors.did | 28 +++++ .../tests/assets/ok/combine_actors.imports | 1 + rust/candid_parser/tests/parse_type.rs | 52 +++++++- 6 files changed, 206 insertions(+), 12 deletions(-) create mode 100644 rust/candid_parser/tests/assets/combine_actors.did create mode 100644 rust/candid_parser/tests/assets/ok/combine_actors.did create mode 100644 rust/candid_parser/tests/assets/ok/combine_actors.imports diff --git a/rust/candid_parser/src/typing.rs b/rust/candid_parser/src/typing.rs index 60bd26ba..c11bf867 100644 --- a/rust/candid_parser/src/typing.rs +++ b/rust/candid_parser/src/typing.rs @@ -181,6 +181,45 @@ fn check_decs(env: &mut Env, decs: &[Dec]) -> Result<()> { Ok(()) } +fn combine_actors(env: &Env, actor1: &Option, actor2: &Option) -> Result> { + match (actor1, actor2) { + (None, None) => Ok(None), + (Some(t1), None) => Ok(Some(t1.clone())), + (None, Some(t2)) => Ok(Some(t2.clone())), + (Some(t1), Some(t2)) => { + if let TypeInner::Class(_, _) = t1.as_ref() { + return Err(Error::Custom(anyhow::Error::msg(format!( + "Cannot combine class type: {:?}", + t1 + )))); + } else if let TypeInner::Class(_, _) = t2.as_ref() { + return Err(Error::Custom(anyhow::Error::msg(format!( + "Cannot combine class type: {:?}", + t2 + )))); + } + + let s1 = env.te.as_service(t1)?; + let s2 = env.te.as_service(t2)?; + // check that both actors don't have any overlapping names + let names1 = s1.iter().map(|(n, _)| n).collect::>(); + let names2 = s2.iter().map(|(n, _)| n).collect::>(); + let intersection = names1.intersection(&names2).collect::>(); + if !intersection.is_empty() { + return Err(Error::Custom(anyhow::Error::msg(format!( + "duplicate method names: {:?} {:?} {:?}", + intersection, names1, names2 + )))); + } + + let mut ret = s1.to_vec(); + ret.extend(s2.iter().cloned()); + + Ok(Some(TypeInner::Service(ret).into())) + } + } +} + fn check_actor(env: &Env, actor: &Option) -> Result> { match actor { None => Ok(None), @@ -216,7 +255,7 @@ fn load_imports( base: &Path, visited: &mut BTreeSet, prog: &IDLProg, - list: &mut Vec, + list: &mut Vec<(PathBuf, IDLProg)>, ) -> Result<()> { for dec in prog.decs.iter() { if let Dec::ImportD(file) = dec { @@ -231,7 +270,7 @@ fn load_imports( }; let base = path.parent().unwrap(); load_imports(is_pretty, base, visited, &code, list)?; - list.push(path); + list.push((path, code)); } } } @@ -262,7 +301,9 @@ pub fn check_init_args( Ok(args) } -fn check_file_(file: &Path, is_pretty: bool) -> Result<(TypeEnv, Option)> { +fn check_file_(file: &Path, options: &CheckFileOptions) -> Result { + let is_pretty = options.pretty_errors; + let base = if file.is_absolute() { file.parent().unwrap().to_path_buf() } else { @@ -286,20 +327,71 @@ fn check_file_(file: &Path, is_pretty: bool) -> Result<(TypeEnv, Option)> te: &mut te, pre: false, }; - for import in imports.iter() { - let code = std::fs::read_to_string(import)?; - let code = code.parse::()?; - check_decs(&mut env, &code.decs)?; + let mut actor: Option = None; + for (_, import_prog) in imports.iter() { + check_decs(&mut env, &import_prog.decs)?; + if options.combine_actors { + actor = combine_actors(&env, &actor, &check_actor(&env, &import_prog.actor)?)?; + } } check_decs(&mut env, &prog.decs)?; - let actor = check_actor(&env, &prog.actor)?; - Ok((te, actor)) + let actor = if options.combine_actors { + combine_actors(&env, &actor, &check_actor(&env, &prog.actor)?)? + } else { + check_actor(&env, &prog.actor)? + }; + + let types = te; + + Ok(CheckFileResult { + types, + actor, + imports: imports.into_iter().map(|e| e.0).collect(), + }) } /// Type check did file including the imports. pub fn check_file(file: &Path) -> Result<(TypeEnv, Option)> { - check_file_(file, false) + let result = check_file_with_options( + file, + &CheckFileOptions { + pretty_errors: false, + ..Default::default() + }, + )?; + Ok((result.types, result.actor)) } + pub fn pretty_check_file(file: &Path) -> Result<(TypeEnv, Option)> { - check_file_(file, true) + let result = check_file_with_options( + file, + &CheckFileOptions { + pretty_errors: true, + ..Default::default() + }, + )?; + Ok((result.types, result.actor)) +} + +/// Return type of `check_file_with_options` +#[derive(Debug, Default, Clone)] +pub struct CheckFileResult { + pub types: TypeEnv, + pub actor: Option, + pub imports: BTreeSet, +} + +/// Options for `check_file_with_options` +#[derive(Debug, Default, Clone)] +pub struct CheckFileOptions { + pub pretty_errors: bool, + pub combine_actors: bool, +} + +/// Type check did file including the imports. This variant +/// takes options to pretty check the file, and also combine +/// actors across imports into a single actor (useful for modular) +/// did files. +pub fn check_file_with_options(file: &Path, options: &CheckFileOptions) -> Result { + check_file_(file, options) } diff --git a/rust/candid_parser/tests/assets/combine_actors.did b/rust/candid_parser/tests/assets/combine_actors.did new file mode 100644 index 00000000..5b95880f --- /dev/null +++ b/rust/candid_parser/tests/assets/combine_actors.did @@ -0,0 +1,18 @@ +import "import/a.did"; +import "import/b/b.did"; +type my_type = principal; +type List = opt record { head: int; tail: List }; +type f = func (List, func (int32) -> (int64)) -> (opt List); +type broker = service { + find : (name: text) -> + (service {up:() -> (); current:() -> (nat32)}); +}; +type nested = record { nat; nat; record {nat;int;}; record { nat; 0x2a:nat; nat8; }; 42:nat; 40:nat; variant{ A; 0x2a; B; C }; }; + +service server : { + f : (test: blob, opt bool) -> () oneway; + g : (my_type, List, opt List, nested) -> (int, broker) query; + h : (vec opt text, variant { A: nat; B: opt text }, opt List) -> (record { id: nat; 0x2a: record {} }); + i : f; + x : (a,b) -> (opt a, opt b); +} diff --git a/rust/candid_parser/tests/assets/import/a.did b/rust/candid_parser/tests/assets/import/a.did index d4ed4870..edba9fb5 100644 --- a/rust/candid_parser/tests/assets/import/a.did +++ b/rust/candid_parser/tests/assets/import/a.did @@ -1,2 +1,7 @@ import "b/b.did"; type a = variant {a;b:b}; + +service server : { + a : (test: blob, opt bool) -> () oneway; + c : (b) -> (int) query; +} diff --git a/rust/candid_parser/tests/assets/ok/combine_actors.did b/rust/candid_parser/tests/assets/ok/combine_actors.did new file mode 100644 index 00000000..51aff1a5 --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/combine_actors.did @@ -0,0 +1,28 @@ +type List = opt record { head : int; tail : List }; +type a = variant { a; b : b }; +type b = record { int; nat }; +type broker = service { + find : (text) -> (service { current : () -> (nat32); up : () -> () }); +}; +type f = func (List, func (int32) -> (int64)) -> (opt List); +type my_type = principal; +type nested = record { + 0 : nat; + 1 : nat; + 2 : record { nat; int }; + 3 : record { 0 : nat; 42 : nat; 43 : nat8 }; + 40 : nat; + 41 : variant { 42; A; B; C }; + 42 : nat; +}; +service : { + a : (vec nat8, opt bool) -> () oneway; + c : (b) -> (int) query; + f : (vec nat8, opt bool) -> () oneway; + g : (my_type, List, opt List, nested) -> (int, broker) query; + h : (vec opt text, variant { A : nat; B : opt text }, opt List) -> ( + record { 42 : record {}; id : nat }, + ); + i : f; + x : (a, b) -> (opt a, opt b); +} diff --git a/rust/candid_parser/tests/assets/ok/combine_actors.imports b/rust/candid_parser/tests/assets/ok/combine_actors.imports new file mode 100644 index 00000000..061da7d3 --- /dev/null +++ b/rust/candid_parser/tests/assets/ok/combine_actors.imports @@ -0,0 +1 @@ +["a.did", "b.did"] diff --git a/rust/candid_parser/tests/parse_type.rs b/rust/candid_parser/tests/parse_type.rs index 7dfb2fb6..e0aaac45 100644 --- a/rust/candid_parser/tests/parse_type.rs +++ b/rust/candid_parser/tests/parse_type.rs @@ -2,7 +2,7 @@ use candid::pretty::candid::compile; use candid::types::TypeEnv; use candid_parser::bindings::{javascript, motoko, rust, typescript}; use candid_parser::types::IDLProg; -use candid_parser::typing::{check_file, check_prog}; +use candid_parser::typing::{check_file, check_prog, check_file_with_options, CheckFileOptions}; use goldenfile::Mint; use std::io::Write; use std::path::Path; @@ -38,6 +38,10 @@ fn compiler_test(resource: &str) { let filename = Path::new(Path::new(resource).file_name().unwrap()); let candid_path = base_path.join(filename); + if filename.file_name().unwrap().to_str().unwrap() == "combine_actors.did" { + return; + } + match check_file(&candid_path) { Ok((env, actor)) => { { @@ -93,6 +97,52 @@ fn compiler_test(resource: &str) { } } +#[test] +fn test_combine_actors() { + let base_path = std::env::current_dir().unwrap().join("tests/assets"); + let mut mint = Mint::new(base_path.join("ok")); + let filename = Path::new("combine_actors.did"); + let candid_path = base_path.join(filename); + + match check_file_with_options( + &candid_path, + &CheckFileOptions { + pretty_errors: false, + combine_actors: true, + }, + ) { + Ok(result) => { + { + let mut output = mint.new_goldenfile(filename.with_extension("did")).unwrap(); + let content = compile(&result.types, &result.actor); + // Type check output + let ast = content.parse::().unwrap(); + check_prog(&mut TypeEnv::new(), &ast).unwrap(); + writeln!(output, "{}", content).unwrap(); + } + + { + let mut output = mint + .new_goldenfile(filename.with_extension("imports")) + .unwrap(); + let mut imports = result + .imports + .into_iter() + .map(|f| f.file_name().unwrap().to_str().unwrap().to_owned()) + .collect::>(); + imports.sort(); + writeln!(output, "{:?}", imports).unwrap(); + } + } + Err(e) => { + let mut fail_output = mint + .new_goldenfile(filename.with_extension("fail")) + .unwrap(); + writeln!(fail_output, "{}", e).unwrap(); + } + } +} + fn check_error R + std::panic::UnwindSafe, R>(f: F, str: &str) { assert_eq!( std::panic::catch_unwind(f)