Skip to content

Commit

Permalink
fix: workaround for #1838
Browse files Browse the repository at this point in the history
This is currently not ideal in form but should inform a larger fix
  • Loading branch information
ludamad committed Jul 5, 2023
1 parent e5773e4 commit 0bb7b53
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 21 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
fn path(&mut self, file_id: FileId) -> &Path {
pub fn path(&self, file_id: FileId) -> &Path {
// Unwrap as we ensure that all file_ids are created by the file manager
// So all file_ids will points to a corresponding path
self.id_to_path.get(&file_id).unwrap().0.as_path()
Expand Down
1 change: 1 addition & 0 deletions crates/lsp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ noirc_driver.workspace = true
noirc_errors.workspace = true
noirc_frontend.workspace = true
serde_json.workspace = true
toml.workspace = true
tower.workspace = true
async-lsp = { version = "0.0.4", default-features = false, features = ["omni-trait"] }

Expand Down
112 changes: 92 additions & 20 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use std::{
collections::HashMap,
fs,
future::Future,
ops::{self, ControlFlow},
path::{Path, PathBuf},
pin::Pin,
task::{Context, Poll},
};
Expand All @@ -20,7 +23,7 @@ use lsp_types::{
};
use noirc_driver::Driver;
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::graph::CrateType;
use noirc_frontend::graph::{CrateName, CrateType};
use serde_json::Value as JsonValue;
use tower::Service;

Expand Down Expand Up @@ -134,13 +137,8 @@ fn on_code_lens_request(
params: CodeLensParams,
) -> impl Future<Output = Result<Option<Vec<CodeLens>>, ResponseError>> {
async move {
// TODO: Requiring `Language` and `is_opcode_supported` to construct a driver makes for some real stinky code
// The driver should not require knowledge of the backend; instead should be implemented as an independent pass (in nargo?)
let mut driver = Driver::new(&Language::R1CS, Box::new(|_op| false));

let file_path = &params.text_document.uri.to_file_path().unwrap();

driver.create_local_crate(file_path, CrateType::Binary);
let actual_path = params.text_document.uri.to_file_path().unwrap();
let mut driver = create_driver_at_path(actual_path);

// We ignore the warnings and errors produced by compilation for producing codelenses

Check warning on line 143 in crates/lsp/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (codelenses)
// because we can still get the test functions even if compilation fails
Expand Down Expand Up @@ -218,33 +216,76 @@ fn on_did_close_text_document(
ControlFlow::Continue(())
}

/// Find the nearest parent file with given names.
fn find_nearest_parent_file(path: &Path, filenames: &[&str]) -> Option<PathBuf> {
let mut current_path = path;

while let Some(parent_path) = current_path.parent() {
for filename in filenames {
let mut possible_file_path = parent_path.to_path_buf();
possible_file_path.push(filename);
if possible_file_path.is_file() {
return Some(possible_file_path);
}
}
current_path = parent_path;
}

None
}

fn read_dependencies(
nargo_toml_path: &Path,
) -> Result<HashMap<String, String>, Box<dyn std::error::Error>> {
let content: String = fs::read_to_string(nargo_toml_path)?;
let value: toml::Value = toml::from_str(&content)?;

let mut dependencies = HashMap::new();

if let Some(toml::Value::Table(table)) = value.get("dependencies") {
for (key, value) in table {
if let toml::Value::Table(inner_table) = value {
if let Some(toml::Value::String(path)) = inner_table.get("path") {
dependencies.insert(key.clone(), path.clone());
}
}
}
}

Ok(dependencies)
}

fn on_did_save_text_document(
state: &mut LspState,
params: DidSaveTextDocumentParams,
) -> ControlFlow<Result<(), async_lsp::Error>> {
// TODO: Requiring `Language` and `is_opcode_supported` to construct a driver makes for some real stinky code
// The driver should not require knowledge of the backend; instead should be implemented as an independent pass (in nargo?)
let mut driver = Driver::new(&Language::R1CS, Box::new(|_op| false));

let file_path = &params.text_document.uri.to_file_path().unwrap();

driver.create_local_crate(file_path, CrateType::Binary);

let mut diagnostics = Vec::new();
let actual_path = params.text_document.uri.to_file_path().unwrap();
let mut driver = create_driver_at_path(actual_path.clone());

let file_diagnostics = match driver.check_crate(false) {
Ok(warnings) => warnings,
Err(errors_and_warnings) => errors_and_warnings,
};
let mut diagnostics = Vec::new();

if !file_diagnostics.is_empty() {
let fm = driver.file_manager();
let files = fm.as_simple_files();

for FileDiagnostic { file_id, diagnostic } in file_diagnostics {
// TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file
if file_id.as_usize() != 0 {
continue;
// TODO(AD): HACK, undo these total hacks once we have a proper approach
if file_id.as_usize() == 0 {
// main.nr case
if actual_path.file_name().unwrap().to_str() != Some("main.nr")
&& actual_path.file_name().unwrap().to_str() != Some("lib.nr")
{
continue;
}
} else if fm.path(file_id).file_name().unwrap().to_str().unwrap()
!= actual_path.file_name().unwrap().to_str().unwrap().replace(".nr", "")
{
// every other file case
continue; // TODO(AD): HACK, we list all errors, filter by hacky final path component
}

let mut range = Range::default();
Expand Down Expand Up @@ -278,6 +319,37 @@ fn on_did_save_text_document(
ControlFlow::Continue(())
}

fn create_driver_at_path(actual_path: PathBuf) -> Driver {
// TODO: Requiring `Language` and `is_opcode_supported` to construct a driver makes for some real stinky code
// The driver should not require knowledge of the backend; instead should be implemented as an independent pass (in nargo?)
let mut driver = Driver::new(&Language::R1CS, Box::new(|_op| false));

let mut file_path: PathBuf = actual_path;
// TODO better naming/unhacking

Check warning on line 328 in crates/lsp/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unhacking)
if let Some(new_path) = find_nearest_parent_file(&file_path, &["lib.nr", "main.nr"]) {
file_path = new_path.clone(); // TODO unhack

Check warning on line 330 in crates/lsp/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unhack)
}
let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]);

driver.create_local_crate(file_path, CrateType::Binary);

// TODO(AD): undo hacky dependency resolution
if let Some(nargo_toml_path) = nargo_toml_path {
let dependencies = read_dependencies(&nargo_toml_path);
if let Ok(dependencies) = dependencies {
for (crate_name, dependency_path) in dependencies.iter() {
let path_to_lib = nargo_toml_path
.parent()
.unwrap() // TODO
.join(PathBuf::from(&dependency_path).join("src").join("lib.nr"));
let library_crate = driver.create_non_local_crate(path_to_lib, CrateType::Library);
driver.propagate_dep(library_crate, &CrateName::new(crate_name).unwrap());
}
}
}
driver
}

fn on_exit(_state: &mut LspState, _params: ()) -> ControlFlow<Result<(), async_lsp::Error>> {
ControlFlow::Continue(())
}
Expand Down

0 comments on commit 0bb7b53

Please sign in to comment.