Resolve bound name of bare import statically (#5773)

This commit is contained in:
Laurenz 2025-01-29 15:20:30 +01:00 committed by GitHub
parent 9665eecdb6
commit 1b2719c94c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 234 additions and 84 deletions

View File

@ -6,7 +6,7 @@ use typst_library::diag::{
use typst_library::engine::Engine;
use typst_library::foundations::{Content, Module, Value};
use typst_library::World;
use typst_syntax::ast::{self, AstNode};
use typst_syntax::ast::{self, AstNode, BareImportError};
use typst_syntax::package::{PackageManifest, PackageSpec};
use typst_syntax::{FileId, Span, VirtualPath};
@ -16,11 +16,11 @@ impl Eval for ast::ModuleImport<'_> {
type Output = Value;
fn eval(self, vm: &mut Vm) -> SourceResult<Self::Output> {
let source = self.source();
let source_span = source.span();
let mut source = source.eval(vm)?;
let new_name = self.new_name();
let imports = self.imports();
let source_expr = self.source();
let source_span = source_expr.span();
let mut source = source_expr.eval(vm)?;
let mut is_str = false;
match &source {
Value::Func(func) => {
@ -32,6 +32,7 @@ impl Eval for ast::ModuleImport<'_> {
Value::Module(_) => {}
Value::Str(path) => {
source = Value::Module(import(&mut vm.engine, path, source_span)?);
is_str = true;
}
v => {
bail!(
@ -42,9 +43,12 @@ impl Eval for ast::ModuleImport<'_> {
}
}
// Source itself is imported if there is no import list or a rename.
let bare_name = self.bare_name();
let new_name = self.new_name();
if let Some(new_name) = new_name {
if let ast::Expr::Ident(ident) = self.source() {
if ident.as_str() == new_name.as_str() {
if let Ok(source_name) = &bare_name {
if source_name == new_name.as_str() {
// Warn on `import x as x`
vm.engine.sink.warn(warning!(
new_name.span(),
@ -58,12 +62,33 @@ impl Eval for ast::ModuleImport<'_> {
}
let scope = source.scope().unwrap();
match imports {
match self.imports() {
None => {
// Only import here if there is no rename.
if new_name.is_none() {
let name: EcoString = source.name().unwrap().into();
vm.scopes.top.define(name, source);
match self.bare_name() {
// Bare dynamic string imports are not allowed.
Ok(name)
if !is_str || matches!(source_expr, ast::Expr::Str(_)) =>
{
if matches!(source_expr, ast::Expr::Ident(_)) {
vm.engine.sink.warn(warning!(
source_expr.span(),
"this import has no effect",
));
}
vm.scopes.top.define_spanned(name, source, source_span);
}
Ok(_) | Err(BareImportError::Dynamic) => bail!(
source_span, "dynamic import requires an explicit name";
hint: "you can name the import with `as`"
),
Err(BareImportError::PathInvalid) => bail!(
source_span, "module name would not be a valid identifier";
hint: "you can rename the import with `as`",
),
// Bad package spec would have failed the import already.
Err(BareImportError::PackageInvalid) => unreachable!(),
}
}
}
Some(ast::Imports::Wildcard) => {

View File

@ -1,7 +1,7 @@
use ecow::EcoString;
use typst::foundations::{Module, Value};
use typst::syntax::ast::AstNode;
use typst::syntax::{ast, LinkedNode, Span, SyntaxKind, SyntaxNode};
use typst::syntax::{ast, LinkedNode, Span, SyntaxKind};
use crate::{analyze_import, IdeWorld};
@ -30,38 +30,38 @@ pub fn named_items<T>(
if let Some(v) = node.cast::<ast::ModuleImport>() {
let imports = v.imports();
let source = node
.children()
.find(|child| child.is::<ast::Expr>())
.and_then(|source: LinkedNode| {
Some((analyze_import(world, &source)?, source))
});
let source = source.as_ref();
let source = v.source();
let source_value = node
.find(source.span())
.and_then(|source| analyze_import(world, &source));
let source_value = source_value.as_ref();
let module = source_value.and_then(|value| match value {
Value::Module(module) => Some(module),
_ => None,
});
let name_and_span = match (imports, v.new_name()) {
// ```plain
// import "foo" as name
// import "foo" as name: ..
// ```
(_, Some(name)) => Some((name.get().clone(), name.span())),
// ```plain
// import "foo"
// ```
(None, None) => v.bare_name().ok().map(|name| (name, source.span())),
// ```plain
// import "foo": ..
// ```
(Some(..), None) => None,
};
// Seeing the module itself.
if let Some((value, source)) = source {
let site = match (imports, v.new_name()) {
// ```plain
// import "foo" as name;
// import "foo" as name: ..;
// ```
(_, Some(name)) => Some(name.to_untyped()),
// ```plain
// import "foo";
// ```
(None, None) => Some(source.get()),
// ```plain
// import "foo": ..;
// ```
(Some(..), None) => None,
};
if let Some((site, value)) =
site.zip(value.clone().cast::<Module>().ok())
{
if let Some(res) = recv(NamedItem::Module(&value, site)) {
return Some(res);
}
if let Some((name, span)) = name_and_span {
if let Some(res) = recv(NamedItem::Module(&name, span, module)) {
return Some(res);
}
}
@ -75,7 +75,7 @@ pub fn named_items<T>(
// import "foo": *;
// ```
Some(ast::Imports::Wildcard) => {
if let Some(scope) = source.and_then(|(value, _)| value.scope()) {
if let Some(scope) = source_value.and_then(Value::scope) {
for (name, value, span) in scope.iter() {
let item = NamedItem::Import(name, span, Some(value));
if let Some(res) = recv(item) {
@ -92,7 +92,7 @@ pub fn named_items<T>(
let bound = item.bound_name();
let (span, value) = item.path().iter().fold(
(bound.span(), source.map(|(value, _)| value)),
(bound.span(), source_value),
|(span, value), path_ident| {
let scope = value.and_then(|v| v.scope());
let span = scope
@ -175,8 +175,8 @@ pub enum NamedItem<'a> {
Var(ast::Ident<'a>),
/// A function item.
Fn(ast::Ident<'a>),
/// A (imported) module item.
Module(&'a Module, &'a SyntaxNode),
/// A (imported) module.
Module(&'a EcoString, Span, Option<&'a Module>),
/// An imported item.
Import(&'a EcoString, Span, Option<&'a Value>),
}
@ -186,7 +186,7 @@ impl<'a> NamedItem<'a> {
match self {
NamedItem::Var(ident) => ident.get(),
NamedItem::Fn(ident) => ident.get(),
NamedItem::Module(value, _) => value.name(),
NamedItem::Module(name, _, _) => name,
NamedItem::Import(name, _, _) => name,
}
}
@ -194,7 +194,7 @@ impl<'a> NamedItem<'a> {
pub(crate) fn value(&self) -> Option<Value> {
match self {
NamedItem::Var(..) | NamedItem::Fn(..) => None,
NamedItem::Module(value, _) => Some(Value::Module((*value).clone())),
NamedItem::Module(_, _, value) => value.cloned().map(Value::Module),
NamedItem::Import(_, _, value) => value.cloned(),
}
}
@ -202,7 +202,7 @@ impl<'a> NamedItem<'a> {
pub(crate) fn span(&self) -> Span {
match *self {
NamedItem::Var(name) | NamedItem::Fn(name) => name.span(),
NamedItem::Module(_, site) => site.span(),
NamedItem::Module(_, span, _) => span,
NamedItem::Import(_, span, _) => span,
}
}
@ -356,7 +356,17 @@ mod tests {
#[test]
fn test_named_items_import() {
test("#import \"foo.typ\": a; #(a);", 2).must_include(["a"]);
test("#import \"foo.typ\"", 2).must_include(["foo"]);
test("#import \"foo.typ\" as bar", 2)
.must_include(["bar"])
.must_exclude(["foo"]);
}
#[test]
fn test_named_items_import_items() {
test("#import \"foo.typ\": a; #(a);", 2)
.must_include(["a"])
.must_exclude(["foo"]);
let world = TestWorld::new("#import \"foo.typ\": a.b; #(b);")
.with_source("foo.typ", "#import \"a.typ\"")

View File

@ -122,8 +122,8 @@ pub(super) fn define(global: &mut Scope, inputs: Dict, features: &Features) {
if features.is_enabled(Feature::Html) {
global.define_func::<target>();
}
global.define_module(calc::module());
global.define_module(sys::module(inputs));
global.define("calc", calc::module());
global.define("sys", sys::module(inputs));
}
/// Fails with an error.

View File

@ -32,7 +32,7 @@ use crate::foundations::{repr, ty, Content, Scope, Value};
#[allow(clippy::derived_hash_with_manual_eq)]
pub struct Module {
/// The module's name.
name: EcoString,
name: Option<EcoString>,
/// The reference-counted inner fields.
inner: Arc<Repr>,
}
@ -52,14 +52,22 @@ impl Module {
/// Create a new module.
pub fn new(name: impl Into<EcoString>, scope: Scope) -> Self {
Self {
name: name.into(),
name: Some(name.into()),
inner: Arc::new(Repr { scope, content: Content::empty(), file_id: None }),
}
}
/// Create a new anonymous module without a name.
pub fn anonymous(scope: Scope) -> Self {
Self {
name: None,
inner: Arc::new(Repr { scope, content: Content::empty(), file_id: None }),
}
}
/// Update the module's name.
pub fn with_name(mut self, name: impl Into<EcoString>) -> Self {
self.name = name.into();
self.name = Some(name.into());
self
}
@ -82,8 +90,8 @@ impl Module {
}
/// Get the module's name.
pub fn name(&self) -> &EcoString {
&self.name
pub fn name(&self) -> Option<&EcoString> {
self.name.as_ref()
}
/// Access the module's scope.
@ -105,8 +113,9 @@ impl Module {
/// Try to access a definition in the module.
pub fn field(&self, name: &str) -> StrResult<&Value> {
self.scope().get(name).ok_or_else(|| {
eco_format!("module `{}` does not contain `{name}`", self.name())
self.scope().get(name).ok_or_else(|| match &self.name {
Some(module) => eco_format!("module `{module}` does not contain `{name}`"),
None => eco_format!("module does not contain `{name}`"),
})
}
@ -131,7 +140,10 @@ impl Debug for Module {
impl repr::Repr for Module {
fn repr(&self) -> EcoString {
eco_format!("<module {}>", self.name())
match &self.name {
Some(module) => eco_format!("<module {module}>"),
None => "<module>".into(),
}
}
}

View File

@ -12,8 +12,8 @@ use typst_utils::Static;
use crate::diag::{bail, HintedStrResult, HintedString, StrResult};
use crate::foundations::{
Element, Func, IntoValue, Module, NativeElement, NativeFunc, NativeFuncData,
NativeType, Type, Value,
Element, Func, IntoValue, NativeElement, NativeFunc, NativeFuncData, NativeType,
Type, Value,
};
use crate::Library;
@ -252,11 +252,6 @@ impl Scope {
self.define(data.name, Element::from(data));
}
/// Define a module.
pub fn define_module(&mut self, module: Module) {
self.define(module.name().clone(), module);
}
/// Try to access a variable immutably.
pub fn get(&self, var: &str) -> Option<&Value> {
self.map.get(var).map(Slot::read)

View File

@ -181,16 +181,6 @@ impl Value {
}
}
/// The name, if this is a function, type, or module.
pub fn name(&self) -> Option<&str> {
match self {
Self::Func(func) => func.name(),
Self::Type(ty) => Some(ty.short_name()),
Self::Module(module) => Some(module.name()),
_ => None,
}
}
/// Try to extract documentation for the value.
pub fn docs(&self) -> Option<&'static str> {
match self {
@ -730,6 +720,11 @@ mod tests {
assert_eq!(value.into_value().repr(), exp);
}
#[test]
fn test_value_size() {
assert!(std::mem::size_of::<Value>() <= 32);
}
#[test]
fn test_value_debug() {
// Primitives.

View File

@ -244,7 +244,7 @@ fn global(math: Module, inputs: Dict, features: &Features) -> Module {
self::model::define(&mut global);
self::text::define(&mut global);
global.reset_category();
global.define_module(math);
global.define("math", math);
self::layout::define(&mut global);
self::visualize::define(&mut global);
self::introspection::define(&mut global);
@ -253,7 +253,7 @@ fn global(math: Module, inputs: Dict, features: &Features) -> Module {
self::pdf::define(&mut global);
global.reset_category();
if features.is_enabled(Feature::Html) {
global.define_module(self::html::module());
global.define("html", self::html::module());
}
prelude(&mut global);
Module::new("global", global)

View File

@ -13,7 +13,7 @@ pub static PDF: Category;
/// Hook up the `pdf` module.
pub(super) fn define(global: &mut Scope) {
global.category(PDF);
global.define_module(module());
global.define("pdf", module());
}
/// Hook up all `pdf` definitions.

View File

@ -4,11 +4,14 @@
use std::num::NonZeroUsize;
use std::ops::Deref;
use std::path::Path;
use std::str::FromStr;
use ecow::EcoString;
use unscanny::Scanner;
use crate::{is_newline, Span, SyntaxKind, SyntaxNode};
use crate::package::PackageSpec;
use crate::{is_ident, is_newline, Span, SyntaxKind, SyntaxNode};
/// A typed AST node.
pub trait AstNode<'a>: Sized {
@ -2064,6 +2067,41 @@ impl<'a> ModuleImport<'a> {
})
}
/// The name that will be bound for a bare import. This name must be
/// statically known. It can come from:
/// - an identifier
/// - a field access
/// - a string that is a valid file path where the file stem is a valid
/// identifier
/// - a string that is a valid package spec
pub fn bare_name(self) -> Result<EcoString, BareImportError> {
match self.source() {
Expr::Ident(ident) => Ok(ident.get().clone()),
Expr::FieldAccess(access) => Ok(access.field().get().clone()),
Expr::Str(string) => {
let string = string.get();
let name = if string.starts_with('@') {
PackageSpec::from_str(&string)
.map_err(|_| BareImportError::PackageInvalid)?
.name
} else {
Path::new(string.as_str())
.file_stem()
.and_then(|path| path.to_str())
.ok_or(BareImportError::PathInvalid)?
.into()
};
if !is_ident(&name) {
return Err(BareImportError::PathInvalid);
}
Ok(name)
}
_ => Err(BareImportError::Dynamic),
}
}
/// The name this module was assigned to, if it was renamed with `as`
/// (`renamed` in `import "..." as renamed`).
pub fn new_name(self) -> Option<Ident<'a>> {
@ -2074,6 +2112,18 @@ impl<'a> ModuleImport<'a> {
}
}
/// Reasons why a bare name cannot be determined for an import source.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
pub enum BareImportError {
/// There is no statically resolvable binding name.
Dynamic,
/// The import source is not a valid path or the path stem not a valid
/// identifier.
PathInvalid,
/// The import source is not a valid package spec.
PackageInvalid,
}
/// The items that ought to be imported from a file.
#[derive(Debug, Copy, Clone, Hash)]
pub enum Imports<'a> {

View File

@ -145,6 +145,34 @@
#test(module.item(1, 2), 3)
#test(module.push(2), 3)
--- import-from-file-bare-invalid ---
// Error: 9-33 module name would not be a valid identifier
// Hint: 9-33 you can rename the import with `as`
#import "modules/with space.typ"
--- import-from-file-bare-dynamic ---
// Error: 9-26 dynamic import requires an explicit name
// Hint: 9-26 you can name the import with `as`
#import "mod" + "ule.typ"
--- import-from-var-bare ---
#let p = "module.typ"
// Error: 9-10 dynamic import requires an explicit name
// Hint: 9-10 you can name the import with `as`
#import p
#test(p.b, 1)
--- import-from-dict-field-bare ---
#let d = (p: "module.typ")
// Error: 9-12 dynamic import requires an explicit name
// Hint: 9-12 you can name the import with `as`
#import d.p
#test(p.b, 1)
--- import-from-file-renamed-dynamic ---
#import "mod" + "ule.typ" as mod
#test(mod.b, 1)
--- import-from-file-renamed ---
// A renamed module import without items.
#import "module.typ" as other
@ -160,6 +188,10 @@
#test(item(1, 2), 3)
#test(newname.item(1, 2), 3)
--- import-from-function-scope-bare ---
// Warning: 9-13 this import has no effect
#import enum
--- import-from-function-scope-renamed ---
// Renamed module import with function scopes.
#import enum as othernum
@ -171,6 +203,23 @@
#import asrt: ne as asne
#asne(1, 2)
--- import-from-module-bare ---
#import "modules/chap1.typ" as mymod
// Warning: 9-14 this import has no effect
#import mymod
// The name `chap1` is not bound.
// Error: 2-7 unknown variable: chap1
#chap1
--- import-module-nested ---
#import std.calc: pi
#test(pi, calc.pi)
--- import-module-nested-bare ---
#import "module.typ"
#import module.chap2
#test(chap2.name, "Peter")
--- import-module-item-name-mutating ---
// Edge case for module access that isn't fixed.
#import "module.typ"
@ -214,10 +263,14 @@
// Warning: 31-35 unnecessary import rename to same name
#import enum as enum: item as item
--- import-item-rename-unnecessary-but-ok ---
// No warning on a case that isn't obviously pathological
--- import-item-rename-unnecessary-string ---
// Warning: 25-31 unnecessary import rename to same name
#import "module.typ" as module
--- import-item-rename-unnecessary-but-ok ---
#import "modul" + "e.typ" as module
#test(module.b, 1)
--- import-from-closure-invalid ---
// Can't import from closures.
#let f(x) = x
@ -359,6 +412,15 @@ This is never reached.
#import "@test/adder:0.1.0"
#test(adder.add(2, 8), 10)
--- import-from-package-dynamic ---
// Error: 9-33 dynamic import requires an explicit name
// Hint: 9-33 you can name the import with `as`
#import "@test/" + "adder:0.1.0"
--- import-from-package-renamed-dynamic ---
#import "@test/" + "adder:0.1.0" as adder
#test(adder.add(2, 8), 10)
--- import-from-package-items ---
// Test import with items.
#import "@test/adder:0.1.0": add

View File

@ -0,0 +1 @@
// SKIP