diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f19aa153db7..09938378f8ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Enhancements -- [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) now reports all string concatenations. +- [useTemplate](https://biomejs.dev/linter/rules/use-template/) now reports all string concatenations. Previously, the rule ignored concatenation of a value and a newline or a backquote. For example, the following concatenation was not reported: @@ -49,7 +49,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + `\`${v}\``; ``` -- [useExponentiationOperator](https://biomejs.dev/linter/rules/use_exponentiation_operator/) suggests better code fixes. +- [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) suggests better code fixes. The rule now preserves any comment preceding the exponent, and it preserves any parenthesis around the base or the exponent. @@ -63,7 +63,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Bug fixes -- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/noDuplicateJsxProps/) case-insensitive. +- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) case-insensitive. Some frameworks, such as Material UI, rely on the case-sensitivity of JSX properties. For example, [TextField has two properties with the same name, but distinct cases](https://mui.com/material-ui/api/text-field/#TextField-prop-inputProps): @@ -74,7 +74,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#4713](https://github.com/rome/tools/issues/4713). - Previously, [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) made the following suggestion: + Previously, [useTemplate](https://biomejs.dev/linter/rules/use-template/) made the following suggestion: ```diff - a + b + "px" @@ -92,7 +92,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#4109](https://github.com/rome/tools/issues/4109) - Previously, [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) suggested an invalid code fix when a leading or trailing single-line comment was present: + Previously, [useTemplate](https://biomejs.dev/linter/rules/use-template/) suggested an invalid code fix when a leading or trailing single-line comment was present: ```diff // leading comment @@ -113,7 +113,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#3850](https://github.com/rome/tools/issues/3850) - Previously [useExponentiationOperator](https://biomejs.dev/linter/rules/use_exponentiation_operator/) suggested invalid code in a specific edge case: + Previously [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) suggested invalid code in a specific edge case: ```diff - 1 +Math.pow(++a, 2) @@ -129,7 +129,12 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [#106](https://github.com/biomejs/biome/issues/106) - [noUndeclaredVariables](https://biomejs.dev/linter/rules/noUndeclaredVariables/) now correctly recognizes some TypeScript types such as `Uppercase`. + [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/) now correctly recognizes some TypeScript types such as `Uppercase`. + +- Fix [rome#4616](https://github.com/rome/tools/issues/4616) + + Previously [noUnreachableSuper](https://biomejs.dev/linter/rules/no-unreacheable-super/) reported valid codes with complex nesting of control flow structures. + ### Parser ### VSCode @@ -324,11 +329,11 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Remove `useCamelCase` - Use [useNamingConvention](https://biomejs.dev/linter/rules/useCamelCase/) instead. + Use [useNamingConvention](https://biomejs.dev/linter/rules/use-camel-case/) instead. #### New rules -- Add [noExcessiveComplexity](https://biomejs.dev/linter/rules/noExcessiveComplexity/) +- Add [noExcessiveComplexity](https://biomejs.dev/linter/rules/no-excessive-complexity/) - Add [useImportRestrictions](https://biomejs.dev/linter/rules/use-import-restrictions/) @@ -359,23 +364,23 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom This rule disallow duplicate keys in a JSON object. -- Add [noVoid](https://biomejs.dev/linter/rules/novoid/) +- Add [noVoid](https://biomejs.dev/linter/rules/no-void/) This rules disallow the use of `void`. -- Add [noNonoctalDecimalEscape](https://biomejs.dev/linter/rules/nononoctaldecimalescape/) +- Add [noNonoctalDecimalEscape](https://biomejs.dev/linter/rules/no-nonoctal-decimal-escape/) This rule disallows `\8` and `\9` escape sequences in string literals. -- Add [noUselessEmptyExport](https://biomejs.dev/linter/rules/noUselessEmptyExport/) +- Add [noUselessEmptyExport](https://biomejs.dev/linter/rules/no-useless-empty-export/) This rule disallows useless `export {}`. -- Add [useIsArray](https://biomejs.dev/linter/rules/useIsArray/) +- Add [useIsArray](https://biomejs.dev/linter/rules/use-is-array/) This rule proposes using `Array.isArray()` instead of `instanceof Array`. -- Add [useGetterReturn](https://biomejs.dev/linter/rules/useGetterReturn/) +- Add [useGetterReturn](https://biomejs.dev/linter/rules/use-getter-return/) This rule enforces the presence of non-empty return statements in getters. This makes the following code incorrect: @@ -390,27 +395,27 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom New rules are promoted, please check [#4750](https://github.com/rome/tools/discussions/4750) for more details: -- [a11y/useHeadingContent](https://biomejs.dev/linter/rules/useHeadingContent/) -- [complexity/noForEach](https://biomejs.dev/linter/rules/noForEach/) -- [complexity/useLiteralKeys](https://biomejs.dev/linter/rules/useLiteralKeys/) -- [complexity/useSimpleNumberKeys](https://biomejs.dev/linter/rules/useSimpleNumberKeys/) -- [correctness/useIsNan](https://biomejs.dev/linter/rules/useIsNan/) -- [suspicious/noConsoleLog](https://biomejs.dev/linter/rules/noConsoleLog/) -- [suspicious/noDuplicateJsxProps](https://biomejs.dev/linter/rules/noDuplicateJsxProps/) +- [a11y/useHeadingContent](https://biomejs.dev/linter/rules/use-heading-content/) +- [complexity/noForEach](https://biomejs.dev/linter/rules/no-for-each/) +- [complexity/useLiteralKeys](https://biomejs.dev/linter/rules/use-literal-keys/) +- [complexity/useSimpleNumberKeys](https://biomejs.dev/linter/rules/use-simple-number-keys/) +- [correctness/useIsNan](https://biomejs.dev/linter/rules/use-is-nan/) +- [suspicious/noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/) +- [suspicious/noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) The following rules are now recommended: -- [noUselessFragments](https://biomejs.dev/linter/rules/noUselessFragments/) -- [noRedundantUseStrict](https://biomejs.dev/linter/rules/noRedundantUseStrict/) -- [useExponentiationOperator](https://biomejs.dev/linter/rules/useExponentiationOperator/) +- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) +- [noRedundantUseStrict](https://biomejs.dev/linter/rules/no-redundant-use-strict/) +- [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) #### Other changes - Add new TypeScript globals (`AsyncDisposable`, `Awaited`, `DecoratorContext`, and others) [4643](https://github.com/rome/tools/issues/4643). -- [noRedeclare](https://biomejs.dev/linter/rules/noredeclare/): allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) +- [noRedeclare](https://biomejs.dev/linter/rules/no-redeclare/): allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) -- Improve [noConsoleLog](https://biomejs.dev/linter/rules/noconsolelog/), [noGlobalObjectCalls](https://biomejs.dev/linter/rules/noglobalobjectcalls/), [useIsNan](https://biomejs.dev/linter/rules/useisnan/), and [useNumericLiterals](https://biomejs.dev/linter/rules/usenumericliterals/) by handling `globalThis` and `window` namespaces. +- Improve [noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/), [noGlobalObjectCalls](https://biomejs.dev/linter/rules/no-global-object-calls/), [useIsNan](https://biomejs.dev/linter/rules/useisnan/), and [useNumericLiterals](https://biomejs.dev/linter/rules/use-numeric-literals/) by handling `globalThis` and `window` namespaces. For instance, the following code is now reported by `noConsoleLog`: @@ -418,9 +423,9 @@ The following rules are now recommended: globalThis.console.log("log") ``` -- Improve [noDuplicateParameters](https://biomejs.dev/linter/rules/noduplicateparameters/) to manage constructor parameters. +- Improve [noDuplicateParameters](https://biomejs.dev/linter/rules/no-duplicate-parameters/) to manage constructor parameters. -- Improve [noInnerDeclarations](https://biomejs.dev/linter/rules/noInnerDeclarations/) +- Improve [noInnerDeclarations](https://biomejs.dev/linter/rules/no-inner-declarations/) Now, the rule doesn't report false-positives about ambient _TypeScript_ declarations. For example, the following code is no longer reported by the rule: @@ -429,7 +434,7 @@ The following rules are now recommended: declare var foo; ``` -- Improve [useEnumInitializers](https://biomejs.dev/linter/rules/useEnumInitializers/) +- Improve [useEnumInitializers](https://biomejs.dev/linter/rules/use-enum-initializers/) The rule now reports all uninitialized members of an enum in a single diagnostic. @@ -443,7 +448,7 @@ The following rules are now recommended: } ``` -- Relax [noBannedTypes](https://biomejs.dev/linter/rules/nobannedtypes/) and improve documentation +- Relax [noBannedTypes](https://biomejs.dev/linter/rules/no-banned-types/) and improve documentation The rule no longer reports a user type that reuses a banned type name. The following code is now allowed: @@ -467,7 +472,7 @@ The following rules are now recommended: type NonNullableMyType = MyType & {}; ``` -- Improve [noConstantCondition](https://biomejs.dev/linter/rules/noConstantCondition/) +- Improve [noConstantCondition](https://biomejs.dev/linter/rules/no-constant-condition/) The rule now allows `while(true)`. This recognizes a common pattern in the web community: @@ -480,13 +485,13 @@ The following rules are now recommended: } ``` -- Improve the diagnostic and the code action of [useDefaultParameterLast](https://biomejs.dev/linter/rules/usedefaultparameterlast/). +- Improve the diagnostic and the code action of [useDefaultParameterLast](https://biomejs.dev/linter/rules/use-default-parameter-last/). The diagnostic now reports the last required parameter which should precede optional and default parameters. The code action now removes any whitespace between the parameter name and its initialization. -- Relax [noConfusingArrow](https://biomejs.dev/linter/rules/noconfusingarrow/) +- Relax [noConfusingArrow](https://biomejs.dev/linter/rules/no-confusing-arrow/) All arrow functions that enclose its parameter with parenthesis are allowed. Thus, the following snippet no longer trigger the rule: @@ -501,7 +506,7 @@ The following rules are now recommended: var x = a => 1 ? 2 : 3; ``` -- Relax [useLiteralEnumMembers](https://biomejs.dev/linter/rules/useLiteralEnumMembers/) +- Relax [useLiteralEnumMembers](https://biomejs.dev/linter/rules/use-literal-enum-members/) Enum members that refer to previous enum members are now allowed. This allows a common pattern in enum flags like in the following example: @@ -526,7 +531,7 @@ The following rules are now recommended: } ``` -- Improve [useLiteralKeys](https://biomejs.dev/linter/rules/useLiteralKeys/). +- Improve [useLiteralKeys](https://biomejs.dev/linter/rules/use-literal-keys/). Now, the rule suggests simplifying computed properties to string literal properties: @@ -548,11 +553,11 @@ The following rules are now recommended: These suggestions are made in object literals, classes, interfaces, and object types. -- Improve [noNewSymbol](https://biomejs.dev/linter/rules/noNewSymbol/). +- Improve [noNewSymbol](https://biomejs.dev/linter/rules/no-new-symbol/). The rule now handles cases where `Symbol` is namespaced with the global `globalThis` or `window`. -- The rules [useExhaustiveDependencies](https://biomejs.dev/linter/rules/useexhaustivedependencies/) and [useHookAtTopLevel](https://biomejs.dev/linter/rules/usehookattoplevel/) accept a different shape of options +- The rules [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies/) and [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) accept a different shape of options Old configuration: @@ -600,43 +605,43 @@ The following rules are now recommended: } ``` -- [noRedundantUseStrict](https://biomejs.dev/linter/rules/noredundantusestrict/) check only `'use strict'` directive to resolve false positive diagnostics. +- [noRedundantUseStrict](https://biomejs.dev/linter/rules/no-redundant-use-strict/) check only `'use strict'` directive to resolve false positive diagnostics. React introduced new directives, "use client" and "use server". The rule raises false positive errors about these directives. -- Fix a crash in the [NoParameterAssign](https://biomejs.dev/linter/rules/noparameterassign/) rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323) +- Fix a crash in the [NoParameterAssign](https://biomejs.dev/linter/rules/no-parameter-assign/) rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323) -- Fix [useExhaustiveDependencies](https://biomejs.dev/linter/rules/useexhaustivedependencies/) in the following cases [#4330](https://github.com/rome/tools/issues/4330): +- Fix [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies/) in the following cases [#4330](https://github.com/rome/tools/issues/4330): - when the first argument of hooks is a named function - inside an export default function - for React.use* hooks -- Fix [noInvalidConstructorSuper](https://biomejs.dev/linter/rules/noinvalidconstructorsuper/) that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624). +- Fix [noInvalidConstructorSuper](https://biomejs.dev/linter/rules/no-invalid-constructor-super/) that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624). - Fix [noDuplicateCase](https://biomejs.dev/linter/rules/noDuplicateCase/) that erroneously reported as equals the strings literals `"'"` and `'"'` [#4706](https://github.com/rome/tools/issues/4706). -- Fix [NoUnreachableSuper](https://biomejs.dev/linter/rules/nounreachablesuper/)'s false positive diagnostics ([#4483](https://github.com/rome/tools/issues/4483)) caused to nested if statement. +- Fix [NoUnreachableSuper](https://biomejs.dev/linter/rules/no-unreachable-super/)'s false positive diagnostics ([#4483](https://github.com/rome/tools/issues/4483)) caused to nested if statement. The rule no longer reports `This constructor calls super() in a loop` when using nested if statements in a constructor. -- Fix [useHookAtTopLevel](https://biomejs.dev/linter/rules/usehookattoplevel/)'s false positive diagnostics ([#4637](https://github.com/rome/tools/issues/4637)) +- Fix [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/)'s false positive diagnostics ([#4637](https://github.com/rome/tools/issues/4637)) The rule no longer reports false positive diagnostics when accessing properties directly from a hook and calling a hook inside function arguments. -- Fix [noUselessConstructor](https://biomejs.dev/linter/rules/noUselessConstructor/) which erroneously reported constructors with default parameters [rome#4781](https://github.com/rome/tools/issues/4781) +- Fix [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) which erroneously reported constructors with default parameters [rome#4781](https://github.com/rome/tools/issues/4781) - Fix [noUselessFragments](https://biomejs.dev/linter/rules/nouselessfragments/)'s panics when running `biome check --apply-unsafe` ([#4637](https://github.com/rome/tools/issues/4639)) This rule's code action emits an invalid AST, so I fixed using JsxString instead of JsStringLiteral -- Fix [noUndeclaredVariables](https://biomejs.dev/linter/rules/noundeclaredvariables/)'s false positive diagnostics ([#4675](https://github.com/rome/tools/issues/4675)) +- Fix [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/)'s false positive diagnostics ([#4675](https://github.com/rome/tools/issues/4675)) The semantic analyzer no longer handles `this` reference identifier. -- Fix [noUnusedVariables](https://biomejs.dev/linter/rules/nounusedvariables/)'s false positive diagnostics ([#4688](https://github.com/rome/tools/issues/4688)) +- Fix [noUnusedVariables](https://biomejs.dev/linter/rules/no-unused-variables/)'s false positive diagnostics ([#4688](https://github.com/rome/tools/issues/4688)) The semantic analyzer handles ts export declaration clause correctly. diff --git a/crates/rome_js_analyze/src/analyzers/correctness/no_unreachable_super.rs b/crates/rome_js_analyze/src/analyzers/correctness/no_unreachable_super.rs index fb0cac9d6621..23b565979d59 100644 --- a/crates/rome_js_analyze/src/analyzers/correctness/no_unreachable_super.rs +++ b/crates/rome_js_analyze/src/analyzers/correctness/no_unreachable_super.rs @@ -1,16 +1,14 @@ -use std::{collections::VecDeque, iter, slice}; - -use roaring::bitmap::RoaringBitmap; use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic}; use rome_console::markup; -use rome_control_flow::InstructionKind; +use rome_control_flow::{ExceptionHandlerKind, InstructionKind}; use rome_js_syntax::{ - AnyJsClass, AnyJsExpression, JsConstructorClassMember, JsSuperExpression, JsSyntaxElement, - JsThisExpression, JsThrowStatement, TextRange, WalkEvent, + AnyJsClass, AnyJsExpression, JsCallExpression, JsConstructorClassMember, JsSyntaxKind, + JsThrowStatement, TextRange, WalkEvent, }; -use rome_rowan::AstNode; +use rome_rowan::{AstNode, NodeOrToken}; +use rustc_hash::FxHashSet; -use crate::control_flow::{AnyJsControlFlowRoot, BasicBlock, ControlFlowGraph}; +use crate::control_flow::{AnyJsControlFlowRoot, ControlFlowGraph}; declare_rule! { /// Ensures the `super()` constructor is called exactly once on every code @@ -71,14 +69,21 @@ declare_rule! { #[allow(clippy::enum_variant_names)] pub(crate) enum RuleState { - /// The constructor reads or write from `this` before calling `super` - ThisBeforeSuper { this: TextRange, super_: TextRange }, /// The constructor may call `super` multiple times DuplicateSuper { first: TextRange, second: TextRange }, /// The constructor may read or write from `this` without calling `super` ThisWithoutSuper { this: TextRange }, /// The constructor may return without calling `super` - ReturnWithoutSuper { return_: Option }, + ReturnWithoutSuper { return_statement: Option }, +} + +/// A [BlockContext] consists of a block id and a context. +/// The context registers if a super() call was seen. +/// This allows traversing a control flow graph and retaining contextual information. +#[derive(Debug, Copy, Clone)] +struct BlockContext { + block_index: u32, + super_call: Option, } impl Rule for NoUnreachableSuper { @@ -107,140 +112,137 @@ impl Rule for NoUnreachableSuper { return None; } - // Iterate over all the blocks, performing block-local checks and - // collecting metadata on the control flow graph in various - // acceleration structures - let mut outgoing_edges = BlockMap::default(); - let mut incoming_edges = BlockMap::default(); - - let mut this_blocks = BlockMap::default(); - let mut super_blocks = BlockMap::default(); - let mut return_blocks = BlockMap::default(); - - for (block_index, block) in cfg.blocks.iter().enumerate() { - let signal = inspect_block( - &mut outgoing_edges, - &mut incoming_edges, - &mut this_blocks, - &mut super_blocks, - &mut return_blocks, - block, - block_index.try_into().expect("integer overflow"), - ); - - if let Some(signal) = signal { - return Some(signal); - } - } - - // Traverse the control flow graph "downwards" starting from blocks - // containing a `super()` call towards the return points of the constructor - let mut queue = VecDeque::new(); - - for (block_id, super_expression) in &super_blocks { - if let Some(outgoing_edges) = outgoing_edges.get(block_id) { - for next_block in outgoing_edges { - queue.push_back((next_block, block_id, super_expression)); - } - } - } - - // During the traversal, all the `super()` expressions that precede a - // given block are collected into the `predecessors` - let mut predecessors = BlockMap::default(); - - while let Some((block_id, prev_block, super_expression)) = queue.pop_front() { - let visited = predecessors - .entry(block_id) - .get_or_insert_with(BlockMap::<&JsSuperExpression>::default); - - let previous_node = visited - .insert(prev_block, super_expression) - .filter(|previous_node| *previous_node == super_expression); - - if previous_node.is_some() { + // We traverse the control flow graph and register in a context if we saw `super`. + // The context follows the traversal of every path (we process [BlockContext]). + // + // A block may be visiter twice: one time with an empty context, + // and another time with a `super` in its context. + // + // As soon as we detect the use of this without calling first `super` + + // stack of block contexts to process + let mut block_context_stack = Vec::new(); + let mut visited_block_contexts = FxHashSet::default(); + block_context_stack.push(BlockContext { + block_index: 0, + super_call: None, + }); + // Set of couples (block_index, was_visited_with_super_call) + visited_block_contexts.insert((0u32, false)); + while let Some(BlockContext { + block_index, + super_call: mut super_, + }) = block_context_stack.pop() + { + let had_super = super_.is_some(); + // SAFETY: this is a safe conversion because it is already an index for `cfg.blocks`. + let block_index = block_index as usize; + let Some(block) = cfg.blocks.get(block_index) else { continue; - } - - if let Some(outgoing_edges) = outgoing_edges.get(block_id) { - for next_block in outgoing_edges { - queue.push_back((next_block, block_id, super_expression)); - } - } - } - - // Check all the blocks containing a `super()` expression and emit an - // error if they have a predecessor (as it means `super()` may have - // already been called) - for (block_id, second) in &super_blocks { - if let Some(predecessors) = predecessors.get(block_id) { - if let Some(first) = predecessors.values().next() { - return Some(RuleState::DuplicateSuper { - first: first.syntax().text_trimmed_range(), - second: second.syntax().text_trimmed_range(), - }); + }; + for instruction in block.instructions.iter() { + if let Some(NodeOrToken::Node(ref block_node)) = instruction.node { + let mut iter = block_node.preorder(); + while let Some(event) = iter.next() { + let WalkEvent::Enter(node) = event else { + continue; + }; + match node.kind() { + JsSyntaxKind::JS_SUPER_EXPRESSION => { + let Some(parent) = node.parent() else { + continue; + }; + if !JsCallExpression::can_cast(parent.kind()) { + // Ignore `super.method()` calls + continue; + } + let range = node.text_trimmed_range(); + if let Some(first) = super_ { + return Some(RuleState::DuplicateSuper { + first, + second: range, + }); + } + super_ = Some(range); + } + JsSyntaxKind::JS_THIS_EXPRESSION => { + if super_.is_none() { + return Some(RuleState::ThisWithoutSuper { + this: node.text_trimmed_range(), + }); + } + } + _ if AnyJsControlFlowRoot::can_cast(node.kind()) => { + iter.skip_subtree(); + } + _ => {} + } + } } - } - } - - // For each block containing a `this`, check that it has a predecessor for each of its incoming edges - for (block_id, this_expression) in &this_blocks { - if super_blocks.contains_key(block_id) { - continue; - } - - if let Some(predecessors) = predecessors.get(block_id) { - if let Some(incoming_edges) = incoming_edges.get(block_id) { - if predecessors.len() != incoming_edges.len() { - return Some(RuleState::ThisWithoutSuper { - this: this_expression.syntax().text_trimmed_range(), - }); + match instruction.kind { + InstructionKind::Statement => {} + InstructionKind::Jump { + block, conditional, .. + } => { + let jump_block_index = block.index(); + // Avoid cycles and redundant checks. + if visited_block_contexts.insert((jump_block_index, super_.is_some())) { + block_context_stack.push(BlockContext { + block_index: jump_block_index, + super_call: super_, + }); + } + if !conditional { + // The next instructions are unreachable. + break; + } + } + InstructionKind::Return => { + if super_.is_none() { + let return_ = instruction.node.as_ref(); + // ignore Throws + if return_.is_some_and(|node| JsThrowStatement::can_cast(node.kind())) { + break; + } + let return_ = return_.map(|node| node.text_trimmed_range()); + return Some(RuleState::ReturnWithoutSuper { + return_statement: return_, + }); + } + // The next instructions are unreachable. + break; } } - } else { - return Some(RuleState::ThisWithoutSuper { - this: this_expression.syntax().text_trimmed_range(), - }); } - } - - // For each block containing a return instruction, check that it has a predecessor for each of its incoming edges - for (block_id, return_range) in &return_blocks { - if super_blocks.contains_key(block_id) { - continue; - } - - if let Some(predecessors) = predecessors.get(block_id) { - if let Some(incoming_edges) = incoming_edges.get(block_id) { - if predecessors.len() != incoming_edges.len() { - return Some(RuleState::ReturnWithoutSuper { - return_: *return_range, + for exception_handler in block.exception_handlers.iter() { + // Ignore finally handler: they are already in the Control Flow Graph. + if matches!(exception_handler.kind, ExceptionHandlerKind::Catch) { + // First, assume that an exception was raised at the start of the block + if !had_super && super_.is_some() { + // Avoid cycles and redundant checks. + if visited_block_contexts.insert((exception_handler.target, had_super)) { + block_context_stack.push(BlockContext { + block_index: exception_handler.target, + super_call: None, + }); + } + } + // Now, assume that an exception was raised at the end of the block + // Avoid cycles and redundant checks. + if visited_block_contexts.insert((exception_handler.target, super_.is_some())) { + block_context_stack.push(BlockContext { + block_index: exception_handler.target, + super_call: super_, }); } } - } else { - return Some(RuleState::ReturnWithoutSuper { - return_: *return_range, - }); } } - None } fn diagnostic(ctx: &RuleContext, state: &Self::State) -> Option { match state { - RuleState::ThisBeforeSuper { this, super_ } => Some( - RuleDiagnostic::new( - rule_category!(), - ctx.query().node.text_trimmed_range(), - markup! { "This constructor has code paths accessing `""this""` before `""super()""` is called." }, - ) - .detail(this, markup! { "`""this""` is accessed here:" }) - .detail(super_, markup! { "`""super()""` is only called here:" }) - .note("If this is intentional, add an explicit throw statement in unsupported paths."), - ), - RuleState::ThisWithoutSuper { this } => Some( RuleDiagnostic::new( rule_category!(), @@ -250,7 +252,6 @@ impl Rule for NoUnreachableSuper { .detail(this, markup! { "`""this""` is accessed here:" }) .note("If this is intentional, add an explicit throw statement in unsupported paths."), ), - RuleState::DuplicateSuper { first, second } if *first == *second => Some( RuleDiagnostic::new( rule_category!(), @@ -268,8 +269,7 @@ impl Rule for NoUnreachableSuper { .detail(first, markup! { "`""super()""` is first called here:" }) .detail(second, markup! { "`""super()""` is then called again here:" }), ), - - RuleState::ReturnWithoutSuper { return_: Some(range) } => Some( + RuleState::ReturnWithoutSuper { return_statement: Some(range) } => Some( RuleDiagnostic::new( rule_category!(), ctx.query().node.text_trimmed_range(), @@ -278,7 +278,7 @@ impl Rule for NoUnreachableSuper { .detail(range, markup! { "This statement returns from the constructor before `""super()""` has been called:" }) .note("If this is intentional, add an explicit throw statement in unsupported paths."), ), - RuleState::ReturnWithoutSuper { return_: None } => Some( + RuleState::ReturnWithoutSuper { return_statement: None } => Some( RuleDiagnostic::new( rule_category!(), ctx.query().node.text_trimmed_range(), @@ -289,210 +289,3 @@ impl Rule for NoUnreachableSuper { } } } - -/// Performs block-local control flow checks to ensure `super()` is only called once, and -/// always before all `this` expressions or return instructions within a single block -/// -/// This function also collects various acceleration structures for the graph-wide analysis step: -/// - `outgoing_edges` and `incoming_edges` map the index of a block to the set of the indices of -/// all the blocks that have a jump coming from- or pointing to this block respectively -/// - `this_blocks` maps the index of a block to the first `this` expression it contains if it has any -/// - `super_blocks` maps the index of a block to the first `super()` expression it contains if it has any -/// - `return_blocks` maps the index of a block to the first return instruction it contains if it has any, -/// with an optional text range if the instruction was created from an explicit `return` -fn inspect_block( - outgoing_edges: &mut BlockMap, - incoming_edges: &mut BlockMap, - this_blocks: &mut BlockMap, - super_blocks: &mut BlockMap, - return_blocks: &mut BlockMap>, - block: &BasicBlock, - block_index: u32, -) -> Option { - let mut has_this = None; - let mut has_super: Option<(usize, JsSuperExpression)> = None; - let mut has_return = None; - - // Iterator over all the instructions in the block - for (inst_index, inst) in block.instructions.iter().enumerate() { - // If the instruction has a corresponding node, visit its descendants - // to detect any `super()` or `this` expression - if let Some(node) = inst.node.as_ref().and_then(JsSyntaxElement::as_node) { - let mut iter = node.preorder(); - while let Some(event) = iter.next() { - let node = match event { - WalkEvent::Enter(node) => { - if AnyJsControlFlowRoot::can_cast(node.kind()) { - iter.skip_subtree(); - continue; - } - - node - } - WalkEvent::Leave(_) => continue, - }; - - // If we find a `super()` node but the block already has one, exit with an error immediately - if let Some(super_node) = JsSuperExpression::cast_ref(&node) { - if let Some((prev_index, prev_super)) = &has_super { - if *prev_index < inst_index { - return Some(RuleState::DuplicateSuper { - first: prev_super.syntax().text_trimmed_range(), - second: super_node.syntax().text_trimmed_range(), - }); - } - } else { - has_super = Some((inst_index, super_node)); - } - } - - has_this = has_this.or_else(|| { - let node = JsThisExpression::cast_ref(&node)?; - Some((inst_index, node)) - }); - } - } - - match inst.kind { - InstructionKind::Statement => {} - - // If the instruction is a jump, stores the metadata about this edge - // and stop analyzing the block if its unconditional - InstructionKind::Jump { - block, conditional, .. - } => { - outgoing_edges - .entry(block_index) - .get_or_insert_with(RoaringBitmap::default) - .insert(block.index()); - - incoming_edges - .entry(block.index()) - .get_or_insert_with(RoaringBitmap::default) - .insert(block_index); - - if !conditional { - break; - } - } - - // If the instruction is a return, store its optional text range and stop analyzing the block - InstructionKind::Return => { - if let Some(node) = &inst.node { - if !JsThrowStatement::can_cast(node.kind()) { - has_return = Some(Some(node.text_trimmed_range())); - } - } else { - has_return = Some(None); - } - break; - } - } - } - - // If the block has a `super()` node and at least one `this` expression, - // check that the first `this` node comes after the call to `super()` - // - // NOTE: The CFG has no representation of control flow within expressions - // at the moment, meaning the ordering of `super()` and `this` within the - // same expression statement is *NOT* checked (for instance the statement - // `this.value && super();` is allowed) - if let (Some((this_index, this_node)), Some((super_index, super_node))) = - (&has_this, &has_super) - { - if this_index < super_index { - return Some(RuleState::ThisBeforeSuper { - this: this_node.syntax().text_trimmed_range(), - super_: super_node.syntax().text_trimmed_range(), - }); - } - } - - if let Some((_, node)) = has_this { - this_blocks.insert(block_index, node); - } - if let Some((_, node)) = has_super { - super_blocks.insert(block_index, node); - } - if let Some(return_range) = has_return { - return_blocks.insert(block_index, return_range); - } - - None -} - -/// Fast implementation of `Map` backed by a vector -struct BlockMap { - storage: Vec>, -} - -impl Default for BlockMap { - fn default() -> Self { - Self { - storage: Vec::new(), - } - } -} - -impl BlockMap { - /// Insert `value` into the map at the position `key` - /// - /// If the map did not have this key present, None is returned. - /// - /// If the map did have this key present, the value is updated, and the old value is returned. - fn insert(&mut self, key: u32, value: T) -> Option { - let index = usize::try_from(key).expect("integer overflow"); - - if self.storage.len() <= index { - self.storage.resize_with(index + 1, || None); - } - - self.storage[index].replace(value) - } - - /// Gets the given key’s corresponding entry in the map for in-place manipulation. - fn entry(&mut self, key: u32) -> &mut Option { - let index = usize::try_from(key).expect("integer overflow"); - - if self.storage.len() <= index { - self.storage.resize_with(index + 1, || None); - } - - &mut self.storage[index] - } - - /// Returns a reference to the value corresponding to the key. - fn get(&self, key: u32) -> Option<&T> { - let index = usize::try_from(key).expect("integer overflow"); - self.storage.get(index)?.as_ref() - } - - /// Returns true if the map contains a value for the specified key. - fn contains_key(&self, key: u32) -> bool { - self.get(key).is_some() - } - - /// Returns the number of elements in the map. - fn len(&self) -> u64 { - self.values().count().try_into().expect("integer overflow") - } - - /// An iterator visiting all values in the map, sorted by their key in ascending order - fn values(&self) -> impl Iterator { - self.storage.iter().filter_map(Option::as_ref) - } -} - -impl<'a, T> IntoIterator for &'a BlockMap { - type Item = (u32, &'a T); - type IntoIter = iter::FilterMap< - iter::Enumerate>>, - fn((usize, &Option)) -> Option<(u32, &T)>, - >; - - fn into_iter(self) -> Self::IntoIter { - self.storage.iter().enumerate().filter_map(|(index, slot)| { - Some((index.try_into().expect("integer overflow"), slot.as_ref()?)) - }) - } -} diff --git a/crates/rome_js_analyze/src/control_flow.rs b/crates/rome_js_analyze/src/control_flow.rs index 654711e47c4a..80b06225f8fc 100644 --- a/crates/rome_js_analyze/src/control_flow.rs +++ b/crates/rome_js_analyze/src/control_flow.rs @@ -5,7 +5,6 @@ use rome_js_syntax::JsLanguage; use rome_js_syntax::TextRange; pub type JsControlFlowGraph = rome_control_flow::ControlFlowGraph; -pub(crate) type BasicBlock = rome_control_flow::BasicBlock; pub(crate) type FunctionBuilder = rome_control_flow::builder::FunctionBuilder; mod nodes; diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js index 4c1745399e8c..25a28708caf3 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js @@ -23,7 +23,6 @@ class D extends A { if (cond) { super(true); } - super(); } } @@ -59,3 +58,25 @@ class G extends A { } } } + +// invalid +class G extends A { + constructor(condA, condB) { + try { + super() + } catch { + super() + } + } +} + +// invalid +class G extends A { + constructor(condA, condB) { + try { + super() + } finally { + super() + } + } +} diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js.snap index 5b9d91b2c090..76c97dc93842 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/duplicateSuper.js.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 96 expression: duplicateSuper.js --- # Input @@ -30,7 +29,6 @@ class D extends A { if (cond) { super(true); } - super(); } } @@ -67,6 +65,28 @@ class G extends A { } } +// invalid +class G extends A { + constructor(condA, condB) { + try { + super() + } catch { + super() + } + } +} + +// invalid +class G extends A { + constructor(condA, condB) { + try { + super() + } finally { + super() + } + } +} + ``` # Diagnostics @@ -117,12 +137,13 @@ duplicateSuper.js:22:5 lint/correctness/noUnreachableSuper ━━━━━━━ > 22 │ constructor(cond) { │ ^^^^^^^^^^^^^^^^^^^ > 23 │ if (cond) { - ... - > 27 │ super(); - > 28 │ } + > 24 │ super(true); + > 25 │ } + > 26 │ super(); + > 27 │ } │ ^ - 29 │ } - 30 │ + 28 │ } + 29 │ i `super()` is first called here: @@ -131,112 +152,165 @@ duplicateSuper.js:22:5 lint/correctness/noUnreachableSuper ━━━━━━━ > 24 │ super(true); │ ^^^^^ 25 │ } - 26 │ + 26 │ super(); i `super()` is then called again here: + 24 │ super(true); 25 │ } - 26 │ - > 27 │ super(); + > 26 │ super(); │ ^^^^^ - 28 │ } - 29 │ } + 27 │ } + 28 │ } ``` ``` -duplicateSuper.js:33:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +duplicateSuper.js:32:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This constructor calls `super()` in a loop. - 31 │ // invalid - 32 │ class E extends A { - > 33 │ constructor(cond) { + 30 │ // invalid + 31 │ class E extends A { + > 32 │ constructor(cond) { │ ^^^^^^^^^^^^^^^^^^^ - > 34 │ do { - > 35 │ super(); - > 36 │ } while (cond); - > 37 │ } + > 33 │ do { + > 34 │ super(); + > 35 │ } while (cond); + > 36 │ } │ ^ - 38 │ } - 39 │ + 37 │ } + 38 │ i `super()` is called here: - 33 │ constructor(cond) { - 34 │ do { - > 35 │ super(); + 32 │ constructor(cond) { + 33 │ do { + > 34 │ super(); │ ^^^^^ - 36 │ } while (cond); - 37 │ } + 35 │ } while (cond); + 36 │ } + + +``` + +``` +duplicateSuper.js:41:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This constructor has code paths that return without calling `super()`. + + 39 │ // invalid + 40 │ class F extends A { + > 41 │ constructor(condA, condB) { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 42 │ if (condA) { + ... + > 47 │ } + > 48 │ } + │ ^ + 49 │ } + 50 │ + + i If this is intentional, add an explicit throw statement in unsupported paths. + + +``` + +``` +duplicateSuper.js:53:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This constructor has code paths that return without calling `super()`. + + 51 │ // invalid + 52 │ class G extends A { + > 53 │ constructor(condA, condB) { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 54 │ while (condA) { + ... + > 58 │ } + > 59 │ } + │ ^ + 60 │ } + 61 │ + + i If this is intentional, add an explicit throw statement in unsupported paths. ``` ``` -duplicateSuper.js:42:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +duplicateSuper.js:64:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This constructor has code paths where `super()` is called more than once. - 40 │ // invalid - 41 │ class F extends A { - > 42 │ constructor(condA, condB) { + 62 │ // invalid + 63 │ class G extends A { + > 64 │ constructor(condA, condB) { │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - > 43 │ if (condA) { + > 65 │ try { ... - > 48 │ } - > 49 │ } + > 69 │ } + > 70 │ } │ ^ - 50 │ } - 51 │ + 71 │ } + 72 │ i `super()` is first called here: - 42 │ constructor(condA, condB) { - 43 │ if (condA) { - > 44 │ super(true); + 64 │ constructor(condA, condB) { + 65 │ try { + > 66 │ super() │ ^^^^^ - 45 │ } - 46 │ if (condB) { + 67 │ } catch { + 68 │ super() i `super()` is then called again here: - 45 │ } - 46 │ if (condB) { - > 47 │ super(true); + 66 │ super() + 67 │ } catch { + > 68 │ super() │ ^^^^^ - 48 │ } - 49 │ } + 69 │ } + 70 │ } ``` ``` -duplicateSuper.js:54:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +duplicateSuper.js:75:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This constructor calls `super()` in a loop. + ! This constructor has code paths where `super()` is called more than once. - 52 │ // invalid - 53 │ class G extends A { - > 54 │ constructor(condA, condB) { + 73 │ // invalid + 74 │ class G extends A { + > 75 │ constructor(condA, condB) { │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - > 55 │ while (condA) { + > 76 │ try { ... - > 59 │ } - > 60 │ } + > 80 │ } + > 81 │ } │ ^ - 61 │ } - 62 │ + 82 │ } + 83 │ - i `super()` is called here: + i `super()` is first called here: + + 75 │ constructor(condA, condB) { + 76 │ try { + > 77 │ super() + │ ^^^^^ + 78 │ } finally { + 79 │ super() - 55 │ while (condA) { - 56 │ if (condB) { - > 57 │ super(); - │ ^^^^^ - 58 │ } - 59 │ } + i `super()` is then called again here: + + 77 │ super() + 78 │ } finally { + > 79 │ super() + │ ^^^^^ + 80 │ } + 81 │ } ``` diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js index b2638a1d3992..2972458ff182 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js @@ -42,7 +42,6 @@ class E extends A { if (cond) { return; } - super(true); } } @@ -67,7 +66,6 @@ class G extends A { } else { throw new Error(); } - this.field = "value"; } } @@ -94,4 +92,11 @@ class I extends A { } } } - } +} + +// invalid +class I extends A { + constructor() { + super.method(); + } +} diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js.snap index 115607ecda33..63688f6b3091 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/missingSuper.js.snap @@ -48,7 +48,6 @@ class E extends A { if (cond) { return; } - super(true); } } @@ -73,7 +72,6 @@ class G extends A { } else { throw new Error(); } - this.field = "value"; } } @@ -100,7 +98,14 @@ class I extends A { } } } - } +} + +// invalid +class I extends A { + constructor() { + super.method(); + } +} ``` @@ -159,12 +164,13 @@ missingSuper.js:41:5 lint/correctness/noUnreachableSuper ━━━━━━━ > 41 │ constructor(cond) { │ ^^^^^^^^^^^^^^^^^^^ > 42 │ if (cond) { - ... - > 46 │ super(true); - > 47 │ } + > 43 │ return; + > 44 │ } + > 45 │ super(true); + > 46 │ } │ ^ - 48 │ } - 49 │ + 47 │ } + 48 │ i This statement returns from the constructor before `super()` has been called: @@ -173,7 +179,7 @@ missingSuper.js:41:5 lint/correctness/noUnreachableSuper ━━━━━━━ > 43 │ return; │ ^^^^^^^ 44 │ } - 45 │ + 45 │ super(true); i If this is intentional, add an explicit throw statement in unsupported paths. @@ -181,39 +187,59 @@ missingSuper.js:41:5 lint/correctness/noUnreachableSuper ━━━━━━━ ``` ``` -missingSuper.js:89:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +missingSuper.js:87:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This constructor has code paths where `super()` is called more than once. - 87 │ // invalid - 88 │ class I extends A { - > 89 │ constructor() { + 85 │ // invalid + 86 │ class I extends A { + > 87 │ constructor() { │ ^^^^^^^^^^^^^^^ - > 90 │ super(); + > 88 │ super(); ... - > 95 │ } - > 96 │ } + > 93 │ } + > 94 │ } │ ^ - 97 │ } - 98 │ + 95 │ } + 96 │ i `super()` is first called here: - 88 │ class I extends A { - 89 │ constructor() { - > 90 │ super(); + 86 │ class I extends A { + 87 │ constructor() { + > 88 │ super(); │ ^^^^^ - 91 │ if (flag1) { - 92 │ if (flag2) { + 89 │ if (flag1) { + 90 │ if (flag2) { i `super()` is then called again here: - 91 │ if (flag1) { - 92 │ if (flag2) { - > 93 │ super(); + 89 │ if (flag1) { + 90 │ if (flag2) { + > 91 │ super(); │ ^^^^^ - 94 │ } - 95 │ } + 92 │ } + 93 │ } + + +``` + +``` +missingSuper.js:99:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This constructor has code paths that return without calling `super()`. + + 97 │ // invalid + 98 │ class I extends A { + > 99 │ constructor() { + │ ^^^^^^^^^^^^^^^ + > 100 │ super.method(); + > 101 │ } + │ ^ + 102 │ } + 103 │ + + i If this is intentional, add an explicit throw statement in unsupported paths. ``` diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js index f02ec3367f70..f4bc6c023ca3 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js @@ -14,7 +14,6 @@ class C extends A { } else { super(false); } - this.field = "value"; } } @@ -31,7 +30,6 @@ class D extends A { class E extends A { constructor(cond) { this.field = "value"; - if (cond) { super(true); } else { @@ -46,7 +44,28 @@ class F extends A { if (cond) { super(true); } - this.field = "value"; } } + +// invalid +class G extends A { + constructor(condA, condB) { + try { + super(); + } catch { + this.prop = 0; + } + } +} + +// invalid +class G extends A { + constructor(condA, condB) { + try { + this.prop = 0; + } catch { + super(); + } + } +} diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js.snap index 1bd924f4a368..4a2d881b2cbb 100644 --- a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js.snap +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/thisBeforeSuper.js.snap @@ -1,6 +1,5 @@ --- source: crates/rome_js_analyze/tests/spec_tests.rs -assertion_line: 96 expression: thisBeforeSuper.js --- # Input @@ -21,7 +20,6 @@ class C extends A { } else { super(false); } - this.field = "value"; } } @@ -38,7 +36,6 @@ class D extends A { class E extends A { constructor(cond) { this.field = "value"; - if (cond) { super(true); } else { @@ -53,47 +50,59 @@ class F extends A { if (cond) { super(true); } - this.field = "value"; } } +// invalid +class G extends A { + constructor(condA, condB) { + try { + super(); + } catch { + this.prop = 0; + } + } +} + +// invalid +class G extends A { + constructor(condA, condB) { + try { + this.prop = 0; + } catch { + super(); + } + } +} + ``` # Diagnostics ``` -thisBeforeSuper.js:24:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +thisBeforeSuper.js:23:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - ! This constructor has code paths accessing `this` before `super()` is called. + ! This constructor has code paths accessing `this` without calling `super()` first. - 22 │ // invalid - 23 │ class D extends A { - > 24 │ constructor() { + 21 │ // invalid + 22 │ class D extends A { + > 23 │ constructor() { │ ^^^^^^^^^^^^^^^ - > 25 │ this.field = "value"; - > 26 │ super(); - > 27 │ } + > 24 │ this.field = "value"; + > 25 │ super(); + > 26 │ } │ ^ - 28 │ } - 29 │ + 27 │ } + 28 │ i `this` is accessed here: - 23 │ class D extends A { - 24 │ constructor() { - > 25 │ this.field = "value"; + 22 │ class D extends A { + 23 │ constructor() { + > 24 │ this.field = "value"; │ ^^^^ - 26 │ super(); - 27 │ } - - i `super()` is only called here: - - 24 │ constructor() { - 25 │ this.field = "value"; - > 26 │ super(); - │ ^^^^^ - 27 │ } - 28 │ } + 25 │ super(); + 26 │ } i If this is intentional, add an explicit throw statement in unsupported paths. @@ -101,30 +110,30 @@ thisBeforeSuper.js:24:5 lint/correctness/noUnreachableSuper ━━━━━━ ``` ``` -thisBeforeSuper.js:32:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +thisBeforeSuper.js:31:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This constructor has code paths accessing `this` without calling `super()` first. - 30 │ // invalid - 31 │ class E extends A { - > 32 │ constructor(cond) { + 29 │ // invalid + 30 │ class E extends A { + > 31 │ constructor(cond) { │ ^^^^^^^^^^^^^^^^^^^ - > 33 │ this.field = "value"; + > 32 │ this.field = "value"; ... - > 39 │ } - > 40 │ } + > 37 │ } + > 38 │ } │ ^ - 41 │ } - 42 │ + 39 │ } + 40 │ i `this` is accessed here: - 31 │ class E extends A { - 32 │ constructor(cond) { - > 33 │ this.field = "value"; + 30 │ class E extends A { + 31 │ constructor(cond) { + > 32 │ this.field = "value"; │ ^^^^ - 34 │ - 35 │ if (cond) { + 33 │ if (cond) { + 34 │ super(true); i If this is intentional, add an explicit throw statement in unsupported paths. @@ -132,30 +141,93 @@ thisBeforeSuper.js:32:5 lint/correctness/noUnreachableSuper ━━━━━━ ``` ``` -thisBeforeSuper.js:45:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +thisBeforeSuper.js:43:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! This constructor has code paths accessing `this` without calling `super()` first. - 43 │ // invalid - 44 │ class F extends A { - > 45 │ constructor(cond) { + 41 │ // invalid + 42 │ class F extends A { + > 43 │ constructor(cond) { │ ^^^^^^^^^^^^^^^^^^^ - > 46 │ if (cond) { - ... - > 50 │ this.field = "value"; - > 51 │ } + > 44 │ if (cond) { + > 45 │ super(true); + > 46 │ } + > 47 │ this.field = "value"; + > 48 │ } │ ^ - 52 │ } - 53 │ + 49 │ } + 50 │ i `this` is accessed here: - 48 │ } - 49 │ - > 50 │ this.field = "value"; + 45 │ super(true); + 46 │ } + > 47 │ this.field = "value"; │ ^^^^ - 51 │ } - 52 │ } + 48 │ } + 49 │ } + + i If this is intentional, add an explicit throw statement in unsupported paths. + + +``` + +``` +thisBeforeSuper.js:53:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This constructor has code paths accessing `this` without calling `super()` first. + + 51 │ // invalid + 52 │ class G extends A { + > 53 │ constructor(condA, condB) { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 54 │ try { + ... + > 58 │ } + > 59 │ } + │ ^ + 60 │ } + 61 │ + + i `this` is accessed here: + + 55 │ super(); + 56 │ } catch { + > 57 │ this.prop = 0; + │ ^^^^ + 58 │ } + 59 │ } + + i If this is intentional, add an explicit throw statement in unsupported paths. + + +``` + +``` +thisBeforeSuper.js:64:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! This constructor has code paths accessing `this` without calling `super()` first. + + 62 │ // invalid + 63 │ class G extends A { + > 64 │ constructor(condA, condB) { + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 65 │ try { + ... + > 69 │ } + > 70 │ } + │ ^ + 71 │ } + 72 │ + + i `this` is accessed here: + + 64 │ constructor(condA, condB) { + 65 │ try { + > 66 │ this.prop = 0; + │ ^^^^ + 67 │ } catch { + 68 │ super(); i If this is intentional, add an explicit throw statement in unsupported paths. diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js new file mode 100644 index 000000000000..484e6c306185 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js @@ -0,0 +1,20 @@ +class B extends A { + constructor() { + super(); + if (foo) { + if (bar) { + return; + } else { + return; + } + } + } +} + +class B extends A { + constructor() { + super(); + this.prop = 0; + super.method(); + } +} \ No newline at end of file diff --git a/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js.snap b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js.snap new file mode 100644 index 000000000000..3ceb24927473 --- /dev/null +++ b/crates/rome_js_analyze/tests/specs/correctness/noUnreachableSuper/valid.js.snap @@ -0,0 +1,29 @@ +--- +source: crates/rome_js_analyze/tests/spec_tests.rs +expression: valid.js +--- +# Input +```js +class B extends A { + constructor() { + super(); + if (foo) { + if (bar) { + return; + } else { + return; + } + } + } +} + +class B extends A { + constructor() { + super(); + this.prop = 0; + super.method(); + } +} +``` + + diff --git a/website/src/content/docs/internals/changelog.mdx b/website/src/content/docs/internals/changelog.mdx index 4cc7753f40d7..168a7b3511a6 100644 --- a/website/src/content/docs/internals/changelog.mdx +++ b/website/src/content/docs/internals/changelog.mdx @@ -36,7 +36,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Enhancements -- [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) now reports all string concatenations. +- [useTemplate](https://biomejs.dev/linter/rules/use-template/) now reports all string concatenations. Previously, the rule ignored concatenation of a value and a newline or a backquote. For example, the following concatenation was not reported: @@ -55,7 +55,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom + `\`${v}\``; ``` -- [useExponentiationOperator](https://biomejs.dev/linter/rules/use_exponentiation_operator/) suggests better code fixes. +- [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) suggests better code fixes. The rule now preserves any comment preceding the exponent, and it preserves any parenthesis around the base or the exponent. @@ -69,7 +69,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom #### Bug fixes -- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/noDuplicateJsxProps/) case-insensitive. +- Fix [#80](https://github.com/biomejs/biome/issues/95), making [noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) case-insensitive. Some frameworks, such as Material UI, rely on the case-sensitivity of JSX properties. For example, [TextField has two properties with the same name, but distinct cases](https://mui.com/material-ui/api/text-field/#TextField-prop-inputProps): @@ -80,7 +80,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#4713](https://github.com/rome/tools/issues/4713). - Previously, [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) made the following suggestion: + Previously, [useTemplate](https://biomejs.dev/linter/rules/use-template/) made the following suggestion: ```diff - a + b + "px" @@ -98,7 +98,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#4109](https://github.com/rome/tools/issues/4109) - Previously, [useTemplate](https://biomejs.dev/linter/rules/useTemplate/) suggested an invalid code fix when a leading or trailing single-line comment was present: + Previously, [useTemplate](https://biomejs.dev/linter/rules/use-template/) suggested an invalid code fix when a leading or trailing single-line comment was present: ```diff // leading comment @@ -119,7 +119,7 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [rome#3850](https://github.com/rome/tools/issues/3850) - Previously [useExponentiationOperator](https://biomejs.dev/linter/rules/use_exponentiation_operator/) suggested invalid code in a specific edge case: + Previously [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) suggested invalid code in a specific edge case: ```diff - 1 +Math.pow(++a, 2) @@ -135,7 +135,12 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Fix [#106](https://github.com/biomejs/biome/issues/106) - [noUndeclaredVariables](https://biomejs.dev/linter/rules/noUndeclaredVariables/) now correctly recognizes some TypeScript types such as `Uppercase`. + [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/) now correctly recognizes some TypeScript types such as `Uppercase`. + +- Fix [rome#4616](https://github.com/rome/tools/issues/4616) + + Previously [noUnreachableSuper](https://biomejs.dev/linter/rules/no-unreacheable-super/) reported valid codes with complex nesting of control flow structures. + ### Parser ### VSCode @@ -330,11 +335,11 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom - Remove `useCamelCase` - Use [useNamingConvention](https://biomejs.dev/linter/rules/useCamelCase/) instead. + Use [useNamingConvention](https://biomejs.dev/linter/rules/use-camel-case/) instead. #### New rules -- Add [noExcessiveComplexity](https://biomejs.dev/linter/rules/noExcessiveComplexity/) +- Add [noExcessiveComplexity](https://biomejs.dev/linter/rules/no-excessive-complexity/) - Add [useImportRestrictions](https://biomejs.dev/linter/rules/use-import-restrictions/) @@ -365,23 +370,23 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom This rule disallow duplicate keys in a JSON object. -- Add [noVoid](https://biomejs.dev/linter/rules/novoid/) +- Add [noVoid](https://biomejs.dev/linter/rules/no-void/) This rules disallow the use of `void`. -- Add [noNonoctalDecimalEscape](https://biomejs.dev/linter/rules/nononoctaldecimalescape/) +- Add [noNonoctalDecimalEscape](https://biomejs.dev/linter/rules/no-nonoctal-decimal-escape/) This rule disallows `\8` and `\9` escape sequences in string literals. -- Add [noUselessEmptyExport](https://biomejs.dev/linter/rules/noUselessEmptyExport/) +- Add [noUselessEmptyExport](https://biomejs.dev/linter/rules/no-useless-empty-export/) This rule disallows useless `export {}`. -- Add [useIsArray](https://biomejs.dev/linter/rules/useIsArray/) +- Add [useIsArray](https://biomejs.dev/linter/rules/use-is-array/) This rule proposes using `Array.isArray()` instead of `instanceof Array`. -- Add [useGetterReturn](https://biomejs.dev/linter/rules/useGetterReturn/) +- Add [useGetterReturn](https://biomejs.dev/linter/rules/use-getter-return/) This rule enforces the presence of non-empty return statements in getters. This makes the following code incorrect: @@ -396,27 +401,27 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom New rules are promoted, please check [#4750](https://github.com/rome/tools/discussions/4750) for more details: -- [a11y/useHeadingContent](https://biomejs.dev/linter/rules/useHeadingContent/) -- [complexity/noForEach](https://biomejs.dev/linter/rules/noForEach/) -- [complexity/useLiteralKeys](https://biomejs.dev/linter/rules/useLiteralKeys/) -- [complexity/useSimpleNumberKeys](https://biomejs.dev/linter/rules/useSimpleNumberKeys/) -- [correctness/useIsNan](https://biomejs.dev/linter/rules/useIsNan/) -- [suspicious/noConsoleLog](https://biomejs.dev/linter/rules/noConsoleLog/) -- [suspicious/noDuplicateJsxProps](https://biomejs.dev/linter/rules/noDuplicateJsxProps/) +- [a11y/useHeadingContent](https://biomejs.dev/linter/rules/use-heading-content/) +- [complexity/noForEach](https://biomejs.dev/linter/rules/no-for-each/) +- [complexity/useLiteralKeys](https://biomejs.dev/linter/rules/use-literal-keys/) +- [complexity/useSimpleNumberKeys](https://biomejs.dev/linter/rules/use-simple-number-keys/) +- [correctness/useIsNan](https://biomejs.dev/linter/rules/use-is-nan/) +- [suspicious/noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/) +- [suspicious/noDuplicateJsxProps](https://biomejs.dev/linter/rules/no-duplicate-jsx-props/) The following rules are now recommended: -- [noUselessFragments](https://biomejs.dev/linter/rules/noUselessFragments/) -- [noRedundantUseStrict](https://biomejs.dev/linter/rules/noRedundantUseStrict/) -- [useExponentiationOperator](https://biomejs.dev/linter/rules/useExponentiationOperator/) +- [noUselessFragments](https://biomejs.dev/linter/rules/no-useless-fragments/) +- [noRedundantUseStrict](https://biomejs.dev/linter/rules/no-redundant-use-strict/) +- [useExponentiationOperator](https://biomejs.dev/linter/rules/use-exponentiation-operator/) #### Other changes - Add new TypeScript globals (`AsyncDisposable`, `Awaited`, `DecoratorContext`, and others) [4643](https://github.com/rome/tools/issues/4643). -- [noRedeclare](https://biomejs.dev/linter/rules/noredeclare/): allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) +- [noRedeclare](https://biomejs.dev/linter/rules/no-redeclare/): allow redeclare of index signatures are in different type members [#4478](https://github.com/rome/tools/issues/4478) -- Improve [noConsoleLog](https://biomejs.dev/linter/rules/noconsolelog/), [noGlobalObjectCalls](https://biomejs.dev/linter/rules/noglobalobjectcalls/), [useIsNan](https://biomejs.dev/linter/rules/useisnan/), and [useNumericLiterals](https://biomejs.dev/linter/rules/usenumericliterals/) by handling `globalThis` and `window` namespaces. +- Improve [noConsoleLog](https://biomejs.dev/linter/rules/no-console-log/), [noGlobalObjectCalls](https://biomejs.dev/linter/rules/no-global-object-calls/), [useIsNan](https://biomejs.dev/linter/rules/useisnan/), and [useNumericLiterals](https://biomejs.dev/linter/rules/use-numeric-literals/) by handling `globalThis` and `window` namespaces. For instance, the following code is now reported by `noConsoleLog`: @@ -424,9 +429,9 @@ The following rules are now recommended: globalThis.console.log("log") ``` -- Improve [noDuplicateParameters](https://biomejs.dev/linter/rules/noduplicateparameters/) to manage constructor parameters. +- Improve [noDuplicateParameters](https://biomejs.dev/linter/rules/no-duplicate-parameters/) to manage constructor parameters. -- Improve [noInnerDeclarations](https://biomejs.dev/linter/rules/noInnerDeclarations/) +- Improve [noInnerDeclarations](https://biomejs.dev/linter/rules/no-inner-declarations/) Now, the rule doesn't report false-positives about ambient _TypeScript_ declarations. For example, the following code is no longer reported by the rule: @@ -435,7 +440,7 @@ The following rules are now recommended: declare var foo; ``` -- Improve [useEnumInitializers](https://biomejs.dev/linter/rules/useEnumInitializers/) +- Improve [useEnumInitializers](https://biomejs.dev/linter/rules/use-enum-initializers/) The rule now reports all uninitialized members of an enum in a single diagnostic. @@ -449,7 +454,7 @@ The following rules are now recommended: } ``` -- Relax [noBannedTypes](https://biomejs.dev/linter/rules/nobannedtypes/) and improve documentation +- Relax [noBannedTypes](https://biomejs.dev/linter/rules/no-banned-types/) and improve documentation The rule no longer reports a user type that reuses a banned type name. The following code is now allowed: @@ -473,7 +478,7 @@ The following rules are now recommended: type NonNullableMyType = MyType & {}; ``` -- Improve [noConstantCondition](https://biomejs.dev/linter/rules/noConstantCondition/) +- Improve [noConstantCondition](https://biomejs.dev/linter/rules/no-constant-condition/) The rule now allows `while(true)`. This recognizes a common pattern in the web community: @@ -486,13 +491,13 @@ The following rules are now recommended: } ``` -- Improve the diagnostic and the code action of [useDefaultParameterLast](https://biomejs.dev/linter/rules/usedefaultparameterlast/). +- Improve the diagnostic and the code action of [useDefaultParameterLast](https://biomejs.dev/linter/rules/use-default-parameter-last/). The diagnostic now reports the last required parameter which should precede optional and default parameters. The code action now removes any whitespace between the parameter name and its initialization. -- Relax [noConfusingArrow](https://biomejs.dev/linter/rules/noconfusingarrow/) +- Relax [noConfusingArrow](https://biomejs.dev/linter/rules/no-confusing-arrow/) All arrow functions that enclose its parameter with parenthesis are allowed. Thus, the following snippet no longer trigger the rule: @@ -507,7 +512,7 @@ The following rules are now recommended: var x = a => 1 ? 2 : 3; ``` -- Relax [useLiteralEnumMembers](https://biomejs.dev/linter/rules/useLiteralEnumMembers/) +- Relax [useLiteralEnumMembers](https://biomejs.dev/linter/rules/use-literal-enum-members/) Enum members that refer to previous enum members are now allowed. This allows a common pattern in enum flags like in the following example: @@ -532,7 +537,7 @@ The following rules are now recommended: } ``` -- Improve [useLiteralKeys](https://biomejs.dev/linter/rules/useLiteralKeys/). +- Improve [useLiteralKeys](https://biomejs.dev/linter/rules/use-literal-keys/). Now, the rule suggests simplifying computed properties to string literal properties: @@ -554,11 +559,11 @@ The following rules are now recommended: These suggestions are made in object literals, classes, interfaces, and object types. -- Improve [noNewSymbol](https://biomejs.dev/linter/rules/noNewSymbol/). +- Improve [noNewSymbol](https://biomejs.dev/linter/rules/no-new-symbol/). The rule now handles cases where `Symbol` is namespaced with the global `globalThis` or `window`. -- The rules [useExhaustiveDependencies](https://biomejs.dev/linter/rules/useexhaustivedependencies/) and [useHookAtTopLevel](https://biomejs.dev/linter/rules/usehookattoplevel/) accept a different shape of options +- The rules [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies/) and [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/) accept a different shape of options Old configuration: @@ -606,43 +611,43 @@ The following rules are now recommended: } ``` -- [noRedundantUseStrict](https://biomejs.dev/linter/rules/noredundantusestrict/) check only `'use strict'` directive to resolve false positive diagnostics. +- [noRedundantUseStrict](https://biomejs.dev/linter/rules/no-redundant-use-strict/) check only `'use strict'` directive to resolve false positive diagnostics. React introduced new directives, "use client" and "use server". The rule raises false positive errors about these directives. -- Fix a crash in the [NoParameterAssign](https://biomejs.dev/linter/rules/noparameterassign/) rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323) +- Fix a crash in the [NoParameterAssign](https://biomejs.dev/linter/rules/no-parameter-assign/) rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323) -- Fix [useExhaustiveDependencies](https://biomejs.dev/linter/rules/useexhaustivedependencies/) in the following cases [#4330](https://github.com/rome/tools/issues/4330): +- Fix [useExhaustiveDependencies](https://biomejs.dev/linter/rules/use-exhaustive-dependencies/) in the following cases [#4330](https://github.com/rome/tools/issues/4330): - when the first argument of hooks is a named function - inside an export default function - for React.use* hooks -- Fix [noInvalidConstructorSuper](https://biomejs.dev/linter/rules/noinvalidconstructorsuper/) that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624). +- Fix [noInvalidConstructorSuper](https://biomejs.dev/linter/rules/no-invalid-constructor-super/) that erroneously reported generic parents [#4624](https://github.com/rome/tools/issues/4624). - Fix [noDuplicateCase](https://biomejs.dev/linter/rules/noDuplicateCase/) that erroneously reported as equals the strings literals `"'"` and `'"'` [#4706](https://github.com/rome/tools/issues/4706). -- Fix [NoUnreachableSuper](https://biomejs.dev/linter/rules/nounreachablesuper/)'s false positive diagnostics ([#4483](https://github.com/rome/tools/issues/4483)) caused to nested if statement. +- Fix [NoUnreachableSuper](https://biomejs.dev/linter/rules/no-unreachable-super/)'s false positive diagnostics ([#4483](https://github.com/rome/tools/issues/4483)) caused to nested if statement. The rule no longer reports `This constructor calls super() in a loop` when using nested if statements in a constructor. -- Fix [useHookAtTopLevel](https://biomejs.dev/linter/rules/usehookattoplevel/)'s false positive diagnostics ([#4637](https://github.com/rome/tools/issues/4637)) +- Fix [useHookAtTopLevel](https://biomejs.dev/linter/rules/use-hook-at-top-level/)'s false positive diagnostics ([#4637](https://github.com/rome/tools/issues/4637)) The rule no longer reports false positive diagnostics when accessing properties directly from a hook and calling a hook inside function arguments. -- Fix [noUselessConstructor](https://biomejs.dev/linter/rules/noUselessConstructor/) which erroneously reported constructors with default parameters [rome#4781](https://github.com/rome/tools/issues/4781) +- Fix [noUselessConstructor](https://biomejs.dev/linter/rules/no-useless-constructor/) which erroneously reported constructors with default parameters [rome#4781](https://github.com/rome/tools/issues/4781) - Fix [noUselessFragments](https://biomejs.dev/linter/rules/nouselessfragments/)'s panics when running `biome check --apply-unsafe` ([#4637](https://github.com/rome/tools/issues/4639)) This rule's code action emits an invalid AST, so I fixed using JsxString instead of JsStringLiteral -- Fix [noUndeclaredVariables](https://biomejs.dev/linter/rules/noundeclaredvariables/)'s false positive diagnostics ([#4675](https://github.com/rome/tools/issues/4675)) +- Fix [noUndeclaredVariables](https://biomejs.dev/linter/rules/no-undeclared-variables/)'s false positive diagnostics ([#4675](https://github.com/rome/tools/issues/4675)) The semantic analyzer no longer handles `this` reference identifier. -- Fix [noUnusedVariables](https://biomejs.dev/linter/rules/nounusedvariables/)'s false positive diagnostics ([#4688](https://github.com/rome/tools/issues/4688)) +- Fix [noUnusedVariables](https://biomejs.dev/linter/rules/no-unused-variables/)'s false positive diagnostics ([#4688](https://github.com/rome/tools/issues/4688)) The semantic analyzer handles ts export declaration clause correctly. diff --git a/website/src/content/docs/linter/rules/no-unreachable-super.md b/website/src/content/docs/linter/rules/no-unreachable-super.md index e1790bdbfc27..a8e03c25e5e1 100644 --- a/website/src/content/docs/linter/rules/no-unreachable-super.md +++ b/website/src/content/docs/linter/rules/no-unreachable-super.md @@ -46,7 +46,7 @@ class A extends B {
correctness/noUnreachableSuper.js:2:5 lint/correctness/noUnreachableSuper ━━━━━━━━━━━━━━━━━━━━━━━━━━
 
-   This constructor has code paths accessing `this` before `super()` is called.
+   This constructor has code paths accessing `this` without calling `super()` first.
   
     1 │ class A extends B {
   > 2 │     constructor(value) {
@@ -67,15 +67,6 @@ class A extends B {
     4 │         super();
     5 │     }
   
-   `super()` is only called here:
-  
-    2 │     constructor(value) {
-    3 │         this.prop = value;
-  > 4 │         super();
-           ^^^^^
-    5 │     }
-    6 │ }
-  
    If this is intentional, add an explicit throw statement in unsupported paths.