Skip to content

Commit

Permalink
feat: implement LSP code action "Implement missing members" (#6020)
Browse files Browse the repository at this point in the history
# Description

## Problem

Part of #1579

## Summary

Adds a code action to add missing trait impl methods and types. Default
methods are not includeded.

Here it's working for `Add`:

![lsp-implement-missing-members-add](https://github.com/user-attachments/assets/0b3b4afc-c1bf-4c1e-9c9e-44186c7bb01b)

Here for `BigField`:

![lsp-implement-missing-members-big-field](https://github.com/user-attachments/assets/22ec63b2-9fff-4824-b9c5-2aad85cc2fce)

Here for a complex type in Aztec-Packages:

![lsp-implement-missing-members-aztec](https://github.com/user-attachments/assets/de822bcc-1397-456a-8175-58613ffa1f0e)

## Additional Context

I found this code action in Rust very useful! It saves a lot of time,
plus there's no need to copy-paste :-)

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Sep 12, 2024
1 parent c2e4a9a commit 9bf2dcb
Show file tree
Hide file tree
Showing 7 changed files with 954 additions and 49 deletions.
71 changes: 64 additions & 7 deletions tooling/lsp/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::BTreeMap;

use noirc_frontend::{
ast::ItemVisibility,
graph::CrateId,
graph::{CrateId, Dependency},
hir::def_map::{CrateDefMap, ModuleId},
macros_api::{ModuleDefId, NodeInterner},
node_interner::ReferenceId,
Expand Down Expand Up @@ -41,7 +41,7 @@ pub(crate) fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> Refer
/// Returns the fully-qualified path of the given `ModuleDefId` relative to `current_module_id`:
/// - If `ModuleDefId` is a module, that module's path is returned
/// - Otherwise, that item's parent module's path is returned
pub(crate) fn module_full_path(
pub(crate) fn relative_module_full_path(
module_def_id: ModuleDefId,
visibility: ItemVisibility,
current_module_id: ModuleId,
Expand All @@ -55,8 +55,12 @@ pub(crate) fn module_full_path(
return None;
}

full_path =
module_id_path(module_id, &current_module_id, current_module_parent_id, interner);
full_path = relative_module_id_path(
module_id,
&current_module_id,
current_module_parent_id,
interner,
);
} else {
let Some(parent_module) = get_parent_module(interner, module_def_id) else {
return None;
Expand All @@ -66,15 +70,19 @@ pub(crate) fn module_full_path(
return None;
}

full_path =
module_id_path(parent_module, &current_module_id, current_module_parent_id, interner);
full_path = relative_module_id_path(
parent_module,
&current_module_id,
current_module_parent_id,
interner,
);
}
Some(full_path)
}

/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`.
/// Returns a relative path if possible.
pub(crate) fn module_id_path(
pub(crate) fn relative_module_id_path(
target_module_id: ModuleId,
current_module_id: &ModuleId,
current_module_parent_id: Option<ModuleId>,
Expand Down Expand Up @@ -130,3 +138,52 @@ pub(crate) fn module_id_path(
segments.reverse();
segments.join("::")
}

pub(crate) fn module_full_path(
module: &ModuleId,
interner: &NodeInterner,
crate_id: CrateId,
crate_name: &str,
dependencies: &Vec<Dependency>,
) -> String {
let mut segments: Vec<String> = Vec::new();

if let Some(module_attributes) = interner.try_module_attributes(module) {
segments.push(module_attributes.name.clone());

let mut current_attributes = module_attributes;
loop {
let Some(parent_local_id) = current_attributes.parent else {
break;
};

let Some(parent_attributes) = interner.try_module_attributes(&ModuleId {
krate: module.krate,
local_id: parent_local_id,
}) else {
break;
};

segments.push(parent_attributes.name.clone());
current_attributes = parent_attributes;
}
}

// We don't record module attributes for the root module,
// so we handle that case separately
if module.krate.is_root() {
if module.krate == crate_id {
segments.push(crate_name.to_string());
} else {
for dep in dependencies {
if dep.crate_id == crate_id {
segments.push(dep.name.to_string());
break;
}
}
}
};

segments.reverse();
segments.join("::")
}
9 changes: 8 additions & 1 deletion tooling/lsp/src/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use lsp_types::{
};
use noirc_errors::Span;
use noirc_frontend::{
ast::{ConstructorExpression, Path, Visitor},
ast::{ConstructorExpression, NoirTraitImpl, Path, Visitor},
graph::CrateId,
hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
macros_api::NodeInterner,
Expand All @@ -26,6 +26,7 @@ use crate::{utils, LspState};
use super::{process_request, to_lsp_location};

mod fill_struct_fields;
mod implement_missing_members;
mod import_or_qualify;
#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -219,4 +220,10 @@ impl<'a> Visitor for CodeActionFinder<'a> {

true
}

fn visit_noir_trait_impl(&mut self, noir_trait_impl: &NoirTraitImpl, span: Span) -> bool {
self.implement_missing_members(noir_trait_impl, span);

true
}
}
Loading

0 comments on commit 9bf2dcb

Please sign in to comment.