Allow renaming imports with `as` (#1923)

8 files changed, 230 insertions, 13 deletions

diff --git a/crates/typst-syntax/src/ast.rs b/crates/typst-syntax/src/ast.rs
index 7cf0cbd5..fd3bfebe 100644
--- a/crates/typst-syntax/src/ast.rs
+++ b/crates/typst-syntax/src/ast.rs
@@ -1988,6 +1988,15 @@ impl<'a> ModuleImport<'a> {
             _ => Option::None,
         })
     }
+
+    /// 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>> {
+        self.0
+            .children()
+            .skip_while(|child| child.kind() != SyntaxKind::As)
+            .find_map(SyntaxNode::cast)
+    }
 }
 
 /// The items that ought to be imported from a file.
@@ -2005,9 +2014,65 @@ node! {
 }
 
 impl<'a> ImportItems<'a> {
-    /// The items to import from the module.
-    pub fn idents(self) -> impl DoubleEndedIterator<Item = Ident<'a>> {
-        self.0.children().filter_map(SyntaxNode::cast)
+    /// Returns an iterator over the items to import from the module.
+    pub fn iter(self) -> impl DoubleEndedIterator<Item = ImportItem<'a>> {
+        self.0.children().filter_map(|child| match child.kind() {
+            SyntaxKind::RenamedImportItem => child.cast().map(ImportItem::Renamed),
+            SyntaxKind::Ident => child.cast().map(ImportItem::Simple),
+            _ => Option::None,
+        })
+    }
+}
+
+/// An imported item, potentially renamed to another identifier.
+#[derive(Debug, Copy, Clone, Hash)]
+pub enum ImportItem<'a> {
+    /// A non-renamed import (the item's name in the scope is the same as its
+    /// name).
+    Simple(Ident<'a>),
+    /// A renamed import (the item was bound to a different name in the scope
+    /// than the one it was defined as).
+    Renamed(RenamedImportItem<'a>),
+}
+
+impl<'a> ImportItem<'a> {
+    /// The original name of the imported item, at its source. This will be the
+    /// equal to the bound name if the item wasn't renamed with 'as'.
+    pub fn original_name(self) -> Ident<'a> {
+        match self {
+            Self::Simple(name) => name,
+            Self::Renamed(renamed_item) => renamed_item.original_name(),
+        }
+    }
+
+    /// The name which this import item was bound to. Corresponds to the new
+    /// name, if it was renamed; otherwise, it's just its original name.
+    pub fn bound_name(self) -> Ident<'a> {
+        match self {
+            Self::Simple(name) => name,
+            Self::Renamed(renamed_item) => renamed_item.new_name(),
+        }
+    }
+}
+
+node! {
+    /// A renamed import item: `a as d`
+    RenamedImportItem
+}
+
+impl<'a> RenamedImportItem<'a> {
+    /// The original name of the imported item (`a` in `a as d`).
+    pub fn original_name(self) -> Ident<'a> {
+        self.0.cast_first_match().unwrap_or_default()
+    }
+
+    /// The new name of the imported item (`d` in `a as d`).
+    pub fn new_name(self) -> Ident<'a> {
+        self.0
+            .children()
+            .filter_map(SyntaxNode::cast)
+            .nth(1)
+            .unwrap_or_default()
     }
 }
 
