fix(eslint-plugin): [no-inferrable] fix optional param to valid code #5342

Josh-Cena · 2022-07-13T03:52:05Z

PR Checklist

Addresses an existing open issue: fixes Bug: no-inferrable-types fixer does not remove ? when removing the type #5340
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

(I have a feeling we need more test cases, but I can't think of any edge-cases.)

nx-cloud · 2022-07-13T03:52:06Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 7598abb. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 47 targets

Sent with 💌 from NxCloud.

typescript-eslint · 2022-07-13T03:52:08Z

Thanks for the PR, @Josh-Cena!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day.

netlify · 2022-07-13T03:52:12Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`7598abb`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/62cf90e68d27480009027eaa
😎 Deploy Preview	https://deploy-preview-5342--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

codecov · 2022-07-13T04:05:55Z

Codecov Report

Merging #5342 (7598abb) into main (75a86d9) will increase coverage by 2.09%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5342      +/-   ##
==========================================
+ Coverage   91.67%   93.77%   +2.09%     
==========================================
  Files         363      287      -76     
  Lines       12227     9875    -2352     
  Branches     3549     2956     -593     
==========================================
- Hits        11209     9260    -1949     
+ Misses        668      335     -333     
+ Partials      350      280      -70

Flag	Coverage Δ
unittest	`93.77% <100.00%> (+2.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ges/eslint-plugin/src/rules/no-inferrable-types.ts	`95.00% <100.00%> (+0.35%)`	⬆️
packages/type-utils/src/getTokenAtPosition.ts
packages/type-utils/src/containsAllTypesByName.ts
...ges/type-utils/src/getConstrainedTypeAtLocation.ts
...estree/src/create-program/createIsolatedProgram.ts
packages/type-utils/src/propertyTypes.ts
...ges/typescript-estree/src/create-program/shared.ts
packages/utils/src/ast-utils/eslint-utils/index.ts
...escript-estree/src/create-program/getScriptKind.ts
...escript-estree/src/semantic-or-syntactic-errors.ts
... and 67 more

armano2 · 2022-07-13T14:14:04Z

packages/eslint-plugin/tests/rules/no-inferrable-types.test.ts

-
+    {
+      // This is invalid TS semantic, but it's trivial to make valid anyway
+      code: 'const fn = (a?: number = 5) => {};',


not sure if we want to handle this in here, but there is one more use case that is broken.

playground

class Test { x!: number = 5 abstract y!: number = 5 }

maybe something like this could be better:

if ( node.type === AST_NODE_TYPES.AssignmentPattern && node.left.optional ) { yield fixer.remove(sourceCode.getTokenBefore(typeNode)!); } else if ( (node.type === AST_NODE_TYPES.TSAbstractPropertyDefinition || node.type === AST_NODE_TYPES.PropertyDefinition) && node.definite ) { yield fixer.remove(sourceCode.getTokenBefore(typeNode)!); }

we can make separate ticket for this, if you like

there is also

class Test2 { constructor(private a: number = 5) { } }

witch is currently not reported

I'll add definite assignment to this PR, but the private one seems tangential to this. Does this one also seem worth reporting?

class Test2 { x?: number = 5; }

Also, the abstract y!: number = 5 is too messed up to be autofixed. The initializer needs to be removed as well. It isn't even reported by the rule under the status quo, so I'll just ignore that.

bradzacher

thanks for fixing this!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | patch | [`5.30.6` -> `5.30.7`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.30.6/5.30.7) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | patch | [`5.30.6` -> `5.30.7`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.30.6/5.30.7) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.30.7`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5307-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5306v5307-2022-07-18) [Compare Source](typescript-eslint/typescript-eslint@v5.30.6...v5.30.7) ##### Bug Fixes - **eslint-plugin:** \[no-inferrable] fix optional param to valid code ([#5342](typescript-eslint/typescript-eslint#5342)) ([98f6d5e](typescript-eslint/typescript-eslint@98f6d5e)) - **eslint-plugin:** \[no-unused-vars] highlight last write reference ([#5267](typescript-eslint/typescript-eslint#5267)) ([c3f199a](typescript-eslint/typescript-eslint@c3f199a)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.30.7`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5307-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5306v5307-2022-07-18) [Compare Source](typescript-eslint/typescript-eslint@v5.30.6...v5.30.7) ##### Bug Fixes - expose types supporting old versions of typescript ([#5339](typescript-eslint/typescript-eslint#5339)) ([4ba9bdb](typescript-eslint/typescript-eslint@4ba9bdb)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1467 Reviewed-by: 6543 <6543@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.30.7` -> `5.31.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.30.7/5.31.0) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.30.7` -> `5.31.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.30.7/5.31.0) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.31.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5310-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5307v5310-2022-07-25) [Compare Source](typescript-eslint/typescript-eslint@v5.30.7...v5.31.0) ##### Bug Fixes - **eslint-plugin:** \[typedef] Support nested array destructuring with type annotation ([#5311](typescript-eslint/typescript-eslint#5311)) ([6d19efe](typescript-eslint/typescript-eslint@6d19efe)) - **scope-manager:** handle typeParameters of TSInstantiationExpression ([#5355](typescript-eslint/typescript-eslint#5355)) ([2595ccf](typescript-eslint/typescript-eslint@2595ccf)) ##### Features - **eslint-plugin:** \[consistent-generic-ctors] check class field declaration ([#5288](typescript-eslint/typescript-eslint#5288)) ([48f996e](typescript-eslint/typescript-eslint@48f996e)) - **eslint-plugin:** \[prefer-nullish-coalescing] add ignoreTernaryTests option ([#4965](typescript-eslint/typescript-eslint#4965)) ([f82727f](typescript-eslint/typescript-eslint@f82727f)) #### [5.30.7](typescript-eslint/typescript-eslint@v5.30.6...v5.30.7) (2022-07-18) ##### Bug Fixes - **eslint-plugin:** \[no-inferrable] fix optional param to valid code ([#5342](typescript-eslint/typescript-eslint#5342)) ([98f6d5e](typescript-eslint/typescript-eslint@98f6d5e)) - **eslint-plugin:** \[no-unused-vars] highlight last write reference ([#5267](typescript-eslint/typescript-eslint#5267)) ([c3f199a](typescript-eslint/typescript-eslint@c3f199a)) #### [5.30.6](typescript-eslint/typescript-eslint@v5.30.5...v5.30.6) (2022-07-11) **Note:** Version bump only for package [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin) #### [5.30.5](typescript-eslint/typescript-eslint@v5.30.4...v5.30.5) (2022-07-04) ##### Bug Fixes - **eslint-plugin:** \[consistent-indexed-object-style] fix record mode fixer for generics with a default value ([#5280](typescript-eslint/typescript-eslint#5280)) ([57f032c](typescript-eslint/typescript-eslint@57f032c)) #### [5.30.4](typescript-eslint/typescript-eslint@v5.30.3...v5.30.4) (2022-07-03) **Note:** Version bump only for package [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin) #### [5.30.3](typescript-eslint/typescript-eslint@v5.30.2...v5.30.3) (2022-07-01) **Note:** Version bump only for package [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin) #### [5.30.2](typescript-eslint/typescript-eslint@v5.30.1...v5.30.2) (2022-07-01) **Note:** Version bump only for package [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin) #### [5.30.1](typescript-eslint/typescript-eslint@v5.30.0...v5.30.1) (2022-07-01) ##### Bug Fixes - **eslint-plugin:** \[no-base-to-string] add missing apostrophe to message ([#5270](typescript-eslint/typescript-eslint#5270)) ([d320174](typescript-eslint/typescript-eslint@58034e3)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.31.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5310-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5307v5310-2022-07-25) [Compare Source](typescript-eslint/typescript-eslint@v5.30.7...v5.31.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.30.7](typescript-eslint/typescript-eslint@v5.30.6...v5.30.7) (2022-07-18) ##### Bug Fixes - expose types supporting old versions of typescript ([#5339](typescript-eslint/typescript-eslint#5339)) ([4ba9bdb](typescript-eslint/typescript-eslint@4ba9bdb)) #### [5.30.6](typescript-eslint/typescript-eslint@v5.30.5...v5.30.6) (2022-07-11) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.30.5](typescript-eslint/typescript-eslint@v5.30.4...v5.30.5) (2022-07-04) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.30.4](typescript-eslint/typescript-eslint@v5.30.3...v5.30.4) (2022-07-03) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.30.3](typescript-eslint/typescript-eslint@v5.30.2...v5.30.3) (2022-07-01) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.30.2](typescript-eslint/typescript-eslint@v5.30.1...v5.30.2) (2022-07-01) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### 5.30.1 (2022-07-01) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1480 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

