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(lsp): add goto definition for functions #3656

Merged
merged 17 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
31 changes: 20 additions & 11 deletions compiler/fm/src/file_map.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use codespan_reporting::files::{Error, Files, SimpleFile, SimpleFiles};
use serde::{Deserialize, Serialize};
use std::collections::HashMap;
use std::{ops::Range, path::PathBuf};

// XXX: File and FileMap serve as opaque types, so that the rest of the library does not need to import the dependency
// or worry about when we change the dep

#[derive(Clone, Debug)]
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
pub struct PathString(PathBuf);

impl std::fmt::Display for PathString {
Expand All @@ -30,7 +31,10 @@ impl From<&PathBuf> for PathString {
}
}
#[derive(Debug)]
pub struct FileMap(SimpleFiles<PathString, String>);
pub struct FileMap {
files: SimpleFiles<PathString, String>,
name_to_id: HashMap<PathString, FileId>,
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
}

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(
Expand Down Expand Up @@ -59,17 +63,22 @@ impl<'input> File<'input> {

impl FileMap {
pub fn add_file(&mut self, file_name: PathString, code: String) -> FileId {
let file_id = self.0.add(file_name, code);
FileId(file_id)
let file_id = FileId(self.files.add(file_name.clone(), code));
self.name_to_id.insert(file_name, file_id);
file_id
}

pub fn get_file(&self, file_id: FileId) -> Option<File> {
self.0.get(file_id.0).map(File).ok()
self.files.get(file_id.0).map(File).ok()
}
}

pub fn get_file_id(&self, file_name: &PathString) -> Option<FileId> {
self.name_to_id.get(file_name).cloned()
}
}
impl Default for FileMap {
fn default() -> Self {
FileMap(SimpleFiles::new())
FileMap { files: SimpleFiles::new(), name_to_id: HashMap::new() }
}
}

Expand All @@ -79,18 +88,18 @@ impl<'a> Files<'a> for FileMap {
type Source = &'a str;

fn name(&self, file_id: Self::FileId) -> Result<Self::Name, Error> {
Ok(self.0.get(file_id.as_usize())?.name().clone())
Ok(self.files.get(file_id.as_usize())?.name().clone())
}

fn source(&'a self, file_id: Self::FileId) -> Result<Self::Source, Error> {
Ok(self.0.get(file_id.as_usize())?.source().as_ref())
Ok(self.files.get(file_id.as_usize())?.source().as_ref())
}

fn line_index(&self, file_id: Self::FileId, byte_index: usize) -> Result<usize, Error> {
self.0.get(file_id.as_usize())?.line_index((), byte_index)
self.files.get(file_id.as_usize())?.line_index((), byte_index)
}

fn line_range(&self, file_id: Self::FileId, line_index: usize) -> Result<Range<usize>, Error> {
self.0.get(file_id.as_usize())?.line_range((), line_index)
self.files.get(file_id.as_usize())?.line_range((), line_index)
}
}
4 changes: 4 additions & 0 deletions compiler/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ impl FileManager {

self.add_file(&candidate).ok_or_else(|| candidate.as_os_str().to_string_lossy().to_string())
}

pub fn name_to_id(&self, file_name: PathBuf) -> Option<FileId> {
self.file_map.get_file_id(&PathString::from_path(file_name))
}
}

/// Returns true if a module's child module's are expected to be in the same directory.
Expand Down
10 changes: 10 additions & 0 deletions compiler/noirc_errors/src/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ impl Span {
pub fn end(&self) -> u32 {
self.0.end().into()
}

pub fn contains(&self, other: &Span) -> bool {
self.start() <= other.start() && self.end() >= other.end()
}

pub fn is_smaller(&self, other: &Span) -> bool {
let self_distance = self.end() - self.start();
let other_distance = other.end() - other.start();
self_distance < other_distance
}
}

impl From<Span> for Range<usize> {
Expand Down
14 changes: 14 additions & 0 deletions compiler/noirc_frontend/src/hir/def_map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,20 @@ impl CrateDefMap {
})
}

