Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
124 changes: 104 additions & 20 deletions rust/rubydex/src/indexing/rbs_indexer.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
//! Visit the RBS AST and create type definitions.

use ruby_rbs::node::{self, ClassNode, ModuleNode, Node, TypeNameNode, Visit};
use ruby_rbs::node::{self, ClassNode, CommentNode, ConstantNode, ModuleNode, Node, TypeNameNode, Visit};

use crate::diagnostic::Rule;
use crate::indexing::local_graph::LocalGraph;
use crate::model::comment::Comment;
use crate::model::definitions::{ClassDefinition, Definition, DefinitionFlags, ModuleDefinition};
use crate::model::definitions::{ClassDefinition, ConstantDefinition, Definition, DefinitionFlags, ModuleDefinition};
use crate::model::document::Document;
use crate::model::ids::{DefinitionId, NameId, UriId};
use crate::model::name::{Name, ParentScope};
Expand Down Expand Up @@ -112,6 +112,16 @@ impl<'a> RBSIndexer<'a> {
}
}

fn collect_comments(comment_node: Option<CommentNode>) -> Vec<Comment> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should split the comment into lines. A RBS comment has multi line contents, but a Rubydex comment only has single-line content.

I think I have an implementation for this. Will share it later.

Copy link
Member

Choose a reason for hiding this comment

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

Do you mean splitting into multiple lines at the RBS parser level?

The challenge with splitting during indexing is when the comment's content includes a line break, which ends up requiring more careful parsing. E.g.:

# something \n

comment_node
.into_iter()
.map(|comment| {
let text = Self::bytes_to_string(comment.string().as_bytes());
Comment::new(Offset::from_rbs_location(&comment.location()), text)
})
.collect()
}

