Skip to content

Commit 5c53c2f

Browse files
committed
Fix imports with packaged standalone scripts
1 parent 7cff37b commit 5c53c2f

3 files changed

Lines changed: 101 additions & 4 deletions

File tree

crates/oak_db/src/file_imports.rs

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,14 @@ impl File {
6969
Some(package) if is_testthat_file(self, db) => {
7070
testthat_imports(self, db, package, None)
7171
},
72-
Some(package) => package_imports(self, db, package),
73-
None => script_imports(self, db),
72+
Some(package) if is_package_source(self, db, package) => {
73+
package_imports(self, db, package)
74+
},
75+
// A file with a package back-pointer that isn't a loadable `R/`
76+
// file (`data-raw/`, `inst/`, a non-collated `R/` file) lives in
77+
// the package but isn't loaded with it. Resolve it as a standalone
78+
// script, same as a file with no package at all.
79+
_ => script_imports(self, db),
7480
}
7581
}
7682

@@ -113,12 +119,25 @@ impl File {
113119
Some(package) if is_testthat_file(self, db) => {
114120
testthat_imports(self, db, package, Some(offset))
115121
},
116-
Some(package) => narrow_package_top_level(self, db, package),
117-
None => narrow_script_top_level(self, db, offset),
122+
Some(package) if is_package_source(self, db, package) => {
123+
narrow_package_top_level(self, db, package)
124+
},
125+
// A packaged file that isn't a loadable `R/` file narrows like a
126+
// standalone script.
127+
_ => narrow_script_top_level(self, db, offset),
118128
}
119129
}
120130
}
121131

132+
/// Whether `file` is one of its package's loadable `R/` files, the ones in
133+
/// `package.files()`. A file can carry a package back-pointer without being
134+
/// loadable: `data-raw/`, `inst/`, and `R/` files left out of a `Collate:`
135+
/// directive all land in `package.scripts()` instead and resolve as
136+
/// standalone scripts.
137+
fn is_package_source(file: File, db: &dyn Db, package: Package) -> bool {
138+
package.files(db).contains(&file)
139+
}
140+
122141
fn narrow_script_top_level(file: File, db: &dyn Db, offset: TextSize) -> Vec<ImportLayer> {
123142
let mut layers = attach_layers(file, db, Some(offset));
124143
extend_with_default_search_path(db, &mut layers);

crates/oak_db/src/tests/file_imports.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,56 @@ fn test_package_file_emits_namespace_and_collation_layers() {
203203
]);
204204
}
205205

206+
#[test]
207+
fn test_package_script_resolves_as_standalone_script() {
208+
// A `data-raw/` file carries a package back-pointer but lives in
209+
// `scripts`, not `files`. It isn't loaded with the package, so its
210+
// imports must be the standalone-script view, never the package view
211+
// (no `R/` `File` layers, no namespace layers). See #1270.
212+
let mut db = TestDb::new();
213+
let installed = install_packages(&mut db, &["dplyr", "base"]);
214+
let dplyr = installed[0];
215+
216+
let workspace = workspace_root(&db, "w");
217+
let pkg = Package::new(
218+
&db,
219+
file_path("w/pkg/DESCRIPTION"),
220+
"pkg".to_string(),
221+
FileRevision::zero(),
222+
FileRevision::zero(),
223+
None,
224+
Vec::new(),
225+
Vec::new(),
226+
);
227+
let r_file = File::new(
228+
&db,
229+
file_path("w/pkg/R/a.R"),
230+
FileRevision::zero(),
231+
Some("internal <- 1\n".to_string()),
232+
Some(pkg),
233+
);
234+
let data_raw = File::new(
235+
&db,
236+
file_path("w/pkg/data-raw/prep.R"),
237+
FileRevision::zero(),
238+
Some("library(dplyr)\n".to_string()),
239+
Some(pkg),
240+
);
241+
pkg.set_files(&mut db).to(vec![r_file]);
242+
pkg.set_scripts(&mut db).to(vec![data_raw]);
243+
workspace.set_packages(&mut db).to(vec![pkg]);
244+
db.workspace_roots().set_roots(&mut db).to(vec![workspace]);
245+
246+
let _ = dplyr;
247+
// Only its own `library(dplyr)` and `base` from the default search path
248+
// (the only other registered package). `R/a.R` and the namespace are
249+
// absent because the script isn't part of the package's namespace.
250+
assert_eq!(shape(&db, data_raw.imports(&db)), vec![
251+
"Package(dplyr)".to_string(),
252+
"Package(base)".to_string(),
253+
]);
254+
}
255+
206256
#[test]
207257
fn test_testthat_file_sees_helpers_package_and_testthat() {
208258
let mut db = TestDb::new();

crates/oak_db/src/tests/file_imports_at.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,34 @@ fn test_package_namespace_and_base_layers_always_visible() {
230230
assert!(attached_packages(&layers).contains(&base));
231231
}
232232

233+
#[test]
234+
fn test_package_script_top_level_resolves_as_standalone_script() {
235+
// A `data-raw/` file carries a package back-pointer but lives in
236+
// `scripts`, not `files`. At top level it must narrow like a standalone
237+
// script and never take the package path, which would log the spurious
238+
// "back-pointer but not in its files" warning from #1270.
239+
let mut db = TestDb::new();
240+
let dplyr = install_packages(&mut db, &["dplyr"])[0];
241+
let pkg = install_workspace_package(&mut db, "pkg");
242+
243+
let r_file = make_package_file(&mut db, "/workspace/pkg/R/a.R", "internal <- 1\n", pkg);
244+
let source = "library(dplyr)\nx <- 1\n";
245+
let data_raw = make_package_file(&mut db, "/workspace/pkg/data-raw/prep.R", source, pkg);
246+
pkg.set_files(&mut db).to(vec![r_file]);
247+
pkg.set_scripts(&mut db).to(vec![data_raw]);
248+
249+
// Before the `library()` call: nothing attached, and no `R/` `File`
250+
// layers (the script isn't part of the package's namespace).
251+
let before = data_raw.imports_at(&db, TextSize::from(0));
252+
assert_eq!(attached_packages(&before), Vec::<Package>::new());
253+
assert_eq!(package_files(&before), Vec::<File>::new());
254+
255+
// After the call: dplyr attached, still no `R/` `File` layers.
256+
let after = data_raw.imports_at(&db, TextSize::from(source.len() as u32));
257+
assert_eq!(attached_packages(&after), vec![dplyr]);
258+
assert_eq!(package_files(&after), Vec::<File>::new());
259+
}
260+
233261
#[test]
234262
fn test_testthat_top_level_library_narrows_by_offset() {
235263
// A test file's own top-level `library()` call narrows like a script's:

0 commit comments

Comments
 (0)