/// Go through all modules in this crate, and find all definition
/// matching Position
// pub fn get_definition_location<'a>(
// &'a self,
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
// interner: &'a NodeInterner,
// ) {
// for (_, module_data) in self.modules.iter() {
// println!("Module {:?}", module_data);
// for definition in module_data.value_definitions() {
// println!("Definition {:?}", definition);
// }
// }
// }

/// Go through all modules in this crate, find all `contract ... { ... }` declarations,
/// and collect them all into a Vec.
pub fn get_all_contracts(&self, interner: &NodeInterner) -> Vec<Contract> {
Expand Down
18 changes: 16 additions & 2 deletions compiler/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::graph::{CrateGraph, CrateId};
use crate::hir_def::function::FuncMeta;
use crate::node_interner::{FuncId, NodeInterner, StructId};
use def_map::{Contract, CrateDefMap};
use fm::FileManager;
use noirc_errors::Location;
use fm::{FileId, FileManager};
use noirc_errors::{Location, Span};
use std::collections::BTreeMap;

use self::def_map::TestFunction;
Expand Down Expand Up @@ -180,6 +180,20 @@ impl Context {
.collect()
}

kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
/// Returns the [Location] of the definition of the given Ident found at [Span]
/// Return [None] when definition is not found.
pub fn find_definition_location(
&self,
file: FileId,
query_location: &Span,
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
) -> Option<Location> {
let interner = &self.def_interner;

interner
.find_location(file, query_location)
.and_then(|index| interner.resolve_location(index))
}

/// Return a Vec of all `contract` declarations in the source code and the functions they contain
pub fn get_all_contracts(&self, crate_id: &CrateId) -> Vec<Contract> {
self.def_map(crate_id)
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir_def/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ pub struct Trait {
pub self_type_typevar_id: TypeVariableId,
pub self_type_typevar: TypeVariable,
}

#[derive(Debug)]
pub struct TraitImpl {
pub ident: Ident,
pub typ: Type,
Expand Down
69 changes: 67 additions & 2 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::ast::Ident;
use crate::graph::CrateId;
use crate::hir::def_collector::dc_crate::{UnresolvedStruct, UnresolvedTrait, UnresolvedTypeAlias};
use crate::hir::def_map::{LocalModuleId, ModuleId};

use crate::hir_def::stmt::HirLetStatement;
use crate::hir_def::traits::TraitImpl;
use crate::hir_def::traits::{Trait, TraitConstraint};
Expand Down Expand Up @@ -36,6 +37,7 @@ type StructAttributes = Vec<SecondaryAttribute>;
/// each definition or struct, etc. Because it is used on the Hir, the NodeInterner is
/// useful in passes where the Hir is used - name resolution, type checking, and
/// monomorphization - and it is not useful afterward.
#[derive(Debug)]
pub struct NodeInterner {
nodes: Arena<Node>,
func_meta: HashMap<FuncId, FuncMeta>,
Expand Down Expand Up @@ -156,7 +158,7 @@ pub enum TraitImplKind {
///
/// Additionally, types can define specialized impls with methods of the same name
/// as long as these specialized impls do not overlap. E.g. `impl Struct<u32>` and `impl Struct<u64>`
#[derive(Default)]
#[derive(Default, Debug)]
pub struct Methods {
direct: Vec<FuncId>,
trait_impl_methods: Vec<FuncId>,
Expand All @@ -165,6 +167,7 @@ pub struct Methods {
/// All the information from a function that is filled out during definition collection rather than
/// name resolution. As a result, if information about a function is needed during name resolution,
/// this is the only place where it is safe to retrieve it (where all fields are guaranteed to be initialized).
#[derive(Debug, Clone)]
pub struct FunctionModifiers {
pub name: String,

Expand Down Expand Up @@ -360,6 +363,15 @@ impl DefinitionInfo {
pub fn is_global(&self) -> bool {
self.kind.is_global()
}

pub fn get_index(&self) -> Option<Index> {
kobyhallx marked this conversation as resolved.
Show resolved Hide resolved
match self.kind {
DefinitionKind::Function(func_id) => Some(func_id.0),
DefinitionKind::Global(stmt_id) => Some(stmt_id.0),
DefinitionKind::Local(expr_id) => expr_id.map(|id| Some(id.0)).unwrap_or(None),
DefinitionKind::GenericType(_) => None,
}
}
}

#[derive(Debug, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -451,6 +463,24 @@ impl NodeInterner {
self.id_to_location.insert(expr_id.into(), Location::new(span, file));
}

/// Scans the interner for the location which contains the span
pub fn find_location(&self, file: FileId, location_span: &Span) -> Option<&Index> {
let mut location_candidate: Option<(&Index, &Location)> = None;

for (index, location) in self.id_to_location.iter() {
if location.file == file && location.span.contains(location_span) {
if let Some(current_location) = location_candidate {
if location.span.is_smaller(&current_location.1.span) {
location_candidate = Some((index, location));
}
} else {
location_candidate = Some((index, location));
}
}
}
location_candidate.map(|(index, _location)| index)
}

/// Interns a HIR Function.
pub fn push_fn(&mut self, func: HirFunction) -> FuncId {
FuncId(self.nodes.insert(Node::Function(func)))
Expand Down Expand Up @@ -1190,6 +1220,41 @@ impl NodeInterner {
pub fn get_selected_impl_for_ident(&self, ident_id: ExprId) -> Option<TraitImplKind> {
self.selected_trait_implementations.get(&ident_id).cloned()
}

pub fn resolve_location(&self, index: &Index) -> Option<Location> {
self.nodes.get(*index).and_then(|def| match def {
Node::Function(func) => {
self.resolve_location(&func.as_expr().0)
}
Node::Expression(expression) => {
match expression {
HirExpression::Ident(ident) => {
let definition_info = self.definition(ident.id);

match definition_info.kind {
DefinitionKind::Function(func_id) => {
Some(self.function_meta(&func_id).location)
}
_ => {
eprintln!(
"\n ---> Not Implemented for Ident, Definition Kind {expression:?} and {definition_info:?}\n"
);
None
}
}
},
_ => {
eprintln!("\n --> Not Implemented for {expression:?}\n");
None
}
}
},
_ => {
eprintln!("\n -> Not Implemented for {def:?}\n");
None
}
})
}
}

impl Methods {
Expand Down Expand Up @@ -1242,7 +1307,7 @@ impl Methods {
}

/// These are the primitive type variants that we support adding methods to
#[derive(Copy, Clone, Hash, PartialEq, Eq)]
#[derive(Copy, Clone, Hash, PartialEq, Eq, Debug)]
enum TypeMethodKey {
/// Fields and integers share methods for ease of use. These methods may still
/// accept only fields or integers, it is just that their names may not clash.
Expand Down
5 changes: 3 additions & 2 deletions tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use notifications::{
on_did_open_text_document, on_did_save_text_document, on_exit, on_initialized,
};
use requests::{
on_code_lens_request, on_formatting, on_initialize, on_profile_run_request, on_shutdown,
on_test_run_request, on_tests_request,
on_code_lens_request, on_formatting, on_goto_definition_request, on_initialize,
on_profile_run_request, on_shutdown, on_test_run_request, on_tests_request,
};
use serde_json::Value as JsonValue;
use tower::Service;
Expand Down Expand Up @@ -76,6 +76,7 @@ impl NargoLspService {
.request::<request::NargoTests, _>(on_tests_request)
.request::<request::NargoTestRun, _>(on_test_run_request)
.request::<request::NargoProfileRun, _>(on_profile_run_request)
.request::<request::GotoDefinition, _>(on_goto_definition_request)
.notification::<notification::Initialized>(on_initialized)
.notification::<notification::DidChangeConfiguration>(on_did_change_configuration)
.notification::<notification::DidOpenTextDocument>(on_did_open_text_document)
Expand Down
Loading
Loading