Skip to content

Add DeclarationDescriptor and ReferenceDescriptor #159

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Add DeclarationDescriptor and ReferenceDescriptor #159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
203d2ec
Add DeclarationDescriptor and ReferenceDescriptor
kjeremy Oct 24, 2018
6ef52b3
Simplify find_all_refs by always resolving a ast::BindPat
kjeremy Oct 24, 2018
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
49 changes: 49 additions & 0 deletions crates/ra_analysis/src/descriptors/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
pub(crate) mod module;

use ra_editor::resolve_local_name;

use ra_syntax::{
ast::{self, AstNode, NameOwner},
text_utils::is_subrange,
TextRange
};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -61,3 +64,49 @@ impl FnDescriptor {
res
}
}

#[derive(Debug)]
pub struct ReferenceDescriptor {
pub range: TextRange,
pub name: String
}

#[derive(Debug)]
pub struct DeclarationDescriptor<'a> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Descriptors should be owned values and, ideally, should not directly reference syntax trees.

The idea is that, when processing code, we convert from syntax trees to a descriptor tree (which is more compact) and then throw away syntax to save memory.

We should have a method to map between descriptors and syntax on as needed basis. For example, we can store some sort of syntax node id in the descriptor, and use it to retrieve the node.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrestled with this but found that it was really the only way to implement find_all_refs without passing in the syntax. If we did that then we need to make sure that the syntax is the right one for the stored TextRange (maybe by storing the FileId as well?).

Can you sketch out a little what this would look like? I'm assuming that we do not have the concept of a syntax node id. If we had one where would it look for the corresponding syntax?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you sketch out a little what this would look like?

I won't have time until next week to give significant advice unfortunately, but I want to document/refactor/flesh-out the descriptor infrastructure more in the coming weeks.

The motivation for descriptors is that there isn't a 1-to-1 correspondance between rust source code and rust semantic model. If you have

#[path="a.rs"]
mod foo;
#[path="a.rs"]
mod bar;

Then the source of a.rs is present in the semantic model as two unrelated modules.

So, while SyntaxTrees correspond to source files, descriptors should correspond to the semantic model, and most of the analysis should work solely with descriptors.

Practically speaking, I think we should give ids to all syntax nodes and descriptors, and then have a free-form mapping between these ids.

I think a good star might be just using a triple of (FIleId, TextRange, SyntaxKind) as a syntax node ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. I'll have to think about how this should work when converting from id to node inside find_all_refs. It looks like we'll need to access (pass in?) the db somehow.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #169 for the syntax ptr stuff. I'll try to move some of the desciptors to this.

pat: ast::BindPat<'a>,
pub range: TextRange
}

impl<'a> DeclarationDescriptor<'a> {
pub fn new(pat: ast::BindPat) -> DeclarationDescriptor {
let range = pat.syntax().range();

DeclarationDescriptor {
pat,
range
}
}

pub fn find_all_refs(&self) -> Vec<ReferenceDescriptor> {
let name = match self.pat.name() {
Some(name) => name,
None => return Default::default()
};

let fn_def = match name.syntax().ancestors().find_map(ast::FnDef::cast) {
Some(def) => def,
None => return Default::default()
};

let refs : Vec<_> = fn_def.syntax().descendants()
.filter_map(ast::NameRef::cast)
.filter(|name_ref| resolve_local_name(*name_ref) == Some((name.text(), name.syntax().range())))
.map(|name_ref| ReferenceDescriptor {
name: name_ref.syntax().text().to_string(),
range : name_ref.syntax().range(),
})
.collect();

refs
}
}
35 changes: 15 additions & 20 deletions crates/ra_analysis/src/imp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::{
},
input::{SourceRootId, FilesDatabase, SourceRoot, WORKSPACE},
descriptors::module::{ModulesDatabase, ModuleTree, Problem},
descriptors::{FnDescriptor},
descriptors::{FnDescriptor, DeclarationDescriptor},
CrateGraph, CrateId, Diagnostic, FileId, FileResolver, FileSystemEdit, Position,
Query, SourceChange, SourceFileEdit, Cancelable,
};
Expand Down Expand Up @@ -309,29 +309,24 @@ impl AnalysisImpl {
let file = self.db.file_syntax(file_id);
let syntax = file.syntax();

let mut ret = vec![];
// Find the binding associated with the offset
let maybe_binding = find_node_at_offset::<ast::BindPat>(syntax, offset)
.or_else(||
find_node_at_offset::<ast::NameRef>(syntax, offset)
.and_then(|name_ref| resolve_local_name(name_ref))
.and_then(|resolved| find_node_at_offset::<ast::BindPat>(syntax, resolved.1.end())));

// Find the symbol we are looking for
if let Some(name_ref) = find_node_at_offset::<ast::NameRef>(syntax, offset) {

// We are only handing local references for now
if let Some(resolved) = resolve_local_name(name_ref) {

ret.push((file_id, resolved.1));
if maybe_binding.is_none() {
return Vec::new();
}

if let Some(fn_def) = find_node_at_offset::<ast::FnDef>(syntax, offset) {
let binding = maybe_binding.unwrap();

let refs : Vec<_> = fn_def.syntax().descendants()
.filter_map(ast::NameRef::cast)
.filter(|&n: &ast::NameRef| resolve_local_name(n) == Some(resolved.clone()))
.collect();
let decl = DeclarationDescriptor::new(binding);

for r in refs {
ret.push((file_id, r.syntax().range()));
}
}
}
}
let mut ret = vec![(file_id, decl.range)];
ret.extend(decl.find_all_refs().into_iter()
.map(|ref_desc| (file_id, ref_desc.range )));

ret
}
Expand Down
11 changes: 11 additions & 0 deletions crates/ra_analysis/tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,17 @@ fn test_find_all_refs_for_param_inside() {
assert_eq!(refs.len(), 2);
}

#[test]
fn test_find_all_refs_for_fn_param() {
let code = r#"
fn foo(i<|> : u32) -> u32 {
i
}"#;

let refs = get_all_refs(code);
assert_eq!(refs.len(), 2);
}

#[test]
fn test_complete_crate_path() {
let snap = analysis(&[
Expand Down