From 732a32fe3691b7a53af9a0d18c9512846134a7a5 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Thu, 24 Nov 2022 07:50:13 +0100 Subject: [PATCH] feat(rome_js_analyze): useDefaultSwitchClauseLast (#3791) --- .../src/categories.rs | 1 + .../rome_js_analyze/src/analyzers/nursery.rs | 3 +- .../nursery/use_default_switch_clause_last.rs | 110 ++++++++++++ .../useDefaultSwitchClauseLast/invalid.js | 30 ++++ .../invalid.js.snap | 156 ++++++++++++++++++ .../useDefaultSwitchClauseLast/valid.js | 28 ++++ .../useDefaultSwitchClauseLast/valid.js.snap | 38 +++++ .../src/configuration/linter/rules.rs | 5 +- editors/vscode/configuration_schema.json | 11 ++ npm/backend-jsonrpc/src/workspace.ts | 5 + npm/rome/configuration_schema.json | 11 ++ website/src/pages/lint/rules/index.mdx | 6 + .../lint/rules/useDefaultSwitchClauseLast.md | 148 +++++++++++++++++ 13 files changed, 550 insertions(+), 2 deletions(-) create mode 100644 crates/rome_js_analyze/src/analyzers/nursery/use_default_switch_clause_last.rs create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js.snap create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js create mode 100644 crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js.snap create mode 100644 website/src/pages/lint/rules/useDefaultSwitchClauseLast.md diff --git a/crates/rome_diagnostics_categories/src/categories.rs b/crates/rome_diagnostics_categories/src/categories.rs index 216285fa7bc..f3d4d37cb53 100644 --- a/crates/rome_diagnostics_categories/src/categories.rs +++ b/crates/rome_diagnostics_categories/src/categories.rs @@ -95,6 +95,7 @@ define_dategories! { "lint/nursery/noVoidTypeReturn": "https://docs.rome.tools/lint/rules/noVoidTypeReturn", "lint/nursery/useCamelCase": "https://docs.rome.tools/lint/rules/useCamelCase", "lint/nursery/useConst":"https://docs.rome.tools/lint/rules/useConst", + "lint/nursery/useDefaultSwitchClauseLast":"https://docs.rome.tools/lint/rules/useDefaultSwitchClauseLast", "lint/nursery/useExhaustiveDependencies": "https://docs.rome.tools/lint/rules/useExhaustiveDependencies", "lint/nursery/useFlatMap": "https://docs.rome.tools/lint/rules/useFlatMap", "lint/nursery/useNumericLiterals": "https://docs.rome.tools/lint/rules/useNumericLiterals", diff --git a/crates/rome_js_analyze/src/analyzers/nursery.rs b/crates/rome_js_analyze/src/analyzers/nursery.rs index cb249fc31de..41c42ba8cb5 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery.rs @@ -17,7 +17,8 @@ mod no_setter_return; mod no_string_case_mismatch; mod no_unsafe_finally; mod no_void_type_return; +mod use_default_switch_clause_last; mod use_flat_map; mod use_numeric_literals; mod use_valid_for_direction; -declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_banned_types :: NoBannedTypes , self :: no_conditional_assignment :: NoConditionalAssignment , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_dupe_keys :: NoDupeKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_explicit_any :: NoExplicitAny , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_precision_loss :: NoPrecisionLoss , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } +declare_group! { pub (crate) Nursery { name : "nursery" , rules : [self :: no_access_key :: NoAccessKey , self :: no_banned_types :: NoBannedTypes , self :: no_conditional_assignment :: NoConditionalAssignment , self :: no_constructor_return :: NoConstructorReturn , self :: no_distracting_elements :: NoDistractingElements , self :: no_dupe_keys :: NoDupeKeys , self :: no_empty_interface :: NoEmptyInterface , self :: no_explicit_any :: NoExplicitAny , self :: no_extra_non_null_assertion :: NoExtraNonNullAssertion , self :: no_header_scope :: NoHeaderScope , self :: no_invalid_constructor_super :: NoInvalidConstructorSuper , self :: no_precision_loss :: NoPrecisionLoss , self :: no_setter_return :: NoSetterReturn , self :: no_string_case_mismatch :: NoStringCaseMismatch , self :: no_unsafe_finally :: NoUnsafeFinally , self :: no_void_type_return :: NoVoidTypeReturn , self :: use_default_switch_clause_last :: UseDefaultSwitchClauseLast , self :: use_flat_map :: UseFlatMap , self :: use_numeric_literals :: UseNumericLiterals , self :: use_valid_for_direction :: UseValidForDirection ,] } } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/use_default_switch_clause_last.rs b/crates/rome_js_analyze/src/analyzers/nursery/use_default_switch_clause_last.rs new file mode 100644 index 00000000000..73f7ec017b5 --- /dev/null +++ b/crates/rome_js_analyze/src/analyzers/nursery/use_default_switch_clause_last.rs @@ -0,0 +1,110 @@ +use rome_analyze::context::RuleContext; +use rome_analyze::{declare_rule, Ast, Rule, RuleDiagnostic}; +use rome_console::markup; +use rome_js_syntax::{JsCaseClause, JsDefaultClause}; +use rome_rowan::{AstNode, Direction}; + +declare_rule! { + /// Enforce default clauses in switch statements to be last + /// + /// A switch statement can optionally have a default clause. + /// + /// If present, it’s usually the last clause, but it doesn’t need to be. It is also allowed to put the default clause before all case clauses, or anywhere between. The behavior is mostly the same as if it was the last clause. The default block will be still executed only if there is no match in the case clauses (including those defined after the default), but there is also the ability to “fall through” from the default clause to the following clause in the list. However, such flow is not common and it would be confusing to the readers. + /// + /// Even if there is no "fall through" logic, it’s still unexpected to see the default clause before or between the case clauses. By convention, it is expected to be the last clause. + /// + /// Source: https://eslint.org/docs/latest/rules/default-case-last + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```js,expect_diagnostic + /// switch (foo) { + /// default: + /// break; + /// case 0: + /// break; + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// switch (foo) { + /// default: + /// f(); + /// case 0: + /// break; + /// } + /// ``` + /// + /// ```js,expect_diagnostic + /// switch (foo) { + /// case 0: + /// break; + /// default: + /// case 1: + /// break; + /// } + /// ``` + /// + /// ### Valid + /// + /// ```js + /// switch (foo) { + /// case 0: + /// break; + /// case 1: + /// default: + /// break; + /// } + /// ``` + /// + /// ```js + /// switch (foo) { + /// case 0: + /// break; + /// } + /// ``` + /// + pub(crate) UseDefaultSwitchClauseLast { + version: "11.0.0", + name: "useDefaultSwitchClauseLast", + recommended: false, + } +} + +impl Rule for UseDefaultSwitchClauseLast { + type Query = Ast; + type State = JsCaseClause; + type Signals = Option; + type Options = (); + + fn run(ctx: &RuleContext) -> Self::Signals { + let default_clause = ctx.query(); + let next_case = default_clause + .syntax() + .siblings(Direction::Next) + .find_map(JsCaseClause::cast); + next_case + } + + fn diagnostic(ctx: &RuleContext, next_case: &Self::State) -> Option { + let default_clause = ctx.query(); + Some(RuleDiagnostic::new( + rule_category!(), + default_clause.range(), + markup! { + "The ""default"" clause should be the last ""switch"" clause." + }, + ).detail( + next_case.range(), + markup! { + "The following ""case"" clause is here:" + }, + ).note( + markup! { + "Regardless its position, the ""default"" clause is always executed when there is no match. To avoid confusion, the ""default"" clause should be the last ""switch"" clause." + } + )) + } +} diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js new file mode 100644 index 00000000000..21e0a986e27 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js @@ -0,0 +1,30 @@ +switch (foo) { + default: + break; + case 0: + break; +} + +switch (foo) { + case 0: + break; + default: + break; + case 1: + break; +} + +switch (foo) { + default: + f(); + case 0: + break; +} + +switch (foo) { + case 0: + break; + default: + case 1: + break; +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js.snap new file mode 100644 index 00000000000..b4bf0a24219 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/invalid.js.snap @@ -0,0 +1,156 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 73 +expression: invalid.js +--- +# Input +```js +switch (foo) { + default: + break; + case 0: + break; +} + +switch (foo) { + case 0: + break; + default: + break; + case 1: + break; +} + +switch (foo) { + default: + f(); + case 0: + break; +} + +switch (foo) { + case 0: + break; + default: + case 1: + break; +} +``` + +# Diagnostics +``` +invalid.js:2:2 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default clause should be the last switch clause. + + 1 │ switch (foo) { + > 2 │ default: + │ ^^^^^^^^ + > 3 │ break; + │ ^^^^^^ + 4 │ case 0: + 5 │ break; + + i The following case clause is here: + + 2 │ default: + 3 │ break; + > 4 │ case 0: + │ ^^^^^^^ + > 5 │ break; + │ ^^^^^^ + 6 │ } + 7 │ + + i Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause. + + +``` + +``` +invalid.js:11:2 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default clause should be the last switch clause. + + 9 │ case 0: + 10 │ break; + > 11 │ default: + │ ^^^^^^^^ + > 12 │ break; + │ ^^^^^^ + 13 │ case 1: + 14 │ break; + + i The following case clause is here: + + 11 │ default: + 12 │ break; + > 13 │ case 1: + │ ^^^^^^^ + > 14 │ break; + │ ^^^^^^ + 15 │ } + 16 │ + + i Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause. + + +``` + +``` +invalid.js:18:2 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default clause should be the last switch clause. + + 17 │ switch (foo) { + > 18 │ default: + │ ^^^^^^^^ + > 19 │ f(); + │ ^^^^ + 20 │ case 0: + 21 │ break; + + i The following case clause is here: + + 18 │ default: + 19 │ f(); + > 20 │ case 0: + │ ^^^^^^^ + > 21 │ break; + │ ^^^^^^ + 22 │ } + 23 │ + + i Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause. + + +``` + +``` +invalid.js:27:2 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! The default clause should be the last switch clause. + + 25 │ case 0: + 26 │ break; + > 27 │ default: + │ ^^^^^^^^ + 28 │ case 1: + 29 │ break; + + i The following case clause is here: + + 26 │ break; + 27 │ default: + > 28 │ case 1: + │ ^^^^^^^ + > 29 │ break; + │ ^^^^^^ + 30 │ } + + i Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause. + + +``` + + diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js new file mode 100644 index 00000000000..1e1b41ddb20 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js @@ -0,0 +1,28 @@ +switch (foo) { + case 0: + break; + case 1: + break; + default: + break; +} + +switch (foo) { + case 0: + break; + case 1: + default: + break; +} + +switch (foo) { + default: + break; +} + +switch (foo) {} + +switch (foo) { + case 0: + break; +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js.snap b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js.snap new file mode 100644 index 00000000000..917c5214296 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/nursery/useDefaultSwitchClauseLast/valid.js.snap @@ -0,0 +1,38 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +assertion_line: 73 +expression: valid.js +--- +# Input +```js +switch (foo) { + case 0: + break; + case 1: + break; + default: + break; +} + +switch (foo) { + case 0: + break; + case 1: + default: + break; +} + +switch (foo) { + default: + break; +} + +switch (foo) {} + +switch (foo) { + case 0: + break; +} +``` + + diff --git a/crates/rome_service/src/configuration/linter/rules.rs b/crates/rome_service/src/configuration/linter/rules.rs index 87003e09573..e7972f3bfd9 100644 --- a/crates/rome_service/src/configuration/linter/rules.rs +++ b/crates/rome_service/src/configuration/linter/rules.rs @@ -769,6 +769,8 @@ struct NurserySchema { use_camel_case: Option, #[doc = "Require const declarations for variables that are never reassigned after declared."] use_const: Option, + #[doc = "Enforce default clauses in switch statements to be last"] + use_default_switch_clause_last: Option, #[doc = "Enforce all dependencies are correctly specified."] use_exhaustive_dependencies: Option, #[doc = "Promotes the use of .flatMap() when map().flat() are used together."] @@ -780,7 +782,7 @@ struct NurserySchema { } impl Nursery { const CATEGORY_NAME: &'static str = "nursery"; - pub(crate) const CATEGORY_RULES: [&'static str; 24] = [ + pub(crate) const CATEGORY_RULES: [&'static str; 25] = [ "noAccessKey", "noBannedTypes", "noConditionalAssignment", @@ -801,6 +803,7 @@ impl Nursery { "noVoidTypeReturn", "useCamelCase", "useConst", + "useDefaultSwitchClauseLast", "useExhaustiveDependencies", "useFlatMap", "useNumericLiterals", diff --git a/editors/vscode/configuration_schema.json b/editors/vscode/configuration_schema.json index 8d1dda14399..0566c1a8426 100644 --- a/editors/vscode/configuration_schema.json +++ b/editors/vscode/configuration_schema.json @@ -980,6 +980,17 @@ } ] }, + "useDefaultSwitchClauseLast": { + "description": "Enforce default clauses in switch statements to be last", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "useExhaustiveDependencies": { "description": "Enforce all dependencies are correctly specified.", "anyOf": [ diff --git a/npm/backend-jsonrpc/src/workspace.ts b/npm/backend-jsonrpc/src/workspace.ts index deba2b226bb..33e692e4756 100644 --- a/npm/backend-jsonrpc/src/workspace.ts +++ b/npm/backend-jsonrpc/src/workspace.ts @@ -431,6 +431,10 @@ export interface Nursery { * Require const declarations for variables that are never reassigned after declared. */ useConst?: RuleConfiguration; + /** + * Enforce default clauses in switch statements to be last + */ + useDefaultSwitchClauseLast?: RuleConfiguration; /** * Enforce all dependencies are correctly specified. */ @@ -671,6 +675,7 @@ export type Category = | "lint/nursery/noVoidTypeReturn" | "lint/nursery/useCamelCase" | "lint/nursery/useConst" + | "lint/nursery/useDefaultSwitchClauseLast" | "lint/nursery/useExhaustiveDependencies" | "lint/nursery/useFlatMap" | "lint/nursery/useNumericLiterals" diff --git a/npm/rome/configuration_schema.json b/npm/rome/configuration_schema.json index 8d1dda14399..0566c1a8426 100644 --- a/npm/rome/configuration_schema.json +++ b/npm/rome/configuration_schema.json @@ -980,6 +980,17 @@ } ] }, + "useDefaultSwitchClauseLast": { + "description": "Enforce default clauses in switch statements to be last", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "useExhaustiveDependencies": { "description": "Enforce all dependencies are correctly specified.", "anyOf": [ diff --git a/website/src/pages/lint/rules/index.mdx b/website/src/pages/lint/rules/index.mdx index a8a47c36dca..f7a2a9d666b 100644 --- a/website/src/pages/lint/rules/index.mdx +++ b/website/src/pages/lint/rules/index.mdx @@ -565,6 +565,12 @@ Enforce camel case naming convention. Require const declarations for variables that are never reassigned after declared.
+

+ useDefaultSwitchClauseLast +

+Enforce default clauses in switch statements to be last +
+

useExhaustiveDependencies

diff --git a/website/src/pages/lint/rules/useDefaultSwitchClauseLast.md b/website/src/pages/lint/rules/useDefaultSwitchClauseLast.md new file mode 100644 index 00000000000..b2a09515224 --- /dev/null +++ b/website/src/pages/lint/rules/useDefaultSwitchClauseLast.md @@ -0,0 +1,148 @@ +--- +title: Lint Rule useDefaultSwitchClauseLast +parent: lint/rules/index +--- + +# useDefaultSwitchClauseLast (since v11.0.0) + +Enforce default clauses in switch statements to be last + +A switch statement can optionally have a default clause. + +If present, it’s usually the last clause, but it doesn’t need to be. It is also allowed to put the default clause before all case clauses, or anywhere between. The behavior is mostly the same as if it was the last clause. The default block will be still executed only if there is no match in the case clauses (including those defined after the default), but there is also the ability to “fall through” from the default clause to the following clause in the list. However, such flow is not common and it would be confusing to the readers. + +Even if there is no "fall through" logic, it’s still unexpected to see the default clause before or between the case clauses. By convention, it is expected to be the last clause. + +Source: https://eslint.org/docs/latest/rules/default-case-last + +## Examples + +### Invalid + +```jsx +switch (foo) { + default: + break; + case 0: + break; +} +``` + +
nursery/useDefaultSwitchClauseLast.js:2:5 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━
+
+  ⚠ The default clause should be the last switch clause.
+  
+    1 │ switch (foo) {
+  > 2 │     default:
+      │     ^^^^^^^^
+  > 3 │         break;
+      │         ^^^^^^
+    4 │     case 0:
+    5 │         break;
+  
+  ℹ The following case clause is here:
+  
+    2 │     default:
+    3 │         break;
+  > 4 │     case 0:
+      │     ^^^^^^^
+  > 5 │         break;
+      │         ^^^^^^
+    6 │ }
+    7 │ 
+  
+  ℹ Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
+  
+
+ +```jsx +switch (foo) { + default: + f(); + case 0: + break; +} +``` + +
nursery/useDefaultSwitchClauseLast.js:2:5 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━
+
+  ⚠ The default clause should be the last switch clause.
+  
+    1 │ switch (foo) {
+  > 2 │     default:
+      │     ^^^^^^^^
+  > 3 │         f();
+      │         ^^^^
+    4 │     case 0:
+    5 │         break;
+  
+  ℹ The following case clause is here:
+  
+    2 │     default:
+    3 │         f();
+  > 4 │     case 0:
+      │     ^^^^^^^
+  > 5 │         break;
+      │         ^^^^^^
+    6 │ }
+    7 │ 
+  
+  ℹ Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
+  
+
+ +```jsx +switch (foo) { + case 0: + break; + default: + case 1: + break; +} +``` + +
nursery/useDefaultSwitchClauseLast.js:4:5 lint/nursery/useDefaultSwitchClauseLast ━━━━━━━━━━━━━━━━━━
+
+  ⚠ The default clause should be the last switch clause.
+  
+    2 │     case 0:
+    3 │         break;
+  > 4 │     default:
+      │     ^^^^^^^^
+    5 │     case 1:
+    6 │         break;
+  
+  ℹ The following case clause is here:
+  
+    3 │         break;
+    4 │     default:
+  > 5 │     case 1:
+      │     ^^^^^^^
+  > 6 │         break;
+      │         ^^^^^^
+    7 │ }
+    8 │ 
+  
+  ℹ Regardless its position, the default clause is always executed when there is no match. To avoid confusion, the default clause should be the last switch clause.
+  
+
+ +### Valid + +```jsx +switch (foo) { + case 0: + break; + case 1: + default: + break; +} +``` + +```jsx +switch (foo) { + case 0: + break; +} +``` +