Skip to content

Commit

Permalink
feat(linter): add fix emoji to rules table and doc pages (#4715)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Aug 10, 2024
1 parent a266b45 commit 3d40528
Show file tree
Hide file tree
Showing 9 changed files with 165 additions and 43 deletions.
62 changes: 57 additions & 5 deletions crates/oxc_linter/src/fixer/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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)
}

Expand All @@ -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
Expand Down Expand Up @@ -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();
}
}
30 changes: 25 additions & 5 deletions crates/oxc_linter/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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() };
Expand All @@ -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()
}
}

Expand Down
28 changes: 23 additions & 5 deletions crates/oxc_linter/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
31 changes: 14 additions & 17 deletions tasks/website/src/linter/rules/doc_page.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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()
))
})?;
}

Expand All @@ -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("🛠️"),
}
}
36 changes: 32 additions & 4 deletions tasks/website/src/linter/rules/html.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(non_snake_case)]

use std::{
cell::RefCell,
fmt::{self, Write},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions tasks/website/src/linter/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions tasks/website/src/linter/rules/table.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
Loading

0 comments on commit 3d40528

Please sign in to comment.