Skip to content

Commit 958f415

Browse files
committed
feat(linter): Enforce schema documentation for rules with configuration options.
This was mostly built using GitHub Copilot w/ Claude Sonnet 4.5. This is primarily a port of the existing check logic I had written in JavaScript for detecting rules that violated, but somewhat more robust. Due to how this is checked, it runs the website generation as part of this, and also uses `git` in order to run the website generation command. This is _probably_ not desirable and there is likely a much better way to programmatically detect whether the rule is set up correctly *without* needing to generate the full docs as part of the test. But this does work for now and the other attempts I tried weren't detecting all the problematic rules, so I'll push it up as a draft. It's also using a somewhat-naive regex to implement the check for whether the schema is documented in the rule's documentation, which should probably also be improved by doing the check programmatically instead.
1 parent 8b14ec9 commit 958f415

File tree

1 file changed

+264
-0
lines changed

1 file changed

+264
-0
lines changed
Lines changed: 264 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,264 @@
1+
//! Test to ensure rules with configuration options have proper documentation
2+
//!
3+
//! This test verifies that all linter rules with configuration options
4+
//! have a "Configuration" section in their generated documentation.
5+
//! This helps ensure that users can understand how to configure rules properly.
6+
7+
#![cfg(feature = "ruledocs")]
8+
9+
/// Test to ensure that all rules with `from_configuration` implementations
10+
/// also have a schema and proper documentation.
11+
///
12+
/// This test:
13+
/// 1. Scans source code to find rules with from_configuration but no config = parameter
14+
/// 2. Generates the actual website documentation
15+
/// 3. Verifies that rules with from_configuration have a Configuration section in their docs
16+
///
17+
/// The Configuration section in generated docs is auto-created from the schema,
18+
/// so rules with custom configuration must have a schema defined.
19+
#[test]
20+
fn test_rules_with_custom_configuration_have_schema() {
21+
let mut failures = Vec::new();
22+
23+
// Rules that have from_configuration but no schema yet.
24+
// These were found by running the source code scanner in this test.
25+
// TODO: Remove rules from this list as they get fixed. Do NOT add new rules to this
26+
// list, newly-created rules should always be documented before being merged!
27+
let exceptions: &[&str] = &[
28+
// eslint (15)
29+
"eslint/arrow-body-style",
30+
"eslint/default-case",
31+
"eslint/func-names",
32+
"eslint/new-cap",
33+
"eslint/no-bitwise",
34+
"eslint/no-cond-assign",
35+
"eslint/no-console",
36+
"eslint/no-else-return",
37+
"eslint/no-empty-function",
38+
"eslint/no-fallthrough",
39+
"eslint/no-inner-declarations",
40+
"eslint/no-restricted-globals",
41+
"eslint/no-restricted-imports",
42+
"eslint/no-self-assign",
43+
"eslint/no-warning-comments",
44+
"eslint/yoda",
45+
// jest (3)
46+
"jest/consistent-test-it",
47+
"jest/prefer-lowercase-title",
48+
"jest/valid-title",
49+
// jsdoc (2)
50+
"jsdoc/require-param",
51+
"jsdoc/require-returns",
52+
// jsx_a11y (3)
53+
"jsx_a11y/label-has-associated-control",
54+
"jsx_a11y/media-has-caption",
55+
"jsx_a11y/no-noninteractive-tabindex",
56+
// promise (3)
57+
"promise/no-callback-in-promise",
58+
"promise/param-names",
59+
"promise/spec-only",
60+
// react (4)
61+
"react/forbid-dom-props",
62+
"react/forbid-elements",
63+
"react/jsx-handler-names",
64+
"react/prefer-es6-class",
65+
// typescript (3)
66+
"typescript/ban-ts-comment",
67+
"typescript/consistent-generic-constructors",
68+
"typescript/consistent-type-imports",
69+
// unicorn (3)
70+
"unicorn/catch-error-name",
71+
"unicorn/filename-case",
72+
"unicorn/switch-case-braces",
73+
// vue (2)
74+
"vue/define-emits-declaration",
75+
"vue/define-props-declaration",
76+
];
77+
78+
let exception_set: std::collections::HashSet<&str> = exceptions.iter().copied().collect();
79+
80+
// Step 1: Scan source code to find rules with from_configuration but no config =
81+
let workspace_root =
82+
std::path::Path::new(env!("CARGO_MANIFEST_DIR")).parent().unwrap().parent().unwrap();
83+
let rules_dir = workspace_root.join("crates/oxc_linter/src/rules");
84+
85+
let mut rules_with_from_config_no_schema = Vec::new();
86+
87+
// Recursively scan rule files
88+
fn scan_rules_dir(
89+
dir: &std::path::Path,
90+
base_dir: &std::path::Path,
91+
results: &mut Vec<String>,
92+
) -> std::io::Result<()> {
93+
for entry in std::fs::read_dir(dir)? {
94+
let entry = entry?;
95+
let path = entry.path();
96+
97+
if path.is_dir() {
98+
scan_rules_dir(&path, base_dir, results)?;
99+
} else if path.extension().and_then(|s| s.to_str()) == Some("rs") {
100+
let content = std::fs::read_to_string(&path)?;
101+
102+
let has_declare_lint = content.contains("declare_oxc_lint!(");
103+
let has_from_config = content.contains("fn from_configuration(");
104+
// Look for "config =" as a parameter in declare_oxc_lint macro
105+
// Use regex to be more precise
106+
let has_config_param = content.contains("declare_oxc_lint!")
107+
&& content.split("declare_oxc_lint!").any(|section| {
108+
// Check if this section (after declare_oxc_lint!) contains "config ="
109+
// before the closing of the macro (the semicolon after the paren)
110+
if let Some(macro_end) = section.find(");") {
111+
let macro_content = &section[..macro_end];
112+
macro_content.contains("config =")
113+
} else {
114+
false
115+
}
116+
});
117+
118+
if has_declare_lint && has_from_config && !has_config_param {
119+
let rel_path = path.strip_prefix(base_dir).unwrap();
120+
results.push(rel_path.to_string_lossy().to_string());
121+
}
122+
}
123+
}
124+
Ok(())
125+
}
126+
127+
scan_rules_dir(&rules_dir, &rules_dir, &mut rules_with_from_config_no_schema)
128+
.expect("Failed to scan rules directory");
129+
130+
// Convert file paths to rule names (plugin/rule-name format)
131+
let mut rules_needing_schema: Vec<String> = rules_with_from_config_no_schema
132+
.iter()
133+
.filter_map(|path| {
134+
// Path format: "plugin/rule_file.rs" or "plugin/rule_name/mod.rs"
135+
// Always use '/' as separator regardless of OS
136+
let normalized_path = path.replace(std::path::MAIN_SEPARATOR, "/");
137+
let parts: Vec<&str> = normalized_path.split('/').collect();
138+
if parts.len() >= 2 {
139+
let plugin = parts[0];
140+
let rule_file = if parts.len() == 2 {
141+
// plugin/rule_file.rs
142+
parts[1].strip_suffix(".rs")?
143+
} else {
144+
// plugin/rule_name/mod.rs
145+
parts[1]
146+
};
147+
// Convert underscores to hyphens for rule name
148+
let rule_name = rule_file.replace('_', "-");
149+
Some(format!("{}/{}", plugin, rule_name))
150+
} else {
151+
None
152+
}
153+
})
154+
.collect();
155+
156+
rules_needing_schema.sort();
157+
rules_needing_schema.dedup();
158+
159+
// Step 2: Generate documentation
160+
let temp_dir = std::env::temp_dir().join(format!("oxc-rule-docs-test-{}", std::process::id()));
161+
std::fs::create_dir_all(&temp_dir).expect("Failed to create temp directory");
162+
163+
let git_ref = std::process::Command::new("git")
164+
.args(["rev-parse", "HEAD"])
165+
.current_dir(workspace_root)
166+
.output()
167+
.expect("Failed to get git ref")
168+
.stdout;
169+
let git_ref = String::from_utf8_lossy(&git_ref).trim().to_string();
170+
171+
let output = std::process::Command::new("cargo")
172+
.args([
173+
"run",
174+
"-p",
175+
"website",
176+
"--",
177+
"linter-rules",
178+
"--rule-docs",
179+
temp_dir.to_str().unwrap(),
180+
"--git-ref",
181+
&git_ref,
182+
])
183+
.current_dir(workspace_root)
184+
.output()
185+
.expect("Failed to generate documentation");
186+
187+
if !output.status.success() {
188+
panic!("Failed to generate documentation:\n{}", String::from_utf8_lossy(&output.stderr));
189+
}
190+
191+
// Step 3: Check generated documentation for Configuration sections
192+
for rule_name in &rules_needing_schema {
193+
if exception_set.contains(rule_name.as_str()) {
194+
// Expected - this rule is a known exception
195+
continue;
196+
}
197+
198+
// Read the generated markdown file
199+
let doc_file = temp_dir.join(format!("{}.md", rule_name));
200+
201+
if !doc_file.exists() {
202+
failures.push(format!(
203+
"Rule '{rule_name}' has from_configuration in source but no documentation file was generated at {}",
204+
doc_file.display()
205+
));
206+
continue;
207+
}
208+
209+
let doc_content = std::fs::read_to_string(&doc_file)
210+
.unwrap_or_else(|_| panic!("Failed to read {}", doc_file.display()));
211+
212+
// Check if the documentation has a Configuration section
213+
let has_config_section = doc_content.contains("## Configuration");
214+
215+
if !has_config_section {
216+
failures.push(format!(
217+
"Rule '{rule_name}' has from_configuration in source code but no Configuration section in generated docs.\n\
218+
This means the rule accepts configuration options but they are not documented.\n\
219+
\n\
220+
To fix:\n\
221+
1. Create a config struct that derives JsonSchema (and typically Deserialize, Debug, Clone)\n\
222+
2. Add `config = YourConfigStruct` to the declare_oxc_lint! macro\n\
223+
3. Add documentation comments to the config struct and its fields\n\
224+
\n\
225+
Or if this rule should not have configuration docs, add it to the exceptions list in this test.\n\
226+
\n\
227+
Example:\n\
228+
#[derive(Debug, Clone, JsonSchema)]\n\
229+
#[serde(rename_all = \"camelCase\", default)]\n\
230+
pub struct YourRuleConfig {{\n\
231+
/// Description of this option\n\
232+
some_option: bool,\n\
233+
}}\n\
234+
\n\
235+
declare_oxc_lint!(\n\
236+
YourRule,\n\
237+
plugin,\n\
238+
category,\n\
239+
config = YourRuleConfig,\n\
240+
);",
241+
));
242+
}
243+
}
244+
245+
// Verify exception list is accurate
246+
for &exception_rule in exceptions {
247+
if !rules_needing_schema.contains(&exception_rule.to_string()) {
248+
failures.push(format!(
249+
"Exception rule '{exception_rule}' is in the exceptions list but was not found by source code analysis.\n\
250+
This rule may have been fixed or removed. Please remove it from the exceptions list."
251+
));
252+
}
253+
}
254+
255+
// Clean up temp directory
256+
let _ = std::fs::remove_dir_all(&temp_dir);
257+
258+
assert!(
259+
failures.is_empty(),
260+
"Found {} rules with configuration issues:\n\n{}",
261+
failures.len(),
262+
failures.join("\n\n")
263+
);
264+
}

0 commit comments

Comments
 (0)