8000 [no-unsafe-call] Expand checks to include parameters · Issue #1968 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

[no-unsafe-call] Expand checks to include parameters #1968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Raynos opened this issue May 2, 2020 · 6 comments
Closed

[no-unsafe-call] Expand checks to include parameters #1968

Raynos opened this issue May 2, 2020 · 6 comments
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look

Comments

@Raynos
Copy link
Raynos commented May 2, 2020

Currently no-unsafe-call checks for calling a function whose type is any and lets you know it's unsafe.

Repro

// this.httpServer is http.Server
// The on('request') defn is req: any, res: any
this.httpServer.on('request', (req, res) => {
  // This method exists and its type are ServerRequest & ServerResponse
  // here I am calling a method that i've implemented with any, any
  this.handleServerRequest(req, res)
})

Smaller reproduction

  /**
   * @returns {void}
   */
  function foo () {
    y(JSON.parse('y'))

    /**
     * @param {string} s
     * @returns {string}
     */
    function y (s) {
      return s
    }
  }

Basically if i have a function with type definition for arguments and i call it with an any that i received from somewhere else then it should fail.

I expect the rule no-unsafe-call to catch this because it's about making all CallExpression in the AST safe ( no unsafe )

Expected Result

Warning from typescript-eslint

Actual Result

no warning

Additional Info

Versions

package version
@typescript-eslint/eslint-plugin X.Y.Z
@typescript-eslint/parser X.Y.Z
TypeScript X.Y.Z
ESLint X.Y.Z
node X.Y.Z
npm X.Y.Z
@Raynos Raynos added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look labels May 2, 2020
@Raynos
Copy link
Author
Raynos commented May 2, 2020

From another issue maybe you wanted a new rule called no-unsafe-argument to cover this.

@Raynos
Copy link
Author
Raynos commented May 2, 2020

That being said a rule like no-unsafe-argument might be useful to protect against

declare function foo(x: any): void

const y = 'text'
foo(y)
const z = 42
foo(z)

Aka the potential type error of passing a well typed value into an argument who's type is any . it's undefined behavior whether foo handles both string & number.

@bradzacher
Copy link
Member

my general thinking with the no-unsafe-* rules has been to create new rules to let people progressively opt-in to the checks.

arguments are a funny thing. They could be included in either no-unsafe-call or no-unsafe-assignment:

  • no-unsafe-call "feels" right because the arguments belong to the call
  • no-unsafe-assignment works as well logically because the checks required for it are the same as the checks already done by the rule (get contextual type and check for any), and arguments are essentially an assignment.
    • I'll admit that this one is probably confusing for the end user.

TBH I think that a whole new rule is probably not worth it just for arguments, I think the best place would be adding them to no-unsafe-call.

Merging this into #791

@Raynos
Copy link
Author
Raynos commented May 2, 2020

Would it be possible to keep this issue open as it’s a “bug” in “no-unsafe-call” and not a new rule that needs to be created.

The issue that is open which marks this issue as duplicate also does not mention this issue in the body as an action item

@bradzacher bradzacher mentioned this issue May 2, 2020
Closed
@bradzacher
Copy link
Member

it's not a bug though? it's a feature request.

This feature is one of the two reasons #791 is still open.
I've stated explicitly within that issue that this needs to be addressed still.

@Raynos
Copy link
Author
Raynos commented May 2, 2020

Yeah, it's a feature request. Thanks for clarifying.

@bradzacher bradzacher mentioned this issue May 6, 2020
Closed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for team members to take a look
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Raynos @bradzacher
0