Skip to content

Commit

Permalink
chore: pass in file reader to FileManager at runtime (#2649)
Browse files Browse the repository at this point in the history
Co-authored-by: kevaundray <kevtheappdev@gmail.com>
  • Loading branch information
TomAFrench and kevaundray authored Sep 29, 2023
1 parent 00ee14f commit 8d67997
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 51 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion compiler/fm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ license.workspace = true

[dependencies]
codespan-reporting.workspace = true
cfg-if.workspace = true
rust-embed = "6.6.0"
serde.workspace = true

Expand Down
34 changes: 6 additions & 28 deletions compiler/fm/src/file_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::path::Path;
// Based on the environment, we either read files using the rust standard library or we
// read files using the javascript host function

pub type FileReader = dyn Fn(&Path) -> std::io::Result<String> + Send;

#[derive(RustEmbed)]
#[folder = "../../noir_stdlib/src"]
#[cfg_attr(not(target_os = "windows"), prefix = "std/")]
Expand Down Expand Up @@ -35,37 +37,13 @@ fn get_stdlib_asset(path: &Path) -> std::io::Result<String> {
}
}

pub(crate) fn read_file_to_string(path_to_file: &Path) -> std::io::Result<String> {
pub(crate) fn read_file_to_string(
path_to_file: &Path,
get_non_stdlib_asset: &impl Fn(&Path) -> std::io::Result<String>,
) -> std::io::Result<String> {
if is_stdlib_asset(path_to_file) {
get_stdlib_asset(path_to_file)
} else {
get_non_stdlib_asset(path_to_file)
}
}

cfg_if::cfg_if! {
if #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] {
use wasm_bindgen::{prelude::*, JsValue};

#[wasm_bindgen(module = "@noir-lang/source-resolver")]
extern "C" {

#[wasm_bindgen(catch)]
fn read_file(path: &str) -> Result<String, JsValue>;

}

fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
let path_str = path_to_file.to_str().unwrap();
match read_file(path_str) {
Ok(buffer) => Ok(buffer),
Err(_) => Err(Error::new(ErrorKind::Other, "could not read file using wasm")),
}
}
} else {

fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
std::fs::read_to_string(path_to_file)
}
}
}
27 changes: 20 additions & 7 deletions compiler/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod file_reader;

pub use file_map::{File, FileId, FileMap, PathString};
use file_reader::is_stdlib_asset;
pub use file_reader::FileReader;

use std::{
collections::HashMap,
Expand All @@ -19,21 +20,33 @@ pub const FILE_EXTENSION: &str = "nr";
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
struct VirtualPath(PathBuf);

#[derive(Debug)]
pub struct FileManager {
root: PathBuf,
file_map: file_map::FileMap,
id_to_path: HashMap<FileId, VirtualPath>,
path_to_id: HashMap<VirtualPath, FileId>,
file_reader: Box<FileReader>,
}

impl std::fmt::Debug for FileManager {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("FileManager")
.field("root", &self.root)
.field("file_map", &self.file_map)
.field("id_to_path", &self.id_to_path)
.field("path_to_id", &self.path_to_id)
.finish()
}
}

impl FileManager {
pub fn new(root: &Path) -> Self {
pub fn new(root: &Path, file_reader: Box<FileReader>) -> Self {
Self {
root: root.normalize(),
file_map: Default::default(),
id_to_path: Default::default(),
path_to_id: Default::default(),
file_reader,
}
}

Expand All @@ -58,7 +71,7 @@ impl FileManager {
}

// Otherwise we add the file
let source = file_reader::read_file_to_string(&resolved_path).ok()?;
let source = file_reader::read_file_to_string(&resolved_path, &self.file_reader).ok()?;
let file_id = self.file_map.add_file(resolved_path.into(), source);
self.register_path(file_id, path_to_file);
Some(file_id)
Expand Down Expand Up @@ -225,7 +238,7 @@ mod tests {
let entry_file_name = Path::new("my_dummy_file.nr");
create_dummy_file(&dir, entry_file_name);

let mut fm = FileManager::new(dir.path());
let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path)));

let file_id = fm.add_file(entry_file_name).unwrap();

