fix(no-unused-vars): consider functions within conditional expressions usable #6553
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s usable Previously, a function declared directly within an `AstKind::ConditionalExpression` would be considered unused. This happened because `is_function_or_class_declaration_used` did not directly handle conditional expression parents, letting such cases fall into the default branch which just returned `false`, marking the function as unused. This change fixes that by considering such functions used when the conditional expression itself is used.
Your org has enabled the Graphite merge queue for merging into mainAdd the label “0-merge” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “hotfix” to add to the merge queue as a hot fix. You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link. |
CodSpeed Performance ReportMerging #6553 will not alter performanceComparing Summary
|
|
Seems like a pathological case but otherwise LGTM |
DonIsaac
reviewed
Oct 14, 2024
crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/eslint.rs
Outdated
Show resolved
Hide resolved
This actually came up as a false positive in my company's codebase. We name the generator function there because our ESLint config enables the |
Per oxc-project#6553 (comment), the `eslint` suite is for tests ported directly from ESLint.
camc314
approved these changes
Oct 14, 2024
|
@eventualbuddha i love it when people make a PR to fix a bug they find in their tooling instead of tweeting at me :) tyvm! |
DonIsaac
approved these changes
Oct 15, 2024
DonIsaac
added a commit
that referenced
this pull request
Oct 19, 2024
## [0.10.0] - 2024-10-18 - 7f6b219 editor/vscode: [**BREAKING**] Unify configuration logic (#6630) (DonIsaac) - 782f0a7 codegen: [**BREAKING**] Rename `print_char` method to `print_ascii_byte` (#6512) (overlookmotel) - 7645e5c codegen: [**BREAKING**] Remove CommentOptions API (#6451) (Boshen) - 5200960 oxc: [**BREAKING**] Remove passing `Trivias` around (#6446) (Boshen) - 80266d8 linter: [**BREAKING**] Support plugins in oxlint config files (#6088) (DonIsaac) ### Features - 6f22538 ecmascript: Add `ToBoolean`, `ToNumber`, `ToString` (#6502) (Boshen) - 1e7fab3 linter: Implement `no-callback-in-promise` (#6157) (dalaoshu) - c56343d linter: Promote `no_unsafe_optional_chaining` to correctness (#6491) (Boshen) - 454874a linter: Implement `react/iframe-missing-sandbox` (#6383) (Radu Baston) - c8174e2 linter: Add suggestions for `no-plusplus` (#6376) (camchenry) - 6e3224d linter: Configure by category in config files (#6120) (DonIsaac) - c5e66e1 linter/no-unused-vars: Report own type references within class, interface, and type alias declarations (#6557) (DonIsaac) - 8c78f97 linter/node: Implement no-new-require (#6165) (Jelle van der Waa) ### Bug Fixes - cf92730 editor: Use human-readable output channel names (#6629) (DonIsaac) - d9159a2 editor: Misaligned command prefixes (#6628) (DonIsaac) - b9c94bb editors/vscode: Temporarily solve oxc_language_server issue on windows (#6384) (dalaoshu) - e340424 linter: Support import type with namespaced import in `import/no-duplicates` (#6650) (Dmitry Zakharov) - a668397 linter: Panic in `no-else-return` (#6648) (dalaoshu) - 41dc8e3 linter: Stack overflow in `oxc/no-async-endpoint-handlers` (#6614) (DonIsaac) - d07a9b0 linter: Panic in `no-zero-fractions` (#6607) (dalaoshu) - d6a0d2e linter: Fix file name checking behavior of `unicorn/filename-case` (#6463) (camchenry) - 0784e74 linter: Error fixer of `switch-case-braces` (#6474) (dalaoshu) - e811812 linter: Error diagnostic message based on parameter length of valid-expect (#6455) (dalaoshu) - f71c91e linter: Move `eslint/sort-keys` to `style` category (#6377) (DonIsaac) - 2b86de9 linter/no-control-regex: False negative for flags in template literals (#6531) (DonIsaac) - 685a590 linter/no-control-regex: Better diagnostic messages (#6530) (DonIsaac) - 6d5a9f2 linter/no-control-regex: Allow capture group references (#6529) (DonIsaac) - ba53bc9 linter/no-unused-vars: False positives in TS type assertions (#6397) (DonIsaac) - d3e59c6 linter/no-unused-vars: False positive in some default export cases (#6395) (DonIsaac) - e08f956 linter/no-unused-vars: False positive for functions and classes in arrays (#6394) (DonIsaac) - b9d7c5f no-unused-vars: Consider functions within conditional expressions usable (#6553) (Brian Donovan) ### Performance - 0cbd4d0 linter: Avoid megamorphism in `RuleFixer` methods (#6606) (DonIsaac) - 725f9f6 linter: Get fewer parent nodes in `unicorn/prefer-dom-node-text-content` (#6467) (camchenry) - c00f669 linter: Use NonZeroUsize for pending module cache entries (#6439) (DonIsaac) - a1a2721 linter: Replace `ToString::to_string` with `CompactStr` in remaining rules (#6407) (camchenry) - c5c69d6 linter: Use `CompactStr` in `valid-title` (#6406) (camchenry) - d66e826 linter: Use `CompactStr` in `prefer-lowercase-title` (#6405) (camchenry) - 889400c linter: Use `CompactStr` for `get_node_name` in Jest rules (#6403) (camchenry) - 9906849 linter: Use `CompactStr` in `no-large-snapshots` (#6402) (camchenry) - c382ec4 linter: Use `CompactStr` in `no-hooks` (#6401) (camchenry) - 24a5d9b linter: Use `CompactStr` in `expect-expect` (#6400) (camchenry) - 71dbdad linter: Use `CompactStr` in `no-console` (#6399) (camchenry) - f5f00a1 linter: Use `CompactStr` in `no-bitwise` (#6398) (camchenry) - 62afaa9 linter/jsx-no-comment-textnodes: Remove regex for checking comment patterns (#6534) (camchenry) - b3d0cce linter/no-unescaped-entities: Add fast path to check if char should be replaced (#6594) (camchenry) - ee73f56 linter/no-unused-vars: Do not construct `Regex` for default ignore pattern (#6590) (camchenry) - 77ddab8 linter/numeric-separators-style: Replace regex with number parser (#6546) (camchenry) - 8f47cd0 linter/react: Remove regex patterns in `no-unknown-property` (#6536) (camchenry) ### Documentation - 557f941 linter: Add docs to no-unused-vars and Tester (#6558) (DonIsaac) ### Refactor - ecce5c5 linter: Improve recursive argument handling and diagnostics creation (#6513) (no-yan) - f960e9e linter: Add suggested file names for `unicorn/filename-case` (#6465) (camchenry) - 7240ee2 linter: Make advertised fix kinds consistent (#6461) (Alexander S.) - b48c368 linter: `no_global_assign` rule: reduce name lookups (#6460) (overlookmotel) - 2566ce7 linter: Remove OxlintOptions (#6098) (DonIsaac) - 002078a linter: Make Runtime's members private (#6440) (DonIsaac) - 6a0a533 linter: Move module cache logic out of Runtime (#6438) (DonIsaac) - c18c6e9 linter: Split service code into separate modules (#6437) (DonIsaac) - 5ea9ef7 linter: Improve labels and help message for `eslint/no-useless-constructor` (#6389) (DonIsaac) - 2c32dac linter/no-control-regex: Remove duplicate code (#6527) (DonIsaac) - 435a89c oxc: Remove useless `allocator.alloc(program)` calls (#6571) (Boshen) - f70e93b oxc: Ban index methods on std::str::Chars (#6075) (dalaoshu) ### Testing - a6cae98 linter: Make sure all auto-fixing rules have fixer test (#6378) (DonIsaac) - 06b09b2 linter/no-unused-vars: Enable now-passing tests (#6556) (DonIsaac) - badd11c linter/no-unused-vars: Ignored catch parameters (#6555) (DonIsaac) - 84aa2a2 linter/no-useless-constructor: Add cases for initializers in subclass constructors (#6390) (DonIsaac) --------- Co-authored-by: DonIsaac <[email protected]> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, a function declared directly within an
AstKind::ConditionalExpressionwould be considered unused. This happened becauseis_function_or_class_declaration_useddid not directly handle conditional expression parents, letting such cases fall into the default branch which just returnedfalse, marking the function as unused. This change fixes that by considering such functions used when the conditional expression itself is used.