fix(eslint-plugin): [no-inferrable] fix optional param to valid code

7236104

armano2 reviewed Jul 13, 2022

View reviewed changes

Add definite assignment

7598abb

bradzacher approved these changes Jul 18, 2022

View reviewed changes

bradzacher added the bug Something isn't working label Jul 18, 2022

bradzacher merged commit 98f6d5e into typescript-eslint:main Jul 18, 2022

Josh-Cena deleted the inferrable-param branch August 13, 2022 10:43

github-actions bot locked as resolved and limited conversation to collaborators Sep 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [no-inferrable] fix optional param to valid code #5342

fix(eslint-plugin): [no-inferrable] fix optional param to valid code #5342

Josh-Cena commented Jul 13, 2022

nx-cloud bot commented Jul 13, 2022 •

edited

Loading

typescript-eslint bot commented Jul 13, 2022

netlify bot commented Jul 13, 2022 •

edited

Loading

codecov bot commented Jul 13, 2022 •

edited

Loading

armano2 Jul 13, 2022 •

edited

Loading

Josh-Cena Jul 14, 2022 •

edited

Loading

Josh-Cena Jul 14, 2022

bradzacher left a comment

fix(eslint-plugin): [no-inferrable] fix optional param to valid code #5342

fix(eslint-plugin): [no-inferrable] fix optional param to valid code #5342

Conversation

Josh-Cena commented Jul 13, 2022

PR Checklist

Overview

nx-cloud bot commented Jul 13, 2022 • edited Loading

☁️ Nx Cloud Report

typescript-eslint bot commented Jul 13, 2022

netlify bot commented Jul 13, 2022 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

codecov bot commented Jul 13, 2022 • edited Loading

Codecov Report

armano2 Jul 13, 2022 • edited Loading

Choose a reason for hiding this comment

Josh-Cena Jul 14, 2022 • edited Loading

Choose a reason for hiding this comment

Josh-Cena Jul 14, 2022

Choose a reason for hiding this comment

bradzacher left a comment

Choose a reason for hiding this comment

nx-cloud bot commented Jul 13, 2022 •

edited

Loading

netlify bot commented Jul 13, 2022 •

edited

Loading

codecov bot commented Jul 13, 2022 •

edited

Loading

armano2 Jul 13, 2022 •

edited

Loading

Josh-Cena Jul 14, 2022 •

edited

Loading