Expand All @@ -240,7 +253,7 @@ mod tests {
let file_name = Path::new("foo.nr");
create_dummy_file(&dir, file_name);

let mut fm = FileManager::new(dir.path());
let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path)));

let file_id = fm.add_file(file_name).unwrap();

Expand All @@ -250,7 +263,7 @@ mod tests {
#[test]
fn path_resolve_sub_module() {
let dir = tempdir().unwrap();
let mut fm = FileManager::new(dir.path());
let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path)));

// Create a lib.nr file at the root.
// we now have dir/lib.nr
Expand Down Expand Up @@ -296,7 +309,7 @@ mod tests {
let sub_dir = TempDir::new_in(&dir).unwrap();
let sub_sub_dir = TempDir::new_in(&sub_dir).unwrap();

let mut fm = FileManager::new(dir.path());
let mut fm = FileManager::new(dir.path(), Box::new(|path| std::fs::read_to_string(path)));

// Create a lib.nr file at the root.
let file_name = Path::new("lib.nr");
Expand Down
1 change: 1 addition & 0 deletions compiler/wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ noirc_driver.workspace = true
noirc_frontend.workspace = true
wasm-bindgen.workspace = true
serde.workspace = true
cfg-if.workspace = true

console_error_panic_hook = "0.1.7"
gloo-utils = { version = "0.1", features = ["serde"] }
Expand Down
26 changes: 25 additions & 1 deletion compiler/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ pub fn compile(args: JsValue) -> JsValue {
debug!("Compiler configuration {:?}", &options);

let root = Path::new("/");
let fm = FileManager::new(root);
let fm = FileManager::new(root, Box::new(get_non_stdlib_asset));
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);

Expand Down Expand Up @@ -137,3 +137,27 @@ pub fn compile(args: JsValue) -> JsValue {
<JsValue as JsValueSerdeExt>::from_serde(&optimized_program).unwrap()
}
}

cfg_if::cfg_if! {
if #[cfg(target_os = "wasi")] {
fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
std::fs::read_to_string(path_to_file)
}
} else {
use std::io::{Error, ErrorKind};

#[wasm_bindgen(module = "@noir-lang/source-resolver")]
extern "C" {
#[wasm_bindgen(catch)]
fn read_file(path: &str) -> Result<String, JsValue>;
}

fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
let path_str = path_to_file.to_str().unwrap();
match read_file(path_str) {
Ok(buffer) => Ok(buffer),
Err(_) => Err(Error::new(ErrorKind::Other, "could not read file using wasm")),
}
}
}
}
1 change: 0 additions & 1 deletion compiler/wasm/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
#![forbid(unsafe_code)]
#![warn(unused_crate_dependencies, unused_extern_crates)]
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]
Expand Down
1 change: 1 addition & 0 deletions tooling/lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ noirc_frontend.workspace = true
serde.workspace = true
serde_json.workspace = true
tower.workspace = true
cfg-if.workspace = true
async-lsp = { version = "0.0.5", default-features = false, features = ["omni-trait"] }

