Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: pass in file reader to FileManager at runtime #2649

Merged
merged 7 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -136,3 +136,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 @@ -24,6 +24,7 @@ serde.workspace = true
serde_json.workspace = true
toml.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
@@ -1,7 +1,7 @@
use std::{
future::{self, Future},
ops::{self, ControlFlow},
path::PathBuf,
path::{Path, PathBuf},
pin::Pin,
task::{self, Poll},
};
Expand Down Expand Up @@ -232,7 +232,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 @@ -328,7 +328,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 @@ -401,7 +401,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 @@ -620,7 +620,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 @@ -804,3 +805,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 @@ -160,7 +160,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 @@ -201,7 +202,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
Loading