Resolve bound name of bare import statically (#5773)

11 files changed, 234 insertions, 84 deletions

diff --git a/crates/typst-eval/src/import.rs b/crates/typst-eval/src/import.rs
index 2060d25f..2bbc7e41 100644
--- a/crates/typst-eval/src/import.rs
+++ b/crates/typst-eval/src/import.rs
@@ -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) => {
diff --git a/crates/typst-ide/src/matchers.rs b/crates/typst-ide/src/matchers.rs
index b92cbf55..ef8288f2 100644
--- a/crates/typst-ide/src/matchers.rs
+++ b/crates/typst-ide/src/matchers.rs
@@ -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\"")
diff --git a/crates/typst-library/src/foundations/mod.rs b/crates/typst-library/src/foundations/mod.rs
index 2c3730d5..2921481b 100644
--- a/crates/typst-library/src/foundations/mod.rs
+++ b/crates/typst-library/src/foundations/mod.rs
@@ -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.
diff --git a/crates/typst-library/src/foundations/module.rs b/crates/typst-library/src/foundations/module.rs
index a476d6af..2001aca1 100644
--- a/crates/typst-library/src/foundations/module.rs
+++ b/crates/typst-library/src/foundations/module.rs
@@ -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(),
+        }
     }
 }
 
diff --git a/crates/typst-library/src/foundations/scope.rs b/crates/typst-library/src/foundations/scope.rs
index b51f8caa..99c9a37e 100644
--- a/crates/typst-library/src/foundations/scope.rs
+++ b/crates/typst-library/src/foundations/scope.rs
@@ -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)
diff --git a/crates/typst-library/src/foundations/value.rs b/crates/typst-library/src/foundations/value.rs
index 8d9f5933..d9902772 100644
--- a/crates/typst-library/src/foundations/value.rs
+++ b/crates/typst-library/src/foundations/value.rs
@@ -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 {
@@ -731,6 +721,11 @@ mod tests {
     }
 
     #[test]
+    fn test_value_size() {
+        assert!(std::mem::size_of::<Value>() <= 32);
+    }
+
+    #[test]
     fn test_value_debug() {
         // Primitives.
         test(Value::None, "none");
diff --git a/crates/typst-library/src/lib.rs b/crates/typst-library/src/lib.rs
index 2ea77eaa..22f3a62a 100644
--- a/crates/typst-library/src/lib.rs
+++ b/crates/typst-library/src/lib.rs
@@ -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)
diff --git a/crates/typst-library/src/pdf/mod.rs b/crates/typst-library/src/pdf/mod.rs
index 669835d4..ec075463 100644
--- a/crates/typst-library/src/pdf/mod.rs
+++ b/crates/typst-library/src/pdf/mod.rs
@@ -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.
diff --git a/crates/typst-syntax/src/ast.rs b/crates/typst-syntax/src/ast.rs
index 014e8392..640138e7 100644
--- a/crates/typst-syntax/src/ast.rs
+++ b/crates/typst-syntax/src/ast.rs
@@ -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> {
diff --git a/tests/suite/scripting/import.typ b/tests/suite/scripting/import.typ
index 95214db7..03e2efc6 100644
--- a/tests/suite/scripting/import.typ
+++ b/tests/suite/scripting/import.typ
@@ -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
diff --git a/tests/suite/scripting/modules/with space.typ b/tests/suite/scripting/modules/with space.typ
new file mode 100644
index 00000000..9138f3c3
--- /dev/null
+++ b/tests/suite/scripting/modules/with space.typ
@@ -0,0 +1 @@
+// SKIP