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

feat: let nargo and LSP work well in the stdlib #5969

Merged
merged 3 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 12 additions & 10 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,23 +212,25 @@ fn add_debug_source_to_file_manager(file_manager: &mut FileManager) {

/// Adds the file from the file system at `Path` to the crate graph as a root file
///
/// Note: This methods adds the stdlib as a dependency to the crate.
/// This assumes that the stdlib has already been added to the file manager.
/// Note: If the stdlib dependency has not been added yet, it's added. Otherwise
/// this method assumes the root crate is the stdlib (useful for running tests
/// in the stdlib, getting LSP stuff for the stdlib, etc.).
pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId {
let path_to_std_lib_file = Path::new(STD_CRATE_NAME).join("lib.nr");
let std_file_id = context
.file_manager
.name_to_id(path_to_std_lib_file)
.expect("stdlib file id is expected to be present");
let std_crate_id = context.crate_graph.add_stdlib(std_file_id);
let std_file_id = context.file_manager.name_to_id(path_to_std_lib_file);
let std_crate_id = std_file_id.map(|std_file_id| context.crate_graph.add_stdlib(std_file_id));

let root_file_id = context.file_manager.name_to_id(file_name.to_path_buf()).unwrap_or_else(|| panic!("files are expected to be added to the FileManager before reaching the compiler file_path: {file_name:?}"));

let root_crate_id = context.crate_graph.add_crate_root(root_file_id);
if let Some(std_crate_id) = std_crate_id {
let root_crate_id = context.crate_graph.add_crate_root(root_file_id);

add_dep(context, root_crate_id, std_crate_id, STD_CRATE_NAME.parse().unwrap());
add_dep(context, root_crate_id, std_crate_id, STD_CRATE_NAME.parse().unwrap());

root_crate_id
root_crate_id
} else {
context.crate_graph.add_crate_root_and_stdlib(root_file_id)
}
}

pub fn link_to_debug_crate(context: &mut Context, root_crate_id: CrateId) {
Expand Down
38 changes: 35 additions & 3 deletions compiler/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ pub enum CrateId {
Root(usize),
Crate(usize),
Stdlib(usize),
/// The special case of running the compiler against the stdlib.
/// In that case there's only one crate, and it's both the root
/// crate and the stdlib crate.
RootAndStdlib(usize),
Dummy,
}

Expand All @@ -25,11 +29,17 @@ impl CrateId {
}

pub fn is_stdlib(&self) -> bool {
matches!(self, CrateId::Stdlib(_))
match self {
CrateId::Stdlib(_) | CrateId::RootAndStdlib(_) => true,
CrateId::Root(_) | CrateId::Crate(_) | CrateId::Dummy => false,
}
}

pub fn is_root(&self) -> bool {
matches!(self, CrateId::Root(_))
match self {
CrateId::Root(_) | CrateId::RootAndStdlib(_) => true,
CrateId::Stdlib(_) | CrateId::Crate(_) | CrateId::Dummy => false,
}
}
}

Expand Down Expand Up @@ -178,7 +188,7 @@ impl CrateGraph {
Some((CrateId::Root(_), _)) => {
panic!("ICE: Tried to re-add the root crate as a regular crate")
}
Some((CrateId::Stdlib(_), _)) => {
Some((CrateId::Stdlib(_), _)) | Some((CrateId::RootAndStdlib(_), _)) => {
panic!("ICE: Tried to re-add the stdlib crate as a regular crate")
}
Some((CrateId::Dummy, _)) => {
Expand Down Expand Up @@ -212,6 +222,28 @@ impl CrateGraph {
crate_id
}

pub fn add_crate_root_and_stdlib(&mut self, file_id: FileId) -> CrateId {
for (crate_id, crate_data) in self.arena.iter() {
if crate_id.is_root() {
panic!("ICE: Cannot add two crate roots to a graph - use `add_crate` instead");
}

if crate_id.is_stdlib() {
panic!("ICE: Cannot add two stdlib crates to a graph - use `add_crate` instead");
}

if crate_data.root_file_id == file_id {
panic!("ICE: This FileId was already added to the CrateGraph")
}
}

let data = CrateData { root_file_id: file_id, dependencies: Vec::new() };
let crate_id = CrateId::RootAndStdlib(self.arena.len());
let prev = self.arena.insert(crate_id, data);
assert!(prev.is_none());
crate_id
}

pub fn iter_keys(&self) -> impl Iterator<Item = CrateId> + '_ {
self.arena.keys().copied()
}
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ pub(crate) fn module_id_path(
if !is_relative {
// We don't record module attributes for the root module,
// so we handle that case separately
if let CrateId::Root(_) = target_module_id.krate {
if target_module_id.krate.is_root() {
segments.push("crate");
}
}
Expand Down
5 changes: 3 additions & 2 deletions tooling/lsp/src/notifications/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use fm::{FileManager, FileMap};
use fxhash::FxHashMap as HashMap;
use lsp_types::{DiagnosticTag, Url};
use noirc_driver::{check_crate, file_manager_with_stdlib};
use noirc_driver::check_crate;
use noirc_errors::{DiagnosticKind, FileDiagnostic};

use crate::types::{
Expand Down Expand Up @@ -120,7 +120,8 @@ pub(crate) fn process_workspace_for_noir_document(
ResponseError::new(ErrorCode::REQUEST_FAILED, lsp_error.to_string())
})?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();

insert_all_files_for_workspace_into_file_manager(
state,
&workspace,
Expand Down
5 changes: 2 additions & 3 deletions tooling/lsp/src/requests/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind};
use noirc_frontend::{
ast::Visibility,
graph::CrateId,
hir::def_map::ModuleId,
hir_def::{stmt::HirPattern, traits::Trait},
macros_api::{NodeInterner, StructId},
Expand Down Expand Up @@ -351,9 +350,9 @@
}
}

// We don't record module attriubtes for the root module,
// We don't record module attributes for the root module,
// so we handle that case separately
if let CrateId::Root(_) = module.krate {
if module.krate.is_root() {
segments.push(&args.crate_name);
};

Expand Down Expand Up @@ -595,7 +594,7 @@

#[test]
async fn hover_on_struct_member() {
assert_hover(

Check warning on line 597 in tooling/lsp/src/requests/hover.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (subone)
"workspace",
"two/src/lib.nr",
Position { line: 9, character: 35 },
Expand All @@ -606,7 +605,7 @@
}

#[test]
async fn hover_on_trait() {

Check warning on line 608 in tooling/lsp/src/requests/hover.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (subone)
assert_hover(
"workspace",
"two/src/lib.nr",
Expand All @@ -621,7 +620,7 @@
async fn hover_on_global() {
assert_hover(
"workspace",
"two/src/lib.nr",

Check warning on line 623 in tooling/lsp/src/requests/hover.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (subone)
Position { line: 15, character: 25 },
r#" one::subone
global some_global: Field"#,
Expand All @@ -634,7 +633,7 @@
assert_hover(
"workspace",
"two/src/lib.nr",
Position { line: 3, character: 4 },

Check warning on line 636 in tooling/lsp/src/requests/hover.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (subone)
r#" one
fn function_one<A, B>()"#,
)
Expand All @@ -646,7 +645,7 @@
assert_hover(
"workspace",
"two/src/lib.nr",
Position { line: 2, character: 7 },

Check warning on line 648 in tooling/lsp/src/requests/hover.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (subone)
r#" two
fn function_two()"#,
)
Expand Down
4 changes: 2 additions & 2 deletions tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
WorkDoneProgressOptions,
};
use nargo_fmt::Config;
use noirc_driver::file_manager_with_stdlib;

use noirc_frontend::graph::CrateId;
use noirc_frontend::hir::def_map::CrateDefMap;
use noirc_frontend::{graph::Dependency, macros_api::NodeInterner};
Expand Down Expand Up @@ -248,14 +248,14 @@
signature_help_provider: Some(lsp_types::OneOf::Right(
lsp_types::SignatureHelpOptions {
trigger_characters: Some(vec!["(".to_string(), ",".to_string()]),
retrigger_characters: None,

Check warning on line 251 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (retrigger)
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
},
)),
code_action_provider: Some(lsp_types::OneOf::Right(lsp_types::CodeActionOptions {
code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]),

Check warning on line 258 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (QUICKFIX)
work_done_progress_options: WorkDoneProgressOptions {
work_done_progress: None,
},
Expand Down Expand Up @@ -407,7 +407,7 @@
location: noirc_errors::Location,
files: &'a FileMap,
interner: &'a NodeInterner,
interners: &'a HashMap<PathBuf, NodeInterner>,

Check warning on line 410 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
crate_id: CrateId,
crate_name: String,
dependencies: &'a Vec<Dependency>,
Expand All @@ -432,7 +432,7 @@
ResponseError::new(ErrorCode::REQUEST_FAILED, "Could not find package for file")
})?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(
state,
&workspace,
Expand Down Expand Up @@ -467,7 +467,7 @@
location,
files,
interner,
interners: &state.cached_definitions,

Check warning on line 470 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
crate_id,
crate_name: package.name.to_string(),
dependencies: &context.crate_graph[context.root_crate_id()].dependencies,
Expand All @@ -477,7 +477,7 @@
pub(crate) fn find_all_references_in_workspace(
location: noirc_errors::Location,
interner: &NodeInterner,
cached_interners: &HashMap<PathBuf, NodeInterner>,

Check warning on line 480 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
files: &FileMap,
include_declaration: bool,
include_self_type_name: bool,
Expand Down
6 changes: 2 additions & 4 deletions tooling/lsp/src/requests/profile_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use async_lsp::{ErrorCode, ResponseError};
use nargo::ops::report_errors;
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_artifacts::debug::DebugArtifact;
use noirc_driver::{
file_manager_with_stdlib, CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING,
};
use noirc_driver::{CompileOptions, DebugFile, NOIR_ARTIFACT_VERSION_STRING};
use noirc_errors::{debug_info::OpCodesCount, Location};

use crate::{
Expand Down Expand Up @@ -53,7 +51,7 @@ fn on_profile_run_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(
state,
&workspace,
Expand Down
6 changes: 2 additions & 4 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use crate::insert_all_files_for_workspace_into_file_manager;
use async_lsp::{ErrorCode, ResponseError};
use nargo::ops::{run_test, TestStatus};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{
check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING,
};
use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING};
use noirc_frontend::hir::FunctionNameMatch;

use crate::{
Expand Down Expand Up @@ -48,7 +46,7 @@ fn on_test_run_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(
state,
&workspace,
Expand Down
4 changes: 2 additions & 2 deletions tooling/lsp/src/requests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::insert_all_files_for_workspace_into_file_manager;
use async_lsp::{ErrorCode, LanguageClient, ResponseError};
use lsp_types::{LogMessageParams, MessageType};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{check_crate, file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING};
use noirc_driver::{check_crate, NOIR_ARTIFACT_VERSION_STRING};

use crate::{
get_package_tests_in_crate, parse_diff,
Expand Down Expand Up @@ -50,7 +50,7 @@ fn on_tests_request_inner(
ResponseError::new(ErrorCode::REQUEST_FAILED, err)
})?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(
state,
&workspace,
Expand Down
18 changes: 18 additions & 0 deletions tooling/nargo/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use std::{
slice,
};

use fm::FileManager;
use noirc_driver::file_manager_with_stdlib;

use crate::{
constants::{CONTRACT_DIR, EXPORT_DIR, PROOFS_DIR, TARGET_DIR},
package::Package,
Expand Down Expand Up @@ -46,6 +49,21 @@ impl Workspace {
pub fn export_directory_path(&self) -> PathBuf {
self.root_dir.join(EXPORT_DIR)
}

/// Returns a new `FileManager` for the root directory of this workspace.
/// If the root directory is not the standard library, the standard library
/// is added to the returned `FileManager`.
pub fn new_file_manager(&self) -> FileManager {
if self.is_stdlib() {
FileManager::new(&self.root_dir)
} else {
file_manager_with_stdlib(&self.root_dir)
}
}

fn is_stdlib(&self) -> bool {
self.members.len() == 1 && self.members[0].name.to_string() == "std"
}
}

pub enum IntoIter<'a, T> {
Expand Down
5 changes: 2 additions & 3 deletions tooling/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use nargo::{
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
use noirc_driver::{
check_crate, compute_function_abi, file_manager_with_stdlib, CompileOptions,
NOIR_ARTIFACT_VERSION_STRING,
check_crate, compute_function_abi, CompileOptions, NOIR_ARTIFACT_VERSION_STRING,
};
use noirc_frontend::{
graph::{CrateId, CrateName},
Expand Down Expand Up @@ -52,7 +51,7 @@ pub(crate) fn run(args: CheckCommand, config: NargoConfig) -> Result<(), CliErro
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

Expand Down
4 changes: 2 additions & 2 deletions tooling/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use nargo::package::Package;
use nargo::workspace::Workspace;
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::DEFAULT_EXPRESSION_WIDTH;
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
use noirc_driver::{file_manager_with_stdlib, DEFAULT_EXPRESSION_WIDTH};
use noirc_driver::{CompilationResult, CompileOptions, CompiledContract};

use noirc_frontend::graph::CrateName;
Expand Down Expand Up @@ -114,7 +114,7 @@ pub(super) fn compile_workspace_full(
workspace: &Workspace,
compile_options: &CompileOptions,
) -> Result<(), CliError> {
let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

Expand Down
5 changes: 2 additions & 3 deletions tooling/nargo_cli/src/cli/export_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ use nargo::workspace::Workspace;
use nargo::{insert_all_files_for_workspace_into_file_manager, parse_all};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{
compile_no_check, file_manager_with_stdlib, CompileOptions, CompiledProgram,
NOIR_ARTIFACT_VERSION_STRING,
compile_no_check, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING,
};

use noirc_frontend::graph::CrateName;
Expand Down Expand Up @@ -54,7 +53,7 @@ pub(crate) fn run(args: ExportCommand, config: NargoConfig) -> Result<(), CliErr
Some(NOIR_ARTIFACT_VERSION_STRING.to_owned()),
)?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

Expand Down
4 changes: 2 additions & 2 deletions tooling/nargo_cli/src/cli/fmt_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{fs::DirEntry, path::Path};
use clap::Args;
use nargo::{insert_all_files_for_workspace_into_file_manager, ops::report_errors};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{file_manager_with_stdlib, NOIR_ARTIFACT_VERSION_STRING};
use noirc_driver::NOIR_ARTIFACT_VERSION_STRING;
use noirc_errors::CustomDiagnostic;
use noirc_frontend::{hir::def_map::parse_file, parser::ParserError};

Expand All @@ -29,7 +29,7 @@ pub(crate) fn run(args: FormatCommand, config: NargoConfig) -> Result<(), CliErr
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);

let config = nargo_fmt::Config::read(&config.program_dir)
Expand Down
6 changes: 2 additions & 4 deletions tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use nargo::{
prepare_package,
};
use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
use noirc_driver::{
check_crate, file_manager_with_stdlib, CompileOptions, NOIR_ARTIFACT_VERSION_STRING,
};
use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING};
use noirc_frontend::{
graph::CrateName,
hir::{FunctionNameMatch, ParsedFiles},
Expand Down Expand Up @@ -65,7 +63,7 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError
Some(NOIR_ARTIFACT_VERSION_STRING.to_string()),
)?;

let mut workspace_file_manager = file_manager_with_stdlib(&workspace.root_dir);
let mut workspace_file_manager = workspace.new_file_manager();
insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager);
let parsed_files = parse_all(&workspace_file_manager);

Expand Down
Loading