Skip to content
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

💅 Evolving any is considered implicit any by noImplicitAnyLet #1883

Closed
1 task done
bgenia opened this issue Feb 21, 2024 · 9 comments
Closed
1 task done

💅 Evolving any is considered implicit any by noImplicitAnyLet #1883

bgenia opened this issue Feb 21, 2024 · 9 comments
Assignees
Labels
A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature

Comments

@bgenia
Copy link

bgenia commented Feb 21, 2024

Environment information

CLI:
  Version:                      1.5.3
  Color support:                true

Platform:
  CPU Architecture:             x86_64
  OS:                           linux

Environment:
  BIOME_LOG_DIR:                unset
  NO_COLOR:                     unset
  TERM:                         "xterm-256color"
  JS_RUNTIME_VERSION:           "v21.5.0"
  JS_RUNTIME_NAME:              "node"
  NODE_PACKAGE_MANAGER:         "npm/10.4.0"

Biome Configuration:
  Status:                       Loaded successfully
  Formatter disabled:           true
  Linter disabled:              false
  Organize imports disabled:    false
  VCS disabled:                 true

Workspace:
  Open Documents:               0

Rule name

lint/suspicious/noImplicitAnyLet

Playground link

https://biomejs.dev/playground/?code=LwAvACAARQB2AG8AbAB2AGkAbgBnACAAYQBuAHkAIABpAHMAIABjAG8AbgBzAGkAZABlAHIAZQBkACAAaQBtAHAAbABpAGMAaQB0ACAAYQBuAHkACgBsAGUAdAAgAGIACgBiACAAPQAgADEACgAKAC8ALwAgAEUAdgBvAGwAdgBpAG4AZwAgAGEAcgByAGEAeQAgAGkAcwAgAGkAZwBuAG8AcgBlAGQACgBjAG8AbgBzAHQAIABhAHIAcgAgAD0AIABbAF0ACgBhAHIAcgAuAHAAdQBzAGgAKAAxACkACgBhAHIAcgAuAHAAdQBzAGgAKAAiAGgAZQBsAGwAbwAiACkACgA%3D

Expected result

Evolving any

let b
b = 1

This case should be only marked as "implicit any" if noImplicitAny tsconfig option is set to false. If the purpose of this rule is specifically to forbid using evolving any I suggest at least changing the rationale in the docs because currently it does not make sense.

The docs say that this is a "dangerous escape hatch" and typescript's noImplicitAny doesn't report this case, this is because it's not actually an any. This is a separate feature which allows inferring variable type from assignments. This is type safe.

Example 1
Example 2

See microsoft/TypeScript#11263 microsoft/TypeScript#45369 microsoft/TypeScript#54414

Evolving arrays

This behavior also exists in a form of evolving arrays, for which biome does not check.

const arr = []
arr.push(1)
arr.push("hello")

Example 1
Example 2

If this rule is meant to ban evolving types, I suggest banning this too.

See microsoft/TypeScript#11432 microsoft/TypeScript#43752

Code of Conduct

  • I agree to follow Biome's Code of Conduct
@Conaclos
Copy link
Member

Good point. We should certainly rename the rule and add more cases.

@Conaclos Conaclos added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature labels Feb 21, 2024
@bgenia
Copy link
Author

bgenia commented Feb 22, 2024

Another case of control flow inference for which biome does not check:

class Example {
       b
    // ^? inferred as number

    constructor() {
        this.b = 2
    }
}

Playground
Example 1
Example 2

This is practically the same case as implicitAnyLet

@fujiyamaorange
Copy link
Contributor

@Conaclos
May I try this issue?

@Conaclos
Copy link
Member

@fujiyamaorange
Which changes would you like to introduce? It is not clear to me for now...

@fujiyamaorange
Copy link
Contributor

fujiyamaorange commented Mar 11, 2024

I plan to redefine this case as valid because it seems a kind of evolving any type according to @bgenia .

let someVar1;
someVar1 = '123';

I also referred this article.

@Conaclos
Copy link
Member

Conaclos commented Mar 11, 2024

I plan to redefine this case as valid because it seems a kind of evolving any type

This could make the rule useless?

Instead, I could create a new rule noEvolvingAny and I could deprecate noImplicitAny (once noEvolvingAny is stabilized).
The new rule could report these cases:

let a; // the current `noImplicitAny`
const b = [];
let c = null;

If you would like to work on this new rule, you are welcomed :)

@fujiyamaorange
Copy link
Contributor

Oh, fantastic!
Thank you for explaining me about the problem. I understand.

Let me try this!

@fujiyamaorange
Copy link
Contributor

@Conaclos
Could you please assign me ?? 🙏

@unvalley
Copy link
Member

We have noEvolvingAny lint rule now, so I close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Linter Area: linter L-JavaScript Language: JavaScript and super languages S-Enhancement Status: Improve an existing feature
Projects
None yet
Development

No branches or pull requests

4 participants