fix: review follow-ups for #5 and the Swift lexical preview #8
4 changed files with 172 additions and 2 deletions
fix(dart): honour import prefixes, show/hide and one level of export
Problem: a bare call in a Dart file resolved against every imported library. An `import 'a.dart' as p;` still bound `foo()` to a.dart's foo, `hide` and `show` combinators were ignored, and a name re-exported through a barrel library (`export 'impl.dart';`) never resolved. Fix: record each Dart import/export directive with its `as` prefix and show/hide lists. A bare name resolves only in libraries imported without a prefix whose combinators let it through, plus that library's own exports that also let it through (one level). Test: tests/dart_imports.rs covers a prefixed, a hiding, a showing and a barrelled import; it failed on the prefixed case before this change.
commit
324900287b
|
|
@ -174,7 +174,8 @@ directory (Dart `package:<own package>/` against `lib/`); missing paths never
|
||||||
fall back to unrelated basenames. Explicit JS/TS named/default import bindings,
|
fall back to unrelated basenames. Explicit JS/TS named/default import bindings,
|
||||||
module-scoped Rust grouped/aliased imports, module-qualified Rust public calls,
|
module-scoped Rust grouped/aliased imports, module-qualified Rust public calls,
|
||||||
and public top-level Dart names from imported libraries can resolve across files
|
and public top-level Dart names from imported libraries can resolve across files
|
||||||
(Dart `show`/`hide` combinators are not modelled). The lexical preview resolves
|
(Dart `show`/`hide` are honoured, `as`-prefixed imports never bind bare names,
|
||||||
|
and a library's own `export`s are followed one level). The lexical preview resolves
|
||||||
Python names in the calling file, then in files it imports; Swift names in the
|
Python names in the calling file, then in files it imports; Swift names in the
|
||||||
calling file, then by a module-wide unique name (Swift files of one module see
|
calling file, then by a module-wide unique name (Swift files of one module see
|
||||||
each other without imports).
|
each other without imports).
|
||||||
|
|
|
||||||
44
src/scan.rs
44
src/scan.rs
|
|
@ -252,6 +252,8 @@ struct ScanState {
|
||||||
/// Imports edges by source file id, built once on first use: scanning every
|
/// Imports edges by source file id, built once on first use: scanning every
|
||||||
/// node and edge per file made resolution O(files x (nodes + edges)).
|
/// node and edge per file made resolution O(files x (nodes + edges)).
|
||||||
imports_by_file: Option<HashMap<String, Vec<String>>>,
|
imports_by_file: Option<HashMap<String, Vec<String>>>,
|
||||||
|
/// Dart file -> its import/export directives (prefixes, show/hide, exports).
|
||||||
|
dart_directives: HashMap<String, Vec<crate::source::DartDirective>>,
|
||||||
/// `name:` of the root pubspec.yaml, so `package:<name>/x.dart` maps to lib/x.dart.
|
/// `name:` of the root pubspec.yaml, so `package:<name>/x.dart` maps to lib/x.dart.
|
||||||
dart_package: Option<String>,
|
dart_package: Option<String>,
|
||||||
}
|
}
|
||||||
|
|
@ -288,6 +290,7 @@ impl ScanState {
|
||||||
references: HashMap::new(),
|
references: HashMap::new(),
|
||||||
imported_paths: HashMap::new(),
|
imported_paths: HashMap::new(),
|
||||||
imports_by_file: None,
|
imports_by_file: None,
|
||||||
|
dart_directives: HashMap::new(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -639,6 +642,10 @@ impl ScanState {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
if language == "dart" {
|
||||||
|
self.dart_directives
|
||||||
|
.insert(path.to_owned(), document.dart_directives.clone());
|
||||||
|
}
|
||||||
for message in &document.diagnostics {
|
for message in &document.diagnostics {
|
||||||
self.diagnostics.push(Diagnostic {
|
self.diagnostics.push(Diagnostic {
|
||||||
level: "warning".into(),
|
level: "warning".into(),
|
||||||
|
|
@ -1356,6 +1363,39 @@ impl ScanState {
|
||||||
paths
|
paths
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Libraries whose public top-level `name` a Dart file can call bare: imports
|
||||||
|
/// without an `as` prefix whose `show`/`hide` let `name` through, plus one level
|
||||||
|
/// of those libraries' own `export`s that also let it through.
|
||||||
|
fn dart_visible_libraries(&self, path: &str, name: &str) -> HashSet<String> {
|
||||||
|
let library_of = |target: &str, from: &str| {
|
||||||
|
self.resolve_import(target, "dart", from)
|
||||||
|
.and_then(|id| self.node(&id))
|
||||||
|
.and_then(|node| node.path.clone())
|
||||||
|
};
|
||||||
|
let mut visible = HashSet::new();
|
||||||
|
for directive in self.dart_directives.get(path).into_iter().flatten() {
|
||||||
|
if directive.export || directive.prefix.is_some() || !directive.allows(name) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let Some(library) = library_of(&directive.target, path) else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
for export in self
|
||||||
|
.dart_directives
|
||||||
|
.get(&library)
|
||||||
|
.into_iter()
|
||||||
|
.flatten()
|
||||||
|
.filter(|export| export.export && export.allows(name))
|
||||||
|
{
|
||||||
|
if let Some(exported) = library_of(&export.target, &library) {
|
||||||
|
visible.insert(exported);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
visible.insert(library);
|
||||||
|
}
|
||||||
|
visible
|
||||||
|
}
|
||||||
|
|
||||||
/// Mark functions that may be used without a resolvable call: named outside
|
/// Mark functions that may be used without a resolvable call: named outside
|
||||||
/// a call position anywhere in the language, or C++ virtual via a prototype.
|
/// a call position anywhere in the language, or C++ virtual via a prototype.
|
||||||
fn annotate_indirect_uses(&mut self) {
|
fn annotate_indirect_uses(&mut self) {
|
||||||
|
|
@ -1434,8 +1474,10 @@ impl ScanState {
|
||||||
if let Some(id) = self.resolve_qualified_symbol(relationship) {
|
if let Some(id) = self.resolve_qualified_symbol(relationship) {
|
||||||
return (id, None, None);
|
return (id, None, None);
|
||||||
}
|
}
|
||||||
let imported = if language == "python" || language == "dart" {
|
let imported = if language == "python" {
|
||||||
self.imported_paths(&relationship.path)
|
self.imported_paths(&relationship.path)
|
||||||
|
} else if language == "dart" {
|
||||||
|
Rc::new(self.dart_visible_libraries(&relationship.path, name))
|
||||||
} else {
|
} else {
|
||||||
Rc::default()
|
Rc::default()
|
||||||
};
|
};
|
||||||
|
|
|
||||||
|
|
@ -38,6 +38,25 @@ pub(crate) struct Declaration {
|
||||||
pub preprocessor_alternative: bool,
|
pub preprocessor_alternative: bool,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A Dart `import` or `export` directive and what it exposes.
|
||||||
|
#[derive(Clone, Debug)]
|
||||||
|
pub(crate) struct DartDirective {
|
||||||
|
pub target: String,
|
||||||
|
/// `as p`: names are reachable only as `p.name`, never bare.
|
||||||
|
pub prefix: Option<String>,
|
||||||
|
pub show: Vec<String>,
|
||||||
|
pub hide: Vec<String>,
|
||||||
|
pub export: bool,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl DartDirective {
|
||||||
|
/// The directive's combinators let `name` through.
|
||||||
|
pub fn allows(&self, name: &str) -> bool {
|
||||||
|
(self.show.is_empty() || self.show.iter().any(|shown| shown == name))
|
||||||
|
&& !self.hide.iter().any(|hidden| hidden == name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
pub(crate) struct ImportBinding {
|
pub(crate) struct ImportBinding {
|
||||||
pub local: String,
|
pub local: String,
|
||||||
pub exported: String,
|
pub exported: String,
|
||||||
|
|
@ -107,6 +126,8 @@ pub(crate) struct Document {
|
||||||
pub diagnostics: Vec<String>,
|
pub diagnostics: Vec<String>,
|
||||||
pub backend: String,
|
pub backend: String,
|
||||||
pub bindings: Vec<ImportBinding>,
|
pub bindings: Vec<ImportBinding>,
|
||||||
|
/// Dart import/export directives with their prefixes and combinators.
|
||||||
|
pub dart_directives: Vec<DartDirective>,
|
||||||
/// C++ qualified names declared (prototype or definition) by this file.
|
/// C++ qualified names declared (prototype or definition) by this file.
|
||||||
pub declared: Vec<String>,
|
pub declared: Vec<String>,
|
||||||
/// C/C++ preprocessor macro names defined by this file.
|
/// C/C++ preprocessor macro names defined by this file.
|
||||||
|
|
@ -221,6 +242,7 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result<Docume
|
||||||
diagnostics: Vec::new(),
|
diagnostics: Vec::new(),
|
||||||
backend: format!("tree-sitter-{version}:brick-source-v1"),
|
backend: format!("tree-sitter-{version}:brick-source-v1"),
|
||||||
bindings: Vec::new(),
|
bindings: Vec::new(),
|
||||||
|
dart_directives: Vec::new(),
|
||||||
declared: Vec::new(),
|
declared: Vec::new(),
|
||||||
macros: Vec::new(),
|
macros: Vec::new(),
|
||||||
parse_incomplete: false,
|
parse_incomplete: false,
|
||||||
|
|
@ -669,6 +691,10 @@ pub(crate) fn extract(language: &str, path: &str, source: &str) -> Result<Docume
|
||||||
{
|
{
|
||||||
doc.bindings.extend(import_bindings(node, &target, source));
|
doc.bindings.extend(import_bindings(node, &target, source));
|
||||||
}
|
}
|
||||||
|
if language == "dart" {
|
||||||
|
doc.dart_directives
|
||||||
|
.push(dart_directive(node, &target, source));
|
||||||
|
}
|
||||||
doc.relationships.push(Relationship {
|
doc.relationships.push(Relationship {
|
||||||
owner: None,
|
owner: None,
|
||||||
target,
|
target,
|
||||||
|
|
@ -1098,6 +1124,35 @@ fn parameter_shadows<'tree>(
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn dart_directive(node: Node<'_>, target: &str, source: &str) -> DartDirective {
|
||||||
|
let mut directive = DartDirective {
|
||||||
|
target: target.to_owned(),
|
||||||
|
prefix: node
|
||||||
|
.child_by_field_name("alias")
|
||||||
|
.map(|alias| text(alias, source).to_owned()),
|
||||||
|
show: Vec::new(),
|
||||||
|
hide: Vec::new(),
|
||||||
|
export: node.kind() == "library_export",
|
||||||
|
};
|
||||||
|
let mut cursor = node.walk();
|
||||||
|
for combinator in node
|
||||||
|
.named_children(&mut cursor)
|
||||||
|
.filter(|child| child.kind() == "combinator")
|
||||||
|
{
|
||||||
|
let mut names = combinator.walk();
|
||||||
|
let identifiers = combinator
|
||||||
|
.named_children(&mut names)
|
||||||
|
.filter(|child| child.kind() == "identifier")
|
||||||
|
.map(|identifier| text(identifier, source).to_owned());
|
||||||
|
if text(combinator, source).trim_start().starts_with("hide") {
|
||||||
|
directive.hide.extend(identifiers);
|
||||||
|
} else {
|
||||||
|
directive.show.extend(identifiers);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
directive
|
||||||
|
}
|
||||||
|
|
||||||
fn import_bindings(node: Node<'_>, module: &str, source: &str) -> Vec<ImportBinding> {
|
fn import_bindings(node: Node<'_>, module: &str, source: &str) -> Vec<ImportBinding> {
|
||||||
let mut result = Vec::new();
|
let mut result = Vec::new();
|
||||||
// Type-only imports must not authorize runtime call resolution.
|
// Type-only imports must not authorize runtime call resolution.
|
||||||
|
|
|
||||||
72
tests/dart_imports.rs
Normal file
72
tests/dart_imports.rs
Normal file
|
|
@ -0,0 +1,72 @@
|
||||||
|
use brick::model::{Building, EdgeKind};
|
||||||
|
use brick::scan;
|
||||||
|
use std::fs;
|
||||||
|
|
||||||
|
fn scan_files(files: &[(&str, &str)]) -> Building {
|
||||||
|
let root = tempfile::tempdir().unwrap();
|
||||||
|
for (path, content) in files {
|
||||||
|
let file = root.path().join(path);
|
||||||
|
fs::create_dir_all(file.parent().unwrap()).unwrap();
|
||||||
|
fs::write(file, content).unwrap();
|
||||||
|
}
|
||||||
|
scan::scan(root.path()).unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Paths of the resolved targets of calls leaving functions named `caller`.
|
||||||
|
fn call_targets(model: &Building, caller: &str) -> Vec<String> {
|
||||||
|
model
|
||||||
|
.edges
|
||||||
|
.iter()
|
||||||
|
.filter(|edge| edge.kind == EdgeKind::Calls)
|
||||||
|
.filter(|edge| {
|
||||||
|
model
|
||||||
|
.nodes
|
||||||
|
.iter()
|
||||||
|
.any(|n| n.id == edge.source && n.name == caller)
|
||||||
|
})
|
||||||
|
.filter_map(|edge| {
|
||||||
|
let target = model.nodes.iter().find(|n| n.id == edge.target)?;
|
||||||
|
Some(format!("{}:{}", target.path.clone()?, target.name))
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn dart_import_prefixes_combinators_and_exports_decide_what_a_bare_name_binds() {
|
||||||
|
// Dart calls resolved through every imported library, ignoring `as` prefixes
|
||||||
|
// and `show`/`hide`, and never through a library's own `export`s.
|
||||||
|
let model = scan_files(&[
|
||||||
|
("lib/a.dart", "void foo() {}\n"),
|
||||||
|
("lib/h.dart", "void hidden() {}\nvoid visible() {}\n"),
|
||||||
|
("lib/s.dart", "void shown() {}\nvoid other() {}\n"),
|
||||||
|
("lib/impl.dart", "void exported() {}\n"),
|
||||||
|
("lib/barrel.dart", "export 'impl.dart';\n"),
|
||||||
|
(
|
||||||
|
"lib/prefixed.dart",
|
||||||
|
"import 'a.dart' as a;\nvoid usePrefix() { foo(); }\n",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"lib/hiding.dart",
|
||||||
|
"import 'h.dart' hide hidden;\nvoid useHide() { hidden(); visible(); }\n",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"lib/showing.dart",
|
||||||
|
"import 's.dart' show shown;\nvoid useShow() { shown(); other(); }\n",
|
||||||
|
),
|
||||||
|
(
|
||||||
|
"lib/barrelled.dart",
|
||||||
|
"import 'barrel.dart';\nvoid useExport() { exported(); }\n",
|
||||||
|
),
|
||||||
|
]);
|
||||||
|
assert!(
|
||||||
|
call_targets(&model, "usePrefix").is_empty(),
|
||||||
|
"{:?}",
|
||||||
|
call_targets(&model, "usePrefix")
|
||||||
|
);
|
||||||
|
assert_eq!(call_targets(&model, "useHide"), ["lib/h.dart:visible"]);
|
||||||
|
assert_eq!(call_targets(&model, "useShow"), ["lib/s.dart:shown"]);
|
||||||
|
assert_eq!(
|
||||||
|
call_targets(&model, "useExport"),
|
||||||
|
["lib/impl.dart:exported"]
|
||||||
|
);
|
||||||
|
}
|
||||||
Loading…
Add table
Add a link
Reference in a new issue