[dev-dependencies]
Expand Down
37 changes: 32 additions & 5 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
use std::{
future::{self, Future},
ops::{self, ControlFlow},
path::PathBuf,
path::{Path, PathBuf},
pin::Pin,
task::{self, Poll},
};
Expand Down Expand Up @@ -237,7 +237,7 @@ fn on_test_run_request(
// Since we filtered on crate name, this should be the only item in the iterator
match workspace.into_iter().next() {
Some(package) => {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset));
if check_crate(&mut context, crate_id, false).is_err() {
let result = NargoTestRunResult {
id: params.id.clone(),
Expand Down Expand Up @@ -330,7 +330,7 @@ fn on_tests_request(
let mut package_tests = Vec::new();

for package in &workspace {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset));
// We ignore the warnings and errors produced by compilation for producing tests
// because we can still get the test functions even if compilation fails
let _ = check_crate(&mut context, crate_id, false);
Expand Down Expand Up @@ -400,7 +400,7 @@ fn on_code_lens_request(
let mut lenses: Vec<CodeLens> = vec![];

for package in &workspace {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) = prepare_package(package, Box::new(get_non_stdlib_asset));
// We ignore the warnings and errors produced by compilation for producing code lenses
// because we can still get the test functions even if compilation fails
let _ = check_crate(&mut context, crate_id, false);
Expand Down Expand Up @@ -619,7 +619,8 @@ fn on_did_save_text_document(
let mut diagnostics = Vec::new();

for package in &workspace {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) =
prepare_package(package, Box::new(|path| std::fs::read_to_string(path)));

let file_diagnostics = match check_crate(&mut context, crate_id, false) {
Ok(((), warnings)) => warnings,
Expand Down Expand Up @@ -803,3 +804,29 @@ mod lsp_tests {
assert!(response.server_info.is_none());
}
}

cfg_if::cfg_if! {
if #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] {
use wasm_bindgen::{prelude::*, JsValue};

#[wasm_bindgen(module = "@noir-lang/source-resolver")]
extern "C" {

#[wasm_bindgen(catch)]
fn read_file(path: &str) -> Result<String, JsValue>;

}

fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
let path_str = path_to_file.to_str().unwrap();
match read_file(path_str) {
Ok(buffer) => Ok(buffer),
Err(_) => Err(Error::new(ErrorKind::Other, "could not read file using wasm")),
}
}
} else {
fn get_non_stdlib_asset(path_to_file: &Path) -> std::io::Result<String> {
std::fs::read_to_string(path_to_file)
}
}
}
6 changes: 3 additions & 3 deletions tooling/nargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub mod workspace;

use std::collections::BTreeMap;

use fm::FileManager;
use fm::{FileManager, FileReader};
use noirc_driver::{add_dep, prepare_crate, prepare_dependency};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateName},
Expand All @@ -42,9 +42,9 @@ pub fn prepare_dependencies(
}
}

pub fn prepare_package(package: &Package) -> (Context, CrateId) {
pub fn prepare_package(package: &Package, file_reader: Box<FileReader>) -> (Context, CrateId) {
// TODO: FileManager continues to leak into various crates
let fm = FileManager::new(&package.root_dir);
let fm = FileManager::new(&package.root_dir, file_reader);
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);

Expand Down
3 changes: 2 additions & 1 deletion tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ pub(crate) fn run(
}

fn check_package(package: &Package, compile_options: &CompileOptions) -> Result<(), CompileError> {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) =
prepare_package(package, Box::new(|path| std::fs::read_to_string(path)));
check_crate_and_report_errors(&mut context, crate_id, compile_options.deny_warnings)?;

if package.is_library() || package.is_contract() {
Expand Down
6 changes: 4 additions & 2 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ fn compile_program(
np_language: Language,
is_opcode_supported: &impl Fn(&Opcode) -> bool,
) -> (FileManager, CompilationResult<CompiledProgram>) {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) =
prepare_package(package, Box::new(|path| std::fs::read_to_string(path)));

let cached_program = if let Ok(preprocessed_program) =
read_program_from_file(workspace.package_build_path(package))
Expand Down Expand Up @@ -231,7 +232,8 @@ fn compile_contract(
np_language: Language,
is_opcode_supported: &impl Fn(&Opcode) -> bool,
) -> (FileManager, CompilationResult<CompiledContract>) {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) =
prepare_package(package, Box::new(|path| std::fs::read_to_string(path)));
let (contract, warnings) =
match noirc_driver::compile_contract(&mut context, crate_id, compile_options) {
Ok(contracts_and_warnings) => contracts_and_warnings,
Expand Down
3 changes: 2 additions & 1 deletion tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ fn run_tests<S: BlackBoxFunctionSolver>(
show_output: bool,
compile_options: &CompileOptions,
) -> Result<(), CliError> {
let (mut context, crate_id) = prepare_package(package);
let (mut context, crate_id) =
prepare_package(package, Box::new(|path| std::fs::read_to_string(path)));
check_crate_and_report_errors(&mut context, crate_id, compile_options.deny_warnings)?;

let test_functions = context.get_all_test_functions_in_crate_matching(&crate_id, test_name);
Expand Down

0 comments on commit 8d67997

Please sign in to comment.