Skip to content

Commit 22617c4

Browse files
authored
Make Graph#index_source language_id optional (#841)
## Problem `Graph#index_source` currently requires callers to always pass a `language_id`, even though Rubydex can infer the language from the URI in the same way file indexing does. This makes callers repeat extension-based dispatch logic and adds friction for common in-memory indexing paths. This follows from the API discussion in https://github.com/shop/world/pull/775250/changes#r3325323365. ## Changes - Made the Ruby API accept an omitted or `nil` `language_id`. - Moved language inference for omitted `language_id` into the Rust FFI boundary, defaulting `.rbs` URIs to RBS and everything else to Ruby. - Updated the RBI signature and `index_source` tests to cover inference, explicit overrides, invalid language IDs, and the resulting graph declarations. ```ruby # Before graph.index_source(uri, source, language_id) # After graph.index_source(uri, source) graph.index_source(uri, source, language_id) # explicit override still works ```
1 parent 88a0d46 commit 22617c4

5 files changed

Lines changed: 110 additions & 23 deletions

File tree

ext/rubydex/graph.c

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,27 @@ static VALUE rdxr_graph_index_all(VALUE self, VALUE file_paths) {
8585
return array;
8686
}
8787

88-
// Indexes a single source string in memory, dispatching to the appropriate indexer based on language_id
88+
// Indexes a single source string in memory, dispatching to the appropriate indexer based on language_id.
8989
//
90-
// Graph#index_source: (String uri, String source, String language_id) -> void
91-
static VALUE rdxr_graph_index_source(VALUE self, VALUE uri, VALUE source, VALUE language_id) {
90+
// Graph#index_source: (String uri, String source, ?String language_id) -> void
91+
// Registered with arity -1 so Ruby can call it with either 2 or 3 arguments.
92+
static VALUE rdxr_graph_index_source(int argc, VALUE *argv, VALUE self) {
93+
VALUE uri, source, language_id;
94+
rb_scan_args(argc, argv, "21", &uri, &source, &language_id);
95+
9296
Check_Type(uri, T_STRING);
9397
Check_Type(source, T_STRING);
94-
Check_Type(language_id, T_STRING);
9598

9699
void *graph;
97100
TypedData_Get_Struct(self, void *, &graph_type, graph);
98101

99102
const char *uri_str = StringValueCStr(uri);
100-
const char *language_id_str = StringValueCStr(language_id);
103+
const char *language_id_str = NULL;
104+
if (!NIL_P(language_id)) {
105+
Check_Type(language_id, T_STRING);
106+
language_id_str = StringValueCStr(language_id);
107+
}
108+
101109
const char *source_str = RSTRING_PTR(source);
102110
size_t source_len = RSTRING_LEN(source);
103111

@@ -746,7 +754,7 @@ void rdxi_initialize_graph(VALUE moduleRubydex) {
746754

747755
rb_define_alloc_func(cGraph, rdxr_graph_alloc);
748756
rb_define_method(cGraph, "index_all", rdxr_graph_index_all, 1);
749-
rb_define_method(cGraph, "index_source", rdxr_graph_index_source, 3);
757+
rb_define_method(cGraph, "index_source", rdxr_graph_index_source, -1);
750758
rb_define_method(cGraph, "document", rdxr_graph_document, 1);
751759
rb_define_method(cGraph, "delete_document", rdxr_graph_delete_document, 1);
752760
rb_define_method(cGraph, "resolve", rdxr_graph_resolve, 0);

rbi/rubydex.rbi

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -298,8 +298,8 @@ class Rubydex::Graph
298298
sig { params(file_paths: T::Array[String]).returns(T::Array[String]) }
299299
def index_all(file_paths); end
300300

301-
sig { params(uri: String, source: String, language_id: String).void }
302-
def index_source(uri, source, language_id); end
301+
sig { params(uri: String, source: String, language_id: T.nilable(String)).void }
302+
def index_source(uri, source, language_id = nil); end
303303

304304
# Index all files and dependencies of the workspace that exists in `@workspace_path`
305305
sig { returns(T::Array[String]) }

rust/rubydex-sys/src/graph_api.rs

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -562,13 +562,15 @@ pub enum IndexSourceResult {
562562
UnsupportedLanguageId = 4,
563563
}
564564

565-
/// Indexes source code from memory using the specified language. Returns `IndexSourceResult::Success` on success
566-
/// or a specific error variant if string conversion or language lookup fails.
565+
/// Indexes source code from memory using the specified language. If `language_id` is null, infers the language from
566+
/// the URI using the same default as file indexing. Returns `IndexSourceResult::Success` on success or a specific error
567+
/// variant if string conversion or language lookup fails.
567568
///
568569
/// # Safety
569570
///
570571
/// - `pointer` must be a valid `GraphPointer` previously returned by this crate.
571-
/// - `uri` and `language_id` must be valid, null-terminated UTF-8 strings.
572+
/// - `uri` must be a valid, null-terminated UTF-8 string.
573+
/// - `language_id` must be either null or a valid, null-terminated UTF-8 string.
572574
/// - `source` must point to a valid UTF-8 byte buffer of at least `source_len` bytes.
573575
/// It may contain null bytes.
574576
#[unsafe(no_mangle)]
@@ -588,12 +590,18 @@ pub unsafe extern "C" fn rdx_index_source(
588590
return IndexSourceResult::InvalidSource;
589591
};
590592

591-
let Ok(language_id_str) = (unsafe { utils::convert_char_ptr_to_string(language_id) }) else {
592-
return IndexSourceResult::InvalidLanguageId;
593-
};
593+
let language = if language_id.is_null() {
594+
LanguageId::from_path(uri_str.as_str())
595+
} else {
596+
let Ok(language_id_str) = (unsafe { utils::convert_char_ptr_to_string(language_id) }) else {
597+
return IndexSourceResult::InvalidLanguageId;
598+
};
599+
600+
let Ok(language) = LanguageId::from_language_id(&language_id_str) else {
601+
return IndexSourceResult::UnsupportedLanguageId;
602+
};
594603

595-
let Ok(language) = LanguageId::from_language_id(&language_id_str) else {
596-
return IndexSourceResult::UnsupportedLanguageId;
604+
language
597605
};
598606

599607
with_mut_graph(pointer, |graph| {

rust/rubydex/src/indexing.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ use crate::{
66
operation::ruby_builder::RubyOperationBuilder,
77
};
88
use crossbeam_channel::{Sender, unbounded};
9-
use std::{ffi::OsStr, fs, path::PathBuf, sync::Arc};
9+
use std::{
10+
ffi::OsStr,
11+
fs,
12+
path::{Path, PathBuf},
13+
sync::Arc,
14+
};
1015
use url::Url;
1116

1217
pub mod local_graph;
@@ -35,6 +40,10 @@ impl From<&OsStr> for LanguageId {
3540
}
3641

3742
impl LanguageId {
43+
pub fn from_path(path: impl AsRef<Path>) -> Self {
44+
path.as_ref().extension().map_or(Self::Ruby, Self::from)
45+
}
46+
3847
/// Determines the language from an LSP language ID string.
3948
///
4049
/// # Errors
@@ -100,7 +109,7 @@ impl Job for IndexingJob {
100109
return;
101110
};
102111

103-
let language = self.path.extension().map_or(LanguageId::Ruby, LanguageId::from);
112+
let language = LanguageId::from_path(&self.path);
104113
let local_graph = build_local_graph(url.to_string(), &source, &language, self.backend);
105114

106115
self.local_graph_tx

test/graph_test.rb

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -529,18 +529,80 @@ def test_delete_uri_with_invalid_argument
529529

530530
def test_index_source_with_ruby
531531
graph = Rubydex::Graph.new
532-
graph.index_source("file:///foo.rb", "class Foo; end", "ruby")
532+
graph.index_source("file:///foo.rb", <<~RUBY, "ruby")
533+
class Foo
534+
def bar; end
535+
end
536+
RUBY
533537
graph.resolve
534538

535-
assert_equal(2, graph.documents.count)
536-
refute_nil(graph["Foo"])
539+
assert_empty(graph.diagnostics)
540+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
541+
end
542+
543+
def test_index_source_infers_ruby_language_id_when_omitted
544+
graph = Rubydex::Graph.new
545+
graph.index_source("file:///foo.rake", <<~RUBY)
546+
class Foo
547+
def bar; end
548+
end
549+
RUBY
550+
graph.resolve
551+
552+
assert_empty(graph.diagnostics)
553+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
554+
end
555+
556+
def test_index_source_infers_ruby_language_id_when_nil
557+
graph = Rubydex::Graph.new
558+
graph.index_source("untitled:Untitled-1", <<~RUBY, nil)
559+
class Foo
560+
def bar; end
561+
end
562+
RUBY
563+
graph.resolve
564+
565+
assert_empty(graph.diagnostics)
566+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
537567
end
538568

539569
def test_index_source_with_rbs
540570
graph = Rubydex::Graph.new
541-
graph.index_source("file:///foo.rbs", "class Foo\nend", "rbs")
571+
graph.index_source("file:///foo.rbs", <<~RBS, "rbs")
572+
class Foo
573+
def bar: () -> void
574+
end
575+
RBS
576+
graph.resolve
542577

543-
assert_equal(2, graph.documents.count)
578+
assert_empty(graph.diagnostics)
579+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
580+
end
581+
582+
def test_index_source_keeps_explicit_rbs_language_id_override
583+
graph = Rubydex::Graph.new
584+
graph.index_source("file:///foo.rb", <<~RBS, "rbs")
585+
class Foo
586+
def bar: () -> void
587+
end
588+
RBS
589+
graph.resolve
590+
591+
assert_empty(graph.diagnostics)
592+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
593+
end
594+
595+
def test_index_source_infers_rbs_language_id_when_omitted
596+
graph = Rubydex::Graph.new
597+
graph.index_source("file:///foo.rbs", <<~RBS)
598+
class Foo
599+
def bar: () -> void
600+
end
601+
RBS
602+
graph.resolve
603+
604+
assert_empty(graph.diagnostics)
605+
assert_equal("Foo#bar()", graph["Foo#bar()"].name)
544606
end
545607

546608
def test_index_source_with_unknown_language_id

0 commit comments

Comments
 (0)