feat(linter): add fix emoji to rules table and doc pages (#4715)

Rules table:
<img width="898" alt="image" src="https://github.com/user-attachments/assets/353052aa-0af3-4c09-8441-ff79f4561ca0">

Doc pages:
<img width="918" alt="image" src="https://github.com/user-attachments/assets/cb43cb2d-15ff-41e6-8523-145cfbc3f484">
This commit is contained in:
DonIsaac 2024-08-10 22:50:47 +00:00
parent a266b45167
commit 3d40528588
9 changed files with 165 additions and 43 deletions

View file

@ -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();
}
}

View file

@ -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<RuleFixMeta> 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()
}
}

View file

@ -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, "| {:<rule_width$} | {:<plugin_width$} | Default |", "Rule name", "Source")
.unwrap();
writeln!(s, "| {x:-<rule_width$} | {x:-<plugin_width$} | {x:-<7} |").unwrap();
writeln!(
s,
"| {:<rule_width$} | {:<plugin_width$} | Default | Fixable? |",
"Rule name", "Source"
)
.unwrap();
writeln!(s, "| {x:-<rule_width$} | {x:-<plugin_width$} | {x:-<7} | {x:-<8} |").unwrap();
for row in rows {
let rule_name = row.name;
let plugin_name = &row.plugin;
let (default, default_width) =
if row.turned_on_by_default { ("", 6) } else { ("", 7) };
if row.turned_on_by_default { ("", DEFAULT - 1) } else { ("", DEFAULT) };
let rendered_name = if let Some(prefix) = link_prefix {
Cow::Owned(format!("[{rule_name}]({prefix}/{plugin_name}/{rule_name}.html)"))
} else {
Cow::Borrowed(rule_name)
};
writeln!(s, "| {rendered_name:<rule_width$} | {plugin_name:<plugin_width$} | {default:<default_width$} |").unwrap();
let (fix_emoji, fix_emoji_width) = row.autofix.emoji().map_or(("", FIX), |emoji| {
let len = emoji.len();
if len > FIX {
(emoji, 0)
} else {
(emoji, FIX - len)
}
});
writeln!(s, "| {rendered_name:<rule_width$} | {plugin_name:<plugin_width$} | {default:<default_width$} | {fix_emoji:<fix_emoji_width$} |").unwrap();
}
s

View file

@ -159,7 +159,7 @@ clone-submodule dir url sha:
cd {{dir}} && git fetch origin {{sha}} && git reset --hard {{sha}}
website path:
cargo run -p website -- linter-rules > {{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

View file

@ -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<String, fmt::Error> {
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<String, fmt::Error>
"<!-- This file is auto-generated by {}. Do not edit it manually. -->\n",
file!()
)?;
writeln!(page, "# {plugin}/{name}\n")?;
writeln!(page, r#"# {plugin}/{name} <Badge type="info" text="{category}" />"#)?;
// 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#"<span class="emoji">✅</span> 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#"<span class="emoji">{}</span> {}"#,
emoji,
autofix.description()
))
})?;
}
@ -47,11 +52,3 @@ pub fn render_rule_docs_page(rule: &RuleTableRow) -> Result<String, fmt::Error>
Ok(page.into())
}
fn fix_emoji(fix: RuleFixMeta) -> Option<&'static str> {
match fix {
RuleFixMeta::None => None,
RuleFixMeta::FixPending => Some("🚧"),
RuleFixMeta::Conditional(_) | RuleFixMeta::Fixable(_) => Some("🛠️"),
}
}

View file

@ -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<S: AsRef<str>>(&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<F>(&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<F>(&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 `<div>` tag.");
make_tag!(Alert);
#[cfg(test)]
mod test {

View file

@ -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;

View file

@ -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`
// <https://oxc.rs/docs/guide/usage/linter/rules.html>
/// 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;

View file

@ -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("<table>"));
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 =