Skip to content

Commit

Permalink
fix: lsp hover wasn't always working (#5515)
Browse files Browse the repository at this point in the history
# Description

## Problem

Resolves #5516

## Summary

It seems hover broke in 70dabfa .
However, our tests still pass. I still don't understand why it only
breaks when the request comes from an actual VS Code instance and not in
tests... but for now I'd rather revert the commit that broke it and
eventually try to do the refactor again, because having hover is pretty
nice :-)

## Additional Context

None.

## 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.

Co-authored-by: Tom French <[email protected]>
  • Loading branch information
asterite and TomAFrench authored Jul 15, 2024
1 parent 8cab4ac commit 951e821
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 35 deletions.
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,11 @@ impl<'a> ModCollector<'a> {

context.def_interner.add_module_attributes(
mod_id,
ModuleAttributes { name: mod_name.0.contents.clone(), location: mod_location },
ModuleAttributes {
name: mod_name.0.contents.clone(),
location: mod_location,
parent: self.module_id,
},
);
}

Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10;
pub struct ModuleAttributes {
pub name: String,
pub location: Location,
pub parent: LocalModuleId,
}

type StructAttributes = Vec<SecondaryAttribute>;
Expand Down Expand Up @@ -1016,6 +1017,10 @@ impl NodeInterner {
self.module_attributes.get(module_id)
}

pub fn try_module_parent(&self, module_id: &ModuleId) -> Option<LocalModuleId> {
self.try_module_attributes(module_id).map(|attrs| attrs.parent)
}

pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] {
&self.global_attributes[global_id]
}
Expand Down
37 changes: 14 additions & 23 deletions tooling/lsp/src/requests/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,14 @@ fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -
}
fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String {
let module_attributes = args.interner.module_attributes(&id);
let parent_module_id = args.def_maps[&id.krate].modules()[id.local_id.0].parent;

let mut string = String::new();
if let Some(parent_module_id) = parent_module_id {
if format_parent_module_from_module_id(
&ModuleId { krate: id.krate, local_id: parent_module_id },
args,
&mut string,
) {
string.push('\n');
}
if format_parent_module_from_module_id(
&ModuleId { krate: id.krate, local_id: module_attributes.parent },
args,
&mut string,
) {
string.push('\n');
}
string.push_str(" ");
string.push_str("mod ");
Expand Down Expand Up @@ -319,6 +316,7 @@ fn format_parent_module_from_module_id(
CrateId::Stdlib(_) => Some("std".to_string()),
CrateId::Dummy => None,
};

let wrote_crate = if let Some(crate_name) = crate_name {
string.push_str(" ");
string.push_str(&crate_name);
Expand All @@ -331,27 +329,20 @@ fn format_parent_module_from_module_id(
return wrote_crate;
};

let modules = args.def_maps[&module.krate].modules();
let module_data = &modules[module.local_id.0];

if wrote_crate {
string.push_str("::");
} else {
string.push_str(" ");
}

let mut segments = Vec::new();
let mut current_parent = module_data.parent;
while let Some(parent) = current_parent {
let parent_attributes = args
.interner
.try_module_attributes(&ModuleId { krate: module.krate, local_id: parent });
if let Some(parent_attributes) = parent_attributes {
segments.push(&parent_attributes.name);
current_parent = modules[module.local_id.0].parent;
} else {
break;
}
let mut current_attributes = module_attributes;
while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId {
krate: module.krate,
local_id: current_attributes.parent,
}) {
segments.push(&parent_attributes.name);
current_attributes = parent_attributes;
}

for segment in segments.iter().rev() {
Expand Down
13 changes: 2 additions & 11 deletions tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
collections::{BTreeMap, HashMap},
future::Future,
};
use std::{collections::HashMap, future::Future};

use crate::{
parse_diff, resolve_workspace_for_source_path,
Expand All @@ -17,11 +14,7 @@ use lsp_types::{
use nargo::insert_all_files_for_workspace_into_file_manager;
use nargo_fmt::Config;
use noirc_driver::file_manager_with_stdlib;
use noirc_frontend::{
graph::{CrateId, Dependency},
hir::def_map::CrateDefMap,
macros_api::NodeInterner,
};
use noirc_frontend::{graph::Dependency, macros_api::NodeInterner};
use serde::{Deserialize, Serialize};

use crate::{
Expand Down Expand Up @@ -285,7 +278,6 @@ pub(crate) struct ProcessRequestCallbackArgs<'a> {
interners: &'a HashMap<String, NodeInterner>,
root_crate_name: String,
root_crate_dependencies: &'a Vec<Dependency>,
def_maps: &'a BTreeMap<CrateId, CrateDefMap>,
}

pub(crate) fn process_request<F, T>(
Expand Down Expand Up @@ -340,7 +332,6 @@ where
interners: &state.cached_definitions,
root_crate_name: package.name.to_string(),
root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies,
def_maps: &context.def_maps,
}))
}
pub(crate) fn find_all_references_in_workspace(
Expand Down

0 comments on commit 951e821

Please sign in to comment.