fn register_definition(
&mut self,
definition: Definition,
Expand All @@ -135,14 +145,7 @@ impl Visit for RBSIndexer<'_> {
let offset = Offset::from_rbs_location(&class_node.location());
let name_offset = Offset::from_rbs_location(&type_name.name().location());

let comments: Vec<_> = class_node
.comment()
.into_iter()
.map(|comment| {
let text = Self::bytes_to_string(comment.string().as_bytes());
Comment::new(Offset::from_rbs_location(&comment.location()), text)
})
.collect();
let comments = Self::collect_comments(class_node.comment());

let superclass_ref = class_node.super_class().as_ref().map(|super_node| {
let type_name = super_node.name();
Expand Down Expand Up @@ -182,14 +185,7 @@ impl Visit for RBSIndexer<'_> {
let offset = Offset::from_rbs_location(&module_node.location());
let name_offset = Offset::from_rbs_location(&type_name.name().location());

let comments: Vec<_> = module_node
.comment()
.into_iter()
.map(|comment| {
let text = Self::bytes_to_string(comment.string().as_bytes());
Comment::new(Offset::from_rbs_location(&comment.location()), text)
})
.collect();
let comments = Self::collect_comments(module_node.comment());

let definition = Definition::Module(Box::new(ModuleDefinition::new(
name_id,
Expand All @@ -210,14 +206,36 @@ impl Visit for RBSIndexer<'_> {

self.nesting_stack.pop();
}

fn visit_constant_node(&mut self, constant_node: &ConstantNode) {
let lexical_nesting_id = self.parent_lexical_scope_id();
let nesting_name_id = self.nesting_name_id(lexical_nesting_id);

let type_name = constant_node.name();
let name_id = self.index_type_name(&type_name, nesting_name_id);
let offset = Offset::from_rbs_location(&constant_node.location());

let comments = Self::collect_comments(constant_node.comment());

let definition = Definition::Constant(Box::new(ConstantDefinition::new(
name_id,
self.uri_id,
offset,
comments,
DefinitionFlags::empty(),
lexical_nesting_id,
)));

self.register_definition(definition, lexical_nesting_id);
}
}

#[cfg(test)]
mod tests {
use crate::test_utils::LocalGraphTest;
use crate::{
assert_def_name_eq, assert_def_name_offset_eq, assert_def_superclass_ref_eq, assert_definition_at,
assert_local_diagnostics_eq, assert_no_local_diagnostics,
assert_def_comments_eq, assert_def_name_eq, assert_def_name_offset_eq, assert_def_superclass_ref_eq,
assert_definition_at, assert_local_diagnostics_eq, assert_no_local_diagnostics,
};

fn index_source(source: &str) -> LocalGraphTest {
Expand Down Expand Up @@ -346,6 +364,72 @@ mod tests {
});
}

#[test]
fn index_constant_node() {
let context = index_source("FOO: String");

assert_no_local_diagnostics!(&context);
assert_eq!(context.graph().definitions().len(), 1);

assert_definition_at!(&context, "1:1-1:12", Constant, |def| {
assert_def_name_eq!(&context, def, "FOO");
assert!(def.lexical_nesting_id().is_none());
});
}

#[test]
fn index_qualified_constant_node() {
let context = index_source("Foo::BAR: String");

assert_no_local_diagnostics!(&context);
assert_eq!(context.graph().definitions().len(), 1);

assert_definition_at!(&context, "1:1-1:17", Constant, |def| {
assert_def_name_eq!(&context, def, "Foo::BAR");
});
}

#[test]
fn index_constant_inside_class() {
let context = index_source({
"
class Foo
FOO: Integer
end
"
});

assert_no_local_diagnostics!(&context);

assert_definition_at!(&context, "1:1-3:4", Class, |class_def| {
assert_def_name_eq!(&context, class_def, "Foo");
assert_eq!(1, class_def.members().len());

assert_definition_at!(&context, "2:3-2:15", Constant, |def| {
assert_def_name_eq!(&context, def, "FOO");
assert_eq!(class_def.id(), def.lexical_nesting_id().unwrap());
assert_eq!(class_def.members()[0], def.id());
});
});
}

#[test]
fn index_constant_node_with_comment() {
let context = index_source({
"
# Some documentation
FOO: String
"
});

assert_no_local_diagnostics!(&context);

assert_definition_at!(&context, "2:1-2:12", Constant, |def| {
assert_def_name_eq!(&context, def, "FOO");
assert_def_comments_eq!(&context, def, ["Some documentation\n"]);
});
}

#[test]
fn index_class_and_module_nesting() {
let context = index_source({
Expand Down
23 changes: 3 additions & 20 deletions rust/rubydex/src/indexing/ruby_indexer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2035,9 +2035,9 @@ impl Visit<'_> for RubyIndexer<'_> {
#[cfg(test)]
mod tests {
use crate::{
assert_def_name_eq, assert_def_name_offset_eq, assert_def_str_eq, assert_def_superclass_ref_eq,
assert_definition_at, assert_local_diagnostics_eq, assert_name_path_eq, assert_no_local_diagnostics,
assert_string_eq,
assert_def_comments_eq, assert_def_name_eq, assert_def_name_offset_eq, assert_def_str_eq,
assert_def_superclass_ref_eq, assert_definition_at, assert_local_diagnostics_eq, assert_name_path_eq,
assert_no_local_diagnostics, assert_string_eq,
model::{
definitions::{Definition, Mixin, Parameter, Receiver},
ids::{StringId, UriId},
Expand All @@ -2046,23 +2046,6 @@ mod tests {
test_utils::LocalGraphTest,
};

/// Asserts that a definition's comments matches the expected comments.
///
/// Usage:
/// - `assert_def_comments_eq!(ctx, def, ["# Comment 1", "# Comment 2"])`
macro_rules! assert_def_comments_eq {
($context:expr, $def:expr, $expected_comments:expr) => {{
let actual_comments: Vec<String> = $def.comments().iter().map(|c| c.string().to_string()).collect();
assert_eq!(
$expected_comments,
actual_comments.as_slice(),
"comments mismatch: expected `{:?}`, got `{:?}`",
$expected_comments,
actual_comments
);
}};
}

/// Asserts that a definition's mixins matches the expected mixins.
///
/// Usage:
Expand Down
31 changes: 31 additions & 0 deletions rust/rubydex/src/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4484,6 +4484,37 @@ mod tests {
assert_ancestors_eq!(context, "Baz::Child", ["Baz::Child", "Baz::Base", "Object"]);
}

#[test]
fn rbs_constant_declarations() {
let mut context = GraphTest::new();
context.index_rbs_uri("file:///test.rbs", {
r"
FOO: String

class Bar
BAZ: Integer
end

Bar::QUX: ::String
"
});
context.resolve();

assert_no_diagnostics!(&context);

assert_declaration_exists!(context, "FOO");
assert_declaration_kind_eq!(context, "FOO", "Constant");
assert_owner_eq!(context, "FOO", "Object");

assert_declaration_exists!(context, "Bar::BAZ");
assert_declaration_kind_eq!(context, "Bar::BAZ", "Constant");
assert_owner_eq!(context, "Bar::BAZ", "Bar");

assert_declaration_exists!(context, "Bar::QUX");
assert_declaration_kind_eq!(context, "Bar::QUX", "Constant");
assert_owner_eq!(context, "Bar::QUX", "Bar");
}

#[test]
fn resolving_meta_programming_class_reopened() {
// It's often not possible to provide first-class support to meta-programming constructs, but we have to prevent
Expand Down
21 changes: 21 additions & 0 deletions rust/rubydex/src/test_utils/local_graph_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,27 @@ macro_rules! assert_def_str_eq {
}};
}

// Comment assertions

#[cfg(test)]
#[macro_export]
/// Asserts that a definition's comments matches the expected comments.
///
/// Usage:
/// - `assert_def_comments_eq!(ctx, def, ["# Comment 1", "# Comment 2"])`
macro_rules! assert_def_comments_eq {
($context:expr, $def:expr, $expected_comments:expr) => {{
let actual_comments: Vec<String> = $def.comments().iter().map(|c| c.string().to_string()).collect();
assert_eq!(
$expected_comments,
actual_comments.as_slice(),
"comments mismatch: expected `{:?}`, got `{:?}`",
$expected_comments,
actual_comments
);
}};
}

// Diagnostic assertions

#[cfg(test)]
Expand Down
Loading