From 704608a63d59d9c4dff4dc5a4960845a78270308 Mon Sep 17 00:00:00 2001 From: l3ops Date: Fri, 9 Dec 2022 15:26:03 +0100 Subject: [PATCH] address PR review and fix tests --- crates/rome_cli/tests/commands/check.rs | 25 ++++++++++--------- crates/rome_cli/tests/configs.rs | 4 +-- .../main_commands_check/upgrade_severity.snap | 23 +++++++++-------- crates/rome_control_flow/src/builder.rs | 3 ++- .../analyzers/nursery/no_unreachable_super.rs | 16 ++++++------ .../noUnreachableSuper/duplicateSuper.js.snap | 20 +++++++-------- .../noUnreachableSuper/missingSuper.js.snap | 6 ++--- .../thisBeforeSuper.js.snap | 6 ++--- crates/rome_service/tests/workspace.rs | 2 +- 9 files changed, 55 insertions(+), 50 deletions(-) diff --git a/crates/rome_cli/tests/commands/check.rs b/crates/rome_cli/tests/commands/check.rs index ed3b6f43a057..933aaa5508d6 100644 --- a/crates/rome_cli/tests/commands/check.rs +++ b/crates/rome_cli/tests/commands/check.rs @@ -65,9 +65,7 @@ const JS_ERRORS_AFTER: &str = "try { } "; -const UPGRADE_SEVERITY_CODE: &str = r#"class A extends B { - constructor() {} -}"#; +const UPGRADE_SEVERITY_CODE: &str = r#"if(!cond) { exprA(); } else { exprB() }"#; const NURSERY_UNSTABLE: &str = r#"if(a = b) {}"#; @@ -579,16 +577,19 @@ fn upgrade_severity() { let messages = &console.out_buffer; + let error_count = messages + .iter() + .filter(|m| m.level == LogLevel::Error) + .filter(|m| { + let content = format!("{:?}", m.content); + content.contains("style/noNegationElse") + }) + .count(); + assert_eq!( - messages - .iter() - .filter(|m| m.level == LogLevel::Error) - .filter(|m| { - let content = format!("{:?}", m.content); - content.contains("nursery/noInvalidConstructorSuper") - }) - .count(), - 1 + error_count, 1, + "expected 1 error-level message in console buffer, found {error_count:?}:\n{:?}", + console.out_buffer ); assert_cli_snapshot(SnapshotPayload::new( diff --git a/crates/rome_cli/tests/configs.rs b/crates/rome_cli/tests/configs.rs index 645af0851305..a56a96880b8e 100644 --- a/crates/rome_cli/tests/configs.rs +++ b/crates/rome_cli/tests/configs.rs @@ -145,8 +145,8 @@ pub const CONFIG_LINTER_UPGRADE_DIAGNOSTIC: &str = r#"{ "linter": { "rules": { "recommended": true, - "nursery": { - "noInvalidConstructorSuper": "error" + "style": { + "noNegationElse": "error" } } } diff --git a/crates/rome_cli/tests/snapshots/main_commands_check/upgrade_severity.snap b/crates/rome_cli/tests/snapshots/main_commands_check/upgrade_severity.snap index 92ca5fc07307..1e0b8915cc04 100644 --- a/crates/rome_cli/tests/snapshots/main_commands_check/upgrade_severity.snap +++ b/crates/rome_cli/tests/snapshots/main_commands_check/upgrade_severity.snap @@ -9,8 +9,8 @@ expression: content "linter": { "rules": { "recommended": true, - "nursery": { - "noInvalidConstructorSuper": "error" + "style": { + "noNegationElse": "error" } } } @@ -20,9 +20,7 @@ expression: content ## `file.js` ```js -class A extends B { - constructor() {} -} +if(!cond) { exprA(); } else { exprB() } ``` # Termination Message @@ -34,14 +32,17 @@ some errors were emitted while running checks # Emitted Messages ```block -file.js:1:9 lint/nursery/noInvalidConstructorSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +file.js:1:1 lint/style/noNegationElse FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - × This class extends another class and a super() call is expected. + × Invert blocks when performing a negation test. + + > 1 │ if(!cond) { exprA(); } else { exprB() } + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + + i Suggested fix: Exchange alternate and consequent of the node - > 1 │ class A extends B { - │ ^^^^^^^^^ - 2 │ constructor() {} - 3 │ } + - if(!cond)·{·exprA();·}·else·{·exprB()·} + + if(cond)·{·exprB()·}·else·{·exprA();·} ``` diff --git a/crates/rome_control_flow/src/builder.rs b/crates/rome_control_flow/src/builder.rs index 131b9795f1b4..8904908ffe43 100644 --- a/crates/rome_control_flow/src/builder.rs +++ b/crates/rome_control_flow/src/builder.rs @@ -39,7 +39,8 @@ impl FunctionBuilder { /// Finishes building the function pub fn finish(mut self) -> ControlFlowGraph { - // Append an implicit return instruction at the end of the function + // Append the implicit return instruction that resumes execution of the + // parent procedure when control flow reaches the end of a function self.append_return(); self.result } diff --git a/crates/rome_js_analyze/src/analyzers/nursery/no_unreachable_super.rs b/crates/rome_js_analyze/src/analyzers/nursery/no_unreachable_super.rs index 45732b4fdb84..1bcfa1fd0b5d 100644 --- a/crates/rome_js_analyze/src/analyzers/nursery/no_unreachable_super.rs +++ b/crates/rome_js_analyze/src/analyzers/nursery/no_unreachable_super.rs @@ -235,7 +235,7 @@ impl Rule for NoUnreachableSuper { RuleDiagnostic::new( rule_category!(), *this, - "`this` can be accessed before `super()` is called", + "`this` is accessed before `super()` is called", ) .detail(super_, "`super()` is only called here"), ), @@ -243,24 +243,26 @@ impl Rule for NoUnreachableSuper { RuleDiagnostic::new( rule_category!(), second, - "`super()` can be called more than once", + "`super()` is called more than once", ) - .detail(first, "`super()` may have already been called here"), + .detail(first, "`super()` has already been called here"), ), RuleState::ThisWithoutSuper { this } => Some(RuleDiagnostic::new( rule_category!(), *this, - "`this` can accessed without calling `super()` first", + "`this` is accessed without calling `super()` first", )), - RuleState::ReturnWithoutSuper { return_: Some(range) } => Some(RuleDiagnostic::new( + RuleState::ReturnWithoutSuper { + return_: Some(range), + } => Some(RuleDiagnostic::new( rule_category!(), *range, - "this statement can return from the constructor without having called `super()` first", + "this statement returns from the constructor without having called `super()` first", )), RuleState::ReturnWithoutSuper { return_: None } => Some(RuleDiagnostic::new( rule_category!(), ctx.query().node.text_trimmed_range(), - "this constructor can return without calling `super()`", + "this constructor returns without calling `super()`", )), } } diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/duplicateSuper.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/duplicateSuper.js.snap index 1dc47747f4be..ea6db6c1f0ca 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/duplicateSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/duplicateSuper.js.snap @@ -72,7 +72,7 @@ class G extends A { ``` duplicateSuper.js:16:9 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `super()` can be called more than once + ! `super()` is called more than once 14 │ constructor() { 15 │ super(1); @@ -81,7 +81,7 @@ duplicateSuper.js:16:9 lint/nursery/noUnreachableSuper ━━━━━━━━ 17 │ } 18 │ } - i `super()` may have already been called here + i `super()` has already been called here 13 │ class C extends A { 14 │ constructor() { @@ -96,7 +96,7 @@ duplicateSuper.js:16:9 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` duplicateSuper.js:27:9 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `super()` can be called more than once + ! `super()` is called more than once 25 │ } 26 │ @@ -105,7 +105,7 @@ duplicateSuper.js:27:9 lint/nursery/noUnreachableSuper ━━━━━━━━ 28 │ } 29 │ } - i `super()` may have already been called here + i `super()` has already been called here 22 │ constructor(cond) { 23 │ if (cond) { @@ -120,7 +120,7 @@ duplicateSuper.js:27:9 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` duplicateSuper.js:35:13 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `super()` can be called more than once + ! `super()` is called more than once 33 │ constructor(cond) { 34 │ do { @@ -129,7 +129,7 @@ duplicateSuper.js:35:13 lint/nursery/noUnreachableSuper ━━━━━━━━ 36 │ } while (cond); 37 │ } - i `super()` may have already been called here + i `super()` has already been called here 33 │ constructor(cond) { 34 │ do { @@ -144,7 +144,7 @@ duplicateSuper.js:35:13 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` duplicateSuper.js:47:13 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `super()` can be called more than once + ! `super()` is called more than once 45 │ } 46 │ if (condB) { @@ -153,7 +153,7 @@ duplicateSuper.js:47:13 lint/nursery/noUnreachableSuper ━━━━━━━━ 48 │ } 49 │ } - i `super()` may have already been called here + i `super()` has already been called here 42 │ constructor(condA, condB) { 43 │ if (condA) { @@ -168,7 +168,7 @@ duplicateSuper.js:47:13 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` duplicateSuper.js:57:17 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `super()` can be called more than once + ! `super()` is called more than once 55 │ while (condA) { 56 │ if (condB) { @@ -177,7 +177,7 @@ duplicateSuper.js:57:17 lint/nursery/noUnreachableSuper ━━━━━━━━ 58 │ } 59 │ } - i `super()` may have already been called here + i `super()` has already been called here 55 │ while (condA) { 56 │ if (condB) { diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/missingSuper.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/missingSuper.js.snap index 6e161caf88ca..9a6374328557 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/missingSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/missingSuper.js.snap @@ -59,7 +59,7 @@ class E extends A { ``` missingSuper.js:19:5 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! this constructor can return without calling `super()` + ! this constructor returns without calling `super()` 17 │ // invalid 18 │ class C extends A { @@ -79,7 +79,7 @@ missingSuper.js:19:5 lint/nursery/noUnreachableSuper ━━━━━━━━━ ``` missingSuper.js:28:5 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! this constructor can return without calling `super()` + ! this constructor returns without calling `super()` 26 │ // invalid 27 │ class D extends A { @@ -99,7 +99,7 @@ missingSuper.js:28:5 lint/nursery/noUnreachableSuper ━━━━━━━━━ ``` missingSuper.js:43:13 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! this statement can return from the constructor without having called `super()` first + ! this statement returns from the constructor without having called `super()` first 41 │ constructor(cond) { 42 │ if (cond) { diff --git a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/thisBeforeSuper.js.snap b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/thisBeforeSuper.js.snap index 55beae16e494..3b1d686f2e49 100644 --- a/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/thisBeforeSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/nursery/noUnreachableSuper/thisBeforeSuper.js.snap @@ -63,7 +63,7 @@ class F extends A { ``` thisBeforeSuper.js:25:9 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `this` can be accessed before `super()` is called + ! `this` is accessed before `super()` is called 23 │ class D extends A { 24 │ constructor() { @@ -87,7 +87,7 @@ thisBeforeSuper.js:25:9 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` thisBeforeSuper.js:33:9 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `this` can accessed without calling `super()` first + ! `this` is accessed without calling `super()` first 31 │ class E extends A { 32 │ constructor(cond) { @@ -102,7 +102,7 @@ thisBeforeSuper.js:33:9 lint/nursery/noUnreachableSuper ━━━━━━━━ ``` thisBeforeSuper.js:50:9 lint/nursery/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! `this` can accessed without calling `super()` first + ! `this` is accessed without calling `super()` first 48 │ } 49 │ diff --git a/crates/rome_service/tests/workspace.rs b/crates/rome_service/tests/workspace.rs index 7595ce4c43c6..1bf6a3ffc13d 100644 --- a/crates/rome_service/tests/workspace.rs +++ b/crates/rome_service/tests/workspace.rs @@ -7,7 +7,7 @@ use rome_service::workspace::{server, FileGuard, Language, OpenFileParams}; fn debug_control_flow() { const SOURCE: &str = "function test () { return; }"; const GRAPH: &str = "flowchart TB - block_0[\"block_0
Return(JS_RETURN_STATEMENT 19..26)\"] + block_0[\"block_0
Return(JS_RETURN_STATEMENT 19..26)
Return\"] ";