Allow renaming imports with as (#1923)

This commit is contained in:
Pg Biel 2023-08-30 08:36:02 -03:00 committed by GitHub
parent 8a0dd88f10
commit 19b91d59d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 230 additions and 13 deletions

View File

@ -1988,6 +1988,15 @@ impl<'a> ModuleImport<'a> {
_ => Option::None, _ => 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. /// The items that ought to be imported from a file.
@ -2005,9 +2014,65 @@ node! {
} }
impl<'a> ImportItems<'a> { impl<'a> ImportItems<'a> {
/// The items to import from the module. /// Returns an iterator over the items to import from the module.
pub fn idents(self) -> impl DoubleEndedIterator<Item = Ident<'a>> { pub fn iter(self) -> impl DoubleEndedIterator<Item = ImportItem<'a>> {
self.0.children().filter_map(SyntaxNode::cast) 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()
} }
} }

View File

@ -242,6 +242,8 @@ pub enum SyntaxKind {
ModuleImport, ModuleImport,
/// Items to import from a module: `a, b, c`. /// Items to import from a module: `a, b, c`.
ImportItems, ImportItems,
/// A renamed import item: `a as d`.
RenamedImportItem,
/// A module include: `include "chapter1.typ"`. /// A module include: `include "chapter1.typ"`.
ModuleInclude, ModuleInclude,
/// A break from a loop: `break`. /// A break from a loop: `break`.
@ -465,6 +467,7 @@ impl SyntaxKind {
Self::ForLoop => "for-loop expression", Self::ForLoop => "for-loop expression",
Self::ModuleImport => "`import` expression", Self::ModuleImport => "`import` expression",
Self::ImportItems => "import items", Self::ImportItems => "import items",
Self::RenamedImportItem => "renamed import item",
Self::ModuleInclude => "`include` expression", Self::ModuleInclude => "`include` expression",
Self::LoopBreak => "`break` expression", Self::LoopBreak => "`break` expression",
Self::LoopContinue => "`continue` expression", Self::LoopContinue => "`continue` expression",

View File

@ -1138,6 +1138,12 @@ fn module_import(p: &mut Parser) {
let m = p.marker(); let m = p.marker();
p.assert(SyntaxKind::Import); p.assert(SyntaxKind::Import);
code_expr(p); 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) { if p.eat_if(SyntaxKind::Colon) && !p.eat_if(SyntaxKind::Star) {
import_items(p); import_items(p);
} }
@ -1147,9 +1153,17 @@ fn module_import(p: &mut Parser) {
fn import_items(p: &mut Parser) { fn import_items(p: &mut Parser) {
let m = p.marker(); let m = p.marker();
while !p.eof() && !p.at(SyntaxKind::Semicolon) { while !p.eof() && !p.at(SyntaxKind::Semicolon) {
let item_marker = p.marker();
if !p.eat_if(SyntaxKind::Ident) { if !p.eat_if(SyntaxKind::Ident) {
p.unexpected(); 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() { if p.current().is_terminator() {
break; break;
} }

View File

@ -544,8 +544,8 @@ impl<'a> CapturesVisitor<'a> {
Some(ast::Expr::Import(expr)) => { Some(ast::Expr::Import(expr)) => {
self.visit(expr.source().to_untyped()); self.visit(expr.source().to_untyped());
if let Some(ast::Imports::Items(items)) = expr.imports() { if let Some(ast::Imports::Items(items)) = expr.imports() {
for item in items.idents() { for item in items.iter() {
self.bind(item); self.bind(item.bound_name());
} }
} }
} }

View File

@ -68,6 +68,7 @@ use std::mem;
use comemo::{Track, Tracked, TrackedMut, Validate}; use comemo::{Track, Tracked, TrackedMut, Validate};
use ecow::{EcoString, EcoVec}; use ecow::{EcoString, EcoVec};
use if_chain::if_chain;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
use unicode_segmentation::UnicodeSegmentation; use unicode_segmentation::UnicodeSegmentation;
@ -1704,15 +1705,30 @@ impl Eval for ast::ForLoop<'_> {
} }
/// Applies imports from `import` to the current scope. /// Applies imports from `import` to the current scope.
fn apply_imports<V: IntoValue>( fn apply_imports<V: IntoValue + Clone>(
imports: Option<ast::Imports>, imports: Option<ast::Imports>,
vm: &mut Vm, vm: &mut Vm,
source_value: V, source_value: V,
name: impl Fn(&V) -> EcoString, new_name: Option<&str>,
name: impl FnOnce(&V) -> EcoString,
scope: impl Fn(&V) -> &Scope, scope: impl Fn(&V) -> &Scope,
) -> SourceResult<()> { ) -> 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 { match imports {
None => { 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); vm.scopes.top.define(name(&source_value), source_value);
} }
Some(ast::Imports::Wildcard) => { Some(ast::Imports::Wildcard) => {
@ -1723,11 +1739,22 @@ fn apply_imports<V: IntoValue>(
Some(ast::Imports::Items(items)) => { Some(ast::Imports::Items(items)) => {
let mut errors = vec![]; let mut errors = vec![];
let scope = scope(&source_value); let scope = scope(&source_value);
for ident in items.idents() { for item in items.iter() {
if let Some(value) = scope.get(&ident) { let original_ident = item.original_name();
vm.define(ident, value.clone()); 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 { } else {
errors.push(error!(ident.span(), "unresolved import")); errors.push(error!(original_ident.span(), "unresolved import"));
} }
} }
if !errors.is_empty() { if !errors.is_empty() {
@ -1746,6 +1773,20 @@ impl Eval for ast::ModuleImport<'_> {
fn eval(self, vm: &mut Vm) -> SourceResult<Self::Output> { fn eval(self, vm: &mut Vm) -> SourceResult<Self::Output> {
let span = self.source().span(); let span = self.source().span();
let source = self.source().eval(vm)?; 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 let Value::Func(func) = source {
if func.info().is_none() { if func.info().is_none() {
bail!(span, "cannot import from user-defined functions"); bail!(span, "cannot import from user-defined functions");
@ -1754,6 +1795,7 @@ impl Eval for ast::ModuleImport<'_> {
self.imports(), self.imports(),
vm, vm,
func, func,
new_name,
|func| func.info().unwrap().name.into(), |func| func.info().unwrap().name.into(),
|func| &func.info().unwrap().scope, |func| &func.info().unwrap().scope,
)?; )?;
@ -1763,6 +1805,7 @@ impl Eval for ast::ModuleImport<'_> {
self.imports(), self.imports(),
vm, vm,
module, module,
new_name,
|module| module.name().clone(), |module| module.name().clone(),
|module| module.scope(), |module| module.scope(),
)?; )?;

View File

@ -503,12 +503,12 @@ fn import_item_completions<'a>(
_ => return, _ => return,
}; };
if existing.idents().next().is_none() { if existing.iter().next().is_none() {
ctx.snippet_completion("*", "*", "Import everything."); ctx.snippet_completion("*", "*", "Import everything.");
} }
for (name, value) in module.scope().iter() { 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); ctx.value_completion(Some(name.clone()), value, false, None);
} }
} }

View File

@ -245,6 +245,7 @@ pub fn highlight(node: &LinkedNode) -> Option<Tag> {
SyntaxKind::ForLoop => None, SyntaxKind::ForLoop => None,
SyntaxKind::ModuleImport => None, SyntaxKind::ModuleImport => None,
SyntaxKind::ImportItems => None, SyntaxKind::ImportItems => None,
SyntaxKind::RenamedImportItem => None,
SyntaxKind::ModuleInclude => None, SyntaxKind::ModuleInclude => None,
SyntaxKind::LoopBreak => None, SyntaxKind::LoopBreak => None,
SyntaxKind::LoopContinue => None, SyntaxKind::LoopContinue => None,

View File

@ -34,6 +34,16 @@
// It exists now! // It exists now!
#test(d, 3) #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. // Test importing from function scopes.
// Ref: true // Ref: true
@ -48,6 +58,11 @@
#eq(10, 10) #eq(10, 10)
#ne(5, 6) #ne(5, 6)
---
// Test renaming items imported from function scopes.
#import assert: eq as aseq
#aseq(10, 10)
--- ---
// A module import without items. // A module import without items.
#import "module.typ" #import "module.typ"
@ -55,6 +70,32 @@
#test(module.item(1, 2), 3) #test(module.item(1, 2), 3)
#test(module.push(2), 3) #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. // Edge case for module access that isn't fixed.
#import "module.typ" #import "module.typ"
@ -78,16 +119,42 @@
#import enum #import enum
#let d = (e: enum) #let d = (e: enum)
#import d.e #import d.e
#import d.e as renamed
#import d.e: item #import d.e: item
#item(2)[a] #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. // Can't import from closures.
#let f(x) = x #let f(x) = x
// Error: 9-10 cannot import from user-defined functions // Error: 9-10 cannot import from user-defined functions
#import f: x #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. // Can't import from closures, despite modifiers.
#let f(x) = x #let f(x) = x
@ -102,14 +169,26 @@
// Error: 9-10 expected path, module or function, found integer // Error: 9-10 expected path, module or function, found integer
#import 5: something #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) // Error: 9-11 failed to load file (is a directory)
#import "": name #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) // Error: 9-20 file not found (searched at typ/compiler/lib/0.2.1)
#import "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. // Some non-text stuff.
// Error: 9-27 file is not valid utf-8 // Error: 9-27 file is not valid utf-8
@ -131,6 +210,18 @@
This is never reached. 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 // Error: 8 expected expression
#import #import