diff --git a/crates/typst-syntax/src/kind.rs b/crates/typst-syntax/src/kind.rs
index 49119720..669ca0f2 100644
--- a/crates/typst-syntax/src/kind.rs
+++ b/crates/typst-syntax/src/kind.rs
@@ -242,6 +242,8 @@ pub enum SyntaxKind {
     ModuleImport,
     /// Items to import from a module: `a, b, c`.
     ImportItems,
+    /// A renamed import item: `a as d`.
+    RenamedImportItem,
     /// A module include: `include "chapter1.typ"`.
     ModuleInclude,
     /// A break from a loop: `break`.
@@ -465,6 +467,7 @@ impl SyntaxKind {
             Self::ForLoop => "for-loop expression",
             Self::ModuleImport => "`import` expression",
             Self::ImportItems => "import items",
+            Self::RenamedImportItem => "renamed import item",
             Self::ModuleInclude => "`include` expression",
             Self::LoopBreak => "`break` expression",
             Self::LoopContinue => "`continue` expression",
diff --git a/crates/typst-syntax/src/parser.rs b/crates/typst-syntax/src/parser.rs
index e0476ec8..306ac798 100644
--- a/crates/typst-syntax/src/parser.rs
+++ b/crates/typst-syntax/src/parser.rs
@@ -1138,6 +1138,12 @@ fn module_import(p: &mut Parser) {
     let m = p.marker();
     p.assert(SyntaxKind::Import);
     code_expr(p);
+    if p.eat_if(SyntaxKind::As) {
+        // Allow renaming a full module import.
+        // If items are included, both the full module and the items are
+        // imported at the same time.
+        p.expect(SyntaxKind::Ident);
+    }
     if p.eat_if(SyntaxKind::Colon) && !p.eat_if(SyntaxKind::Star) {
         import_items(p);
     }
@@ -1147,9 +1153,17 @@ fn module_import(p: &mut Parser) {
 fn import_items(p: &mut Parser) {
     let m = p.marker();
     while !p.eof() && !p.at(SyntaxKind::Semicolon) {
+        let item_marker = p.marker();
         if !p.eat_if(SyntaxKind::Ident) {
             p.unexpected();
         }
+
+        // Rename imported item.
+        if p.eat_if(SyntaxKind::As) {
+            p.expect(SyntaxKind::Ident);
+            p.wrap(item_marker, SyntaxKind::RenamedImportItem);
+        }
+
         if p.current().is_terminator() {
             break;
         }
diff --git a/crates/typst/src/eval/func.rs b/crates/typst/src/eval/func.rs
index d185b319..effafa7b 100644
--- a/crates/typst/src/eval/func.rs
+++ b/crates/typst/src/eval/func.rs
@@ -544,8 +544,8 @@ impl<'a> CapturesVisitor<'a> {
             Some(ast::Expr::Import(expr)) => {
                 self.visit(expr.source().to_untyped());
                 if let Some(ast::Imports::Items(items)) = expr.imports() {
-                    for item in items.idents() {
-                        self.bind(item);
+                    for item in items.iter() {
+                        self.bind(item.bound_name());
                     }
                 }
             }
diff --git a/crates/typst/src/eval/mod.rs b/crates/typst/src/eval/mod.rs
index 706f6b01..3f0e4577 100644
--- a/crates/typst/src/eval/mod.rs
+++ b/crates/typst/src/eval/mod.rs
@@ -68,6 +68,7 @@ use std::mem;
 
 use comemo::{Track, Tracked, TrackedMut, Validate};
 use ecow::{EcoString, EcoVec};
+use if_chain::if_chain;
 use serde::{Deserialize, Serialize};
 use unicode_segmentation::UnicodeSegmentation;
 
@@ -1704,15 +1705,30 @@ impl Eval for ast::ForLoop<'_> {
 }
 
 /// Applies imports from `import` to the current scope.
-fn apply_imports<V: IntoValue>(
+fn apply_imports<V: IntoValue + Clone>(
     imports: Option<ast::Imports>,
     vm: &mut Vm,
     source_value: V,
-    name: impl Fn(&V) -> EcoString,
+    new_name: Option<&str>,
+    name: impl FnOnce(&V) -> EcoString,
     scope: impl Fn(&V) -> &Scope,
 ) -> SourceResult<()> {
+    if let Some(new_name) = new_name {
+        // Renamed module => define it on the scope (possibly with further items).
+        if imports.is_none() {
+            // Avoid unneeded clone when there are no imported items.
+            vm.scopes.top.define(new_name, source_value);
+            return Ok(());
+        } else {
+            vm.scopes.top.define(new_name, source_value.clone());
+        }
+    }
+
     match imports {
         None => {
+            // If the module were renamed and there were no imported items, we
+            // would have returned above. It is therefore safe to import the
+            // module with its original name here.
             vm.scopes.top.define(name(&source_value), source_value);
         }
         Some(ast::Imports::Wildcard) => {
@@ -1723,11 +1739,22 @@ fn apply_imports<V: IntoValue>(
         Some(ast::Imports::Items(items)) => {
             let mut errors = vec![];
             let scope = scope(&source_value);
-            for ident in items.idents() {
-                if let Some(value) = scope.get(&ident) {
-                    vm.define(ident, value.clone());
+            for item in items.iter() {
+                let original_ident = item.original_name();
+                if let Some(value) = scope.get(&original_ident) {
+                    if let ast::ImportItem::Renamed(renamed_item) = &item {
+                        if renamed_item.original_name().as_str()
+                            == renamed_item.new_name().as_str()
+                        {
+                            vm.vt.tracer.warn(warning!(
+                                renamed_item.new_name().span(),
+                                "unnecessary import rename to same name",
+                            ));
+                        }
+                    }
+                    vm.define(item.bound_name(), value.clone());
                 } else {
-                    errors.push(error!(ident.span(), "unresolved import"));
+                    errors.push(error!(original_ident.span(), "unresolved import"));
                 }
             }
             if !errors.is_empty() {
@@ -1746,6 +1773,20 @@ impl Eval for ast::ModuleImport<'_> {
     fn eval(self, vm: &mut Vm) -> SourceResult<Self::Output> {
         let span = self.source().span();
         let source = self.source().eval(vm)?;
+        let new_name_ident = self.new_name();
+        let new_name = new_name_ident.map(ast::Ident::as_str);
+        if_chain! {
+            if let Some(new_name_ident) = new_name_ident;
+            if let ast::Expr::Ident(ident) = self.source();
+            if ident.as_str() == new_name_ident.as_str();
+            then {
+                // warn on `import x as x`
+                vm.vt.tracer.warn(warning!(
+                    new_name_ident.span(),
+                    "unnecessary import rename to same name",
+                ));
+            }
+        }
         if let Value::Func(func) = source {
             if func.info().is_none() {
                 bail!(span, "cannot import from user-defined functions");
@@ -1754,6 +1795,7 @@ impl Eval for ast::ModuleImport<'_> {
                 self.imports(),
                 vm,
                 func,
+                new_name,
                 |func| func.info().unwrap().name.into(),
                 |func| &func.info().unwrap().scope,
             )?;
@@ -1763,6 +1805,7 @@ impl Eval for ast::ModuleImport<'_> {
                 self.imports(),
                 vm,
                 module,
+                new_name,
                 |module| module.name().clone(),
                 |module| module.scope(),
             )?;
diff --git a/crates/typst/src/ide/complete.rs b/crates/typst/src/ide/complete.rs
index ded6205e..4f2bacfd 100644
--- a/crates/typst/src/ide/complete.rs
+++ b/crates/typst/src/ide/complete.rs
@@ -503,12 +503,12 @@ fn import_item_completions<'a>(
         _ => return,
     };
 
-    if existing.idents().next().is_none() {
+    if existing.iter().next().is_none() {
         ctx.snippet_completion("*", "*", "Import everything.");
     }
 
     for (name, value) in module.scope().iter() {
-        if existing.idents().all(|ident| ident.as_str() != name) {
+        if existing.iter().all(|item| item.original_name().as_str() != name) {
             ctx.value_completion(Some(name.clone()), value, false, None);
         }
     }
diff --git a/crates/typst/src/ide/highlight.rs b/crates/typst/src/ide/highlight.rs
index 037106e4..ad746754 100644
--- a/crates/typst/src/ide/highlight.rs
+++ b/crates/typst/src/ide/highlight.rs
@@ -245,6 +245,7 @@ pub fn highlight(node: &LinkedNode) -> Option<Tag> {
         SyntaxKind::ForLoop => None,
         SyntaxKind::ModuleImport => None,
         SyntaxKind::ImportItems => None,
+        SyntaxKind::RenamedImportItem => None,
         SyntaxKind::ModuleInclude => None,
         SyntaxKind::LoopBreak => None,
         SyntaxKind::LoopContinue => None,
diff --git a/tests/typ/compiler/import.typ b/tests/typ/compiler/import.typ
index 7f6baee0..e5a7a858 100644
--- a/tests/typ/compiler/import.typ
+++ b/tests/typ/compiler/import.typ
@@ -35,6 +35,16 @@
 #test(d, 3)
 
 ---
+// A renamed item import.
+#import "module.typ": item as something
+#test(something(1, 2), 3)
+
+// Mixing renamed and not renamed items.
+#import "module.typ": fn, b as val, item as other
+#test(val, 1)
+#test(other(1, 2), 3)
+
+---
 // Test importing from function scopes.
 // Ref: true
 
@@ -49,6 +59,11 @@
 #ne(5, 6)
 
 ---
+// Test renaming items imported from function scopes.
+#import assert: eq as aseq
+#aseq(10, 10)
+
+---
 // A module import without items.
 #import "module.typ"
 #test(module.b, 1)
@@ -56,6 +71,32 @@
 #test(module.push(2), 3)
 
 ---
+// A renamed module import without items.
+#import "module.typ" as other
+#test(other.b, 1)
+#test(other.item(1, 2), 3)
+#test(other.push(2), 3)
+
+---
+// Mixing renamed module and items.
+#import "module.typ" as newname: b as newval, item
+#test(newname.b, 1)
+#test(newval, 1)
+#test(item(1, 2), 3)
+#test(newname.item(1, 2), 3)
+
+---
+// Renamed module import with function scopes.
+#import enum as othernum
+#test(enum, othernum)
+
+---
+// Mixing renamed module import from function with renamed item import.
+#import assert as asrt
+#import asrt: ne as asne
+#asne(1, 2)
+
+---
 // Edge case for module access that isn't fixed.
 #import "module.typ"
 
@@ -78,17 +119,43 @@
 #import enum
 #let d = (e: enum)
 #import d.e
+#import d.e as renamed
 #import d.e: item
 
 #item(2)[a]
 
 ---
+// Warning: 23-27 unnecessary import rename to same name
+#import enum: item as item
+
+---
+// Warning: 17-21 unnecessary import rename to same name
+#import enum as enum
+
+---
+// Warning: 17-21 unnecessary import rename to same name
+#import enum as enum: item
+// Warning: 17-21 unnecessary import rename to same name
+// Warning: 31-35 unnecessary import rename to same name
+#import enum as enum: item as item
+
+---
+// No warning on a case that isn't obviously pathological
+#import "module.typ" as module
+
+---
 // Can't import from closures.
 #let f(x) = x
 // Error: 9-10 cannot import from user-defined functions
 #import f: x
 
 ---
+// Can't import from closures, despite renaming.
+#let f(x) = x
+// Error: 9-10 cannot import from user-defined functions
+#import f as g
+
+---
 // Can't import from closures, despite modifiers.
 #let f(x) = x
 // Error: 9-18 cannot import from user-defined functions
@@ -103,14 +170,26 @@
 #import 5: something
 
 ---
+// Error: 9-10 expected path, module or function, found integer
+#import 5 as x
+
+---
 // Error: 9-11 failed to load file (is a directory)
 #import "": name
 
 ---
+// Error: 9-11 failed to load file (is a directory)
+#import "" as x
+
+---
 // Error: 9-20 file not found (searched at typ/compiler/lib/0.2.1)
 #import "lib/0.2.1"
 
 ---
+// Error: 9-20 file not found (searched at typ/compiler/lib/0.2.1)
+#import "lib/0.2.1" as x
+
+---
 // Some non-text stuff.
 // Error: 9-27 file is not valid utf-8
 #import "/files/rhino.png"
@@ -132,6 +211,18 @@
 This is never reached.
 
 ---
+// Renaming does not import the old name (without items).
+#import "module.typ" as something
+// Error: 7-13 unknown variable: module
+#test(module.b, 1)
+
+---
+// Renaming does not import the old name (with items).
+#import "module.typ" as something: b as other
+// Error: 7-13 unknown variable: module
+#test(module.b, 1)
+
+---
 // Error: 8 expected expression
 #import