fix(rome_js_analyze): handle geenric parent in noInvalidConstructorSuper

rome · Jun 28, 2023 · 21e0dce · 21e0dce
1 parent af25635
commit 21e0dce
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -126,6 +126,8 @@ multiple files:
   - inside an export default function
   - for React.use* hooks
 
+- Fix [`noInvalidConstructorSuper`](https://docs.rome.tools/lint/rules/noinvalidconstructorsuper/) rule that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624).
+
 - Relax [`noBannedTypes`](https://docs.rome.tools/lint/rules/nobannedtypes/) and improve documentation
 
   The rule no longer reports a user type that reuses a banned type name.

diff --git a/crates/rome_js_analyze/src/analyzers/correctness/no_invalid_constructor_super.rs b/crates/rome_js_analyze/src/analyzers/correctness/no_invalid_constructor_super.rs
@@ -152,6 +152,9 @@ impl Rule for NoInvalidConstructorSuper {
 
 fn is_valid_constructor(expression: AnyJsExpression) -> Option<bool> {
     match expression.omit_parentheses() {
+        AnyJsExpression::JsAwaitExpression(await_expression) => {
+            is_valid_constructor(await_expression.argument().ok()?)
+        }
         AnyJsExpression::JsThisExpression(_)
         | AnyJsExpression::JsFunctionExpression(_)
         | AnyJsExpression::JsCallExpression(_)
@@ -168,7 +171,6 @@ fn is_valid_constructor(expression: AnyJsExpression) -> Option<bool> {
         }
         AnyJsExpression::JsAssignmentExpression(assignment) => {
             let operator = assignment.operator().ok()?;
-
             if matches!(
                 operator,
                 JsAssignmentOperator::Assign
@@ -195,6 +197,40 @@ fn is_valid_constructor(expression: AnyJsExpression) -> Option<bool> {
         AnyJsExpression::JsSequenceExpression(sequence_expression) => {
             is_valid_constructor(sequence_expression.right().ok()?)
         }
-        _ => Some(false),
+        AnyJsExpression::JsTemplateExpression(template_expression) => {
+            // Tagged templates can return anything
+            Some(template_expression.tag().is_some())
+        }
+        AnyJsExpression::TsInstantiationExpression(instantiation_expression) => {
+            is_valid_constructor(instantiation_expression.expression().ok()?)
+        }
+        AnyJsExpression::TsAsExpression(type_assertion) => {
+            is_valid_constructor(type_assertion.expression().ok()?)
+        }
+        AnyJsExpression::TsNonNullAssertionExpression(type_assertion) => {
+            is_valid_constructor(type_assertion.expression().ok()?)
+        }
+        AnyJsExpression::TsSatisfiesExpression(type_assertion) => {
+            is_valid_constructor(type_assertion.expression().ok()?)
+        }
+        AnyJsExpression::TsTypeAssertionExpression(type_assertion) => {
+            is_valid_constructor(type_assertion.expression().ok()?)
+        }
+        AnyJsExpression::JsComputedMemberExpression(_)
+        | AnyJsExpression::AnyJsLiteralExpression(_)
+        | AnyJsExpression::JsArrayExpression(_)
+        | AnyJsExpression::JsArrowFunctionExpression(_)
+        | AnyJsExpression::JsBinaryExpression(_)
+        | AnyJsExpression::JsBogusExpression(_)
+        | AnyJsExpression::JsInstanceofExpression(_)
+        | AnyJsExpression::JsObjectExpression(_)
+        | AnyJsExpression::JsPostUpdateExpression(_)
+        | AnyJsExpression::JsPreUpdateExpression(_)
+        | AnyJsExpression::JsSuperExpression(_)
+        | AnyJsExpression::JsUnaryExpression(_)
+        | AnyJsExpression::JsxTagExpression(_) => Some(false),
+        AnyJsExpression::JsInExpression(_) => None,
+        // Should not be triggered because we called `omit_parentheses`
+        AnyJsExpression::JsParenthesizedExpression(_) => None,
     }
 }
diff --git a/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/invalid.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/invalid.js.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
-assertion_line: 96
 expression: invalid.js
 ---
 # Input

diff --git a/...ctness/noInvalidConstructorSuper/valid.js → ...ctness/noInvalidConstructorSuper/valid.ts b/...ctness/noInvalidConstructorSuper/valid.js → ...ctness/noInvalidConstructorSuper/valid.ts
@@ -41,3 +41,41 @@ export class A extends mod.B {
         super();
     }
 }
+
+// Regression test for https://github.com/rome/tools/issues/4624
+class ExtendGeneric
+    extends A<number>
+    implements I {
+
+        constructor() {
+		super();
+	}
+}
+
+class ExtendTaggesTemplate extends tag`something` {
+	constructor() {
+		super();
+	}
+}
+
+class ExtendUntaggedTemplate extends `something` {
+	constructor() {}
+}
+
+class ExtendNullAssertion extends A! {
+    constructor() {
+		super();
+	}
+}
+
+class ExtendTypeAssertion extends (A as A) {
+    constructor() {
+		super();
+	}
+}
+
+class ExtendStatisfiesExpression extends (A satisfies A) {
+    constructor() {
+		super();
+	}
+}
diff --git a/...s/noInvalidConstructorSuper/valid.js.snap → ...s/noInvalidConstructorSuper/valid.ts.snap b/...s/noInvalidConstructorSuper/valid.js.snap → ...s/noInvalidConstructorSuper/valid.ts.snap
@@ -1,7 +1,6 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
-assertion_line: 96
-expression: valid.js
+expression: valid.ts
 ---
 # Input
 ```js
@@ -49,6 +48,44 @@ export class A extends mod.B {
     }
 }
 
+// Regression test for https://github.com/rome/tools/issues/4624
+class ExtendGeneric
+    extends A<number>
+    implements I {
+
+        constructor() {
+		super();
+	}
+}
+
+class ExtendTaggesTemplate extends tag`something` {
+	constructor() {
+		super();
+	}
+}
+
+class ExtendUntaggedTemplate extends `something` {
+	constructor() {}
+}
+
+class ExtendNullAssertion extends A! {
+    constructor() {
+		super();
+	}
+}
+
+class ExtendTypeAssertion extends (A as A) {
+    constructor() {
+		super();
+	}
+}
+
+class ExtendStatisfiesExpression extends (A satisfies A) {
+    constructor() {
+		super();
+	}
+}
+
 ```