From 71a21b7ec1ef73a36f3ce37abeddb94967ca7888 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Sat, 23 Sep 2023 00:29:35 +0200 Subject: [PATCH] Improve import autocompletion Now also works for functions, types, and packages --- crates/typst/src/eval/mod.rs | 195 +++++++++++++------------------ crates/typst/src/ide/analyze.rs | 34 ++++-- crates/typst/src/ide/complete.rs | 22 +--- 3 files changed, 111 insertions(+), 140 deletions(-) diff --git a/crates/typst/src/eval/mod.rs b/crates/typst/src/eval/mod.rs index da0aadd07..cd1f29c66 100644 --- a/crates/typst/src/eval/mod.rs +++ b/crates/typst/src/eval/mod.rs @@ -72,7 +72,6 @@ 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; @@ -256,7 +255,7 @@ pub struct Vm<'a> { impl<'a> Vm<'a> { /// Create a new virtual machine. - fn new( + pub fn new( vt: Vt<'a>, route: Tracked<'a, Route>, file: Option, @@ -1733,120 +1732,87 @@ impl Eval for ast::ForLoop<'_> { } } -/// Applies imports from `import` to the current scope. -fn apply_imports( - imports: Option, - vm: &mut Vm, - source_value: V, - 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) => { - for (var, value) in scope(&source_value).iter() { - vm.scopes.top.define(var.clone(), value.clone()); - } - } - Some(ast::Imports::Items(items)) => { - let mut errors = vec![]; - let scope = scope(&source_value); - 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!(original_ident.span(), "unresolved import")); - } - } - if !errors.is_empty() { - return Err(Box::new(errors)); - } - } - } - - Ok(()) -} - impl Eval for ast::ModuleImport<'_> { type Output = Value; #[tracing::instrument(name = "ModuleImport::eval", skip_all)] fn eval(self, vm: &mut Vm) -> SourceResult { - 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", - )); + 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 name = match &source { + Value::Func(func) => { + func.scope() + .ok_or("cannot import from user-defined functions") + .at(source_span)?; + func.name().unwrap_or_default().into() } + Value::Type(ty) => ty.short_name().into(), + other => { + let module = import(vm, other.clone(), source_span, true)?; + let name = module.name().clone(); + source = Value::Module(module); + name + } + }; + + if let Some(new_name) = &new_name { + if let ast::Expr::Ident(ident) = self.source() { + if ident.as_str() == new_name.as_str() { + // Warn on `import x as x` + vm.vt.tracer.warn(warning!( + new_name.span(), + "unnecessary import rename to same name", + )); + } + } + + // Define renamed module on the scope. + vm.scopes.top.define(new_name.as_str(), source.clone()); } - if let Value::Func(func) = source { - let Some(scope) = func.scope() else { - bail!(span, "cannot import from user-defined functions"); - }; - apply_imports( - self.imports(), - vm, - func, - new_name, - |func| func.name().unwrap_or_default().into(), - |_| scope, - )?; - } else if let Value::Type(ty) = source { - apply_imports( - self.imports(), - vm, - ty, - new_name, - |ty| ty.short_name().into(), - |ty| ty.scope(), - )?; - } else { - let module = import(vm, source, span, true)?; - apply_imports( - self.imports(), - vm, - module, - new_name, - |module| module.name().clone(), - |module| module.scope(), - )?; + + let scope = source.scope().unwrap(); + match imports { + None => { + // Only import here if there is no rename. + if new_name.is_none() { + vm.scopes.top.define(name, source); + } + } + Some(ast::Imports::Wildcard) => { + for (var, value) in scope.iter() { + vm.scopes.top.define(var.clone(), value.clone()); + } + } + Some(ast::Imports::Items(items)) => { + let mut errors = vec![]; + for item in items.iter() { + let original_ident = item.original_name(); + if let Some(value) = scope.get(&original_ident) { + // Warn on `import ...: x as x` + 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!(original_ident.span(), "unresolved import")); + } + } + if !errors.is_empty() { + return Err(Box::new(errors)); + } + } } Ok(Value::None) @@ -1866,7 +1832,7 @@ impl Eval for ast::ModuleInclude<'_> { } /// Process an import of a module relative to the current location. -fn import( +pub(crate) fn import( vm: &mut Vm, source: Value, span: Span, @@ -1875,13 +1841,10 @@ fn import( let path = match source { Value::Str(path) => path, Value::Module(module) => return Ok(module), - v => { - if allow_scopes { - bail!(span, "expected path, module, function, or type, found {}", v.ty()) - } else { - bail!(span, "expected path or module, found {}", v.ty()) - } + v if allow_scopes => { + bail!(span, "expected path, module, function, or type, found {}", v.ty()) } + v => bail!(span, "expected path or module, found {}", v.ty()), }; // Handle package and file imports. diff --git a/crates/typst/src/ide/analyze.rs b/crates/typst/src/ide/analyze.rs index 1a545d24a..769e80adc 100644 --- a/crates/typst/src/ide/analyze.rs +++ b/crates/typst/src/ide/analyze.rs @@ -2,9 +2,9 @@ use comemo::Track; use ecow::{eco_vec, EcoString, EcoVec}; use crate::doc::Frame; -use crate::eval::{eval, Module, Route, Tracer, Value}; -use crate::model::{Introspector, Label}; -use crate::syntax::{ast, LinkedNode, Source, SyntaxKind}; +use crate::eval::{Route, Scopes, Tracer, Value, Vm}; +use crate::model::{DelayedErrors, Introspector, Label, Locator, Vt}; +use crate::syntax::{ast, LinkedNode, Span, SyntaxKind}; use crate::World; /// Try to determine a set of possible values for an expression. @@ -44,12 +44,30 @@ pub fn analyze_expr(world: &dyn World, node: &LinkedNode) -> EcoVec { } /// Try to load a module from the current source file. -pub fn analyze_import(world: &dyn World, source: &Source, path: &str) -> Option { - let route = Route::default(); +pub fn analyze_import(world: &dyn World, source: &LinkedNode) -> Option { + let id = source.span().id()?; + let source = analyze_expr(world, source).into_iter().next()?; + if source.scope().is_some() { + return Some(source); + } + + let mut locator = Locator::default(); + let introspector = Introspector::default(); + let mut delayed = DelayedErrors::new(); let mut tracer = Tracer::new(); - let id = source.id().join(path); - let source = world.source(id).ok()?; - eval(world.track(), route.track(), tracer.track_mut(), &source).ok() + let vt = Vt { + world: world.track(), + introspector: introspector.track(), + locator: &mut locator, + delayed: delayed.track_mut(), + tracer: tracer.track_mut(), + }; + + let route = Route::default(); + let mut vm = Vm::new(vt, route.track(), Some(id), Scopes::new(Some(world.library()))); + crate::eval::import(&mut vm, source, Span::detached(), true) + .ok() + .map(Value::Module) } /// Find all labels and details for them. diff --git a/crates/typst/src/ide/complete.rs b/crates/typst/src/ide/complete.rs index f100de8d8..59a6c9300 100644 --- a/crates/typst/src/ide/complete.rs +++ b/crates/typst/src/ide/complete.rs @@ -457,10 +457,9 @@ fn complete_imports(ctx: &mut CompletionContext) -> bool { if let Some(ast::Expr::Import(import)) = prev.get().cast(); if let Some(ast::Imports::Items(items)) = import.imports(); if let Some(source) = prev.children().find(|child| child.is::()); - if let Some(value) = analyze_expr(ctx.world, &source).into_iter().next(); then { ctx.from = ctx.cursor; - import_item_completions(ctx, items, &value); + import_item_completions(ctx, items, &source); return true; } } @@ -475,10 +474,9 @@ fn complete_imports(ctx: &mut CompletionContext) -> bool { if let Some(ast::Expr::Import(import)) = grand.get().cast(); if let Some(ast::Imports::Items(items)) = import.imports(); if let Some(source) = grand.children().find(|child| child.is::()); - if let Some(value) = analyze_expr(ctx.world, &source).into_iter().next(); then { ctx.from = ctx.leaf.offset(); - import_item_completions(ctx, items, &value); + import_item_completions(ctx, items, &source); return true; } } @@ -490,22 +488,16 @@ fn complete_imports(ctx: &mut CompletionContext) -> bool { fn import_item_completions<'a>( ctx: &mut CompletionContext<'a>, existing: ast::ImportItems<'a>, - value: &Value, + source: &LinkedNode, ) { - let module = match value { - Value::Str(path) => match analyze_import(ctx.world, ctx.source, path) { - Some(module) => module, - None => return, - }, - Value::Module(module) => module.clone(), - _ => return, - }; + let Some(value) = analyze_import(ctx.world, source) else { return }; + let Some(scope) = value.scope() else { return }; if existing.iter().next().is_none() { ctx.snippet_completion("*", "*", "Import everything."); } - for (name, value) in module.scope().iter() { + for (name, value) in scope.iter() { if existing.iter().all(|item| item.original_name().as_str() != name) { ctx.value_completion(Some(name.clone()), value, false, None); } @@ -955,7 +947,6 @@ struct CompletionContext<'a> { world: &'a (dyn World + 'static), frames: &'a [Frame], library: &'a Library, - source: &'a Source, global: &'a Scope, math: &'a Scope, text: &'a str, @@ -985,7 +976,6 @@ impl<'a> CompletionContext<'a> { world, frames, library, - source, global: library.global.scope(), math: library.math.scope(), text,