From 90111a04b4ad5007b2058ac6bd18126b35f6f143 Mon Sep 17 00:00:00 2001 From: Victorien ELVINGER Date: Wed, 28 Jun 2023 12:21:23 +0200 Subject: [PATCH] fix(rome_js_analyze): handle geenric parent in noInvalidConstructorSuper (#4625) --- CHANGELOG.md | 2 + .../no_invalid_constructor_super.rs | 40 +++++++++++++++++- .../noInvalidConstructorSuper/invalid.js.snap | 1 - .../{valid.js => valid.ts} | 38 +++++++++++++++++ .../{valid.js.snap => valid.ts.snap} | 41 ++++++++++++++++++- 5 files changed, 117 insertions(+), 5 deletions(-) rename crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/{valid.js => valid.ts} (57%) rename crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/{valid.js.snap => valid.ts.snap} (58%) diff --git a/CHANGELOG.md b/CHANGELOG.md index c48bd5d8596..9a1489a1613 100644 --- 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 index d3c97c45caf..d010aa01907 100644 --- 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 { 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 { } AnyJsExpression::JsAssignmentExpression(assignment) => { let operator = assignment.operator().ok()?; - if matches!( operator, JsAssignmentOperator::Assign @@ -195,6 +197,40 @@ fn is_valid_constructor(expression: AnyJsExpression) -> Option { 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 index e86007639c8..45ee6f3bad6 100644 --- 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/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js b/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.ts similarity index 57% rename from crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js rename to crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.ts index c2c83fccee3..699a1310f9e 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js +++ b/crates/rome_js_analyze/tests/specs/correctness/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 + 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/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.ts.snap similarity index 58% rename from crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js.snap rename to crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.ts.snap index 48d8a151509..c01c5893176 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noInvalidConstructorSuper/valid.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/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 + 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(); + } +} + ```