Skip to content

Commit

Permalink
feat(website): auto-generate rule docs pages (#4640)
Browse files Browse the repository at this point in the history
> AI-generated description because I'm lazy
### TL;DR

This PR introduces the ability to generate documentation for linter rules and adds new methods and metadata for rule fix capabilities.

To see what this looks like, please check out oxc-project/oxc-project.github.io#165.

## Screenshots
Hyperlinks to rule doc pages in auto-generated rules table
<img width="809" alt="image" src="https://github.com/user-attachments/assets/e09eb47d-e86a-4ed1-b1f9-5034f33c71a2">

Example of a docs page
<img width="1273" alt="image" src="https://github.com/user-attachments/assets/78f7e9e6-f4dd-4cc9-aebc-1cdd64b024ec">

### What changed?

- Added `RuleFixMeta` to indicate rule fix capabilities
- Introduced methods `is_none` and `is_pending` in `RuleFixMeta`
- Modified `render_markdown_table` in `RuleTableSection` to accept an optional link prefix
- Created new modules for rule documentation and HTML rendering
- Updated `print_rules` function to generate markdown for rules and detailed documentation pages

### How to test?

Run the `linter-rules` task with appropriate arguments to generate the markdown table and documentation pages.
Verify the generated files for correctness and that all metadata is correctly displayed.

### Why make this change?

To enhance the project documentation and provide clear rule fix capabilities, thereby improving the developer experience and easing the integration process.

---
  • Loading branch information
DonIsaac committed Aug 10, 2024
1 parent d191823 commit f629514
Show file tree
Hide file tree
Showing 16 changed files with 377 additions and 57 deletions.
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub use crate::{
fixer::FixKind,
frameworks::FrameworkFlags,
options::{AllowWarnDeny, LintOptions},
rule::{RuleCategory, RuleMeta, RuleWithSeverity},
rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleWithSeverity},
service::{LintService, LintServiceOptions},
};
use crate::{
Expand Down Expand Up @@ -146,7 +146,7 @@ impl Linter {
pub fn print_rules<W: Write>(writer: &mut W) {
let table = RuleTable::new();
for section in table.sections {
writeln!(writer, "{}", section.render_markdown_table()).unwrap();
writeln!(writer, "{}", section.render_markdown_table(None)).unwrap();
}
writeln!(writer, "Default: {}", table.turned_on_by_default_count).unwrap();
writeln!(writer, "Total: {}", table.total).unwrap();
Expand Down
21 changes: 16 additions & 5 deletions crates/oxc_linter/src/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl fmt::Display for RuleCategory {

// NOTE: this could be packed into a single byte if we wanted. I don't think
// this is needed, but we could do it if it would have a performance impact.
/// Describes the auto-fixing capabilities of a [`Rule`].
/// Describes the auto-fixing capabilities of a `Rule`.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
pub enum RuleFixMeta {
/// An auto-fix is not available.
Expand All @@ -132,14 +132,24 @@ pub enum RuleFixMeta {
}

impl RuleFixMeta {
/// Does this [`Rule`] have some kind of auto-fix available?
#[inline]
pub fn is_none(self) -> bool {
matches!(self, Self::None)
}

/// Does this `Rule` have some kind of auto-fix available?
///
/// Also returns `true` for suggestions.
#[inline]
pub fn has_fix(self) -> bool {
matches!(self, Self::Fixable(_) | Self::Conditional(_))
}

#[inline]
pub fn is_pending(self) -> bool {
matches!(self, Self::FixPending)
}

pub fn supports_fix(self, kind: FixKind) -> bool {
matches!(self, Self::Fixable(fix_kind) | Self::Conditional(fix_kind) if fix_kind.can_apply(kind))
}
Expand All @@ -163,9 +173,10 @@ impl RuleFixMeta {
let mut message =
if kind.is_dangerous() { format!("dangerous {noun}") } else { noun.into() };

let article = match message.chars().next().unwrap() {
'a' | 'e' | 'i' | 'o' | 'u' => "An",
_ => "A",
let article = match message.chars().next() {
Some('a' | 'e' | 'i' | 'o' | 'u') => "An",
Some(_) => "A",
None => unreachable!(),
};

if matches!(self, Self::Conditional(_)) {
Expand Down
10 changes: 6 additions & 4 deletions crates/oxc_linter/src/rules/import/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ pub struct Namespace {

declare_oxc_lint!(
/// ### What it does
/// Enforces names exist at the time they are dereferenced, when imported as a full namespace (i.e. import * as foo from './foo'; foo.bar(); will report if bar is not exported by ./foo.).
/// Will report at the import declaration if there are no exported names found.
/// Also, will report for computed references (i.e. foo["bar"]()).
/// Reports on assignment to a member of an imported namespace.
/// Enforces names exist at the time they are dereferenced, when imported as
/// a full namespace (i.e. `import * as foo from './foo'; foo.bar();` will
/// report if bar is not exported by `./foo.`). Will report at the import
/// declaration if there are no exported names found. Also, will report for
/// computed references (i.e. `foo["bar"]()`). Reports on assignment to a
/// member of an imported namespace.
Namespace,
correctness
);
Expand Down
22 changes: 4 additions & 18 deletions crates/oxc_linter/src/rules/jsx_a11y/anchor_is_valid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ declare_oxc_lint!(
///
/// Consider the following:
///
/// ```javascript
/// ```jsx
/// <a href="javascript:void(0)" onClick={foo}>Perform action</a>
/// <a href="#" onClick={foo}>Perform action</a>
/// <a onClick={foo}>Perform action</a>
/// ````
///
/// All these anchor implementations indicate that the element is only used to execute JavaScript code. All the above should be replaced with:
///
/// ```javascript
/// ```jsx
/// <button onClick={foo}>Perform action</button>
/// ```
/// `
Expand All @@ -78,33 +78,19 @@ declare_oxc_lint!(
///
/// #### Valid
///
/// ```javascript
/// ```jsx
/// <a href={`https://www.javascript.com`}>navigate here</a>
/// ```
///
/// ```javascript
/// <a href={somewhere}>navigate here</a>
/// ```
///
/// ```javascript
/// <a {...spread}>navigate here</a>
/// ```
///
/// #### Invalid
///
/// ```javascript
/// ```jsx
/// <a href={null}>navigate here</a>
/// ```
/// ```javascript
/// <a href={undefined}>navigate here</a>
/// ```
/// ```javascript
/// <a href>navigate here</a>
/// ```
/// ```javascript
/// <a href="javascript:void(0)">navigate here</a>
/// ```
/// ```javascript
/// <a href="https://example.com" onClick={something}>navigate here</a>
/// ```
///
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_linter/src/rules/jsx_a11y/lang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub struct Lang;
declare_oxc_lint!(
/// ### What it does
///
/// The lang prop on the <html> element must be a valid IETF's BCP 47 language tag.
/// The lang prop on the `<html>` element must be a valid IETF's BCP 47 language tag.
///
/// ### Why is this bad?
///
Expand All @@ -39,13 +39,13 @@ declare_oxc_lint!(
/// ### Example
///
/// // good
/// ```javascript
/// ```jsx
/// <html lang="en">
/// <html lang="en-US">
/// ```
///
/// // bad
/// ```javascript
/// ```jsx
/// <html>
/// <html lang="foo">
/// ````
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,17 @@ declare_oxc_lint!(
///
/// ### Why is this necessary?
///
/// Elements that can be visually distracting can cause accessibility issues with visually impaired users.
/// Such elements are most likely deprecated, and should be avoided. By default, <marquee> and <blink> elements are visually distracting.
/// Elements that can be visually distracting can cause accessibility issues
/// with visually impaired users. Such elements are most likely deprecated,
/// and should be avoided. By default, `<marquee>` and `<blink>` elements
/// are visually distracting.
///
/// ### What it checks
///
/// This rule checks for marquee and blink element.
///
/// ### Example
/// ```javascript
/// ```jsx
/// // Bad
/// <marquee />
/// <marquee {...props} />
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/jsx_a11y/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ pub struct Scope;
declare_oxc_lint!(
/// ### What it does
///
/// The scope prop should be used only on <th> elements.
/// The scope prop should be used only on `<th>` elements.
///
/// ### Why is this bad?
/// The scope attribute makes table navigation much easier for screen reader users, provided that it is used correctly.
/// Incorrectly used, scope can make table navigation much harder and less efficient.
/// A screen reader operates under the assumption that a table has a header and that this header specifies a scope. Because of the way screen readers function, having an accurate header makes viewing a table far more accessible and more efficient for people who use the device.
///
/// ### Example
/// ```javascript
/// ```jsx
/// // Bad
/// <div scope />
///
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_linter/src/rules/nextjs/no_duplicate_head.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ pub struct NoDuplicateHead;

declare_oxc_lint!(
/// ### What it does
/// Prevent duplicate usage of <Head> in pages/_document.js.
/// Prevent duplicate usage of `<Head>` in `pages/_document.js``.
///
/// ### Why is this bad?
/// This can cause unexpected behavior in your application.
///
/// ### Example
/// ```javascript
/// ```jsx
/// import Document, { Html, Head, Main, NextScript } from 'next/document'
/// class MyDocument extends Document {
/// static async getInitialProps(ctx) {
Expand Down
10 changes: 6 additions & 4 deletions crates/oxc_linter/src/rules/typescript/no_this_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ declare_oxc_lint!(
///
/// ### Why is this bad?
///
/// Generic type parameters (<T>) in TypeScript may be "constrained" with an extends keyword.
/// When no extends is provided, type parameters default a constraint to unknown. It is therefore redundant to extend from any or unknown.
/// Generic type parameters (`<T>`) in TypeScript may be "constrained" with
/// an extends keyword. When no extends is provided, type parameters
/// default a constraint to unknown. It is therefore redundant to extend
/// from any or unknown.
///
/// the rule doesn't allow const {allowedName} = this
/// the rule doesn't allow `const {allowedName} = this`
/// this is to keep 1:1 with eslint implementation
/// sampe with obj.<allowedName> = this
/// sampe with `obj.<allowedName> = this`
/// ```
NoThisAlias,
correctness
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ declare_oxc_lint!(
///
/// ### Why is this bad?
///
/// Generic type parameters (<T>) in TypeScript may be "constrained" with an extends keyword.
/// Generic type parameters (`<T>`) in TypeScript may be "constrained" with an extends keyword.
/// When no extends is provided, type parameters default a constraint to unknown. It is therefore redundant to extend from any or unknown.
///
/// ### Example
/// ```javascript
/// ```typescript
/// interface FooAny<T extends any> {}
/// interface FooUnknown<T extends unknown> {}
/// type BarAny<T extends any> = {};
Expand Down
19 changes: 15 additions & 4 deletions crates/oxc_linter/src/table.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::fmt::Write;
use std::{borrow::Cow, fmt::Write};

use rustc_hash::{FxHashMap, FxHashSet};

use crate::{rules::RULES, Linter, RuleCategory};
use crate::{rules::RULES, Linter, RuleCategory, RuleFixMeta};

pub struct RuleTable {
pub sections: Vec<RuleTableSection>,
Expand All @@ -23,6 +23,7 @@ pub struct RuleTableRow {
pub category: RuleCategory,
pub documentation: Option<&'static str>,
pub turned_on_by_default: bool,
pub autofix: RuleFixMeta,
}

impl Default for RuleTable {
Expand All @@ -49,6 +50,7 @@ impl RuleTable {
plugin: rule.plugin_name().to_string(),
category: rule.category(),
turned_on_by_default: default_rules.contains(name),
autofix: rule.fix(),
}
})
.collect::<Vec<_>>();
Expand Down Expand Up @@ -88,7 +90,11 @@ impl RuleTable {
}

impl RuleTableSection {
pub fn render_markdown_table(&self) -> String {
/// Renders all the rules in this section as a markdown table.
///
/// 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 {
let mut s = String::new();
let category = &self.category;
let rows = &self.rows;
Expand All @@ -108,7 +114,12 @@ impl RuleTableSection {
let plugin_name = &row.plugin;
let (default, default_width) =
if row.turned_on_by_default { ("✅", 6) } else { ("", 7) };
writeln!(s, "| {rule_name:<rule_width$} | {plugin_name:<plugin_width$} | {default:<default_width$} |").unwrap();
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();
}

s
Expand Down
57 changes: 57 additions & 0 deletions tasks/website/src/linter/rules/doc_page.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//! Create documentation pages for each rule. Pages are printed as Markdown and
//! get added to the website.
use oxc_linter::{table::RuleTableRow, RuleFixMeta};
use std::fmt::{self, Write};

use crate::linter::rules::html::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 mut page = HtmlWriter::with_capacity(
documentation.map_or(0, str::len) + name.len() + APPROX_FIX_CATEGORY_AND_PLUGIN_LEN,
);

writeln!(
page,
"<!-- This file is auto-generated by {}. Do not edit it manually. -->\n",
file!()
)?;
writeln!(page, "# {plugin}/{name}\n")?;

// 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.")
})?;
}

if let Some(emoji) = fix_emoji(*autofix) {
p.span(r#"class="fix""#, |p| {
p.writeln(format!("{} {}", emoji, autofix.description()))
})?;
}

Ok(())
})?;

// rule documentation
if let Some(docs) = documentation {
writeln!(page, "\n{}", *docs)?;
}

// TODO: link to rule source

Ok(page.into())
}

fn fix_emoji(fix: RuleFixMeta) -> Option<&'static str> {
match fix {
RuleFixMeta::None => None,
RuleFixMeta::FixPending => Some("🚧"),
RuleFixMeta::Conditional(_) | RuleFixMeta::Fixable(_) => Some("🛠️"),
}
}
Loading

0 comments on commit f629514

Please sign in to comment.