Skip to content

Commit

Permalink
feat(lsp): add goto definition for locals (#3705)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves 

feat(lsp): add goto definition for locals #3706

## Summary\*

LSP server now understands goto definition command issued by Client (ie.
vscode). In case of local symbol it will jump to it's definition.

## Additional Context


## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** 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: jfecher <[email protected]>
  • Loading branch information
kobyhallx and jfecher authored Dec 8, 2023
1 parent 9a95fbe commit 9dd465c
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 27 deletions.
21 changes: 15 additions & 6 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ impl<'a> ModCollector<'a> {

for method in r#impl.methods {
let func_id = context.def_interner.push_empty_fn();
context.def_interner.push_function(func_id, &method.def, module_id);
let location = Location::new(method.span(), self.file_id);
context.def_interner.push_function(func_id, &method.def, module_id, location);
unresolved_functions.push_fn(self.module_id, func_id, method);
}

Expand All @@ -152,7 +153,8 @@ impl<'a> ModCollector<'a> {

for (_, func_id, noir_function) in &mut unresolved_functions.functions {
noir_function.def.where_clause.append(&mut trait_impl.where_clause.clone());
context.def_interner.push_function(*func_id, &noir_function.def, module);
let location = Location::new(noir_function.def.span, self.file_id);
context.def_interner.push_function(*func_id, &noir_function.def, module, location);
}

let unresolved_trait_impl = UnresolvedTraitImpl {
Expand Down Expand Up @@ -185,7 +187,8 @@ impl<'a> ModCollector<'a> {
for item in &trait_impl.items {
if let TraitImplItem::Function(impl_method) = item {
let func_id = context.def_interner.push_empty_fn();
context.def_interner.push_function(func_id, &impl_method.def, module);
let location = Location::new(impl_method.span(), self.file_id);
context.def_interner.push_function(func_id, &impl_method.def, module, location);
unresolved_functions.push_fn(self.module_id, func_id, impl_method.clone());
}
}
Expand Down Expand Up @@ -218,7 +221,8 @@ impl<'a> ModCollector<'a> {

// First create dummy function in the DefInterner
// So that we can get a FuncId
context.def_interner.push_function(func_id, &function.def, module);
let location = Location::new(function.span(), self.file_id);
context.def_interner.push_function(func_id, &function.def, module, location);

// Now link this func_id to a crate level map with the noir function and the module id
// Encountering a NoirFunction, we retrieve it's module_data to get the namespace
Expand Down Expand Up @@ -266,7 +270,9 @@ impl<'a> ModCollector<'a> {

// Create the corresponding module for the struct namespace
let id = match self.push_child_module(&name, self.file_id, false, false) {
Ok(local_id) => context.def_interner.new_struct(&unresolved, krate, local_id),
Ok(local_id) => {
context.def_interner.new_struct(&unresolved, krate, local_id, self.file_id)
}
Err(error) => {
definition_errors.push((error.into(), self.file_id));
continue;
Expand Down Expand Up @@ -391,7 +397,10 @@ impl<'a> ModCollector<'a> {
is_internal: None,
};

context.def_interner.push_function_definition(func_id, modifiers, id.0);
let location = Location::new(name.span(), self.file_id);
context
.def_interner
.push_function_definition(func_id, modifiers, id.0, location);

match self.def_collector.def_map.modules[id.0.local_id.0]
.declare_function(name.clone(), func_id)
Expand Down
6 changes: 4 additions & 2 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,9 @@ impl<'a> Resolver<'a> {
return self.add_global_variable_decl(name, definition);
}

let id = self.interner.push_definition(name.0.contents.clone(), mutable, definition);
let location = Location::new(name.span(), self.file);
let id =
self.interner.push_definition(name.0.contents.clone(), mutable, definition, location);
let ident = HirIdent { location, id };
let resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused };

Expand Down Expand Up @@ -300,8 +301,9 @@ impl<'a> Resolver<'a> {
ident = hir_let_stmt.ident();
resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused: true };
} else {
let id = self.interner.push_definition(name.0.contents.clone(), false, definition);
let location = Location::new(name.span(), self.file);
let id =
self.interner.push_definition(name.0.contents.clone(), false, definition, location);
ident = HirIdent { location, id };
resolver_meta = ResolverMeta { num_times_used: 0, ident, warn_if_unused: true };
}
Expand Down
5 changes: 3 additions & 2 deletions compiler/noirc_frontend/src/hir/resolution/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::{BTreeMap, HashSet};

use fm::FileId;
use iter_extended::vecmap;
use noirc_errors::Span;
use noirc_errors::{Location, Span};

use crate::{
graph::CrateId,
Expand Down Expand Up @@ -187,7 +187,8 @@ fn collect_trait_impl_methods(
if let Some(default_impl) = &method.default_impl {
let func_id = interner.push_empty_fn();
let module = ModuleId { local_id: trait_impl.module_id, krate: crate_id };
interner.push_function(func_id, &default_impl.def, module);
let location = Location::new(default_impl.def.span, trait_impl.file_id);
interner.push_function(func_id, &default_impl.def, module, location);
func_ids_in_trait.insert(func_id);
ordered_methods.push((
method.default_impl_module_id,
Expand Down
24 changes: 16 additions & 8 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,24 +255,27 @@ mod test {
fn basic_let() {
let mut interner = NodeInterner::default();

// Safety: The FileId in a location isn't used for tests
let file = FileId::default();
let location = Location::new(Span::default(), file);

// Add a simple let Statement into the interner
// let z = x + y;
//
// Push x variable
let x_id = interner.push_definition("x".into(), false, DefinitionKind::Local(None));

// Safety: The FileId in a location isn't used for tests
let file = FileId::default();
let location = Location::new(Span::default(), file);
let x_id =
interner.push_definition("x".into(), false, DefinitionKind::Local(None), location);

let x = HirIdent { id: x_id, location };

// Push y variable
let y_id = interner.push_definition("y".into(), false, DefinitionKind::Local(None));
let y_id =
interner.push_definition("y".into(), false, DefinitionKind::Local(None), location);
let y = HirIdent { id: y_id, location };

// Push z variable
let z_id = interner.push_definition("z".into(), false, DefinitionKind::Local(None));
let z_id =
interner.push_definition("z".into(), false, DefinitionKind::Local(None), location);
let z = HirIdent { id: z_id, location };

// Push x and y as expressions
Expand Down Expand Up @@ -304,7 +307,12 @@ mod test {

let name = HirIdent {
location,
id: interner.push_definition("test_func".into(), false, DefinitionKind::Local(None)),
id: interner.push_definition(
"test_func".into(),
false,
DefinitionKind::Local(None),
location,
),
};

// Add function meta
Expand Down
9 changes: 5 additions & 4 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
node_interner::{ExprId, NodeInterner, TypeAliasId},
};
use iter_extended::vecmap;
use noirc_errors::Span;
use noirc_errors::{Location, Span};
use noirc_printable_type::PrintableType;

use crate::{node_interner::StructId, Ident, Signedness};
Expand Down Expand Up @@ -166,7 +166,7 @@ pub struct StructType {
fields: Vec<(Ident, Type)>,

pub generics: Generics,
pub span: Span,
pub location: Location,
}

/// Corresponds to generic lists such as `<T, U>` in the source
Expand All @@ -191,11 +191,12 @@ impl StructType {
pub fn new(
id: StructId,
name: Ident,
span: Span,

location: Location,
fields: Vec<(Ident, Type)>,
generics: Generics,
) -> StructType {
StructType { id, fields, name, span, generics }
StructType { id, fields, name, location, generics }
}

/// To account for cyclic references between structs, a struct's
Expand Down
18 changes: 13 additions & 5 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ pub struct DefinitionInfo {
pub name: String,
pub mutable: bool,
pub kind: DefinitionKind,
pub location: Location,
}

impl DefinitionInfo {
Expand Down Expand Up @@ -517,6 +518,7 @@ impl NodeInterner {
typ: &UnresolvedStruct,
krate: CrateId,
local_id: LocalModuleId,
file_id: FileId,
) -> StructId {
let struct_id = StructId(ModuleId { krate, local_id });
let name = typ.struct_def.name.clone();
Expand All @@ -532,7 +534,8 @@ impl NodeInterner {
(id, TypeVariable::unbound(id))
});

let new_struct = StructType::new(struct_id, name, typ.struct_def.span, no_fields, generics);
let location = Location::new(typ.struct_def.span, file_id);
let new_struct = StructType::new(struct_id, name, location, no_fields, generics);
self.structs.insert(struct_id, Shared::new(new_struct));
self.struct_attributes.insert(struct_id, typ.struct_def.attributes.clone());
struct_id
Expand Down Expand Up @@ -661,13 +664,14 @@ impl NodeInterner {
name: String,
mutable: bool,
definition: DefinitionKind,
location: Location,
) -> DefinitionId {
let id = DefinitionId(self.definitions.len());
if let DefinitionKind::Function(func_id) = definition {
self.function_definition_ids.insert(func_id, id);
}

self.definitions.push(DefinitionInfo { name, mutable, kind: definition });
self.definitions.push(DefinitionInfo { name, mutable, kind: definition, location });
id
}

Expand All @@ -678,7 +682,8 @@ impl NodeInterner {
let mut modifiers = FunctionModifiers::new();
modifiers.name = name;
let module = ModuleId::dummy_id();
self.push_function_definition(id, modifiers, module);
let location = Location::dummy();
self.push_function_definition(id, modifiers, module, location);
id
}

Expand All @@ -687,6 +692,7 @@ impl NodeInterner {
id: FuncId,
function: &FunctionDefinition,
module: ModuleId,
location: Location,
) -> DefinitionId {
use ContractFunctionType::*;

Expand All @@ -700,19 +706,20 @@ impl NodeInterner {
contract_function_type: Some(if function.is_open { Open } else { Secret }),
is_internal: Some(function.is_internal),
};
self.push_function_definition(id, modifiers, module)
self.push_function_definition(id, modifiers, module, location)
}

pub fn push_function_definition(
&mut self,
func: FuncId,
modifiers: FunctionModifiers,
module: ModuleId,
location: Location,
) -> DefinitionId {
let name = modifiers.name.clone();
self.function_modifiers.insert(func, modifiers);
self.function_modules.insert(func, module);
self.push_definition(name, false, DefinitionKind::Function(func))
self.push_definition(name, false, DefinitionKind::Function(func), location)
}

pub fn set_function_trait(&mut self, func: FuncId, self_type: Type, trait_id: TraitId) {
Expand Down Expand Up @@ -1275,6 +1282,7 @@ impl NodeInterner {
DefinitionKind::Function(func_id) => {
Some(self.function_meta(&func_id).location)
}
DefinitionKind::Local(_local_id) => Some(definition_info.location),
_ => None,
}
}
Expand Down

0 comments on commit 9dd465c

Please sign in to comment.