From 5d60e48057650a213a791d50c7e1321615d91acf Mon Sep 17 00:00:00 2001 From: PgBiel <9021226+PgBiel@users.noreply.github.com> Date: Mon, 22 Jul 2024 11:57:39 -0300 Subject: [PATCH] use picostr for identifiers --- crates/typst/src/diag.rs | 65 +++++++-------------------------- crates/typst/src/eval/import.rs | 8 ++-- crates/typst/src/eval/markup.rs | 8 +++- crates/typst/src/lib.rs | 4 +- crates/typst/src/text/mod.rs | 4 +- 5 files changed, 28 insertions(+), 61 deletions(-) diff --git a/crates/typst/src/diag.rs b/crates/typst/src/diag.rs index 03f8ef886..2e4437c7b 100644 --- a/crates/typst/src/diag.rs +++ b/crates/typst/src/diag.rs @@ -9,6 +9,7 @@ use std::string::FromUtf8Error; use comemo::Tracked; use ecow::{eco_vec, EcoVec}; +use typst_utils::PicoStr; use crate::syntax::package::{PackageSpec, PackageVersion}; use crate::syntax::{ast, Span, Spanned, SyntaxError}; @@ -116,14 +117,14 @@ macro_rules! __error { macro_rules! __warning { ( $span:expr, - $id:ident, - $fmt:literal $(, $arg:expr)* + id: $id:expr, + message: $fmt:literal $(, $arg:expr)* $(; hint: $hint:literal $(, $hint_arg:expr)*)* $(,)? ) => { $crate::diag::SourceDiagnostic::warning( $span, - ::std::option::Option::Some($crate::diag::CompilerWarning::$id), + $id, $crate::diag::eco_format!($fmt, $($arg),*), ) $(.with_hint($crate::diag::eco_format!($hint, $($hint_arg),*)))* }; @@ -161,7 +162,9 @@ pub struct SourceDiagnostic { /// The span of the relevant node in the source code. pub span: Span, /// The identifier for this diagnostic. - pub identifier: Option, + /// + /// Currently, this field is only used by warnings. + pub identifier: Option, /// A diagnostic message describing the problem. pub message: EcoString, /// The trace of function calls leading to the problem. @@ -196,13 +199,13 @@ impl SourceDiagnostic { /// Create a new, bare warning. pub fn warning( span: Span, - identifier: Option, + identifier: impl Into, message: impl Into, ) -> Self { Self { severity: Severity::Warning, span, - identifier: identifier.map(Identifier::Warn), + identifier: Some(identifier.into()), trace: eco_vec![], message: message.into(), hints: eco_vec![], @@ -240,49 +243,6 @@ impl From for SourceDiagnostic { } } -/// The identifier of a [`SourceDiagnostic`]. -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub enum Identifier { - /// Identifier for a built-in compiler warning. - Warn(CompilerWarning), - /// Identifier for a warning raised by a package. - User(EcoString), -} - -/// Built-in compiler warnings. -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub enum CompilerWarning { - UnnecessaryImportRenaming, - UnnecessaryStars, - UnnecessaryUnderscores, - NonConvergingLayout, - UnknownFontFamilies, -} - -impl CompilerWarning { - /// The name of the warning as a string, following the format of diagnostic - /// identifiers. - pub const fn name(&self) -> &'static str { - match self { - CompilerWarning::UnnecessaryImportRenaming => "unnecessary-import-renaming", - CompilerWarning::UnnecessaryStars => "unnecessary-stars", - CompilerWarning::UnnecessaryUnderscores => "unnecessary-underscores", - CompilerWarning::NonConvergingLayout => "non-converging-layout", - CompilerWarning::UnknownFontFamilies => "unknown-font-families", - } - } -} - -impl Identifier { - /// The identifier's name, e.g. 'unnecessary-stars'. - pub fn name(&self) -> &str { - match self { - Self::Warn(warning_identifier) => warning_identifier.name(), - Self::User(user_identifier) => user_identifier, - } - } -} - /// A part of a diagnostic's [trace](SourceDiagnostic::trace). #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum Tracepoint { @@ -529,6 +489,7 @@ pub fn deduplicate_and_suppress_warnings( return true; }; + let identifier = identifier.resolve(); let should_raise = !is_warning_suppressed(diag.span, world, identifier) && !diag.trace.iter().any(|tracepoint| { is_warning_suppressed(tracepoint.span, world, identifier) @@ -544,7 +505,7 @@ pub fn deduplicate_and_suppress_warnings( /// in. If one of the ancestors of the node where the warning occurred has a /// warning suppression annotation sibling right before it suppressing this /// particular warning, the warning is considered suppressed. -fn is_warning_suppressed(span: Span, world: &dyn World, warning: &Identifier) -> bool { +fn is_warning_suppressed(span: Span, world: &dyn World, warning: &str) -> bool { // Don't suppress detached warnings. let Some(source) = span.id().and_then(|file| world.source(file).ok()) else { return false; @@ -574,14 +535,14 @@ fn is_warning_suppressed(span: Span, world: &dyn World, warning: &Identifier) -> /// identifier to be suppressed. fn check_annotation_suppresses_warning( annotation: ast::Annotation, - warning: &Identifier, + warning: &str, ) -> bool { if annotation.name().as_str() != "allow" { return false; } for argument in annotation.arguments() { - if warning.name() == argument.get() { + if warning == argument.get() { return true; } } diff --git a/crates/typst/src/eval/import.rs b/crates/typst/src/eval/import.rs index 7451e6030..fc0de6c58 100644 --- a/crates/typst/src/eval/import.rs +++ b/crates/typst/src/eval/import.rs @@ -37,8 +37,8 @@ impl Eval for ast::ModuleImport<'_> { // Warn on `import x as x` vm.engine.sink.warn(warning!( new_name.span(), - UnnecessaryImportRenaming, - "unnecessary import rename to same name", + id: "unnecessary-import-renaming", + message: "unnecessary import rename to same name", )); } } @@ -113,8 +113,8 @@ impl Eval for ast::ModuleImport<'_> { { vm.engine.sink.warn(warning!( renamed_item.new_name().span(), - UnnecessaryImportRenaming, - "unnecessary import rename to same name", + id: "unnecessary-import-renaming", + message: "unnecessary import rename to same name", )); } } diff --git a/crates/typst/src/eval/markup.rs b/crates/typst/src/eval/markup.rs index 68a41408f..d9ef28862 100644 --- a/crates/typst/src/eval/markup.rs +++ b/crates/typst/src/eval/markup.rs @@ -136,7 +136,9 @@ impl Eval for ast::Strong<'_> { vm.engine .sink .warn(warning!( - self.span(), UnnecessaryStars, "no text within stars"; + self.span(), + id: "unnecessary-stars", + message: "no text within stars"; hint: "using multiple consecutive stars (e.g. **) has no additional effect", )); } @@ -154,7 +156,9 @@ impl Eval for ast::Emph<'_> { vm.engine .sink .warn(warning!( - self.span(), UnnecessaryUnderscores, "no text within underscores"; + self.span(), + id: "unnecessary-underscores", + message: "no text within underscores"; hint: "using multiple consecutive underscores (e.g. __) has no additional effect" )); } diff --git a/crates/typst/src/lib.rs b/crates/typst/src/lib.rs index 67bf3239d..1fe18c054 100644 --- a/crates/typst/src/lib.rs +++ b/crates/typst/src/lib.rs @@ -161,7 +161,9 @@ fn compile_inner( if iter >= 5 { sink.warn(warning!( - Span::detached(), NonConvergingLayout, "layout did not converge within 5 attempts"; + Span::detached(), + id: "non-converging-layout", + message: "layout did not converge within 5 attempts"; hint: "check if any states or queries are updating themselves" )); break; diff --git a/crates/typst/src/text/mod.rs b/crates/typst/src/text/mod.rs index 125b09877..234422595 100644 --- a/crates/typst/src/text/mod.rs +++ b/crates/typst/src/text/mod.rs @@ -134,8 +134,8 @@ pub struct TextElem { if !book.contains_family(family.as_str()) { engine.sink.warn(warning!( font_list.span, - UnknownFontFamilies, - "unknown font family: {}", + id: "unknown-font-families", + message: "unknown font family: {}", family.as_str(), )); }