Skip to content

Commit 44ec18c

Browse files
authored
Implement package root layers with file collation support (#1152)
Branched from #1151 Part of #1148
2 parents 9168963 + 73e33a2 commit 44ec18c

8 files changed

Lines changed: 376 additions & 17 deletions

File tree

crates/ark/src/lsp/diagnostics.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ pub(crate) fn generate_diagnostics(
170170
if let Some(SourceRoot::Package(root)) = &state.root {
171171
// Add symbols from `importFrom()` directives
172172
for import in &root.namespace.imports {
173-
context.workspace_symbols.insert(import.clone());
173+
context.workspace_symbols.insert(import.name.clone());
174174
}
175175

176176
// Add symbols from `import()` directives

crates/oak_index/src/external.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::HashMap;
22

33
use biome_rowan::TextRange;
44
use oak_package::library::Library;
5+
use oak_package::package_namespace::Namespace;
56
use url::Url;
67

78
use crate::semantic_index::DirectiveKind;
@@ -107,3 +108,27 @@ pub fn file_layers(file: Url, index: &SemanticIndex) -> Vec<BindingSource> {
107108

108109
layers
109110
}
111+
112+
/// Build the root layers for a package from its NAMESPACE.
113+
///
114+
/// These go at the back of every file's scope chain:
115+
/// - `PackageImports` from `importFrom()` directives (name → package)
116+
/// - `PackageExports` from `import()` directives
117+
pub fn package_root_layers(namespace: &Namespace) -> Vec<BindingSource> {
118+
let mut layers = Vec::new();
119+
120+
if !namespace.imports.is_empty() {
121+
let map = namespace
122+
.imports
123+
.iter()
124+
.map(|imp| (imp.name.clone(), imp.package.clone()))
125+
.collect();
126+
layers.push(BindingSource::PackageImports(map));
127+
}
128+
129+
for pkg in &namespace.package_imports {
130+
layers.push(BindingSource::PackageExports(pkg.clone()));
131+
}
132+
133+
layers
134+
}

crates/oak_index/tests/external.rs

Lines changed: 169 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ use biome_rowan::TextRange;
77
use biome_rowan::TextSize;
88
use oak_index::builder::build;
99
use oak_index::external::file_layers;
10+
use oak_index::external::package_root_layers;
1011
use oak_index::external::resolve_external_name;
1112
use oak_index::external::BindingSource;
1213
use oak_index::external::ExternalDefinition;
1314
use oak_package::library::Library;
1415
use oak_package::package::Package;
1516
use oak_package::package_description::Description;
17+
use oak_package::package_namespace::Import;
1618
use oak_package::package_namespace::Namespace;
1719
use url::Url;
1820

@@ -368,11 +370,11 @@ fn test_chained_scope_predecessor_files() {
368370
scope.extend(file_layers(file_url("a.R"), &index_a));
369371

370372
// Resolve from predecessor file b
373+
// Predecessor file export from b.R
371374
let result = resolve_external_name(&library, &scope, "helper_b");
372-
assert!(matches!(
373-
result,
374-
Some(ExternalDefinition::ProjectFile { .. })
375-
));
375+
assert_matches!(result, Some(ExternalDefinition::ProjectFile { file, .. }) => {
376+
assert_eq!(file, file_url("b.R"));
377+
});
376378

377379
// Resolve from predecessor file a
378380
let result = resolve_external_name(&library, &scope, "helper_a");
@@ -391,3 +393,166 @@ fn test_chained_scope_predecessor_files() {
391393
})
392394
);
393395
}
396+
397+
// --- root_layers ---
398+
399+
#[test]
400+
fn test_root_layers_from_namespace_imports() {
401+
let ns = Namespace {
402+
package_imports: vec!["rlang".to_string(), "cli".to_string()],
403+
..Default::default()
404+
};
405+
let layers = package_root_layers(&ns);
406+
assert_eq!(layers.len(), 2);
407+
assert_matches!(&layers[0], BindingSource::PackageExports(pkg) => {
408+
assert_eq!(pkg, "rlang");
409+
});
410+
assert_matches!(&layers[1], BindingSource::PackageExports(pkg) => {
411+
assert_eq!(pkg, "cli");
412+
});
413+
}
414+
415+
#[test]
416+
fn test_root_layers_empty_namespace() {
417+
let ns = Namespace::default();
418+
let layers = package_root_layers(&ns);
419+
assert!(layers.is_empty());
420+
}
421+
422+
#[test]
423+
fn test_root_layers_includes_importfrom() {
424+
let ns = Namespace {
425+
imports: vec![
426+
Import {
427+
name: "median".to_string(),
428+
package: "stats".to_string(),
429+
},
430+
Import {
431+
name: "head".to_string(),
432+
package: "utils".to_string(),
433+
},
434+
],
435+
..Default::default()
436+
};
437+
let layers = package_root_layers(&ns);
438+
assert_eq!(layers.len(), 1);
439+
assert_matches!(&layers[0], BindingSource::PackageImports(map) => {
440+
assert_eq!(map.get("median").unwrap(), "stats");
441+
assert_eq!(map.get("head").unwrap(), "utils");
442+
});
443+
}
444+
445+
#[test]
446+
fn test_root_layers_importfrom_before_package_exports() {
447+
let ns = Namespace {
448+
imports: vec![Import {
449+
name: "filter".to_string(),
450+
package: "stats".to_string(),
451+
}],
452+
package_imports: vec!["dplyr".to_string()],
453+
..Default::default()
454+
};
455+
let layers = package_root_layers(&ns);
456+
assert_eq!(layers.len(), 2);
457+
assert_matches!(&layers[0], BindingSource::PackageImports(_));
458+
assert_matches!(&layers[1], BindingSource::PackageExports(pkg) => {
459+
assert_eq!(pkg, "dplyr");
460+
});
461+
}
462+
463+
// --- scope chain assembly ---
464+
465+
#[test]
466+
fn test_scope_chain_combines_predecessors_and_root() {
467+
let library = test_library(vec![
468+
("rlang", vec!["sym", "expr"]),
469+
("dplyr", vec!["filter", "mutate"]),
470+
]);
471+
472+
let index_a = index_source("helper_a <- 1");
473+
let index_b = index_source("library(dplyr)\nhelper_b <- 2");
474+
475+
let ns = Namespace {
476+
package_imports: vec!["rlang".to_string()],
477+
..Default::default()
478+
};
479+
480+
let mut scope = Vec::new();
481+
scope.extend(file_layers(file_url("b.R"), &index_b));
482+
scope.extend(file_layers(file_url("a.R"), &index_a));
483+
scope.extend(package_root_layers(&ns));
484+
485+
// Predecessor file export
486+
let result = resolve_external_name(&library, &scope, "helper_b");
487+
assert_eq!(
488+
result,
489+
Some(ExternalDefinition::ProjectFile {
490+
file: file_url("b.R"),
491+
name: "helper_b".to_string(),
492+
range: range(15, 23),
493+
})
494+
);
495+
496+
// Predecessor library() directive
497+
let result = resolve_external_name(&library, &scope, "filter");
498+
assert_eq!(
499+
result,
500+
Some(ExternalDefinition::Package {
501+
package: "dplyr".to_string(),
502+
name: "filter".to_string(),
503+
})
504+
);
505+
506+
// Root layer (NAMESPACE import)
507+
let result = resolve_external_name(&library, &scope, "sym");
508+
assert_eq!(
509+
result,
510+
Some(ExternalDefinition::Package {
511+
package: "rlang".to_string(),
512+
name: "sym".to_string(),
513+
})
514+
);
515+
516+
// Miss
517+
assert_eq!(resolve_external_name(&library, &scope, "unknown"), None);
518+
}
519+
520+
#[test]
521+
fn test_scope_chain_predecessors_shadow_root() {
522+
let library = test_library(vec![("rlang", vec!["expr"])]);
523+
524+
let index = index_source("expr <- function() NULL");
525+
526+
let ns = Namespace {
527+
package_imports: vec!["rlang".to_string()],
528+
..Default::default()
529+
};
530+
531+
let mut scope = Vec::new();
532+
scope.extend(file_layers(file_url("utils.R"), &index));
533+
scope.extend(package_root_layers(&ns));
534+
535+
// File export shadows the rlang root layer
536+
let result = resolve_external_name(&library, &scope, "expr");
537+
assert!(matches!(
538+
result,
539+
Some(ExternalDefinition::ProjectFile { .. })
540+
));
541+
}
542+
543+
#[test]
544+
fn test_scope_chain_later_predecessor_shadows_earlier() {
545+
// b.R is loaded after a.R, so b.R's layers come first in the scope
546+
// and its `helper` should shadow a.R's `helper`.
547+
let index_a = index_source("helper <- 1");
548+
let index_b = index_source("helper <- 2");
549+
550+
let mut scope = Vec::new();
551+
scope.extend(file_layers(file_url("b.R"), &index_b));
552+
scope.extend(file_layers(file_url("a.R"), &index_a));
553+
554+
let result = resolve_external_name(&empty_library(), &scope, "helper");
555+
assert_matches!(result, Some(ExternalDefinition::ProjectFile { file, .. }) => {
556+
assert_eq!(file, file_url("b.R"));
557+
});
558+
}
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
use std::collections::HashSet;
2+
3+
use crate::package_description::Description;
4+
5+
/// Determine the order in which R source files should be loaded.
6+
///
7+
/// If the DESCRIPTION has a `Collate` field, its whitespace-separated
8+
/// file list is used. Otherwise files are sorted in C locale order
9+
/// (byte-wise, same as R's default).
10+
pub fn collation_order(description: &Description, files: &[String]) -> Vec<String> {
11+
description
12+
.collate()
13+
.map(|collate| {
14+
let available: HashSet<&str> = files.iter().map(|s| s.as_str()).collect();
15+
collate
16+
.into_iter()
17+
.filter(|name| available.contains(name.as_str()))
18+
.collect()
19+
})
20+
.unwrap_or_else(|| {
21+
let mut sorted = files.to_vec();
22+
sorted.sort();
23+
sorted
24+
})
25+
}
26+
27+
#[cfg(test)]
28+
mod tests {
29+
use super::*;
30+
use crate::package_description::Description;
31+
32+
fn desc_with_collate(collate: &str) -> Description {
33+
let raw = format!("Package: test\nVersion: 1.0\nCollate: {collate}");
34+
Description::parse(&raw).unwrap()
35+
}
36+
37+
fn desc_without_collate() -> Description {
38+
Description::parse("Package: test\nVersion: 1.0").unwrap()
39+
}
40+
41+
#[test]
42+
fn test_collate_field_determines_order() {
43+
let desc = desc_with_collate("zzz.R aaa.R bbb.R");
44+
let files = vec!["aaa.R".into(), "bbb.R".into(), "zzz.R".into()];
45+
assert_eq!(collation_order(&desc, &files), vec![
46+
"zzz.R", "aaa.R", "bbb.R"
47+
]);
48+
}
49+
50+
#[test]
51+
fn test_alphabetical_without_collate() {
52+
let desc = desc_without_collate();
53+
let files = vec!["zzz.R".into(), "aaa.R".into(), "bbb.R".into()];
54+
assert_eq!(collation_order(&desc, &files), vec![
55+
"aaa.R", "bbb.R", "zzz.R"
56+
]);
57+
}
58+
59+
#[test]
60+
fn test_collate_ignores_missing_files() {
61+
let desc = desc_with_collate("aaa.R missing.R bbb.R");
62+
let files = vec!["aaa.R".into(), "bbb.R".into()];
63+
assert_eq!(collation_order(&desc, &files), vec!["aaa.R", "bbb.R"]);
64+
}
65+
66+
#[test]
67+
fn test_collate_multiline() {
68+
// DCF continuation lines have leading whitespace, which the DCF parser
69+
// preserves. split_whitespace handles this naturally.
70+
let desc = desc_with_collate("aaa.R\n bbb.R\n zzz.R");
71+
let files = vec!["aaa.R".into(), "bbb.R".into(), "zzz.R".into()];
72+
assert_eq!(collation_order(&desc, &files), vec![
73+
"aaa.R", "bbb.R", "zzz.R"
74+
]);
75+
}
76+
77+
#[test]
78+
fn test_empty_r_files() {
79+
let desc = desc_without_collate();
80+
let files: Vec<String> = vec![];
81+
assert_eq!(collation_order(&desc, &files), Vec::<String>::new());
82+
}
83+
84+
#[test]
85+
fn test_alphabetical_is_byte_order() {
86+
let desc = desc_without_collate();
87+
// Uppercase sorts before lowercase in C locale (byte order)
88+
let files = vec!["aaa.R".into(), "BBB.R".into(), "ccc.R".into()];
89+
assert_eq!(collation_order(&desc, &files), vec![
90+
"BBB.R", "aaa.R", "ccc.R"
91+
]);
92+
}
93+
}

crates/oak_package/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod collation;
12
pub mod library;
23
pub mod package;
34
pub mod package_description;

crates/oak_package/src/library.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ mod tests {
8181
use tempfile::TempDir;
8282

8383
use super::*;
84+
use crate::package_namespace::Import;
8485

8586
// Helper to create a temporary package directory with DESCRIPTION and NAMESPACE
8687
fn create_temp_package(
@@ -136,6 +137,9 @@ importFrom(pkg, baz)
136137

137138
// Namespace is parsed
138139
assert_eq!(pkg.namespace.exports, vec!["bar", "foo"]);
139-
assert_eq!(pkg.namespace.imports, vec!["baz"]);
140+
assert_eq!(pkg.namespace.imports, vec![Import {
141+
name: "baz".to_string(),
142+
package: "pkg".to_string()
143+
}]);
140144
}
141145
}

crates/oak_package/src/package_description.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ impl Description {
7777
fields,
7878
})
7979
}
80+
81+
/// Parse the `Collate` field, if present, returning the whitespace-separated
82+
/// file names in the order specified.
83+
pub fn collate(&self) -> Option<Vec<String>> {
84+
let collate = self.fields.get("Collate")?;
85+
Some(collate.split_whitespace().map(|s| s.to_string()).collect())
86+
}
8087
}
8188

8289
/// Parse a DCF (Debian Control File) format string into a key-value map.

0 commit comments

Comments
 (0)