address PR review and fix tests

rome · Dec 9, 2022 · e070741 · e070741
1 parent 0d53448
commit e070741
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 50 deletions.
diff --git 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
@@ -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
@@ -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
@@ -39,7 +39,8 @@ impl<L: Language> FunctionBuilder<L> {
 
     /// Finishes building the function
     pub fn finish(mut self) -> ControlFlowGraph<L> {
-        // 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
@@ -235,32 +235,34 @@ 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"),
             ),
             RuleState::DuplicateSuper { first, second } => Some(
                 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
@@ -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
@@ -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
@@ -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
@@ -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[\"<b>block_0</b><br/>Return(JS_RETURN_STATEMENT 19..26)\"]
+    block_0[\"<b>block_0</b><br/>Return(JS_RETURN_STATEMENT 19..26)<br/>Return\"]
 
 ";