Skip to content

Using arguments should be an error with noImplicitAny #25285

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.

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

Closed
bcherny opened this issue Jun 28, 2018 · 9 comments
Closed

Using arguments should be an error with noImplicitAny #25285

bcherny opened this issue Jun 28, 2018 · 9 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@bcherny
Copy link

bcherny commented Jun 28, 2018

TypeScript Version: 3.0.0-dev.20180628

Search Terms: argument, parameter, arguments, list, any, untyped

Code

function sumVariadic() {
  return Array
    .from(arguments)
    .reduce((total, n) => total + n, 0) // any
}

Expected behavior: TSC should complain that total, n, and sumVariadic's return type are implicitly any when noImplicitAny=true.

Actual behavior: total, n, and sumVariadic's return type are any. No errors are thrown.

Playground Link: https://www.typescriptlang.org/play/index.html#src=function%20sumVariadic()%20%7B%0A%20%20return%20Array%0A%20%20%20%20.from(arguments)%0A%20%20%20%20.reduce((total%2C%20n)%20%3D%3E%20total%20%2B%20n%2C%200)%0A%7D%0A

Related Issues:

@bcherny bcherny changed the title arguments should be forbidden with noImplicitAny Using arguments should be an error with noImplicitAny Jun 28, 2018
@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jun 28, 2018
@RyanCavanaugh
Copy link
Member

None of these expressions meet the criteria of noImplicitAny

@bcherny
Copy link
Author

bcherny commented Jun 28, 2018

@RyanCavanaugh I see, because they’re not declarations? What would it take for TSC to catch these? The issue here is that arguments is unsafe, and a user wouldn’t know about it.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jun 28, 2018

@bcherny basically there's a difference between an any that arises from accessing some member (i.e. indexing into an arguments whose index signature returns any), and something where an any implicitly arises from. Last I recall, an implicit any comes from

  1. Any location that has no type annotation or initializer where contextual typing could not provide a type.
  2. Any location where null and undefined widen to any (doesn't happen in strictNullChecks though).

You'd really need a new type like implicitany that does widen to any and causes errors, but I don't know how desirable that is in lib.d.ts when users expected their code to work despite the any.

@RyanCavanaugh
Copy link
Member

Even if implicitAny existed as a type, what could you do with it? Presumably const x = arguments; would be an illegal statement because now arguments has "infected" x?

What would be an error under this schema that isn't an error under a lint rule banning referencing arguments at all, or vice versa?

@bcherny
Copy link
Author

bcherny commented Jun 28, 2018

Got it. For anyone that finds this thread in the future, the issue is that arguments' type is IArguments, which includes this index signature: [index: number]: any.

I'll use a lint rule for now (doesn't look like TSLint has one already, so will add one). I wonder if TSC should have a mode where it's more aggressive about banning any in general, in addition to noImplicitAny. A couple of ideas:

  • Update the index signature for IArguments to unknown as part of TS 3.0
  • A new noUnsafeTypes keyword, like Flow's strict mode, that bans any, Object, Function, and {}

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 28, 2018

@bcherny probably worth referencing the strictAny thread #24737

@bcherny
Copy link
Author

bcherny commented Jun 29, 2018

@RyanCavanaugh Thanks for linking that thread, and thanks to the TS team for documenting the discussion. Do you guys plan to replace any with unknown in lib.d.ts where appropriate, eg. for IArguments?

@DanielRosenwasser
Copy link
Member

Do you guys plan to replace any with unknown in lib.d.ts where appropriate, eg. for IArguments?

No, unknown is strictly a top type and we did consider making it more flexible, but found it would be even more confusing.

But really this is the sort of perfect situation where you want to defer to a library consumer's strictness settings as to how a type can be used.

@NN---
Copy link

NN--- commented Jun 30, 2018

You can try TSLint no-unsafe-any rule
https://palantir.github.io//tslint/rules/no-unsafe-any/

@bcherny bcherny closed this as completed Jun 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants