From 3d4052858858e0867c10ed8602e0b3577c101655 Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Sat, 10 Aug 2024 22:50:47 +0000 Subject: [PATCH] feat(linter): add fix emoji to rules table and doc pages (#4715) Rules table: image Doc pages: image --- crates/oxc_linter/src/fixer/fix.rs | 62 ++++++++++++++++++++-- crates/oxc_linter/src/rule.rs | 30 +++++++++-- crates/oxc_linter/src/table.rs | 28 ++++++++-- justfile | 2 +- tasks/website/src/linter/rules/doc_page.rs | 31 +++++------ tasks/website/src/linter/rules/html.rs | 36 +++++++++++-- tasks/website/src/linter/rules/mod.rs | 1 + tasks/website/src/linter/rules/table.rs | 7 ++- tasks/website/src/linter/rules/test.rs | 11 ++-- 9 files changed, 165 insertions(+), 43 deletions(-) diff --git a/crates/oxc_linter/src/fixer/fix.rs b/crates/oxc_linter/src/fixer/fix.rs index af0696ed8afb9..8296e3201d878 100644 --- a/crates/oxc_linter/src/fixer/fix.rs +++ b/crates/oxc_linter/src/fixer/fix.rs @@ -29,11 +29,14 @@ bitflags! { /// rule category. const Dangerous = 1 << 2; - /// Used to specify that no fixes should be applied. - const None = 0; const SafeFix = Self::Fix.bits(); + const SafeFixOrSuggestion = Self::Fix.bits() | Self::Suggestion.bits(); const DangerousFix = Self::Dangerous.bits() | Self::Fix.bits(); const DangerousSuggestion = Self::Dangerous.bits() | Self::Suggestion.bits(); + const DangerousFixOrSuggestion = Self::Dangerous.bits() | Self::Fix.bits() | Self::Suggestion.bits(); + + /// Used to specify that no fixes should be applied. + const None = 0; /// Fixes and Suggestions that are safe or dangerous. const All = Self::Dangerous.bits() | Self::Fix.bits() | Self::Suggestion.bits(); } @@ -49,17 +52,17 @@ impl Default for FixKind { impl FixKind { #[inline] - pub fn is_none(self) -> bool { + pub const fn is_none(self) -> bool { self.is_empty() } #[inline] - pub fn is_some(self) -> bool { + pub const fn is_some(self) -> bool { self.bits() > 0 } #[inline] - pub fn is_dangerous(self) -> bool { + pub const fn is_dangerous(self) -> bool { self.contains(Self::Dangerous) } @@ -85,6 +88,30 @@ impl FixKind { pub fn can_apply(self, rule_fix: Self) -> bool { self.contains(rule_fix) } + + /// # Panics + /// If this [`FixKind`] is only [`FixKind::Dangerous`] without a + /// [`FixKind::Fix`] or [`FixKind::Suggestion`] qualifier. + pub fn emoji(self) -> &'static str { + if self.is_empty() { + return ""; + } + match self { + Self::Fix => "🛠️", + Self::Suggestion => "💡", + Self::SafeFixOrSuggestion => "🛠️💡", + Self::DangerousFixOrSuggestion => "⚠️🛠️️💡", + Self::DangerousFix => "⚠️🛠️️", + Self::DangerousSuggestion => "⚠️💡", + Self::Dangerous => panic!( + "Fix kinds cannot just be dangerous, they must also be 'Fix' or 'Suggestion'." + ), + _ => { + debug_assert!(false, "Please add an emoji for FixKind: {self:?}"); + "" + } + } + } } // TODO: rename @@ -620,4 +647,29 @@ mod test { f.push(vec![f3.clone(), f3.clone()].into()); assert_eq!(f, CompositeFix::Multiple(vec![f1, f2, f3.clone(), f3])); } + + #[test] + fn test_emojis() { + let tests = vec![ + (FixKind::None, ""), + (FixKind::Fix, "🛠️"), + (FixKind::Suggestion, "💡"), + (FixKind::Suggestion | FixKind::Fix, "🛠️💡"), + (FixKind::DangerousFix, "⚠️🛠️️"), + (FixKind::DangerousSuggestion, "⚠️💡"), + (FixKind::DangerousFix.union(FixKind::Suggestion), "⚠️🛠️️💡"), + ]; + + for (kind, expected) in tests { + assert_eq!(kind.emoji(), expected, "Expected {kind:?} to have emoji '{expected}'."); + } + } + + #[test] + #[should_panic( + expected = "Fix kinds cannot just be dangerous, they must also be 'Fix' or 'Suggestion'." + )] + fn test_emojis_invalid() { + FixKind::Dangerous.emoji(); + } } diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 287bd40a9bfe3..f30f6ab72e647 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -137,6 +137,20 @@ impl RuleFixMeta { matches!(self, Self::None) } + #[inline] + pub const fn fix_kind(self) -> FixKind { + match self { + Self::Conditional(kind) | Self::Fixable(kind) => { + debug_assert!( + !kind.is_none(), + "This lint rule indicates that it provides an auto-fix but its FixKind is None. This is a bug. If this rule does not provide a fix, please use RuleFixMeta::None. Otherwise, please provide a valid FixKind" + ); + kind + } + RuleFixMeta::None | RuleFixMeta::FixPending => FixKind::None, + } + } + /// Does this `Rule` have some kind of auto-fix available? /// /// Also returns `true` for suggestions. @@ -168,7 +182,9 @@ impl RuleFixMeta { (true, true) => "auto-fix and a suggestion are available for this rule", (true, false) => "auto-fix is available for this rule", (false, true) => "suggestion is available for this rule", - _ => unreachable!(), + _ => unreachable!( + "Fix kinds must contain Fix and/or Suggestion, but {self:?} has neither." + ), }; let mut message = if kind.is_dangerous() { format!("dangerous {noun}") } else { noun.into() }; @@ -187,14 +203,18 @@ impl RuleFixMeta { } } } + pub fn emoji(self) -> Option<&'static str> { + match self { + Self::None => None, + Self::Conditional(kind) | Self::Fixable(kind) => Some(kind.emoji()), + Self::FixPending => Some("🚧"), + } + } } impl From for FixKind { fn from(value: RuleFixMeta) -> Self { - match value { - RuleFixMeta::None | RuleFixMeta::FixPending => FixKind::None, - RuleFixMeta::Fixable(kind) | RuleFixMeta::Conditional(kind) => kind, - } + value.fix_kind() } } diff --git a/crates/oxc_linter/src/table.rs b/crates/oxc_linter/src/table.rs index 83c7877c31d25..dc5a4ca3d1d6b 100644 --- a/crates/oxc_linter/src/table.rs +++ b/crates/oxc_linter/src/table.rs @@ -95,6 +95,12 @@ impl RuleTableSection { /// Provide [`Some`] prefix to render the rule name as a link. Provide /// [`None`] to just display the rule name as text. pub fn render_markdown_table(&self, link_prefix: Option<&str>) -> String { + const FIX_EMOJI_COL_WIDTH: usize = 10; + const DEFAULT_EMOJI_COL_WIDTH: usize = 9; + /// text width, leave 2 spaces for padding + const FIX: usize = FIX_EMOJI_COL_WIDTH - 2; + const DEFAULT: usize = DEFAULT_EMOJI_COL_WIDTH - 2; + let mut s = String::new(); let category = &self.category; let rows = &self.rows; @@ -105,21 +111,33 @@ impl RuleTableSection { writeln!(s, "{}", category.description()).unwrap(); let x = ""; - writeln!(s, "| {: FIX { + (emoji, 0) + } else { + (emoji, FIX - len) + } + }); + writeln!(s, "| {rendered_name: {{path}}/src/docs/guide/usage/linter/generated-rules.md + cargo run -p website -- linter-rules --table {{path}}/src/docs/guide/usage/linter/generated-rules.md --rule-docs {{path}}/src/docs/guide/usage/linter/rules cargo run -p website -- linter-cli > {{path}}/src/docs/guide/usage/linter/generated-cli.md cargo run -p website -- linter-schema-markdown > {{path}}/src/docs/guide/usage/linter/generated-config.md diff --git a/tasks/website/src/linter/rules/doc_page.rs b/tasks/website/src/linter/rules/doc_page.rs index 2edfd76587b2a..82bba0fc2de92 100644 --- a/tasks/website/src/linter/rules/doc_page.rs +++ b/tasks/website/src/linter/rules/doc_page.rs @@ -1,14 +1,15 @@ //! Create documentation pages for each rule. Pages are printed as Markdown and //! get added to the website. -use oxc_linter::{table::RuleTableRow, RuleFixMeta}; +use oxc_linter::table::RuleTableRow; use std::fmt::{self, Write}; -use crate::linter::rules::html::HtmlWriter; +use super::HtmlWriter; pub fn render_rule_docs_page(rule: &RuleTableRow) -> Result { const APPROX_FIX_CATEGORY_AND_PLUGIN_LEN: usize = 512; - let RuleTableRow { name, documentation, plugin, turned_on_by_default, autofix, .. } = rule; + let RuleTableRow { name, documentation, plugin, turned_on_by_default, autofix, category } = + rule; let mut page = HtmlWriter::with_capacity( documentation.map_or(0, str::len) + name.len() + APPROX_FIX_CATEGORY_AND_PLUGIN_LEN, @@ -19,19 +20,23 @@ pub fn render_rule_docs_page(rule: &RuleTableRow) -> Result "\n", file!() )?; - writeln!(page, "# {plugin}/{name}\n")?; + writeln!(page, r#"# {plugin}/{name} "#)?; // rule metadata page.div(r#"class="rule-meta""#, |p| { if *turned_on_by_default { - p.span(r#"class="default-on""#, |p| { - p.writeln("✅ This rule is turned on by default.") + p.Alert(r#"class="default-on" type="success""#, |p| { + p.writeln(r#"✅ This rule is turned on by default."#) })?; } - if let Some(emoji) = fix_emoji(*autofix) { - p.span(r#"class="fix""#, |p| { - p.writeln(format!("{} {}", emoji, autofix.description())) + if let Some(emoji) = autofix.emoji() { + p.Alert(r#"class="fix" type="info""#, |p| { + p.writeln(format!( + r#"{} {}"#, + emoji, + autofix.description() + )) })?; } @@ -47,11 +52,3 @@ pub fn render_rule_docs_page(rule: &RuleTableRow) -> Result Ok(page.into()) } - -fn fix_emoji(fix: RuleFixMeta) -> Option<&'static str> { - match fix { - RuleFixMeta::None => None, - RuleFixMeta::FixPending => Some("🚧"), - RuleFixMeta::Conditional(_) | RuleFixMeta::Fixable(_) => Some("🛠️"), - } -} diff --git a/tasks/website/src/linter/rules/html.rs b/tasks/website/src/linter/rules/html.rs index 9441ad5dff622..c3ee9700c5e5f 100644 --- a/tasks/website/src/linter/rules/html.rs +++ b/tasks/website/src/linter/rules/html.rs @@ -1,3 +1,5 @@ +#![allow(non_snake_case)] + use std::{ cell::RefCell, fmt::{self, Write}, @@ -41,14 +43,30 @@ impl HtmlWriter { Self { inner: RefCell::new(String::with_capacity(capacity)) } } + /// Similar to [`Write::write_str`], but doesn't require a mutable borrow. + /// + /// Useful when nesting [`HtmlWriter::html`] calls. pub fn writeln>(&self, line: S) -> fmt::Result { writeln!(self.inner.borrow_mut(), "{}", line.as_ref()) } + /// Finalize this writer's internal buffer and return it as a [`String`]. pub fn into_inner(self) -> String { self.inner.into_inner() } + /// Render an HTML tag with some children. + /// + /// In most cases, you shouldn't use this method directly. Instead, prefer one of the + /// tag-specific convenience methods like [`HtmlWriter::div`]. Feel free to add any missing + /// implementations that you need. + /// + /// Also works with JSX (or really any XML). Does not support self-closing tags. + /// + /// - `tag`: The HTML tag name + /// - `attrs`: Raw `attr="value"` string to insert into the opening tag + /// - `inner`: A closure that produces content to render in between the opening and closing + /// tags pub fn html(&self, tag: &'static str, attrs: &str, inner: F) -> fmt::Result where F: FnOnce(&Self) -> fmt::Result, @@ -84,10 +102,14 @@ impl HtmlWriter { } } +/// Implements a tag factory on [`HtmlWriter`] with optional documentation. macro_rules! make_tag { - ($name:ident) => { + ($name:ident, $($docs:expr),+) => { impl HtmlWriter { - #[inline] + // create a #[doc = $doc] for each item in $docs + $( + #[doc = $docs] + )+ pub fn $name(&self, attrs: &str, inner: F) -> fmt::Result where F: FnOnce(&Self) -> fmt::Result, @@ -96,10 +118,16 @@ macro_rules! make_tag { } } }; + ($name:ident) => { + make_tag!( + $name, + "Render a tag with the same name as this method." + ); + } } -make_tag!(div); -make_tag!(span); +make_tag!(div, "Render a `
` tag."); +make_tag!(Alert); #[cfg(test)] mod test { diff --git a/tasks/website/src/linter/rules/mod.rs b/tasks/website/src/linter/rules/mod.rs index 88cafd8dfd6eb..01b7926c0fc74 100644 --- a/tasks/website/src/linter/rules/mod.rs +++ b/tasks/website/src/linter/rules/mod.rs @@ -12,6 +12,7 @@ use std::{ }; use doc_page::render_rule_docs_page; +use html::HtmlWriter; use oxc_linter::table::RuleTable; use pico_args::Arguments; use table::render_rules_table; diff --git a/tasks/website/src/linter/rules/table.rs b/tasks/website/src/linter/rules/table.rs index 963f899328d1e..68f939cd6f7f8 100644 --- a/tasks/website/src/linter/rules/table.rs +++ b/tasks/website/src/linter/rules/table.rs @@ -1,9 +1,12 @@ use oxc_linter::table::RuleTable; -// `cargo run -p website linter-rules > /path/to/oxc/oxc-project.github.io/src/docs/guide/usage/linter/generated-rules.md` -// +/// Renders the website's Rules page. Each [`category`] gets its own table with +/// links to documentation pages for each rule. +/// /// `docs_prefix` is a path prefix to the base URL all rule documentation pages /// share in common. +/// +/// [`category`]: oxc_linter::RuleCategory pub fn render_rules_table(table: &RuleTable, docs_prefix: &str) -> String { let total = table.total; let turned_on_by_default_count = table.turned_on_by_default_count; diff --git a/tasks/website/src/linter/rules/test.rs b/tasks/website/src/linter/rules/test.rs index 5238ee060afde..672352b7a3403 100644 --- a/tasks/website/src/linter/rules/test.rs +++ b/tasks/website/src/linter/rules/test.rs @@ -1,4 +1,4 @@ -use markdown::{to_html, to_html_with_options, Options}; +use markdown::{to_html_with_options, Options}; use oxc_diagnostics::NamedSource; use scraper::{ElementRef, Html, Selector}; use std::sync::{Arc, OnceLock}; @@ -42,9 +42,13 @@ fn parse_type(filename: &str, source_text: &str, source_type: SourceType) -> Res #[test] fn test_rules_table() { const PREFIX: &str = "/docs/guide/usage/linter/rules"; + let options = Options::gfm(); let rendered_table = render_rules_table(table(), PREFIX); - let html = to_html(&rendered_table); - let jsx = format!("const Table = () => <>{html}"); + let rendered_html = to_html_with_options(&rendered_table, &options).unwrap(); + assert!(rendered_html.contains("")); + let html = Html::parse_fragment(&rendered_html); + assert!(html.errors.is_empty(), "{:#?}", html.errors); + let jsx = format!("const Table = () => <>{rendered_html}"); parse("rules-table", &jsx).unwrap(); } @@ -70,7 +74,6 @@ fn test_doc_pages() { // ensure code examples are valid { let html = Html::parse_fragment(docs); - assert!(html.errors.is_empty(), "HTML parsing errors: {:#?}", html.errors); for code_el in html.select(&code) { let inner = code_el.inner_html(); let inner =