From ed704be6948c082ab44028a8bb7630f7105dcc17 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Wed, 13 Nov 2024 12:01:11 +0000 Subject: [PATCH 01/13] refactor(rules_check): extract DiagnosticWriter from assert_lint() --- xtask/rules_check/src/lib.rs | 131 ++++++++++++++++++++++------------- 1 file changed, 82 insertions(+), 49 deletions(-) diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 44fd6b2109b5..5994eb516e08 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -123,6 +123,7 @@ pub fn check_rules() -> anyhow::Result<()> { Ok(()) } + struct CodeBlockTest { tag: String, expect_diagnostic: bool, @@ -166,6 +167,73 @@ impl FromStr for CodeBlockTest { } } +struct DiagnosticWriter<'a> { + group: &'a str, + rule: &'a str, + test: &'a CodeBlockTest, + code: &'a str, + diagnostic_count: i32, + all_diagnostics: Vec, + has_error: bool, +} + +impl<'a> DiagnosticWriter<'a> { + pub fn new( + group: &'a str, + rule: &'a str, + test: &'a CodeBlockTest, + code: &'a str, + ) -> DiagnosticWriter<'a> { + DiagnosticWriter { + group, + rule, + test, + code, + diagnostic_count: 0, + all_diagnostics: vec![], + has_error: false, + } + } + + pub fn write_diagnostic(&mut self, diag: biome_diagnostics::Error) -> anyhow::Result<()> { + let group = self.group; + let rule = self.rule; + let code = self.code; + + // Record the diagnostic + self.all_diagnostics.push(diag); + + // Fail the test if the analysis returns more diagnostics than expected... + if self.test.expect_diagnostic { + if self.all_diagnostics.len() > 1 { + self.print_all_diagnostics(); + self.has_error = true; + bail!("Analysis of '{group}/{rule}' on the following code block returned multiple diagnostics.\n\n{code}"); + } + } else { + // ...or if the analysis returns a diagnostic when it is expected to not report one. + self.print_all_diagnostics(); + self.has_error = true; + bail!("Analysis of '{group}/{rule}' on the following code block returned an unexpected diagnostic.\n\n{code}"); + } + self.diagnostic_count += 1; + Ok(()) + } + + /// Prints all diagnostics to help the user. + fn print_all_diagnostics(&mut self) { + let mut console = biome_console::EnvConsole::default(); + for diag in self.all_diagnostics.iter() { + console.println( + biome_console::LogLevel::Error, + markup! { + {PrintDiagnostic::verbose(diag)} + }, + ); + } + } +} + /// Parse and analyze the provided code block, and asserts that it emits /// exactly zero or one diagnostic depending on the value of `expect_diagnostic`. /// That diagnostic is then emitted as text into the `content` buffer @@ -177,49 +245,14 @@ fn assert_lint( ) -> anyhow::Result<()> { let file_path = format!("code-block.{}", test.tag); - let mut diagnostic_count = 0; - let mut all_diagnostics = vec![]; - let mut has_error = false; - let mut write_diagnostic = |code: &str, diag: biome_diagnostics::Error| { - all_diagnostics.push(diag); - // Fail the test if the analysis returns more diagnostics than expected - if test.expect_diagnostic { - // Print all diagnostics to help the user - if all_diagnostics.len() > 1 { - let mut console = biome_console::EnvConsole::default(); - for diag in all_diagnostics.iter() { - console.println( - biome_console::LogLevel::Error, - markup! { - {PrintDiagnostic::verbose(diag)} - }, - ); - } - has_error = true; - bail!("Analysis of '{group}/{rule}' on the following code block returned multiple diagnostics.\n\n{code}"); - } - } else { - // Print all diagnostics to help the user - let mut console = biome_console::EnvConsole::default(); - for diag in all_diagnostics.iter() { - console.println( - biome_console::LogLevel::Error, - markup! { - {PrintDiagnostic::verbose(diag)} - }, - ); - } - has_error = true; - bail!("Analysis of '{group}/{rule}' on the following code block returned an unexpected diagnostic.\n\n{code}"); - } - diagnostic_count += 1; - Ok(()) - }; - if test.ignore { return Ok(()); } + // Record the diagnostics emitted by the lint rule to later check if + // what was emitted matches the expectations set for this code block. + let mut diagnostics = DiagnosticWriter::new(group, rule, test, code); + let mut settings = WorkspaceSettings::default(); let key = settings.insert_project(PathBuf::new()); settings.register_current_project(key); @@ -247,7 +280,7 @@ fn assert_lint( if parse.has_errors() { for diag in parse.into_diagnostics() { let error = diag.with_file_path(&file_path).with_file_source_code(code); - write_diagnostic(code, error)?; + diagnostics.write_diagnostic(error)?; } } else { let root = parse.tree(); @@ -283,7 +316,7 @@ fn assert_lint( .with_severity(severity) .with_file_path(&file_path) .with_file_source_code(code); - let res = write_diagnostic(code, error); + let res = diagnostics.write_diagnostic(error); // Abort the analysis on error if let Err(err) = res { @@ -302,7 +335,7 @@ fn assert_lint( if parse.has_errors() { for diag in parse.into_diagnostics() { let error = diag.with_file_path(&file_path).with_file_source_code(code); - write_diagnostic(code, error)?; + diagnostics.write_diagnostic(error)?; } } else { let root = parse.tree(); @@ -334,7 +367,7 @@ fn assert_lint( .with_severity(severity) .with_file_path(&file_path) .with_file_source_code(code); - let res = write_diagnostic(code, error); + let res = diagnostics.write_diagnostic(error); // Abort the analysis on error if let Err(err) = res { @@ -353,7 +386,7 @@ fn assert_lint( if parse.has_errors() { for diag in parse.into_diagnostics() { let error = diag.with_file_path(&file_path).with_file_source_code(code); - write_diagnostic(code, error)?; + diagnostics.write_diagnostic(error)?; } } else { let root = parse.tree(); @@ -385,7 +418,7 @@ fn assert_lint( .with_severity(severity) .with_file_path(&file_path) .with_file_source_code(code); - let res = write_diagnostic(code, error); + let res = diagnostics.write_diagnostic(error); // Abort the analysis on error if let Err(err) = res { @@ -404,7 +437,7 @@ fn assert_lint( if parse.has_errors() { for diag in parse.into_diagnostics() { let error = diag.with_file_path(&file_path).with_file_source_code(code); - write_diagnostic(code, error)?; + diagnostics.write_diagnostic(error)?; } } else { let root = parse.tree(); @@ -436,7 +469,7 @@ fn assert_lint( .with_severity(severity) .with_file_path(&file_path) .with_file_source_code(code); - let res = write_diagnostic(code, error); + let res = diagnostics.write_diagnostic(error); // Abort the analysis on error if let Err(err) = res { @@ -459,12 +492,12 @@ fn assert_lint( if test.expect_diagnostic { // Fail the test if the analysis didn't emit any diagnostic ensure!( - diagnostic_count == 1, + diagnostics.diagnostic_count == 1, "Analysis of '{group}/{rule}' on the following code block returned no diagnostics.\n\n{code}", ); } - if has_error { + if diagnostics.has_error { bail!("A code snippet must emit one single diagnostic, but it seems multiple diagnostics were emitted. Make sure that all the snippets inside the code block 'expect_diagnostic' emit only one diagnostic.") } From 34f1f378574371613f11396186a9af820f464616 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Thu, 14 Nov 2024 13:51:28 +0000 Subject: [PATCH 02/13] refactor(rules_check): create AnalyzerOptions from WorkspaceSettings via resolve_analyzer_options --- Cargo.lock | 1 + xtask/rules_check/Cargo.toml | 1 + xtask/rules_check/src/lib.rs | 67 ++++++++++++++++++++++++------------ 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9ad2ac28851e..5b581f41d000 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3130,6 +3130,7 @@ dependencies = [ "biome_css_parser", "biome_css_syntax", "biome_diagnostics", + "biome_fs", "biome_graphql_analyze", "biome_graphql_parser", "biome_graphql_syntax", diff --git a/xtask/rules_check/Cargo.toml b/xtask/rules_check/Cargo.toml index e3dac5aaddd0..110a057cd208 100644 --- a/xtask/rules_check/Cargo.toml +++ b/xtask/rules_check/Cargo.toml @@ -13,6 +13,7 @@ biome_css_analyze = { workspace = true } biome_css_parser = { workspace = true } biome_css_syntax = { workspace = true } biome_diagnostics = { workspace = true } +biome_fs = { workspace = true } biome_graphql_analyze = { workspace = true } biome_graphql_parser = { workspace = true } biome_graphql_syntax = { workspace = true } diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 5994eb516e08..7879e6fb4dba 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -4,19 +4,20 @@ use anyhow::{bail, ensure}; use biome_analyze::options::JsxRuntime; use biome_analyze::{ - AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, GroupCategory, Queryable, - RegistryVisitor, Rule, RuleCategory, RuleFilter, RuleGroup, RuleMetadata, + AnalysisFilter, AnalyzerOptions, ControlFlow, GroupCategory, Queryable, RegistryVisitor, Rule, + RuleCategory, RuleFilter, RuleGroup, RuleMetadata, }; use biome_console::{markup, Console}; use biome_css_parser::CssParserOptions; use biome_css_syntax::CssLanguage; use biome_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic}; +use biome_fs::BiomePath; use biome_graphql_syntax::GraphqlLanguage; use biome_js_parser::JsParserOptions; use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage}; use biome_json_parser::JsonParserOptions; use biome_json_syntax::JsonLanguage; -use biome_service::settings::WorkspaceSettings; +use biome_service::settings::{ServiceLanguage, WorkspaceSettings}; use biome_service::workspace::DocumentFileSource; use pulldown_cmark::{CodeBlockKind, Event, Parser, Tag, TagEnd}; use std::collections::BTreeMap; @@ -234,6 +235,36 @@ impl<'a> DiagnosticWriter<'a> { } } +fn create_analyzer_options( + workspace_settings: &WorkspaceSettings, + file_path: &String, + test: &CodeBlockTest, +) -> AnalyzerOptions +where + L: ServiceLanguage, +{ + let path = BiomePath::new(PathBuf::from(&file_path)); + let file_source = &test.document_file_source(); + let supression_reason = None; + + let settings = workspace_settings.get_current_settings(); + let linter = settings.map(|s| &s.linter); + let overrides = settings.map(|s| &s.override_settings); + let language_settings = settings + .map(|s| L::lookup_settings(&s.languages)) + .map(|result| &result.linter); + + L::resolve_analyzer_options( + settings, + linter, + overrides, + language_settings, + &path, + file_source, + supression_reason, + ) +} + /// Parse and analyze the provided code block, and asserts that it emits /// exactly zero or one diagnostic depending on the value of `expect_diagnostic`. /// That diagnostic is then emitted as text into the `content` buffer @@ -291,14 +322,12 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = AnalyzerOptions { - configuration: AnalyzerConfiguration { - jsx_runtime: Some(JsxRuntime::default()), - ..Default::default() - }, - file_path: PathBuf::from(&file_path), - suppression_reason: None, + let options = { + let mut o = create_analyzer_options::(&settings, &file_path, &test); + o.configuration.jsx_runtime = Some(JsxRuntime::default()); + o }; + biome_js_analyze::analyze(&root, filter, &options, file_source, None, |signal| { if let Some(mut diag) = signal.diagnostic() { let category = diag.category().expect("linter diagnostic has no code"); @@ -346,10 +375,8 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = AnalyzerOptions { - file_path: PathBuf::from(&file_path), - ..Default::default() - }; + let options = create_analyzer_options::(&settings, &file_path, &test); + biome_json_analyze::analyze(&root, filter, &options, file_source, |signal| { if let Some(mut diag) = signal.diagnostic() { let category = diag.category().expect("linter diagnostic has no code"); @@ -397,10 +424,8 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = AnalyzerOptions { - file_path: PathBuf::from(&file_path), - ..Default::default() - }; + let options = create_analyzer_options::(&settings, &file_path, &test); + biome_css_analyze::analyze(&root, filter, &options, |signal| { if let Some(mut diag) = signal.diagnostic() { let category = diag.category().expect("linter diagnostic has no code"); @@ -448,10 +473,8 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = AnalyzerOptions { - file_path: PathBuf::from(&file_path), - ..Default::default() - }; + let options = create_analyzer_options::(&settings, &file_path, &test); + biome_graphql_analyze::analyze(&root, filter, &options, |signal| { if let Some(mut diag) = signal.diagnostic() { let category = diag.category().expect("linter diagnostic has no code"); From d911213876aa5ce2c0f5c773981b464ab5418b63 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Wed, 13 Nov 2024 12:06:22 +0000 Subject: [PATCH 03/13] feat(rules_check): validate the option examples for rules --- Cargo.lock | 3 + crates/biome_analyze/CONTRIBUTING.md | 2 + .../explain_valid_rule.snap | 33 +++- .../src/lint/a11y/no_blank_target.rs | 31 +++- .../src/lint/a11y/no_label_without_control.rs | 3 +- .../src/lint/a11y/use_valid_aria_role.rs | 3 +- .../no_excessive_cognitive_complexity.rs | 3 +- .../correctness/no_undeclared_dependencies.rs | 3 +- .../correctness/no_undeclared_variables.rs | 2 +- .../use_exhaustive_dependencies.rs | 4 +- .../lint/correctness/use_import_extensions.rs | 3 +- .../src/lint/nursery/no_restricted_imports.rs | 37 +++- .../src/lint/nursery/no_restricted_types.rs | 3 +- .../use_adjacent_overload_signatures.rs | 2 +- .../use_component_export_only_modules.rs | 6 +- .../src/lint/nursery/use_sorted_classes.rs | 26 ++- .../lint/nursery/use_valid_autocomplete.rs | 3 +- .../src/lint/style/no_restricted_globals.rs | 5 +- .../lint/style/use_consistent_array_type.rs | 3 +- .../lint/style/use_filenaming_convention.rs | 3 +- .../src/lint/style/use_naming_convention.rs | 92 ++++++---- .../src/lint/suspicious/no_console.rs | 12 +- xtask/rules_check/Cargo.toml | 3 + xtask/rules_check/src/lib.rs | 172 +++++++++++++++++- 24 files changed, 354 insertions(+), 103 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5b581f41d000..549ec4176083 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3125,10 +3125,12 @@ version = "0.0.0" dependencies = [ "anyhow", "biome_analyze", + "biome_configuration", "biome_console", "biome_css_analyze", "biome_css_parser", "biome_css_syntax", + "biome_deserialize", "biome_diagnostics", "biome_fs", "biome_graphql_analyze", @@ -3138,6 +3140,7 @@ dependencies = [ "biome_js_parser", "biome_js_syntax", "biome_json_analyze", + "biome_json_factory", "biome_json_parser", "biome_json_syntax", "biome_service", diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index b7080fb388c5..fbe5bfed7085 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -710,6 +710,8 @@ The documentation needs to adhere to the following rules: - When adding _invalid_ snippets in the `### Invalid` section, you must use the `expect_diagnostic` code block property. We use this property to generate a diagnostic and attach it to the snippet. A snippet **must emit only ONE diagnostic**. - When adding _valid_ snippets in the `### Valid` section, you can use one single snippet. - You can use the code block property `ignore` to tell the code generation script to **not generate a diagnostic for an invalid snippet**. +- You can use the code block property `options` to tell the code generation script that this is a configuration options example. +- You can use the code block property `use_options` to tell the code generation script to use the options from the most recent `options` block while linting. - Update the `language` field in the `declare_lint_rule!` macro to the language the rule primarily applies to. - If your rule applies to any JavaScript, you can leave it as `js`. - If your rule only makes sense in a specific JavaScript dialect, you should set it to `jsx`, `ts`, or `tsx`, whichever is most appropriate. diff --git a/crates/biome_cli/tests/snapshots/main_commands_explain/explain_valid_rule.snap b/crates/biome_cli/tests/snapshots/main_commands_explain/explain_valid_rule.snap index 87c6d943292e..ca2cf78d1114 100644 --- a/crates/biome_cli/tests/snapshots/main_commands_explain/explain_valid_rule.snap +++ b/crates/biome_cli/tests/snapshots/main_commands_explain/explain_valid_rule.snap @@ -1,6 +1,7 @@ --- source: crates/biome_cli/tests/snap_test.rs expression: content +snapshot_kind: text --- # Emitted Messages @@ -36,11 +37,11 @@ and the [the noopener documentation](https://html.spec.whatwg.org/multipage/link ``` ```jsx,expect_diagnostic -child +child ``` ```jsx,expect_diagnostic -child +child ``` ### Valid @@ -50,7 +51,7 @@ and the [the noopener documentation](https://html.spec.whatwg.org/multipage/link ``` ```jsx -child +child ``` ## Options @@ -58,22 +59,36 @@ and the [the noopener documentation](https://html.spec.whatwg.org/multipage/link The option `allowDomains` allows specific domains to use `target="_blank"` without `rel="noreferrer"`. In the following configuration, it's allowed to use the domains `https://example.com` and `example.org`: -```json,ignore +```json,options { -"//": "...", "options": { "allowDomains": ["https://example.com", "example.org"] } } ``` -```jsx,ignore +```jsx,use_options <> - - + + ``` -Biome doesn't check if the list contains valid URLs. +The diagnostic is applied to all domains not in the allow list: + +```json,options +{ +"options": { +"allowDomains": ["https://example.com"] +} +} ``` +```jsx,expect_diagnostic,use_options +<> + + + +``` +Biome doesn't check if the list contains valid URLs. +``` diff --git a/crates/biome_js_analyze/src/lint/a11y/no_blank_target.rs b/crates/biome_js_analyze/src/lint/a11y/no_blank_target.rs index 1b45e3fa5f79..4898dbd12d23 100644 --- a/crates/biome_js_analyze/src/lint/a11y/no_blank_target.rs +++ b/crates/biome_js_analyze/src/lint/a11y/no_blank_target.rs @@ -33,11 +33,11 @@ declare_lint_rule! { /// ``` /// /// ```jsx,expect_diagnostic - /// child + /// child /// ``` /// /// ```jsx,expect_diagnostic - /// child + /// child /// ``` /// /// ### Valid @@ -47,7 +47,7 @@ declare_lint_rule! { /// ``` /// /// ```jsx - /// child + /// child /// ``` /// /// ## Options @@ -55,22 +55,37 @@ declare_lint_rule! { /// The option `allowDomains` allows specific domains to use `target="_blank"` without `rel="noreferrer"`. /// In the following configuration, it's allowed to use the domains `https://example.com` and `example.org`: /// - /// ```json,ignore + /// ```json,options /// { - /// "//": "...", /// "options": { /// "allowDomains": ["https://example.com", "example.org"] /// } /// } /// ``` /// - /// ```jsx,ignore + /// ```jsx,use_options /// <> - /// - /// + /// + /// /// /// ``` /// + /// The diagnostic is applied to all domains not in the allow list: + /// + /// ```json,options + /// { + /// "options": { + /// "allowDomains": ["https://example.com"] + /// } + /// } + /// ``` + /// + /// ```jsx,expect_diagnostic,use_options + /// <> + /// + /// + /// + /// ``` /// Biome doesn't check if the list contains valid URLs. pub NoBlankTarget { version: "1.0.0", diff --git a/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs b/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs index 497f126a81d0..bae591e668ad 100644 --- a/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs +++ b/crates/biome_js_analyze/src/lint/a11y/no_label_without_control.rs @@ -73,9 +73,8 @@ declare_lint_rule! { /// /// Both options `inputComponents` and `labelComponents` don't have support for namespace components (e.g. ``). /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "inputComponents": ["CustomInput"], /// "labelAttributes": ["label"], diff --git a/crates/biome_js_analyze/src/lint/a11y/use_valid_aria_role.rs b/crates/biome_js_analyze/src/lint/a11y/use_valid_aria_role.rs index 692986a033ad..64495ad70bb6 100644 --- a/crates/biome_js_analyze/src/lint/a11y/use_valid_aria_role.rs +++ b/crates/biome_js_analyze/src/lint/a11y/use_valid_aria_role.rs @@ -44,9 +44,8 @@ declare_lint_rule! { /// /// ## Options /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "allowInvalidRoles": ["invalid-role", "text"], /// "ignoreNonDom": true diff --git a/crates/biome_js_analyze/src/lint/complexity/no_excessive_cognitive_complexity.rs b/crates/biome_js_analyze/src/lint/complexity/no_excessive_cognitive_complexity.rs index 53598c34aaf7..43da9180f2db 100644 --- a/crates/biome_js_analyze/src/lint/complexity/no_excessive_cognitive_complexity.rs +++ b/crates/biome_js_analyze/src/lint/complexity/no_excessive_cognitive_complexity.rs @@ -58,9 +58,8 @@ declare_lint_rule! { /// /// Allows to specify the maximum allowed complexity. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "maxAllowedComplexity": 15 /// } diff --git a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs index 3bb061c5ca57..503e554938b5 100644 --- a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs +++ b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs @@ -51,7 +51,8 @@ declare_lint_rule! { /// - `optionalDependencies`: If set to `false`, then the rule will show an error when `optionalDependencies` are imported. Defaults to `true`. /// /// You can set the options like this: - /// ```json + /// + /// ```json,options /// { /// "options": { /// "devDependencies": false, diff --git a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs index 3f6bcdb432d3..1e9e7fe5e1e5 100644 --- a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs +++ b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_variables.rs @@ -19,7 +19,7 @@ declare_lint_rule! { /// The rule provides a `checkTypes` option that make the rule checks undeclared types. /// The option defaults to `true`. /// - /// ```json + /// ```json,options /// { /// "options": { /// "checkTypes": true diff --git a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs index 5daf84f2bf8e..15da2a2ddf61 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs @@ -176,9 +176,8 @@ declare_lint_rule! { /// /// #### Example /// - /// ```json + /// ```json, options /// { - /// "//": "...", /// "options": { /// "hooks": [ /// { "name": "useLocation", "closureIndex": 0, "dependenciesIndex": 1}, @@ -218,7 +217,6 @@ declare_lint_rule! { /// /// ```json /// { - /// "//": "...", /// "options": { /// "hooks": [ /// { "name": "useDispatch", "stableResult": true } diff --git a/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs b/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs index 3a750b7f35f5..a95205e70474 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_import_extensions.rs @@ -84,9 +84,8 @@ declare_lint_rule! { /// For example, if you want `.ts` files to import other modules as `.js` (or `.jsx`), you should /// configure the following options in your Biome config: /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "suggestedExtensions": { /// "ts": { diff --git a/crates/biome_js_analyze/src/lint/nursery/no_restricted_imports.rs b/crates/biome_js_analyze/src/lint/nursery/no_restricted_imports.rs index ead4e83692a9..cdf525d9f053 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_restricted_imports.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_restricted_imports.rs @@ -12,18 +12,41 @@ declare_lint_rule! { /// /// ## Options /// - /// ```json + /// ```json,options /// { - /// "noRestrictedImports": { - /// "options": { - /// "paths": { - /// "lodash": "Using lodash is not encouraged", - /// "underscore": "Using underscore is not encouraged" - /// } + /// "options": { + /// "paths": { + /// "lodash": "Using lodash is not encouraged", + /// "underscore": "Using underscore is not encouraged" /// } /// } /// } /// ``` + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic,use_options + /// import "lodash"; + /// import "allowed-import"; + /// ``` + /// + /// ```js,expect_diagnostic,use_options + /// const underscore = await import("underscore"); + /// ``` + /// + /// ```js,expect_diagnostic,use_options + /// const lodash = require("lodash"); + /// ``` + /// + /// ### Valid + /// + /// ```js,use_options + /// import "allowed-import"; + /// const myImport = await import("allowed-import"); + /// const myImport = require("allowed-import"); + /// ``` pub NoRestrictedImports { version: "1.6.0", name: "noRestrictedImports", diff --git a/crates/biome_js_analyze/src/lint/nursery/no_restricted_types.rs b/crates/biome_js_analyze/src/lint/nursery/no_restricted_types.rs index 0df43b5de86a..58f3c243fc90 100644 --- a/crates/biome_js_analyze/src/lint/nursery/no_restricted_types.rs +++ b/crates/biome_js_analyze/src/lint/nursery/no_restricted_types.rs @@ -28,9 +28,8 @@ declare_lint_rule! { /// Use the options to specify additional types that you want to restrict in your /// source code. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "types": { /// "Foo": { diff --git a/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs b/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs index 6cfcc1b7e4ee..ed4dff2b571f 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_adjacent_overload_signatures.rs @@ -40,7 +40,7 @@ declare_lint_rule! { /// } /// ``` /// - /// ```ts,expect_diagnostic,ignore + /// ```ts,expect_diagnostic /// class A { /// fooA(s: string): void; /// fooA(n: number): void; diff --git a/crates/biome_js_analyze/src/lint/nursery/use_component_export_only_modules.rs b/crates/biome_js_analyze/src/lint/nursery/use_component_export_only_modules.rs index c32ae1532b99..bdfb63cebebb 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_component_export_only_modules.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_component_export_only_modules.rs @@ -71,9 +71,8 @@ declare_lint_rule! { /// /// Some tools, such as [Vite], allow exporting constants along with components. By enabling the following, the rule will support the pattern. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options":{ /// "allowConstantExport" : true /// } @@ -85,9 +84,8 @@ declare_lint_rule! { /// If you use a framework that handles [Hot Module Replacement(HMR)] of some specific exports, you can use this option to avoid warning for them. /// /// Example for [Remix](https://remix.run/docs/en/main/discussion/hot-module-replacement#supported-exports): - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options":{ /// "allowExportNames": ["json", "loader", "headers", "meta", "links", "scripts"] /// } diff --git a/crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs b/crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs index f01ccc0dab02..2bc14ffc32e2 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_sorted_classes.rs @@ -71,7 +71,7 @@ declare_lint_rule! { /// /// ### Code-related /// - /// ```json + /// ```json,options /// { /// "options": { /// "attributes": ["classList"], @@ -88,19 +88,29 @@ declare_lint_rule! { /// /// If specified, strings in the indicated functions will be sorted. This is useful when working with libraries like [`clsx`](https://github.com/lukeed/clsx) or [`cva`](https://cva.style/). /// - /// ```js,ignore + /// ```js,expect_diagnostic,use_options /// clsx("px-2 foo p-4 bar", { + /// "some-css-class": condition, + /// }); + /// ``` + /// + /// ```js,expect_diagnostic,use_options + /// clsx("some-css-class", { /// "block mx-4": condition, /// }); /// ``` /// /// Tagged template literals are also supported, for example: /// - /// ```js,ignore + /// ```js,use_options /// tw`px-2`; /// tw.div`px-2`; /// ``` /// + /// ```js,expect_diagnostic,use_options + /// tw`px-2 foo p-4 bar`; + /// ``` + /// /// ### Sort-related /// /// :::caution @@ -117,8 +127,14 @@ declare_lint_rule! { /// /// This has two implications: /// - /// - False positives: classes can be wrongly recognized as utilities even though their values are incorrect. For example, if there's a `px-` utility defined in the configuration, it will match all of the following classes: `px-2`, `px-1337`, `px-[not-actually-valid]`, `px-literally-anything`. - /// - No distinction between different utilities that share the same prefix: for example, `text-red-500` and `text-lg` are both interpreted as the same type of utility by this rule, even though the former refers to a color and the latter to a font size. This results in all utilities that share the same prefix being sorted together, regardless of their actual values. + /// - **False positives:** classes can be wrongly recognized as utilities even though their values are incorrect. + /// For example, if there's a `px-` utility defined in the configuration, it will match all of the following classes: + /// `px-2`, `px-1337`, `px-[not-actually-valid]`, `px-literally-anything`. + /// + /// - **No distinction between different utilities that share the same prefix:** for example, + /// `text-red-500` and `text-lg` are both interpreted as the same type of utility by this rule, + /// even though the former refers to a color and the latter to a font size. This results in all + /// utilities that share the same prefix being sorted together, regardless of their actual values. /// /// ### Custom additions must be specified /// diff --git a/crates/biome_js_analyze/src/lint/nursery/use_valid_autocomplete.rs b/crates/biome_js_analyze/src/lint/nursery/use_valid_autocomplete.rs index 3e5d37ace142..701fecd67a04 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_valid_autocomplete.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_valid_autocomplete.rs @@ -32,9 +32,8 @@ declare_lint_rule! { /// /// ## Options /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "inputComponents": ["MyInput"] /// } diff --git a/crates/biome_js_analyze/src/lint/style/no_restricted_globals.rs b/crates/biome_js_analyze/src/lint/style/no_restricted_globals.rs index 7f45ef22e073..1998bf613d36 100644 --- a/crates/biome_js_analyze/src/lint/style/no_restricted_globals.rs +++ b/crates/biome_js_analyze/src/lint/style/no_restricted_globals.rs @@ -11,6 +11,8 @@ use serde::{Deserialize, Serialize}; declare_lint_rule! { /// This rule allows you to specify global variable names that you don’t want to use in your application. /// + /// References to the global identifiers `error` and `event` are always disallowed by this rule. + /// /// > Disallowing usage of specific global variables can be useful if you want to allow a set of /// global variables by enabling an environment, but still want to disallow some of those. /// @@ -33,9 +35,8 @@ declare_lint_rule! { /// Use the options to specify additional globals that you want to restrict in your /// source code. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "deniedGlobals": ["$", "MooTools"] /// } diff --git a/crates/biome_js_analyze/src/lint/style/use_consistent_array_type.rs b/crates/biome_js_analyze/src/lint/style/use_consistent_array_type.rs index 925afb33eced..f152685d0ec7 100644 --- a/crates/biome_js_analyze/src/lint/style/use_consistent_array_type.rs +++ b/crates/biome_js_analyze/src/lint/style/use_consistent_array_type.rs @@ -51,9 +51,8 @@ declare_lint_rule! { /// /// Use the options to specify the syntax of array declarations to use. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "syntax": "shorthand" /// } diff --git a/crates/biome_js_analyze/src/lint/style/use_filenaming_convention.rs b/crates/biome_js_analyze/src/lint/style/use_filenaming_convention.rs index e7b6b573d3ac..9842ae5a82f3 100644 --- a/crates/biome_js_analyze/src/lint/style/use_filenaming_convention.rs +++ b/crates/biome_js_analyze/src/lint/style/use_filenaming_convention.rs @@ -65,9 +65,8 @@ declare_lint_rule! { /// /// The rule provides several options that are detailed in the following subsections. /// - /// ```json5 + /// ```jsonc,options /// { - /// "//": "...", /// "options": { /// "strictCase": false, /// "requireAscii": true, diff --git a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs index 126c22b32108..c1743b439474 100644 --- a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs +++ b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs @@ -232,7 +232,7 @@ declare_lint_rule! { /// /// ### TypeScript `namespace` names /// - /// A _TypeScript_ `namespace` names are in [`camelCase`] or in [`PascalCase`]. + /// A _TypeScript_ `namespace` name is in [`camelCase`] or in [`PascalCase`]. /// /// ```ts /// namespace mathExtra { @@ -248,11 +248,11 @@ declare_lint_rule! { /// /// Note that some declarations are always ignored. /// You cannot apply a convention to them. - /// This is the case of: + /// This is the case for: /// /// - Member names that are not identifiers /// - /// ```js,ignore + /// ```js /// class C { /// ["not an identifier"]() {} /// } @@ -260,27 +260,27 @@ declare_lint_rule! { /// /// - Named imports /// - /// ```js,ignore + /// ```js /// import { an_IMPORT } from "mod" /// ``` /// - /// - destructured object properties + /// - Destructured object properties /// - /// ```js,ignore + /// ```js /// const { destructed_PROP } = obj; /// ``` /// - /// - class member marked with `override` + /// - Class members marked with `override`: /// - /// ```ts,ignore + /// ```ts /// class C extends B { /// override overridden_METHOD() {} /// } /// ``` /// - /// - declarations inside an external TypeScript module + /// - Declarations inside an external TypeScript module /// - /// ```ts,ignore + /// ```ts /// declare module "myExternalModule" { /// export interface my_INTERFACE {} /// } @@ -290,9 +290,8 @@ declare_lint_rule! { /// /// The rule provides several options that are detailed in the following subsections. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "strictCase": false, /// "requireAscii": true, @@ -314,23 +313,50 @@ declare_lint_rule! { /// ### strictCase /// /// When this option is set to `true`, it forbids consecutive uppercase characters in [`camelCase`] and [`PascalCase`]. - /// For instance, when the option is set to `true`, `HTTPServer` or `aHTTPServer` will throw an error. - /// These names should be renamed to `HttpServer` and `aHttpServer` /// - /// When the option is set to `false`, consecutive uppercase characters are allowed. - /// `HTTPServer` and `aHTTPServer` are so valid. + /// **Default:** `true` /// - /// Default: `true` + /// For instance, `HTTPServer` or `aHTTPServer` are not permitted for `strictCase: true`. + /// These names should be renamed to `HttpServer` and `aHttpServer`: + /// + /// ```json,options + /// { + /// "options": { + /// "strictCase": true + /// } + /// } + /// ``` + /// + /// ```js,expect_diagnostic,use_options + /// class HTTPServer { + /// } + /// ``` + /// + /// When `strictCase` is set to `false`, consecutive uppercase characters are allowed. + /// For example, `HTTPServer` and `aHTTPServer` would be considered valid then: + /// + /// ```json,options + /// { + /// "options": { + /// "strictCase": false + /// } + /// } + /// ``` + /// + /// ```js,use_options + /// class HTTPServer { + /// } + /// ``` /// /// ### requireAscii /// - /// When this option is set to `true`, it forbids names that include non-ASCII characters. - /// For instance, when the option is set to `true`, `café` or `안녕하세요` will throw an error. + /// When `true`, names must only consist of ASCII characters only, + /// forbidding names like `café` or `안녕하세요` that include non-ASCII characters. /// - /// When the option is set to `false`, names may include non-ASCII characters. - /// `café` and `안녕하세요` are so valid. + /// When `requireAscii` is set to `false`, names may include non-ASCII characters. + /// For example, `café` and `안녕하세요` would be considered valid then. /// - /// Default: `false` + /// **Default:** `false` /// /// **This option will be turned on by default in Biome 2.0.** /// @@ -342,8 +368,8 @@ declare_lint_rule! { /// You can enforce another convention by setting `enumMemberCase` option. /// The supported cases are: [`PascalCase`], [`CONSTANT_CASE`], and [`camelCase`]. /// - /// This option will be deprecated in the future. - /// Use the `conventions` option instead. + /// **This option will be deprecated in the future.** + /// **Use the [`conventions`](#conventions-since-v180) option instead.** /// /// ### conventions (Since v1.8.0) /// @@ -353,9 +379,8 @@ declare_lint_rule! { /// /// For example, you can enforce the use of [`CONSTANT_CASE`] for global `const` declarations: /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "conventions": [ /// { @@ -440,9 +465,8 @@ declare_lint_rule! { /// - A private property starts with `_` and consists of at least two characters /// - The captured name (the name without the leading `_`) is in [`camelCase`]. /// - /// ```json5 + /// ```json,options /// { - /// // ... /// "options": { /// "conventions": [ /// { @@ -462,9 +486,8 @@ declare_lint_rule! { /// then the part of the name captured by the regular expression is forwarded to the next conventions of the array. /// In the following example, we require that private class members start with `_` and all class members are in ["camelCase"]. /// - /// ```json5 + /// ```jsonc,options /// { - /// // ... /// "options": { /// "conventions": [ /// { @@ -491,9 +514,8 @@ declare_lint_rule! { /// Because the default conventions already ensure that class members are in ["camelCase"], /// the previous example can be simplified to: /// - /// ```json5 + /// ```jsonc,options /// { - /// // ... /// "options": { /// "conventions": [ /// { @@ -516,9 +538,8 @@ declare_lint_rule! { /// /// You can reset all default conventions by adding a convention at the end of the array that accepts anything: /// - /// ```json5 + /// ```jsonc,options /// { - /// // ... /// "options": { /// "conventions": [ /// // your conventions @@ -546,9 +567,8 @@ declare_lint_rule! { /// and to be in [`PascalCase`]. /// - All other names follow the default conventions /// - /// ```json5 + /// ```jsonc,options /// { - /// // ... /// "options": { /// "conventions": [ /// { diff --git a/crates/biome_js_analyze/src/lint/suspicious/no_console.rs b/crates/biome_js_analyze/src/lint/suspicious/no_console.rs index f053bd4e5df3..bc2c7e053fbc 100644 --- a/crates/biome_js_analyze/src/lint/suspicious/no_console.rs +++ b/crates/biome_js_analyze/src/lint/suspicious/no_console.rs @@ -26,17 +26,23 @@ declare_lint_rule! { /// /// ## Options /// - /// Use the options to specify the allowed `console` methods. + /// Use the options to explicitly allow a specific subset of `console` methods. /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "allow": ["assert", "error", "info", "warn"] /// } /// } /// ``` /// + /// ```js,expect_diagnostic,use_options + /// console.error("error message"); // Allowed + /// console.warn("warning message"); // Allowed + /// console.info("info message"); // Allowed + /// console.log("log message"); + /// console.assert(true, "explanation"); // Allowed + /// ``` pub NoConsole { version: "1.6.0", name: "noConsole", diff --git a/xtask/rules_check/Cargo.toml b/xtask/rules_check/Cargo.toml index 110a057cd208..9c99562524e3 100644 --- a/xtask/rules_check/Cargo.toml +++ b/xtask/rules_check/Cargo.toml @@ -8,10 +8,12 @@ version = "0.0.0" [dependencies] anyhow = { workspace = true } biome_analyze = { workspace = true } +biome_configuration = { workspace = true } biome_console = { workspace = true } biome_css_analyze = { workspace = true } biome_css_parser = { workspace = true } biome_css_syntax = { workspace = true } +biome_deserialize = { workspace = true } biome_diagnostics = { workspace = true } biome_fs = { workspace = true } biome_graphql_analyze = { workspace = true } @@ -21,6 +23,7 @@ biome_js_analyze = { workspace = true } biome_js_parser = { workspace = true } biome_js_syntax = { workspace = true } biome_json_analyze = { workspace = true } +biome_json_factory = { workspace = true } biome_json_parser = { workspace = true } biome_json_syntax = { workspace = true } biome_service = { workspace = true } diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 7879e6fb4dba..17c703851e43 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -7,16 +7,19 @@ use biome_analyze::{ AnalysisFilter, AnalyzerOptions, ControlFlow, GroupCategory, Queryable, RegistryVisitor, Rule, RuleCategory, RuleFilter, RuleGroup, RuleMetadata, }; +use biome_configuration::PartialConfiguration; use biome_console::{markup, Console}; use biome_css_parser::CssParserOptions; use biome_css_syntax::CssLanguage; +use biome_deserialize::json::deserialize_from_json_ast; use biome_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic}; use biome_fs::BiomePath; use biome_graphql_syntax::GraphqlLanguage; use biome_js_parser::JsParserOptions; use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage}; +use biome_json_factory::make; use biome_json_parser::JsonParserOptions; -use biome_json_syntax::JsonLanguage; +use biome_json_syntax::{AnyJsonValue, JsonLanguage, JsonObjectValue}; use biome_service::settings::{ServiceLanguage, WorkspaceSettings}; use biome_service::workspace::DocumentFileSource; use pulldown_cmark::{CodeBlockKind, Event, Parser, Tag, TagEnd}; @@ -126,9 +129,22 @@ pub fn check_rules() -> anyhow::Result<()> { } struct CodeBlockTest { + /// The language tag of this code block. tag: String, + + /// True if this is an invalid example that should trigger a diagnostic. expect_diagnostic: bool, + + /// Whether to ignore this code block. ignore: bool, + + /// True if this is a block of configuration options instead + /// of a valid/invalid code example. + options: bool, + + /// Whether to use the last code block that was marked with + /// `options` as the configuration settings for this code block. + use_options: bool, } impl CodeBlockTest { @@ -152,6 +168,8 @@ impl FromStr for CodeBlockTest { tag: String::new(), expect_diagnostic: false, ignore: false, + options: false, + use_options: false, }; for token in tokens { @@ -159,6 +177,8 @@ impl FromStr for CodeBlockTest { // Other attributes "expect_diagnostic" => test.expect_diagnostic = true, "ignore" => test.ignore = true, + "options" => test.options = true, + "use_options" => test.use_options = true, // Regard as language tags, last one wins _ => test.tag = token.to_string(), } @@ -273,6 +293,7 @@ fn assert_lint( rule: &'static str, test: &CodeBlockTest, code: &str, + config: &Option, ) -> anyhow::Result<()> { let file_path = format!("code-block.{}", test.tag); @@ -284,9 +305,22 @@ fn assert_lint( // what was emitted matches the expectations set for this code block. let mut diagnostics = DiagnosticWriter::new(group, rule, test, code); + // Create a synthetic workspace configuration let mut settings = WorkspaceSettings::default(); let key = settings.insert_project(PathBuf::new()); settings.register_current_project(key); + + // Load settings from the preceding `json,options` block if requested + if test.use_options { + let Some(partial_config) = config else { + bail!("Code blocks tagged with 'use_options' must be preceded by a valid 'json,options' code block."); + }; + + settings + .get_current_settings_mut() + .merge_with_configuration(partial_config.clone(), None, None, &[])?; + } + match test.document_file_source() { DocumentFileSource::Js(file_source) => { // Temporary support for astro, svelte and vue code blocks @@ -323,7 +357,7 @@ fn assert_lint( }; let options = { - let mut o = create_analyzer_options::(&settings, &file_path, &test); + let mut o = create_analyzer_options::(&settings, &file_path, test); o.configuration.jsx_runtime = Some(JsxRuntime::default()); o }; @@ -375,7 +409,7 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = create_analyzer_options::(&settings, &file_path, &test); + let options = create_analyzer_options::(&settings, &file_path, test); biome_json_analyze::analyze(&root, filter, &options, file_source, |signal| { if let Some(mut diag) = signal.diagnostic() { @@ -424,7 +458,7 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = create_analyzer_options::(&settings, &file_path, &test); + let options = create_analyzer_options::(&settings, &file_path, test); biome_css_analyze::analyze(&root, filter, &options, |signal| { if let Some(mut diag) = signal.diagnostic() { @@ -473,7 +507,7 @@ fn assert_lint( ..AnalysisFilter::default() }; - let options = create_analyzer_options::(&settings, &file_path, &test); + let options = create_analyzer_options::(&settings, &file_path, test); biome_graphql_analyze::analyze(&root, filter, &options, |signal| { if let Some(mut diag) = signal.diagnostic() { @@ -527,7 +561,124 @@ fn assert_lint( Ok(()) } -/// Parse the documentation fragment for a lint rule (in markdown) and lint the code blcoks. +/// Creates a synthetic JSON AST for an object literal with a single member. +fn make_json_object_with_single_member>( + name: &str, + value: V, +) -> JsonObjectValue { + make::json_object_value( + make::token(biome_json_syntax::JsonSyntaxKind::L_CURLY), + make::json_member_list( + [make::json_member( + make::json_member_name(make::json_string_literal(name)), + make::token(biome_json_syntax::JsonSyntaxKind::COLON), + value.into(), + )], + [], + ), + make::token(biome_json_syntax::JsonSyntaxKind::R_CURLY), + ) +} + +/// Parse the options fragment for a lint rule and return the parsed options. +fn parse_rule_options( + group: &'static str, + rule: &'static str, + test: &CodeBlockTest, + code: &str, +) -> anyhow::Result> { + let file_path = format!("code-block.{}", test.tag); + + // Record the diagnostics emitted during configuration parsing to later check + // if what was emitted matches the expectations set for this code block. + let mut diagnostics = DiagnosticWriter::new(group, rule, test, code); + + match test.document_file_source() { + DocumentFileSource::Json(file_source) => { + let parse = biome_json_parser::parse_json(code, JsonParserOptions::from(&file_source)); + + if parse.has_errors() { + for diag in parse.into_diagnostics() { + let error = diag.with_file_path(&file_path).with_file_source_code(code); + diagnostics.write_diagnostic(error)?; + } + // Parsing failed, but test.expect_diagnostic is true + return Ok(None); + } + + // By convention, the configuration blocks in the documentation + // only contain the settings for the lint rule itself, like so: + // + // ```json,options + // { + // "options": { + // ... + // } + // } + // ``` + let parsed_root = parse.tree(); + + // We therefore extend the JSON AST with some synthetic elements + // to make it match the structure expected by the configuration parse: + // + // { + // "linter": { + // "rules": { + // "": { + // "": {} + // } + // } + // } + // } + let parsed_options = parsed_root.value()?; + let synthetic_tree = make_json_object_with_single_member( + "linter", + make_json_object_with_single_member( + "rules", + make_json_object_with_single_member( + group, + make_json_object_with_single_member(rule, parsed_options), + ), + ), + ); + + // We reuse the original AST where possible so that errors are + // reported at the correct locations in the source code. + let eof_token = parsed_root.eof_token()?; + let mut root_builder = make::json_root(synthetic_tree.into(), eof_token); + if let Some(bom_token) = parsed_root.bom_token() { + root_builder = root_builder.with_bom_token(bom_token); + } + let root = root_builder.build(); + + // Deserialize the configuration from the partially-synthetic AST, + // and report any errors encountered during deserialization. + let deserialized = deserialize_from_json_ast::(&root, ""); + let (partial_configuration, deserialize_diagnostics) = deserialized.consume(); + + if !deserialize_diagnostics.is_empty() { + for diag in deserialize_diagnostics { + let error = diag.with_file_path(&file_path).with_file_source_code(code); + diagnostics.write_diagnostic(error)?; + } + // Deserialization failed, but test.expect_diagnostic is true + return Ok(None); + } + + let Some(result) = partial_configuration else { + bail!("Failed to deserialize configuration options for '{group}/{rule}' from the following code block due to unknown error.\n\n{code}"); + }; + + Ok(Some(result)) + } + _ => { + // Only JSON code blocks can contain configuration options + bail!("The following non-JSON code block for '{group}/{rule}' was marked as containing configuration options. Only JSON code blocks can used to provide configuration options.\n\n{code}"); + } + } +} + +/// Parse the documentation fragment for a lint rule (in markdown) and lint the code blocks. fn parse_documentation( group: &'static str, rule: &'static str, @@ -535,6 +686,9 @@ fn parse_documentation( ) -> anyhow::Result<()> { let parser = Parser::new(docs); + // Track the last configuration options block that was encountered + let mut last_options: Option = None; + // Tracks the content of the current code block if it's using a // language supported for analysis let mut language = None; @@ -548,7 +702,11 @@ fn parse_documentation( } Event::End(TagEnd::CodeBlock) => { if let Some((test, block)) = language.take() { - assert_lint(group, rule, &test, &block)?; + if test.options { + last_options = parse_rule_options(group, rule, &test, &block)?; + } else { + assert_lint(group, rule, &test, &block, &last_options)?; + } } } Event::Text(text) => { From 3cb7b987bd1d4a769200632cb268e66fe4d6b50d Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Thu, 14 Nov 2024 13:55:56 +0000 Subject: [PATCH 04/13] fix(rules_check): report correct offsets for configuration code blocks --- Cargo.lock | 1 + xtask/rules_check/Cargo.toml | 1 + xtask/rules_check/src/lib.rs | 58 +++++++++++++++++++++++++++++++++--- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 549ec4176083..604df6f1eeee 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3143,6 +3143,7 @@ dependencies = [ "biome_json_factory", "biome_json_parser", "biome_json_syntax", + "biome_rowan", "biome_service", "pulldown-cmark", ] diff --git a/xtask/rules_check/Cargo.toml b/xtask/rules_check/Cargo.toml index 9c99562524e3..f43e383edf36 100644 --- a/xtask/rules_check/Cargo.toml +++ b/xtask/rules_check/Cargo.toml @@ -26,6 +26,7 @@ biome_json_analyze = { workspace = true } biome_json_factory = { workspace = true } biome_json_parser = { workspace = true } biome_json_syntax = { workspace = true } +biome_rowan = { workspace = true } biome_service = { workspace = true } pulldown-cmark = "0.12.2" diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 17c703851e43..b52a5be10994 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -16,10 +16,11 @@ use biome_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic}; use biome_fs::BiomePath; use biome_graphql_syntax::GraphqlLanguage; use biome_js_parser::JsParserOptions; -use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage}; +use biome_js_syntax::{EmbeddingKind, JsFileSource, JsLanguage, TextSize}; use biome_json_factory::make; use biome_json_parser::JsonParserOptions; use biome_json_syntax::{AnyJsonValue, JsonLanguage, JsonObjectValue}; +use biome_rowan::AstNode; use biome_service::settings::{ServiceLanguage, WorkspaceSettings}; use biome_service::workspace::DocumentFileSource; use pulldown_cmark::{CodeBlockKind, Event, Parser, Tag, TagEnd}; @@ -196,6 +197,7 @@ struct DiagnosticWriter<'a> { diagnostic_count: i32, all_diagnostics: Vec, has_error: bool, + subtract_offset: TextSize, } impl<'a> DiagnosticWriter<'a> { @@ -213,6 +215,7 @@ impl<'a> DiagnosticWriter<'a> { diagnostic_count: 0, all_diagnostics: vec![], has_error: false, + subtract_offset: TextSize::from(0), } } @@ -222,7 +225,7 @@ impl<'a> DiagnosticWriter<'a> { let code = self.code; // Record the diagnostic - self.all_diagnostics.push(diag); + self.all_diagnostics.push(self.adjust_span_offset(diag)); // Fail the test if the analysis returns more diagnostics than expected... if self.test.expect_diagnostic { @@ -253,6 +256,21 @@ impl<'a> DiagnosticWriter<'a> { ); } } + + /// Adjusts the location of the diagnostic to account for synthetic nodes + /// that arent't present in the source code but only in the AST. + fn adjust_span_offset(&self, diag: biome_diagnostics::Error) -> biome_diagnostics::Error { + if self.subtract_offset != 0.into() { + if let Some(span) = diag.location().span { + let new_span = span.checked_sub(self.subtract_offset); + diag.with_file_span(new_span) + } else { + diag + } + } else { + diag + } + } } fn create_analyzer_options( @@ -580,6 +598,23 @@ fn make_json_object_with_single_member>( ) } +fn get_first_member>(parent: V, expected_name: &str) -> Option { + let parent_value: AnyJsonValue = parent.into(); + let member = parent_value + .as_json_object_value()? + .json_member_list() + .into_iter() + .next()? + .ok()?; + let member_name = member.name().ok()?.inner_string_text().ok()?.to_string(); + + if member_name.as_str() == expected_name { + member.value().ok() + } else { + None + } +} + /// Parse the options fragment for a lint rule and return the parsed options. fn parse_rule_options( group: &'static str, @@ -642,8 +677,7 @@ fn parse_rule_options( ), ); - // We reuse the original AST where possible so that errors are - // reported at the correct locations in the source code. + // Create a new JsonRoot from the synthetic AST let eof_token = parsed_root.eof_token()?; let mut root_builder = make::json_root(synthetic_tree.into(), eof_token); if let Some(bom_token) = parsed_root.bom_token() { @@ -651,6 +685,22 @@ fn parse_rule_options( } let root = root_builder.build(); + // Adjust source code spans to account for the synthetic nodes + // so that errors are reported at the correct source code locations: + let original_offset = parsed_root.value().ok().map(|v| AstNode::range(&v).start()); + let wrapped_offset = root + .value() + .ok() + .and_then(|v| get_first_member(v, "linter")) + .and_then(|v| get_first_member(v, "rules")) + .and_then(|v| get_first_member(v, group)) + .and_then(|v| get_first_member(v, rule)) + .map(|v| AstNode::range(&v).start()); + diagnostics.subtract_offset = wrapped_offset + .zip(original_offset) + .and_then(|(wrapped, original)| wrapped.checked_sub(original)) + .unwrap_or_default(); + // Deserialize the configuration from the partially-synthetic AST, // and report any errors encountered during deserialization. let deserialized = deserialize_from_json_ast::(&root, ""); From 94e699bc06506309ee97bdf91f33b4a63afcccc3 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Thu, 14 Nov 2024 16:06:52 +0000 Subject: [PATCH 05/13] feat(rules_check): strip "# " prefix from code block lines --- xtask/rules_check/src/lib.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index b52a5be10994..5543c6a51967 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -761,7 +761,12 @@ fn parse_documentation( } Event::Text(text) => { if let Some((_, block)) = &mut language { - write!(block, "{text}")?; + if let Some(inner_text) = text.strip_prefix("# ") { + // Lines prefixed with "# " are hidden from the public documentation + write!(block, "{inner_text}")?; + } else { + write!(block, "{text}")?; + } } } // We don't care other events From 2b2d0d289e0d9247520ecf79e614cfd1b4b94f35 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Thu, 14 Nov 2024 19:56:22 +0000 Subject: [PATCH 06/13] docs(js_analyze): improve docs of useConsistentMemberAccessibility lint rule --- .../use_consistent_member_accessibility.rs | 187 ++++++++++++++---- 1 file changed, 144 insertions(+), 43 deletions(-) diff --git a/crates/biome_js_analyze/src/lint/nursery/use_consistent_member_accessibility.rs b/crates/biome_js_analyze/src/lint/nursery/use_consistent_member_accessibility.rs index 8945dd5dcc1c..e75116f02e6d 100644 --- a/crates/biome_js_analyze/src/lint/nursery/use_consistent_member_accessibility.rs +++ b/crates/biome_js_analyze/src/lint/nursery/use_consistent_member_accessibility.rs @@ -26,84 +26,163 @@ declare_lint_rule! { /// /// ### Invalid /// - /// The following patterns are considered incorrect code with the default options `noPublic`: + /// #### `"accessibility": "noPublic"` (default value) /// - /// ```js,ignore + /// Use the following configuration to disallow all explicit `public` modifiers: + /// + /// ```json,options + /// { + /// "options": { + /// "accessibility": "noPublic" + /// } + /// } + /// ``` + /// + /// The following patterns are considered incorrect code with `noPublic`: + /// + /// ```ts,expect_diagnostic,use_options /// class Animal { - /// public constructor( + /// public constructor(breed, name) { + /// // ... + /// } + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Animal { + /// constructor( /// public breed, /// name, /// ) { - /// // Parameter property and constructor - /// this.animalName = name; + /// // ... /// } - /// public animalName: string; // Property + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Animal { + /// public animalName: string; + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Pet { /// public get name(): string { - /// // get accessor /// return this.animalName; /// } + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Pet { /// public set name(value: string) { - /// // set accessor /// this.animalName = value; /// } + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Dog { /// public walk() { - /// // method + /// // ... /// } /// } /// ``` /// - /// The following patterns are considered incorrect code with the accessibility set to `explicit`: + /// #### `"accessibility": "explicit"` + /// + /// Use the following configuration to enforce the presence of explicit modifiers wherever possible: + /// + /// ```json,options + /// { + /// "options": { + /// "accessibility": "explicit" + /// } + /// } + /// ``` + /// + /// The following patterns are considered incorrect code with `accessibility` set to `explicit`: /// - /// ```ts,ignore + /// ```ts,expect_diagnostic,use_options /// class Animal { - /// // Constructor is not set accessibility modifier - /// constructor( + /// constructor( // Invalid: Missing accessibility modifier /// public breed, /// name, /// ) { - /// // Parameter property and constructor /// this.animalName = name; /// } - /// private animalName: string; // Property - /// public get name(): string { - /// // get accessor + /// private animalName: string; // OK: Modifier must be present + /// public get name(): string { // OK: Modifier must be present /// return this.animalName; /// } - /// public set name(value: string) { - /// // set accessor + /// public set name(value: string) { // OK: Modifier must be present /// this.animalName = value; /// } - /// protected walk() { - /// // method + /// protected walk() { // OK: Modifier must be present + /// // ... /// } /// } /// ``` /// - /// The following patterns are considered incorrect code with the accessibility set to `none`: + /// #### `"accessibility": "none"` + /// + /// Use the following configuration to disallow all explicit visibility modifiers: /// - /// ```ts,ignore + /// ```json,options + /// { + /// "options": { + /// "accessibility": "none" + /// } + /// } + /// ``` + /// + /// The following patterns are considered incorrect code with `accessibility` set to `none`: + /// + /// ```ts,expect_diagnostic,use_options + /// class Animal { + /// protected constructor(breed, name) { + /// // ... + /// } + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options /// class Animal { /// constructor( /// protected breed, /// name, /// ) { - /// // Parameter property and constructor - /// this.name = name; + /// // ... /// } - /// // Property is set accessibility modifier - /// private animalName: string; // Property - /// get name(): string { - /// // get accessor + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Animal { + /// private animalName: string; + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Animal { + /// protected get name(): string { /// return this.animalName; /// } - /// // set accessor is set accessibility modifier - /// set name(value: string) { - /// // set accessor + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Pet { + /// private set name(value: string) { /// this.animalName = value; /// } - /// // walk() is set accessibility modifier - /// protected walk() { - /// // method + /// } + /// ``` + /// + /// ```ts,expect_diagnostic,use_options + /// class Dog { + /// public walk() { + /// // ... /// } /// } /// ``` @@ -112,13 +191,20 @@ declare_lint_rule! { /// /// The following patterns are considered correct code with the default options `noPublic`: /// - /// ```ts,ignore + /// ```json,options + /// { + /// "options": { + /// "accessibility": "noPublic" + /// } + /// } + /// ``` + /// + /// ```ts,use_options /// class Animal { /// constructor( - /// public breed, + /// private breed, /// name, /// ) { - /// // Parameter property and constructor /// this.animalName = name; /// } /// private animalName: string; // Property @@ -138,7 +224,15 @@ declare_lint_rule! { /// /// The following patterns are considered correct code with the accessibility set to `explicit`: /// - /// ```ts,ignore + /// ```json,options + /// { + /// "options": { + /// "accessibility": "explicit" + /// } + /// } + /// ``` + /// + /// ```ts,use_options /// class Animal { /// public constructor( /// public breed, @@ -164,7 +258,15 @@ declare_lint_rule! { /// /// The following patterns are considered correct code with the accessibility set to `none`: /// - /// ```ts,ignore + /// ```json,options + /// { + /// "options": { + /// "accessibility": "none" + /// } + /// } + /// ``` + /// + /// ```ts,use_options /// class Animal { /// constructor( /// breed, @@ -192,9 +294,8 @@ declare_lint_rule! { /// /// The rule supports the following options: /// - /// ```json + /// ```json,options /// { - /// "//": "...", /// "options": { /// "accessibility": "explicit" /// } @@ -210,7 +311,7 @@ declare_lint_rule! { /// - `explicit` - requires an accessibility modifier for every member that allows that (a safe fix will add public). /// - `none` - forbid all accessibility modifiers (public, protected, private). /// - /// Default: `noPublic`. + /// **Default:** `noPublic` /// pub UseConsistentMemberAccessibility { version: "1.9.0", From 40d2b1b62898edf2093db7479bacec310077a572 Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 10:49:19 +0000 Subject: [PATCH 07/13] feat(rules_check): validate examples of full biome.json snippets in rules documentation --- xtask/rules_check/src/lib.rs | 167 ++++++++++++++++++++++------------- 1 file changed, 105 insertions(+), 62 deletions(-) diff --git a/xtask/rules_check/src/lib.rs b/xtask/rules_check/src/lib.rs index 5543c6a51967..a221a2b74961 100644 --- a/xtask/rules_check/src/lib.rs +++ b/xtask/rules_check/src/lib.rs @@ -139,15 +139,29 @@ struct CodeBlockTest { /// Whether to ignore this code block. ignore: bool, - /// True if this is a block of configuration options instead - /// of a valid/invalid code example. - options: bool, + /// Whether this is a block of configuration options instead + /// of a valid/invalid code example, and if yes, how that + /// block of configuration options should be parsed: + options: OptionsParsingMode, /// Whether to use the last code block that was marked with /// `options` as the configuration settings for this code block. use_options: bool, } +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum OptionsParsingMode { + /// This code block does not contain configuration options. + #[default] + NoOptions, + + /// This code block contains the options for a single rule only. + RuleOptionsOnly, + + /// This code block contains JSON that adheres to the full `biome.json` schema. + FullConfiguration, +} + impl CodeBlockTest { fn document_file_source(&self) -> DocumentFileSource { DocumentFileSource::from_extension(&self.tag) @@ -169,7 +183,7 @@ impl FromStr for CodeBlockTest { tag: String::new(), expect_diagnostic: false, ignore: false, - options: false, + options: OptionsParsingMode::NoOptions, use_options: false, }; @@ -178,7 +192,8 @@ impl FromStr for CodeBlockTest { // Other attributes "expect_diagnostic" => test.expect_diagnostic = true, "ignore" => test.ignore = true, - "options" => test.options = true, + "options" => test.options = OptionsParsingMode::RuleOptionsOnly, + "full_options" => test.options = OptionsParsingMode::FullConfiguration, "use_options" => test.use_options = true, // Regard as language tags, last one wins _ => test.tag = token.to_string(), @@ -641,65 +656,93 @@ fn parse_rule_options( return Ok(None); } - // By convention, the configuration blocks in the documentation - // only contain the settings for the lint rule itself, like so: - // - // ```json,options - // { - // "options": { - // ... - // } - // } - // ``` let parsed_root = parse.tree(); - - // We therefore extend the JSON AST with some synthetic elements - // to make it match the structure expected by the configuration parse: - // - // { - // "linter": { - // "rules": { - // "": { - // "": {} - // } - // } - // } - // } let parsed_options = parsed_root.value()?; - let synthetic_tree = make_json_object_with_single_member( - "linter", - make_json_object_with_single_member( - "rules", - make_json_object_with_single_member( - group, - make_json_object_with_single_member(rule, parsed_options), - ), - ), - ); - // Create a new JsonRoot from the synthetic AST - let eof_token = parsed_root.eof_token()?; - let mut root_builder = make::json_root(synthetic_tree.into(), eof_token); - if let Some(bom_token) = parsed_root.bom_token() { - root_builder = root_builder.with_bom_token(bom_token); - } - let root = root_builder.build(); - - // Adjust source code spans to account for the synthetic nodes - // so that errors are reported at the correct source code locations: - let original_offset = parsed_root.value().ok().map(|v| AstNode::range(&v).start()); - let wrapped_offset = root - .value() - .ok() - .and_then(|v| get_first_member(v, "linter")) - .and_then(|v| get_first_member(v, "rules")) - .and_then(|v| get_first_member(v, group)) - .and_then(|v| get_first_member(v, rule)) - .map(|v| AstNode::range(&v).start()); - diagnostics.subtract_offset = wrapped_offset - .zip(original_offset) - .and_then(|(wrapped, original)| wrapped.checked_sub(original)) - .unwrap_or_default(); + let root = match test.options { + OptionsParsingMode::NoOptions => { + unreachable!("parse_rule_options should only be called for options blocks") + } + OptionsParsingMode::RuleOptionsOnly => { + // By convention, the configuration blocks in the documentation + // only contain the settings for the lint rule itself, like so: + // + // ```json,options + // { + // "options": { + // ... + // } + // } + // ``` + // + // We therefore extend the JSON AST with some synthetic elements + // to make it match the structure expected by the configuration parse: + // + // { + // "linter": { + // "rules": { + // "": { + // "": {} + // } + // } + // } + // } + let synthetic_tree = make_json_object_with_single_member( + "linter", + make_json_object_with_single_member( + "rules", + make_json_object_with_single_member( + group, + make_json_object_with_single_member(rule, parsed_options), + ), + ), + ); + + // Create a new JsonRoot from the synthetic AST + let eof_token = parsed_root.eof_token()?; + let mut root_builder = make::json_root(synthetic_tree.into(), eof_token); + if let Some(bom_token) = parsed_root.bom_token() { + root_builder = root_builder.with_bom_token(bom_token); + } + let synthetic_root = root_builder.build(); + + // Adjust source code spans to account for the synthetic nodes + // so that errors are reported at the correct source code locations: + let original_offset = + parsed_root.value().ok().map(|v| AstNode::range(&v).start()); + let wrapped_offset = synthetic_root + .value() + .ok() + .and_then(|v| get_first_member(v, "linter")) + .and_then(|v| get_first_member(v, "rules")) + .and_then(|v| get_first_member(v, group)) + .and_then(|v| get_first_member(v, rule)) + .map(|v| AstNode::range(&v).start()); + diagnostics.subtract_offset = wrapped_offset + .zip(original_offset) + .and_then(|(wrapped, original)| wrapped.checked_sub(original)) + .unwrap_or_default(); + + synthetic_root + } + OptionsParsingMode::FullConfiguration => { + // In some rare cases, we want to be able to display full JSON configuration + // instead, e.t. to be able to show off per-file overrides: + // + // ```json,full-options + // { + // "linter": { + // "rules": { + // "": { + // "": {} + // } + // } + // } + // } + // ``` + parsed_root + } + }; // Deserialize the configuration from the partially-synthetic AST, // and report any errors encountered during deserialization. @@ -752,7 +795,7 @@ fn parse_documentation( } Event::End(TagEnd::CodeBlock) => { if let Some((test, block)) = language.take() { - if test.options { + if test.options != OptionsParsingMode::NoOptions { last_options = parse_rule_options(group, rule, &test, &block)?; } else { assert_lint(group, rule, &test, &block, &last_options)?; From 6c80b4bbedd40b8de6c72fd03e6d51d4bfe816bf Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 11:34:51 +0000 Subject: [PATCH 08/13] fix(useNamingConvention): disable failing doctest until #4545 is fixed --- .../src/lint/style/use_naming_convention.rs | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs index c1743b439474..34f011487154 100644 --- a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs +++ b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs @@ -280,7 +280,48 @@ declare_lint_rule! { /// /// - Declarations inside an external TypeScript module /// - /// ```ts + /// :::caution + /// **Bug:** Declarations inside external TypeScript modules are currently not ignored. + /// This is a bug, and is tracked under [#4545](https://github.com/biomejs/biome/issues/4545). + /// + /// Until the bug is fixed, we recommend one of the following workarounds: + /// + /// - Move the type declarations for external modules into separate `.d.ts` files, + /// and use [overrides](https://biomejs.dev/reference/configuration/#overrides) + /// in your [`biome.json`](https://biomejs.dev/reference/configuration/) + /// to disable the `useNamingConvention` rule for those files: + /// + /// ```jsonc,full_options + /// { + /// "linter": { + /// "rules": { + /// "style": { + /// "useNamingConvention": "warn" + /// } + /// // ... + /// } + /// }, + /// // ... + /// "overrides": [ + /// { + /// "include": ["typings/*.d.ts"], + /// "linter": { + /// "rules": { + /// "style": { + /// "useNamingConvention": "off" + /// } + /// } + /// } + /// } + /// ] + /// } + /// ``` + /// + /// - Use [`// biome-ignore lint/style/useNamingConvention: `](https://biomejs.dev/linter/#ignore-code) + /// to ignore the problematic lines. + /// ::: + /// + /// ```ts,ignore /// declare module "myExternalModule" { /// export interface my_INTERFACE {} /// } From dceddc712dd492b4aa48957ef51dd4065481832e Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 13:06:45 +0000 Subject: [PATCH 09/13] docs(biome_analyze): fix spelling and grammar issues & use consistent headings --- crates/biome_analyze/CONTRIBUTING.md | 161 +++++++++++++++++---------- 1 file changed, 105 insertions(+), 56 deletions(-) diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index fbe5bfed7085..e61b2ac54bfa 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -10,6 +10,33 @@ The analyzer allows implementors to create **four different** types of rules: - **Assist**: This rule detects refactoring opportunities and emits code action signals. - **Transformation**: This rule detects transformations that should be applied to the code. +### Contents + +- [Creating a rule](#creating-a-rule) + - [Guideline: Naming Convention for rules](#guideline-naming-convention-for-rules) + - [Guideline: Explain a rule to the user](#guideline-explain-a-rule-to-the-user) + - [Guideline: Placement of new rules](#guideline-placement-of-new-rules) + - [Creating and implementing the rule](#creating-and-implementing-the-rule) + - [Rule options](#rule-options) + - [Coding the rule](#coding-the-rule) + - [`declare_lint_rule!` macro](#declare_lint_rule-macro) + - [`rule_category!` macro](#rule_category-macro) + - [Navigating the CST](#navigating-the-cst) + - [Querying multiple node types via `declare_node_union!`](#querying-multiple-node-types-via-declare_node_union) + - [Semantic Model](#semantic-model) + - [Multiple signals](#multiple-signals) + - [Code Actions](#code-actions) + - [Custom Syntax Tree Visitors](#custom-syntax-tree-visitors) + - [Common Logic Mistakes](#common-logic-mistakes) + - [Testing the rule](#testing-the-rule) + - [Quick Test](#quick-test) + - [Snapshot Tests](#snapshot-tests) + - [Run the snapshot tests](#run-the-snapshot-tests) + - [Documenting the rule](#documenting-the-rule) + - [Code generation](#code-generation) + - [Commiting your work](#commiting-your-work) + - [Sidenote: Deprecating a rule](#sidenote-deprecating-a-rule) + ## Creating a rule When creating or updating a lint rule, you need to be aware that there's a lot of generated code inside our toolchain. @@ -20,38 +47,49 @@ To create a new rule, you have to create and update several files. Because it is a bit tedious, _Biome_ provides an easy way to create and test your rule using [Just](https://just.systems/man/en/). _Just_ is not part of the rust toolchain, you have to install it with [a package manager](https://just.systems/man/en/chapter_4.html). -### Choose a name + +### Guideline: Naming Convention for rules _Biome_ follows a naming convention according to what the rule does: -1. Forbid a concept +1. _Forbid <a concept>_ - ```block + ``` no ``` When a rule's sole intention is to **forbid a single concept** - such as disallowing the use of `debugger` statements - the rule should be named using the `no` prefix. - For example, the rule to disallow the use of `debugger` statements is named `noDebugger`. -1. Mandate a concept + > [!NOTE] + > For example, the rule to disallow the use of `debugger` statements is named `noDebugger`. + +2. _Mandate <a concept>_ - ```block + ``` use ``` - When a rule's sole intention is to **mandate a single concept** - such as forcing the use of camel-casing - the rule should be named using the `use` prefix. - For example, the rule to mandating the use of camel-cased variable names is named `useCamelCase`. + When a rule's sole intention is to **mandate a single concept** - such as forcing the use of correct values for a certain attribute or the use of identifiers following a naming convention - the rule should be named using the `use` prefix. + + > [!NOTE] + > For example, the rule to mandating the use valid values for the HTML `lang` attribute is named `useValidLang`. -### Explain a rule to the user +### Guideline: Explain a rule to the user A rule should be informative to the user, and give as much explanation as possible. When writing a rule, you must adhere to the following **pillars**: -1. Explain to the user the error. Generally, this is the message of the diagnostic. -1. Explain to the user **why** the error is triggered. Generally, this is implemented with an additional node. -1. Tell the user what they should do. Generally, this is implemented using a code action. If a code action is not applicable a note should tell the user what they should do to fix the error. -### Create and implement the rule +1. Explain to the user **what** the error is. + Generally, this is the message of the diagnostic. + +2. Explain to the user ***why*** the error is triggered. + Generally, this is implemented with an additional output node. + +3. Tell the user **what** they **should do**. Generally, this is implemented using a [code action](#code-actions). + If a code action is not applicable a note should tell the user what they should do to fix the error. + +### Guideline: Placement of new rules New rules **must** be placed inside the `nursery` group. This group is meant as an incubation space, exempt from semantic versioning. Once a rule is stable, it's promoted to a group that fits it. This is done in a minor/major release. @@ -60,6 +98,8 @@ New rules **must** be placed inside the `nursery` group. This group is meant as > > If you aren't familiar with Biome's APIs, this is an option that you have. If you decide to use this option, you should make sure to describe your plan in an issue. +### Creating and implementing the rule + Let's say we want to create a new **lint** rule called `useMyRuleName`, follow these steps: 1. Run the command @@ -121,22 +161,22 @@ Let's say we want to create a new **lint** rule called `useMyRuleName`, follow t ``` When returning a code action, you must pass the `category` and the `applicability` fields. `category` must be `ctx.action_category(ctx.category(), ctx.group())`. - `applicability` is derived from the metadata [`fix_kind`](#code-action). + `applicability` is derived from the metadata [`fix_kind`](#code-actions). In other words, the code transformation should always result in code that doesn't change the behavior of the logic. In the case of `noVar`, it is not always safe to turn `var` to `const` or `let`. Don't forget to format your code with `just f` and lint with `just l`. -That's it! Now, let's test the rule. +That's it! Now, let's [test the rule](#testing-the-rule). -### Rule configuration +### Rule options Some rules may allow customization using options. We try to keep rule options to a minimum and only when needed. Before adding an option, it's worth a discussion. Options should follow our [technical philosophy](https://biomejs.dev/internals/philosophy/#technical). -Let's assume that the rule we implement support the following options: +Let's assume that the rule we want to implement supports the following options: - `behavior`: a string among `"A"`, `"B"`, and `"C"`; - `threshold`: an integer between 0 and 255; @@ -245,7 +285,7 @@ pub enum Behavior { Below, there are many tips and guidelines on how to create a lint rule using Biome infrastructure. -#### `declare_lint_rule` +#### `declare_lint_rule!` macro This macro is used to declare an analyzer rule type, and implement the [RuleMeta] trait for it. @@ -306,9 +346,9 @@ declare_lint_rule! { By default, `source_kind` is always `RuleSourceKind::SameLogic`. -#### Category Macro +#### `rule_category!` macro -Declaring a rule using `declare_lint_rule!` will cause a new `rule_category!` +Declaring a rule using [`declare_lint_rule!`](#declare_lint_rule-macro) will cause a new `rule_category!` macro to be declared in the surrounding module. This macro can be used to refer to the corresponding diagnostic category for this lint rule, if it has one. Using this macro instead of getting the category for a diagnostic @@ -361,7 +401,7 @@ Generally, you will end up navigating the CST inside the `run` function, and thi } ``` -#### Query multiple nodes +#### Querying multiple node types via `declare_node_union!` There are times when you might need to query multiple nodes at once. Instead of querying the root of the CST, you can use the macro `declare_node_union!` to "join" multiple nodes into an `enum`: @@ -476,13 +516,13 @@ impl Rule for ForLoopCountReferences { } ``` -#### Code action +#### Code Actions -A rule can implement a code action. A code action provides to the final user the option to fix or change their code. +A rule can provide one or more code actions. Code actions provide the final user with the option to fix or change their code. In a lint rule, for example, it signals an opportunity for the user to fix the diagnostic emitted by the rule. -First, you have to add a new metadata called `fix_kind`, its value is the `FixKind`. +First, you have to add a new metadata called `fix_kind` that specifies whether the fixes emitted by the rule are considered [**"safe"**](https://biomejs.dev/linter/#safe-fixes) or [**"unsafe"**](https://biomejs.dev/linter/#unsafe-fixes). ```rust use biome_analyze::{declare_lint_rule, FixKind}; @@ -499,9 +539,9 @@ declare_lint_rule! { } ``` -Then, you'll have to implement the `action` function of the trait `Rule` and return a `JsRuleAction`. +Then, you'll have to implement the `action` function of the `Rule` trait and return a `JsRuleAction`. -`JsRuleAction` needs, among other things, a `mutation` type, which you will use to store all additions, deletions and replacements that you will execute inside the action: +`JsRuleAction` needs, among other things, a `mutation` type, which you will use to store all additions, deletions and replacements that will be executed when the user applies the action: ```rust impl Rule for ExampleRule { @@ -518,9 +558,9 @@ impl Rule for ExampleRule { } ``` -The function `ctx.metadata().applicability()` will compute the `Applicability` type from the `fix_kind` value you provided at the beginning, inside the macro. +The `ctx.metadata().applicability()` function will compute the `Applicability` type from the `fix_kind` value you provided at the beginning inside the `declare_lint_rule!` macro. -#### Custom Visitors +#### Custom Syntax Tree Visitors Some lint rules may need to deeply inspect the child nodes of a query match before deciding on whether they should emit a signal or not. These rules can be @@ -638,25 +678,26 @@ console.log(); // <-- This should not be reported because `console` is redeclare To avoid this, you should consult the semantic model to check if the variable is global or not. -### Test the rule +### Testing the rule -#### Quick test +#### Quick Test A swift way to test your rule is to go inside the `biome_js_analyze/src/lib.rs` file (this will change based on where you're implementing the rule) and modify the `quick_test` function. -Usually this test is ignored, so remove _comment_ the macro `#[ignore]` macro, change the `let SOURCE` variable to whatever source code you need to test. Then update the rule filter, and add your rule: +Usually this test is ignored, so remove/_comment_ the `#[ignore]` macro and change the `let SOURCE` variable to whatever source code you need to test. Then update the rule filter, and add your rule: ```rust let rule_filter = RuleFilter::Rule("nursery", "useAwesomeTrick"); ``` -Now from your terminal, go inside the `biome_js_analyze` folder and run the test using `cargo`: +Now from your terminal, switch to the `crates/biome_js_analyze` folder and run the test using `cargo`: ```shell +cd crates/biome_js_analyze cargo t quick_test ``` -Remember that, in case you add `dbg!` macros inside your source code, you'll have to use `--show-output`: +Remember that if you added `dbg!` macros inside your source code, you'll have to use `--show-output`: ```shell cargo t quick_test -- --show-output @@ -664,21 +705,23 @@ cargo t quick_test -- --show-output The test is designed to **show** diagnostics and code actions if the rule correctly emits the signal. If nothing is shown, your logic didn't emit any signal. -#### Snapshots +#### Snapshot Tests Inside the `tests/specs/` folder, rules are divided by group and rule name. -The test infrastructure is rigid around the association of the pair "group/rule name", which means that -_**your test cases are placed inside the wrong group, you won't see any diagnostics**_. +The test infrastructure is rigid around the association of the pair "group/rule name", which means that _**if your test cases are placed inside the wrong group, you won't see any diagnostics**_. -Since each new rule will start from `nursery`, that's where we start. -If you used `just new-js-lintrule`, a folder that use the name of the rule should exist. -Otherwise, create a folder called `myRuleName/`, and then create one or more files where you want to create different cases. +Since each new rule will start from `nursery`, that's where we'll start. +If you used `just new-js-lintrule`, a folder with the name of the rule should already exist there. +Otherwise, create a folder called `myRuleName/`, and then create one or more files for the different cases you want to test. -A common pattern is to create files prefixed by `invalid` or `valid`. -The files prefixed by `invalid` contain code that are reported by the rule. -The files prefixed by `valid` contain code that are not reported by the rule. +> [!NOTE] +> A common pattern is to create files prefixed by `invalid` or `valid`. +> - The files prefixed by `invalid` contain code that is reported by the rule. +> - The files prefixed by `valid` contain code that is not reported by the rule. -Files ending with the extension `.jsonc` are differently handled. +##### `.jsonc` files + +Files ending with the extension `.jsonc` are handled differently. These files should contain an array of strings where each string is a code snippet. For instance, for the rule `noVar`, the file `invalidScript.jsonc` contains: @@ -686,9 +729,11 @@ For instance, for the rule `noVar`, the file `invalidScript.jsonc` contains: ["var x = 1; foo(x);", "for (var x of [1,2,3]) { foo(x); }"] ``` -Note that code in a file ending with the extension `.jsonc` are in a _script environment_. +Note that the code in those `.jsonc` files is interpreted in a _script environment_. This means that you cannot use syntax that belongs to _ECMAScript modules_ such as `import` and `export`. +#### Run the snapshot tests + Run the command: ```shell @@ -699,16 +744,21 @@ and if you've done everything correctly, you should see some snapshots emitted w Check our main [contribution document](https://github.com/biomejs/biome/blob/main/CONTRIBUTING.md#testing) to know how to deal with the snapshot tests. -### Document the rule +### Documenting the rule The documentation needs to adhere to the following rules: -- The **first** paragraph of the documentation is used as brief description of the rule, and it **must** be written in one single line. Breaking the paragraph in multiple lines will break the table content of the rules page. +- The **first** paragraph of the documentation is used as a brief description of the rule, + and it **must** be written in one single line. Breaking the paragraph into multiple lines + will break the table contents of the rules overview page. - The next paragraphs can be used to further document the rule with as many details as you see fit. -- The documentation must have a `## Examples` header, followed by two headers: `### Invalid` and `### Valid`. `### Invalid` must go first because we need to show when the rule is triggered. +- The documentation must have a `## Examples` header, followed by two headers: `### Invalid` and `### Valid`. + `### Invalid` must go first because we need to show when the rule is triggered. - Rule options if any, must be documented in the `## Options` section. - Each code block must have a _language_ defined. -- When adding _invalid_ snippets in the `### Invalid` section, you must use the `expect_diagnostic` code block property. We use this property to generate a diagnostic and attach it to the snippet. A snippet **must emit only ONE diagnostic**. -- When adding _valid_ snippets in the `### Valid` section, you can use one single snippet. +- When adding _invalid_ snippets in the `### Invalid` section, you must use the + `expect_diagnostic` code block property. We use this property to generate a diagnostic + and attach it to the snippet. A given snippet **must emit only ONE diagnostic**. +- When adding _valid_ snippets in the `### Valid` section, you can use one single snippet for all different valid cases. - You can use the code block property `ignore` to tell the code generation script to **not generate a diagnostic for an invalid snippet**. - You can use the code block property `options` to tell the code generation script that this is a configuration options example. - You can use the code block property `use_options` to tell the code generation script to use the options from the most recent `options` block while linting. @@ -768,9 +818,9 @@ For simplicity, use `just` to run all the commands with: just gen-lint ``` -### Commit your work +### Commiting your work -Once the rule implemented, tested, and documented, you are ready to open a pull request! +Once the rule is implemented, tested and documented, you are ready to open a pull request! Stage and commit your changes: @@ -779,13 +829,12 @@ Stage and commit your changes: > git commit -m 'feat(biome_js_analyze): myRuleName' ``` +Then push your changes to your forked repository and open a pull request. -### Deprecate a rule - -There are occasions when a rule must be deprecated, to avoid breaking changes. The reason -of deprecation can be multiple. +### Sidenote: Deprecating a rule -In order to do, the macro allows adding additional field to add the reason for deprecation +There are occasions when a rule must be deprecated to avoid breaking changes. +There are different reasons for deprecation, so the `declare_lint_rule!` macro enables you to specify the reason via an additional `deprecated:` field: ```rust use biome_analyze::declare_lint_rule; From f223c09f81b29af961ac427b3cae652d6b1dffde Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 13:28:57 +0000 Subject: [PATCH 10/13] docs(biome_analyze): move the "Rule Options" section to a more logical location --- crates/biome_analyze/CONTRIBUTING.md | 230 +++++++++++++++------------ crates/biome_deserialize/README.md | 2 +- 2 files changed, 127 insertions(+), 105 deletions(-) diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index e61b2ac54bfa..af621622538c 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -17,10 +17,10 @@ The analyzer allows implementors to create **four different** types of rules: - [Guideline: Explain a rule to the user](#guideline-explain-a-rule-to-the-user) - [Guideline: Placement of new rules](#guideline-placement-of-new-rules) - [Creating and implementing the rule](#creating-and-implementing-the-rule) - - [Rule options](#rule-options) - [Coding the rule](#coding-the-rule) - [`declare_lint_rule!` macro](#declare_lint_rule-macro) - [`rule_category!` macro](#rule_category-macro) + - [Rule Options](#rule-options) - [Navigating the CST](#navigating-the-cst) - [Querying multiple node types via `declare_node_union!`](#querying-multiple-node-types-via-declare_node_union) - [Semantic Model](#semantic-model) @@ -169,12 +169,116 @@ Don't forget to format your code with `just f` and lint with `just l`. That's it! Now, let's [test the rule](#testing-the-rule). -### Rule options + +### Coding the rule + +Below, there are many tips and guidelines on how to create a lint rule using Biome infrastructure. + + +#### `declare_lint_rule!` macro + +This macro is used to declare an analyzer rule type, and implement the [RuleMeta] trait for it. + +The macro itself expects the following syntax: + +```rust +use biome_analyze::declare_lint_rule; + +declare_lint_rule! { + /// Documentation + pub(crate) ExampleRule { + version: "next", + name: "myRuleName", + language: "js", + recommended: false, + } +} +``` + +##### Biome lint rules inspired by other lint rules + +If a **lint** rule is inspired by an existing rule from other ecosystems (ESLint, ESLint plugins, clippy, etc.), you can add a new metadata to the macro called `source`. Its value is `&'static [RuleSource]`, which is a reference to a slice of `RuleSource` elements, each representing a different source. + +If you're implementing a lint rule that matches the behaviour of the ESLint rule `no-debugger`, you'll use the variant `::ESLint` and pass the name of the rule: + +```rust +use biome_analyze::{declare_lint_rule, RuleSource}; + +declare_lint_rule! { + /// Documentation + pub(crate) ExampleRule { + version: "next", + name: "myRuleName", + language: "js", + recommended: false, + sources: &[RuleSource::Eslint("no-debugger")], + } +} +``` + +If the rule you're implementing has a different behaviour or option, you can add the `source_kind` metadata and use the `RuleSourceKind::Inspired` type. If there are multiple sources, we assume that each source has the same `source_kind`. + +```rust +use biome_analyze::{declare_lint_rule, RuleSource, RuleSourceKind}; + +declare_lint_rule! { + /// Documentation + pub(crate) ExampleRule { + version: "next", + name: "myRuleName", + language: "js", + recommended: false, + sources: &[RuleSource::Eslint("no-debugger")], + source_kind: RuleSourceKind::Inspired, + } +} +``` + +By default, `source_kind` is always `RuleSourceKind::SameLogic`. + +#### `rule_category!` macro + +Declaring a rule using [`declare_lint_rule!`](#declare_lint_rule-macro) will cause a new `rule_category!` +macro to be declared in the surrounding module. This macro can be used to +refer to the corresponding diagnostic category for this lint rule, if it +has one. Using this macro instead of getting the category for a diagnostic +by dynamically parsing its string name has the advantage of statically +injecting the category at compile time and checking that it is correctly +registered to the `biome_diagnostics` library. + +```rust +declare_lint_rule! { + /// Documentation + pub(crate) ExampleRule { + version: "next", + name: "myRuleName", + language: "js", + recommended: false, + } +} + +impl Rule for ExampleRule { + fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { + Some(RuleDiagnostic::new( + rule_category!(), + ctx.query().text_trimmed_range(), + "message", + )) + } +} +``` + +#### Rule Options Some rules may allow customization using options. -We try to keep rule options to a minimum and only when needed. -Before adding an option, it's worth a discussion. -Options should follow our [technical philosophy](https://biomejs.dev/internals/philosophy/#technical). + +> [!NOTE] +> We try to keep rule options to a minimum and only provide them when needed. +> Before adding an option, it's worth a discussion. +> +> If provided, options should follow our [technical philosophy](https://biomejs.dev/internals/philosophy/#technical). + +##### Options for our example rule Let's assume that the rule we want to implement supports the following options: @@ -201,6 +305,8 @@ We would like to set the options in the `biome.json` configuration file: } ``` +##### Representing the rule options in Rust + The first step is to create the Rust data representation of the rule's options. ```rust @@ -242,12 +348,26 @@ impl Rule for MyRule { } ``` -A rule can retrieve its options with: +##### Retrieving the rule options within a Rule + +A rule can retrieve the options that apply to the location of the currently matched node with: ```rust let options = ctx.options(); ``` +Modifications of the configuration via e.g. [_`extends`_](https://biomejs.dev/reference/configuration/#extends) +and [_`overrides`_](https://biomejs.dev/reference/configuration/#overrides) (in `biome.json`) +that apply only to a subset of files are automatically taken into account, +and do not need to be handled by the rule itself. + +##### Implementing JSON deserialization/serialization support + +> [!WARNING] +> Although we use `serde`s attribute syntax, we do not actually use the `serde` crate for (de)serialization. +> +> We instead provide a ***`serde`-inspired*** implementation in `biome_deserialize` and `biome_deserialize_macros` that [differs in some aspects](../biome_deserialize/README.md), like being fault-tolerant. + The compiler should warn you that `MyRuleOptions` does not implement some required types. We currently require implementing _serde_'s traits `Deserialize`/`Serialize`. @@ -280,104 +400,6 @@ pub enum Behavior { } ``` -### Coding the rule - -Below, there are many tips and guidelines on how to create a lint rule using Biome infrastructure. - - -#### `declare_lint_rule!` macro - -This macro is used to declare an analyzer rule type, and implement the [RuleMeta] trait for it. - -The macro itself expects the following syntax: - -```rust -use biome_analyze::declare_lint_rule; - -declare_lint_rule! { - /// Documentation - pub(crate) ExampleRule { - version: "next", - name: "myRuleName", - language: "js", - recommended: false, - } -} -``` - -##### Biome lint rules inspired by other lint rules - -If a **lint** rule is inspired by an existing rule from other ecosystems (ESLint, ESLint plugins, clippy, etc.), you can add a new metadata to the macro called `source`. Its value is `&'static [RuleSource]`, which is a reference to a slice of `RuleSource` elements, each representing a different source. - -If you're implementing a lint rule that matches the behaviour of the ESLint rule `no-debugger`, you'll use the variant `::ESLint` and pass the name of the rule: - -```rust -use biome_analyze::{declare_lint_rule, RuleSource}; - -declare_lint_rule! { - /// Documentation - pub(crate) ExampleRule { - version: "next", - name: "myRuleName", - language: "js", - recommended: false, - sources: &[RuleSource::Eslint("no-debugger")], - } -} -``` - -If the rule you're implementing has a different behaviour or option, you can add the `source_kind` metadata and use the `RuleSourceKind::Inspired` type. If there are multiple sources, we assume that each source has the same `source_kind`. - -```rust -use biome_analyze::{declare_lint_rule, RuleSource, RuleSourceKind}; - -declare_lint_rule! { - /// Documentation - pub(crate) ExampleRule { - version: "next", - name: "myRuleName", - language: "js", - recommended: false, - sources: &[RuleSource::Eslint("no-debugger")], - source_kind: RuleSourceKind::Inspired, - } -} -``` - -By default, `source_kind` is always `RuleSourceKind::SameLogic`. - -#### `rule_category!` macro - -Declaring a rule using [`declare_lint_rule!`](#declare_lint_rule-macro) will cause a new `rule_category!` -macro to be declared in the surrounding module. This macro can be used to -refer to the corresponding diagnostic category for this lint rule, if it -has one. Using this macro instead of getting the category for a diagnostic -by dynamically parsing its string name has the advantage of statically -injecting the category at compile time and checking that it is correctly -registered to the `biome_diagnostics` library. - -```rust -declare_lint_rule! { - /// Documentation - pub(crate) ExampleRule { - version: "next", - name: "myRuleName", - language: "js", - recommended: false, - } -} - -impl Rule for ExampleRule { - fn diagnostic(ctx: &RuleContext, _state: &Self::State) -> Option { - Some(RuleDiagnostic::new( - rule_category!(), - ctx.query().text_trimmed_range(), - "message", - )) - } -} -``` - #### Navigating the CST When navigating the nodes and tokens of certain nodes, you will notice straight away that the majority of those methods will return a `Result` (`SyntaxResult`). diff --git a/crates/biome_deserialize/README.md b/crates/biome_deserialize/README.md index 537dc52b19ef..80d189e5c149 100644 --- a/crates/biome_deserialize/README.md +++ b/crates/biome_deserialize/README.md @@ -6,7 +6,7 @@ It provides a framework by which these two groups interact with each other, allowing any supported data structure to be deserialized using any supported data format. This crate inspired by [serde](https://serde.rs/). -0ne of the main difference is the fault-tolerant behavior of `biome_deserialize`. +One of the main difference is the fault-tolerant behavior of `biome_deserialize`. _Serde_ uses a fast-fail strategy, while `biome_deserialize` deserialize as much as possible and report several diagnostics (errors, warning, deprecation messages, ...). Also, `biome_deserialize` is intended to deserialize textual data formats. From b3bc62fb394a6d1f02c13eca4cbce5fc67f91bed Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 13:42:52 +0000 Subject: [PATCH 11/13] docs(biome_analyze): improve developer documentation about rule options --- crates/biome_analyze/CONTRIBUTING.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index af621622538c..cad7b1310f16 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -270,7 +270,7 @@ impl Rule for ExampleRule { #### Rule Options -Some rules may allow customization using options. +Some rules may allow customization [using per-rule options in `biome.json`](https://biomejs.dev/linter/#rule-options). > [!NOTE] > We try to keep rule options to a minimum and only provide them when needed. @@ -364,7 +364,7 @@ and do not need to be handled by the rule itself. ##### Implementing JSON deserialization/serialization support > [!WARNING] -> Although we use `serde`s attribute syntax, we do not actually use the `serde` crate for (de)serialization. +> Although we use `serde`s attribute syntax, we do not actually use the `serde` crate for (de)serialization of `biome.json`. > > We instead provide a ***`serde`-inspired*** implementation in `biome_deserialize` and `biome_deserialize_macros` that [differs in some aspects](../biome_deserialize/README.md), like being fault-tolerant. @@ -376,11 +376,16 @@ Also, we use other `serde` macros to adjust the JSON configuration: - `deny_unknown_fields`: it raises an error if the configuration contains extraneous fields. - `default`: it uses the `Default` value when the field is missing from `biome.json`. This macro makes the field optional. -You can simply use a derive macros: +Because we use `schemars`to generate a JSON schema for `biome.json`, our options type must support the `schemars::JsonSchema` trait as well. + +You can simply use the derive macros provided by `serde`, `biome_deserialize` and `schemars` to generate the necessary implementations automatically: ```rust -#[derive(Debug, Default, Clone, Serialize, Deserialize)] -#[cfg_attr(feature = "schemars", derive(JsonSchema))] +use biome_deserialize_macros::Deserializable; +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Default, Clone, Serialize, Deserialize, Deserializable)] +#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields, default)] pub struct MyRuleOptions { #[serde(default, skip_serializing_if = "is_default")] From 6d8b4067684b94e537c7b75249023b9cb9ed6fee Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 14:49:48 +0000 Subject: [PATCH 12/13] docs(biome_analyze): improve developer documentation for rule options --- crates/biome_analyze/CONTRIBUTING.md | 222 +++++++++++++++++++++++++-- 1 file changed, 205 insertions(+), 17 deletions(-) diff --git a/crates/biome_analyze/CONTRIBUTING.md b/crates/biome_analyze/CONTRIBUTING.md index cad7b1310f16..d1d52a71653f 100644 --- a/crates/biome_analyze/CONTRIBUTING.md +++ b/crates/biome_analyze/CONTRIBUTING.md @@ -33,6 +33,11 @@ The analyzer allows implementors to create **four different** types of rules: - [Snapshot Tests](#snapshot-tests) - [Run the snapshot tests](#run-the-snapshot-tests) - [Documenting the rule](#documenting-the-rule) + - [General Structure](#general-structure) + - [Associated Language(s)](#associated-languages) + - [Code Blocks](#code-blocks) + - [Using Rule Options](#using-rule-options) + - [Full Documentation Example](#full-documentation-example) - [Code generation](#code-generation) - [Commiting your work](#commiting-your-work) - [Sidenote: Deprecating a rule](#sidenote-deprecating-a-rule) @@ -295,9 +300,11 @@ We would like to set the options in the `biome.json` configuration file: "recommended": true, "nursery": { "my-rule": { - "behavior": "A", - "threshold": 30, - "behaviorExceptions": ["f"], + "options": { + "behavior": "A", + "threshold": 30, + "behaviorExceptions": ["f"], + } } } } @@ -405,6 +412,10 @@ pub enum Behavior { } ``` +##### Testing & Documenting Rule Options + +As with every other user-facing aspect of a rule, the effect that options have on a rule's operation should be both documented and tested, as explained in more detail in the section [Documenting the rule](#documenting-the-rule). + #### Navigating the CST When navigating the nodes and tokens of certain nodes, you will notice straight away that the majority of those methods will return a `Result` (`SyntaxResult`). @@ -774,6 +785,9 @@ Check our main [contribution document](https://github.com/biomejs/biome/blob/mai ### Documenting the rule The documentation needs to adhere to the following rules: + +#### General Structure + - The **first** paragraph of the documentation is used as a brief description of the rule, and it **must** be written in one single line. Breaking the paragraph into multiple lines will break the table contents of the rules overview page. @@ -781,19 +795,197 @@ The documentation needs to adhere to the following rules: - The documentation must have a `## Examples` header, followed by two headers: `### Invalid` and `### Valid`. `### Invalid` must go first because we need to show when the rule is triggered. - Rule options if any, must be documented in the `## Options` section. -- Each code block must have a _language_ defined. -- When adding _invalid_ snippets in the `### Invalid` section, you must use the - `expect_diagnostic` code block property. We use this property to generate a diagnostic - and attach it to the snippet. A given snippet **must emit only ONE diagnostic**. -- When adding _valid_ snippets in the `### Valid` section, you can use one single snippet for all different valid cases. -- You can use the code block property `ignore` to tell the code generation script to **not generate a diagnostic for an invalid snippet**. -- You can use the code block property `options` to tell the code generation script that this is a configuration options example. -- You can use the code block property `use_options` to tell the code generation script to use the options from the most recent `options` block while linting. + +#### Associated Language(s) + - Update the `language` field in the `declare_lint_rule!` macro to the language the rule primarily applies to. - If your rule applies to any JavaScript, you can leave it as `js`. - If your rule only makes sense in a specific JavaScript dialect, you should set it to `jsx`, `ts`, or `tsx`, whichever is most appropriate. -Here's an example of how the documentation could look like: +#### Code Blocks + +> [!TIP] +> The build process will automatically check each (properly marked) code block in a rule's documentation comment to ensure that: +> +> 1. The `### Valid` examples contain valid, parseable code, and the rule +> does not report any diagnostics for them. +> 2. Each `### Invalid` example reports _exactly one_ diagnostic. +> The output of the diagnostic will also be shown in the [generated documentation +> for that rule](https://biomejs.dev/linter/rules/no-header-scope/#invalid) at [biomejs.dev](https://biomejs.dev/). +> +> To make this work, all code blocks must adhere to a few rules, as listed below: + +- **Language** + + Each code block must have a _language_ defined (so that the correct syntax highlighting and analyzer options are applied). + +- **Valid/Invalid snippets** + + When adding _invalid_ snippets in the `### Invalid` section, you must use the + `expect_diagnostic` code block property. We use this property to generate a diagnostic + and attach it to the snippet. A given snippet **must emit only ONE diagnostic**. + + When adding _valid_ snippets in the `### Valid` section, you can use one single snippet for all different valid cases. + +- **Ignoring snippets** + + You can use the code block property `ignore` to tell the code generation script to **not generate a diagnostic for an invalid snippet** and **exclude it from the automatic validation** described above. + + Please use this sparingly and prefer automatically validated snippets, as this avoids out-of-date documentation when the implementation is changed. + +- **Hiding lines** + + Although usually not necessary, it is possible to prevent code lines from being shown in the output by prefixing them with `# `. + + You should usually prefer to show a concise but complete sample snippet instead. + +- **Ordering of code block properties** + + In addition to the language, a code block can be tagged with a few additional properties like `expect_diagnostic`, `options`, `full_options`, `use_options` and/or `ignore`. + + The parser does not care about the order, but for consistency, modifiers should always be ordered as follows: + + ````rust + /// ```[,expect_diagnostic][,(options|full_options|use_options)][,ignore] + /// ``` + ```` + + e.g. + + ````rust + /// ```tsx,expect_diagnostic,use_options,ignore + /// ``` + ```` + +#### Using Rule Options + +All code blocks are interpreted as sample code that should be analyzed using the rule's default options by default, unless the codeblock is marked with `options`, `full_options` or `use_options`. +Codeblocks can therefore be of one of three types: + +- Valid/Invalid **example snippets** using the **default settings** are marked as described above: + + ````rust + /// ### Valid + /// + /// ```js + /// var some_valid_example = true; + /// ``` + ```` + + ````rust + /// ### Invalid + /// + /// ```ts,expect_diagnostic + /// const some_invalid_example: UndeclaredType = false; + /// ``` + ```` + +- Valid **configuration option snippets** that contain only the settings for the rule itself should be written in `json` or `jsonc` together with the code block property `options`: + + ````rust + /// ### Valid + /// + /// ```json,options + /// { + /// "options": { + /// "behavior": "A", + /// "threshold": 30, + /// "behaviorExceptions": ["f"] + /// } + /// } + /// ``` + ```` + + Although usually not needed, you can show syntactically or semantically invalid configuration option snippets by adding `expect_diagnostic` in addition to `options`. As for normal snippets, a given snippet **must emit only ONE diagnostic**: + + ````rust + /// ### Invalid + /// + /// ```json,expect_diagnostic,options + /// { + /// "options": { + /// "behavior": "invalid-value" + /// } + /// } + /// ``` + ```` + +- Usually, the shown configuration option snippets only need to change rule-specific options. + + If you need to show off a **full `biome.json` configuration** instead, you can use `full_options` instead of `options` to change the parsing mode. + + ````rust + /// ```jsonc,full_options + /// { + /// "linter": { + /// "rules": { + /// "style": { + /// "useNamingConvention": "warn" + /// } + /// } + /// }, + /// // ... + /// "overrides": [ + /// { + /// // Override useNamingConvention for external module typing declarations + /// "include": ["typings/*.d.ts"], + /// "linter": { + /// "rules": { + /// "style": { + /// "useNamingConvention": "off" + /// } + /// } + /// } + /// } + /// ] + /// } + /// ``` + ```` + + Although probably never needed, it is possible to define an expected-to-be-invalid full configuration snippet as follows: + + ````rust + /// ```jsonc,expect_diagnostic,full_options + /// { + /// "linter": { + /// // ... + /// } + /// } + /// ``` + ```` + +- A **valid** configuration option example can be followed by one or more valid/invalid code snippets that use these options, possibly with interleaving text. + Those code snippets have to be marked with `use_options`: + + ````rust + /// ### Valid/Invalid + /// + /// A configuration could look like this: + /// + /// ```json,options + /// { + /// "options": { + /// "your-custom-option": "..." + /// } + /// } + /// ``` + /// + /// And a usage looks like this: + /// + /// ```js,use_options + /// var some_valid_example = true; + /// ``` + /// + /// And an "invalid" usage that triggers the rule looks like this: + /// + /// ```js,expect_diagnostic,use_options + /// var this_should_trigger_the_rule = true; + /// ``` + ```` + +#### Full Documentation Example + +Here's an example of how the final documentation could look like: ```rust use biome_analyze::declare_lint_rule; @@ -802,7 +994,7 @@ declare_lint_rule! { /// /// _ES2015_ allows to create variables with block scope instead of function scope /// using the `let` and `const` keywords. - /// Block scope is common in many other programming languages and help to avoid mistakes. + /// Block scope is common in many other programming languages and helps to avoid mistakes. /// /// Source: https://eslint.org/docs/latest/rules/no-var /// @@ -833,10 +1025,6 @@ declare_lint_rule! { } ``` -This will cause the documentation generator to ensure the rule does emit -exactly one diagnostic for this code, and to include a snapshot for the -diagnostic in the resulting documentation page. - ### Code generation For simplicity, use `just` to run all the commands with: From d5c0c1f83185e0778ad3746e4b49fd017570ef4d Mon Sep 17 00:00:00 2001 From: Lukas Waslowski Date: Fri, 15 Nov 2024 15:29:23 +0000 Subject: [PATCH 13/13] fix(useNamingConvention): fix error in doctest --- crates/biome_js_analyze/src/lint/style/use_naming_convention.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs index 34f011487154..6852b042dd42 100644 --- a/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs +++ b/crates/biome_js_analyze/src/lint/style/use_naming_convention.rs @@ -340,7 +340,7 @@ declare_lint_rule! { /// "conventions": [ /// { /// "selector": { - /// "kind": "memberLike", + /// "kind": "classMember", /// "modifiers": ["private"] /// }, /// "match": "_(.+)",