diff --git a/crates/tinymist-analysis/src/syntax/def.rs b/crates/tinymist-analysis/src/syntax/def.rs index d99f9c6c..3cb235f5 100644 --- a/crates/tinymist-analysis/src/syntax/def.rs +++ b/crates/tinymist-analysis/src/syntax/def.rs @@ -247,6 +247,16 @@ impl Expr { _ => self.span().id(), } } + + /// Returns whether the expression is definitely defined. + pub fn is_defined(&self) -> bool { + match self { + Expr::Ref(refs) => refs.root.is_some() || refs.term.is_some(), + Expr::Decl(decl) => decl.is_def(), + // There are unsure cases, like `x.y`, which may be defined or not. + _ => false, + } + } } impl fmt::Display for Expr { diff --git a/crates/tinymist-lint/src/lib.rs b/crates/tinymist-lint/src/lib.rs index 6a145f44..d0c19a95 100644 --- a/crates/tinymist-lint/src/lib.rs +++ b/crates/tinymist-lint/src/lib.rs @@ -3,7 +3,8 @@ use std::sync::Arc; use tinymist_analysis::{ - syntax::ExprInfo, + adt::interner::Interned, + syntax::{Decl, ExprInfo}, ty::{Ty, TyCtx, TypeInfo}, }; use tinymist_project::LspWorld; @@ -31,28 +32,71 @@ pub struct LintInfo { } /// Performs linting check on file and returns a vector of diagnostics. -pub fn lint_file(world: &LspWorld, expr: &ExprInfo, ti: Arc) -> LintInfo { - let diagnostics = Linter::new(world, ti).lint(expr.source.root()); +pub fn lint_file( + world: &LspWorld, + ei: &ExprInfo, + ti: Arc, + known_issues: KnownIssues, +) -> LintInfo { + let diagnostics = Linter::new(world, ei.clone(), ti, known_issues).lint(ei.source.root()); LintInfo { - revision: expr.revision, - fid: expr.fid, + revision: ei.revision, + fid: ei.fid, diagnostics, } } +/// Information about issues the linter checks for that will already be reported +/// to the user via other means (such as compiler diagnostics), to avoid +/// duplicating warnings. +#[derive(Default, Clone, Hash)] +pub struct KnownIssues { + unknown_vars: EcoVec, +} + +impl KnownIssues { + /// Collects known lint issues from the given compiler diagnostics. + pub fn from_compiler_diagnostics<'a>( + diags: impl Iterator + Clone, + ) -> Self { + let mut unknown_vars = Vec::default(); + for diag in diags { + if diag.message.starts_with("unknown variable") { + unknown_vars.push(diag.span); + } + } + unknown_vars.sort_by_key(|span| span.into_raw()); + let unknown_vars = EcoVec::from(unknown_vars); + Self { unknown_vars } + } + + pub(crate) fn has_unknown_math_ident(&self, ident: ast::MathIdent<'_>) -> bool { + self.unknown_vars.contains(&ident.span()) + } +} + struct Linter<'w> { world: &'w LspWorld, + ei: ExprInfo, ti: Arc, + known_issues: KnownIssues, diag: DiagnosticVec, loop_info: Option, func_info: Option, } impl<'w> Linter<'w> { - fn new(world: &'w LspWorld, ti: Arc) -> Self { + fn new( + world: &'w LspWorld, + ei: ExprInfo, + ti: Arc, + known_issues: KnownIssues, + ) -> Self { Self { world, + ei, ti, + known_issues, diag: EcoVec::new(), loop_info: None, func_info: None, @@ -386,6 +430,27 @@ impl DataFlowVisitor for Linter<'_> { if expr.callee().to_untyped().text() == "text" { self.check_variable_font(expr.args().items()); } + self.exprs(expr.args().to_untyped().exprs().chain(expr.callee().once())); + Some(()) + } + + fn math_ident(&mut self, ident: ast::MathIdent<'_>) -> Option<()> { + let resolved = self.ei.get_def(&Interned::new(Decl::math_ident_ref(ident))); + let is_defined = resolved.is_some_and(|expr| expr.is_defined()); + + if !is_defined && !self.known_issues.has_unknown_math_ident(ident) { + let var = ident.as_str(); + let mut warning = + SourceDiagnostic::warning(ident.span(), eco_format!("unknown variable: {var}")); + + // Tries to produce the same hints as the corresponding Typst compiler error. + // See `unknown_variable_math` in typst-library/src/foundations/scope.rs: + // https://github.com/typst/typst/blob/v0.13.1/crates/typst-library/src/foundations/scope.rs#L386 + let in_global = self.world.library.global.scope().get(var).is_some(); + hint_unknown_variable_math(var, in_global, &mut warning); + self.diag.push(warning); + } + Some(()) } } @@ -1021,3 +1086,32 @@ fn is_compare_op(op: ast::BinOp) -> bool { use ast::BinOp::*; matches!(op, Lt | Leq | Gt | Geq | Eq | Neq) } + +/// The error message when a variable wasn't found it math. +#[cold] +fn hint_unknown_variable_math(var: &str, in_global: bool, diag: &mut SourceDiagnostic) { + if matches!(var, "none" | "auto" | "false" | "true") { + diag.hint(eco_format!( + "if you meant to use a literal, \ + try adding a hash before it: `#{var}`", + )); + } else if in_global { + diag.hint(eco_format!( + "`{var}` is not available directly in math, \ + try adding a hash before it: `#{var}`", + )); + } else { + diag.hint(eco_format!( + "if you meant to display multiple letters as is, \ + try adding spaces between each letter: `{}`", + var.chars() + .flat_map(|c| [' ', c]) + .skip(1) + .collect::() + )); + diag.hint(eco_format!( + "or if you meant to display this as text, \ + try placing it in quotes: `\"{var}\"`" + )); + } +} diff --git a/crates/tinymist-project/src/compiler.rs b/crates/tinymist-project/src/compiler.rs index c0126d1c..7fe1bcd9 100644 --- a/crates/tinymist-project/src/compiler.rs +++ b/crates/tinymist-project/src/compiler.rs @@ -114,7 +114,7 @@ impl CompiledArtifact { } /// Returns the diagnostics. - pub fn diagnostics(&self) -> impl Iterator { + pub fn diagnostics(&self) -> impl Iterator + Clone { self.diag.diagnostics() } diff --git a/crates/tinymist-query/src/analysis.rs b/crates/tinymist-query/src/analysis.rs index 6a890db3..26c4d5bd 100644 --- a/crates/tinymist-query/src/analysis.rs +++ b/crates/tinymist-query/src/analysis.rs @@ -610,6 +610,8 @@ mod call_info_tests { mod lint_tests { use std::collections::BTreeMap; + use tinymist_lint::KnownIssues; + use crate::tests::*; #[test] @@ -617,7 +619,7 @@ mod lint_tests { snapshot_testing("lint", &|ctx, path| { let source = ctx.source_by_path(&path).unwrap(); - let result = ctx.lint(&source); + let result = ctx.lint(&source, &KnownIssues::default()); let result = crate::diagnostics::DiagWorker::new(ctx).convert_all(result.iter()); let result = result .into_iter() diff --git a/crates/tinymist-query/src/analysis/code_action.rs b/crates/tinymist-query/src/analysis/code_action.rs index 10375c44..93ba7b16 100644 --- a/crates/tinymist-query/src/analysis/code_action.rs +++ b/crates/tinymist-query/src/analysis/code_action.rs @@ -206,7 +206,8 @@ impl<'a> CodeActionWorker<'a> { Some(()) } - /// Add spaces between letters in an unknown math identifier: `$xyz$` -> `$x y z$`. + /// Add spaces between letters in an unknown math identifier: `$xyz$` -> `$x + /// y z$`. fn add_spaces_to_math_unknown_variable(&mut self, node: &LinkedNode<'_>) -> Option<()> { let ident = node.cast::()?.get(); @@ -613,7 +614,7 @@ enum AutofixKind { fn match_autofix_kind(msg: &str) -> Option { static PATTERNS: &[(&str, AutofixKind)] = &[ - ("unknown variable", AutofixKind::UnknownVariable), + ("unknown variable", AutofixKind::UnknownVariable), // typst compiler error ("file not found", AutofixKind::FileNotFound), ]; diff --git a/crates/tinymist-query/src/analysis/global.rs b/crates/tinymist-query/src/analysis/global.rs index cec837a0..e722d059 100644 --- a/crates/tinymist-query/src/analysis/global.rs +++ b/crates/tinymist-query/src/analysis/global.rs @@ -13,7 +13,7 @@ use tinymist_analysis::stats::AllocStats; use tinymist_analysis::syntax::classify_def_loosely; use tinymist_analysis::ty::term_value; use tinymist_analysis::{analyze_expr_, analyze_import_}; -use tinymist_lint::LintInfo; +use tinymist_lint::{KnownIssues, LintInfo}; use tinymist_project::{LspComputeGraph, LspWorld, TaskWhen}; use tinymist_std::hash::{FxDashMap, hash128}; use tinymist_std::typst::TypstDocument; @@ -475,8 +475,12 @@ impl LocalContext { cache.get_or_init(|| self.shared.type_check(source)).clone() } - pub(crate) fn lint(&mut self, source: &Source) -> EcoVec { - self.shared.lint(source).diagnostics + pub(crate) fn lint( + &mut self, + source: &Source, + known_issues: &KnownIssues, + ) -> EcoVec { + self.shared.lint(source, known_issues).diagnostics } /// Get the type check information of a source file. @@ -794,13 +798,13 @@ impl SharedContext { /// Get the lint result of a source file. #[typst_macros::time(span = source.root().span())] - pub(crate) fn lint(self: &Arc, source: &Source) -> LintInfo { + pub(crate) fn lint(self: &Arc, source: &Source, issues: &KnownIssues) -> LintInfo { let ei = self.expr_stage(source); let ti = self.type_check(source); let guard = self.query_stat(source.id(), "lint"); - self.slot.lint.compute(hash128(&(&ei, &ti)), |_prev| { + self.slot.lint.compute(hash128(&(&ei, &ti, issues)), |_| { guard.miss(); - tinymist_lint::lint_file(&self.world, &ei, ti) + tinymist_lint::lint_file(&self.world, &ei, ti, issues.clone()) }) } diff --git a/crates/tinymist-query/src/check.rs b/crates/tinymist-query/src/check.rs index c8d90180..12c7b26f 100644 --- a/crates/tinymist-query/src/check.rs +++ b/crates/tinymist-query/src/check.rs @@ -1,3 +1,4 @@ +use tinymist_lint::KnownIssues; use tinymist_project::LspCompiledArtifact; use crate::{DiagWorker, DiagnosticsMap, SemanticRequest, prelude::*}; @@ -14,6 +15,9 @@ impl SemanticRequest for CheckRequest { fn request(self, ctx: &mut LocalContext) -> Option { let worker = DiagWorker::new(ctx); - Some(worker.check().convert_all(self.snap.diagnostics())) + let compiler_diags = self.snap.diagnostics(); + + let known_issues = KnownIssues::from_compiler_diagnostics(compiler_diags.clone()); + Some(worker.check(&known_issues).convert_all(compiler_diags)) } } diff --git a/crates/tinymist-query/src/code_action.rs b/crates/tinymist-query/src/code_action.rs index 6f8247c9..6cf53fdd 100644 --- a/crates/tinymist-query/src/code_action.rs +++ b/crates/tinymist-query/src/code_action.rs @@ -93,6 +93,7 @@ impl SemanticRequest for CodeActionRequest { #[cfg(test)] mod tests { + use tinymist_lint::KnownIssues; use typst::{diag::Warned, layout::PagedDocument}; use super::*; @@ -131,18 +132,15 @@ mod tests { warnings: compiler_warnings, } = typst::compile::(&ctx.world); let compiler_errors = output.err().unwrap_or_default(); + let compiler_diags = compiler_warnings.iter().chain(compiler_errors.iter()); + + let known_issues = KnownIssues::from_compiler_diagnostics(compiler_diags.clone()); + let lint_warnings = ctx.lint(source, &known_issues); - let lint_warnings = ctx.lint(source); let diagnostics = DiagWorker::new(ctx) - .convert_all( - compiler_errors - .iter() - .chain(compiler_warnings.iter()) - .chain(lint_warnings.iter()), - ) + .convert_all(compiler_diags.chain(lint_warnings.iter())) .into_values() .flatten(); - CodeActionContext { // The filtering here matches the LSP specification and VS Code behavior; // see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionContext: diff --git a/crates/tinymist-query/src/diagnostics.rs b/crates/tinymist-query/src/diagnostics.rs index c376064e..463ff492 100644 --- a/crates/tinymist-query/src/diagnostics.rs +++ b/crates/tinymist-query/src/diagnostics.rs @@ -1,5 +1,6 @@ use std::borrow::Cow; +use tinymist_lint::KnownIssues; use tinymist_project::LspWorld; use tinymist_world::vfs::WorkspaceResolver; use typst::syntax::Span; @@ -33,6 +34,7 @@ pub fn convert_diagnostics<'a>( pub(crate) struct DiagWorker<'a> { /// The world surface for Typst compiler. pub ctx: &'a mut LocalContext, + pub source: &'static str, /// Results pub results: DiagnosticsMap, } @@ -42,12 +44,15 @@ impl<'w> DiagWorker<'w> { pub fn new(ctx: &'w mut LocalContext) -> Self { Self { ctx, + source: "typst", results: DiagnosticsMap::default(), } } /// Runs code check on the main document and all its dependencies. - pub fn check(mut self) -> Self { + pub fn check(mut self, known_issues: &KnownIssues) -> Self { + let source = self.source; + self.source = "tinymist-lint"; for dep in self.ctx.world.depended_files() { if WorkspaceResolver::is_package_file(dep) { continue; @@ -57,10 +62,11 @@ impl<'w> DiagWorker<'w> { continue; }; - for diag in self.ctx.lint(&source) { + for diag in self.ctx.lint(&source, known_issues) { self.handle(&diag); } } + self.source = source; self } @@ -121,7 +127,7 @@ impl<'w> DiagWorker<'w> { range: lsp_range, severity: Some(lsp_severity), message: lsp_message, - source: Some("typst".to_owned()), + source: Some(self.source.to_owned()), related_information: (!typst_diagnostic.trace.is_empty()).then(|| { typst_diagnostic .trace diff --git a/crates/tinymist-world/src/compute.rs b/crates/tinymist-world/src/compute.rs index 6b0643ee..72689ed7 100644 --- a/crates/tinymist-world/src/compute.rs +++ b/crates/tinymist-world/src/compute.rs @@ -412,7 +412,7 @@ impl DiagnosticsTask { } /// Gets the diagnostics. - pub fn diagnostics(&self) -> impl Iterator { + pub fn diagnostics(&self) -> impl Iterator + Clone { self.paged .errors .iter()