feat(rome_js_analyze): implement the noUnreachableSuper rule (#4017)

* feat(rome_js_analyze): implement the noUnreachableSuper rule

* address PR review and fix tests

* run lintdoc

* improve the formatting of diagnostics and add additional test cases

* improve the diagnostics

* run lintdoc

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(

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"
         }
     }
   }

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();·}
   
 
 ```

crates/rome_control_flow/src/builder.rs

@@ -1,4 +1,4 @@
-use rome_rowan::{Language, SyntaxElement};
+use rome_rowan::{Language, SyntaxElement, SyntaxNode};
 
 use crate::{
     BasicBlock, ControlFlowGraph, ExceptionHandler, ExceptionHandlerKind, Instruction,
@@ -27,19 +27,21 @@ pub struct FunctionBuilder<L: Language> {
     block_cursor: BlockId,
 }
 
-impl<L: Language> Default for FunctionBuilder<L> {
-    fn default() -> Self {
+impl<L: Language> FunctionBuilder<L> {
+    /// Create a new [FunctionBuilder] instance from a function node
+    pub fn new(node: SyntaxNode<L>) -> Self {
         Self {
-            result: ControlFlowGraph::new(),
+            result: ControlFlowGraph::new(node),
             exception_target: Vec::new(),
             block_cursor: BlockId { index: 0 },
         }
     }
-}
 
-impl<L: Language> FunctionBuilder<L> {
     /// Finishes building the function
-    pub fn finish(self) -> ControlFlowGraph<L> {
+    pub fn finish(mut self) -> ControlFlowGraph<L> {
+        // 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
     }
 

crates/rome_control_flow/src/lib.rs

@@ -5,7 +5,7 @@ use std::{
     fmt::{self, Display, Formatter},
 };
 
-use rome_rowan::{Language, SyntaxElement};
+use rome_rowan::{Language, SyntaxElement, SyntaxNode};
 
 pub mod builder;
 
@@ -18,12 +18,15 @@ use crate::builder::BlockId;
 pub struct ControlFlowGraph<L: Language> {
     /// List of blocks that make up this function
     pub blocks: Vec<BasicBlock<L>>,
+    /// The function node this CFG was built for in the syntax tree
+    pub node: SyntaxNode<L>,
 }
 
 impl<L: Language> ControlFlowGraph<L> {
-    fn new() -> Self {
+    fn new(node: SyntaxNode<L>) -> Self {
         ControlFlowGraph {
             blocks: vec![BasicBlock::new(None, None)],
+            node,
         }
     }
 }

crates/rome_diagnostics_categories/src/categories.rs

@@ -66,6 +66,7 @@ define_dategories! {
     "lint/nursery/noSelfCompare": "https://docs.rome.tools/lint/rules/noSelfCompare",
     "lint/nursery/noSetterReturn": "https://docs.rome.tools/lint/rules/noSetterReturn",
     "lint/nursery/noStringCaseMismatch": "https://docs.rome.tools/lint/rules/noStringCaseMismatch",
+    "lint/nursery/noUnreachableSuper": "https://rome.tools/docs/lint/rules/noUnreachableSuper",
     "lint/nursery/noUnsafeFinally": "https://docs.rome.tools/lint/rules/noUnsafeFinally",
     "lint/nursery/noUselessSwitchCase": "https://docs.rome.tools/lint/rules/noUselessSwitchCase",
     "lint/nursery/noVar": "https://docs.rome.tools/lint/rules/noVar",

crates/rome_js_analyze/src/analyzers/nursery/no_invalid_constructor_super.rs

@@ -15,13 +15,15 @@ declare_rule! {
     /// ### Invalid
     ///
     /// ```js,expect_diagnostic
-    /// class A extends B {
-    ///     constructor() {}
+    /// class A {
+    ///     constructor() {
+    ///         super();
+    ///     }
     /// }
     /// ```
     ///
     /// ```js,expect_diagnostic
-    /// class A {
+    /// class A extends undefined {
     ///     constructor() {
     ///         super();
     ///     }
@@ -52,7 +54,6 @@ declare_rule! {
 }
 
 pub(crate) enum NoInvalidConstructorSuperState {
-    MissingSuper(TextRange),
     UnexpectedSuper(TextRange),
     BadExtends {
         extends_range: TextRange,
@@ -63,17 +64,13 @@ pub(crate) enum NoInvalidConstructorSuperState {
 impl NoInvalidConstructorSuperState {
     fn range(&self) -> &TextRange {
         match self {
-            NoInvalidConstructorSuperState::MissingSuper(range) => range,
             NoInvalidConstructorSuperState::UnexpectedSuper(range) => range,
             NoInvalidConstructorSuperState::BadExtends { super_range, .. } => super_range,
         }
     }
 
     fn message(&self) -> MarkupBuf {
         match self {
-            NoInvalidConstructorSuperState::MissingSuper(_) => {
-                (markup! { "This class extends another class and a "<Emphasis>"super()"</Emphasis>" call is expected." }).to_owned()
-            }
             NoInvalidConstructorSuperState::UnexpectedSuper(_) => {
                 (markup! { "This class should not have a "<Emphasis>"super()"</Emphasis>" call. You should remove it." }).to_owned()
             }
@@ -136,16 +133,6 @@ impl Rule for NoInvalidConstructorSuper {
                     None
                 }
             }
-            (None, Some(extends_clause)) => {
-                let super_class = extends_clause.super_class().ok()?;
-                if !matches!(super_class, AnyJsExpression::AnyJsLiteralExpression(_,)) {
-                    Some(NoInvalidConstructorSuperState::MissingSuper(
-                        extends_clause.syntax().text_trimmed_range(),
-                    ))
-                } else {
-                    None
-                }
-            }
             (Some(super_expression), None) => Some(
                 NoInvalidConstructorSuperState::UnexpectedSuper(super_expression.range()),
             ),