Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Add no-let-undefined rule #2100

Merged
merged 2 commits into from
Jan 22, 2017
Merged

Conversation

andy-hanson
Copy link
Contributor

PR checklist

  • Addresses an existing issue: #0000
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

What changes did you make?

Added the no-let-undefined rule, which forbids let x = undefined;, preferring let x;.

public static metadata: Lint.IRuleMetadata = {
ruleName: "no-let-undefined",
description: "Forbids a 'let' statement to be initialized to 'undefined'.",
optionsDescription: "Not configurable.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasFix: true

@@ -0,0 +1,9 @@
let x: string | undefined = undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add arg initializer test?
add const x = undefined; which shouldn't fail lint

Copy link
Contributor

@nchen63 nchen63 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@nchen63 nchen63 merged commit 59c312e into palantir:master Jan 22, 2017
@andy-hanson andy-hanson deleted the no_let_undefined branch January 22, 2017 19:10
@ajafff
Copy link
Contributor

ajafff commented Jan 22, 2017

IMO this rule is a bit too specialized.
What about var? Why not check parameters and destructuring? Then it could also be renamed to no-unnecessary-initializer

@andy-hanson
Copy link
Contributor Author

andy-hanson commented Jan 22, 2017

Yeah, this could work for var.

It can't be extended to parameters, though: function f(x: string | undefined = undefined) will allow f(), but function f(x: string | undefined) will not. (Besides, one should use x?: string anyway.)

For destructuring you mean to warn on this?

declare function f(): { x?: number };
const { x = undefined } = f(); // Could be `const { x } = f()`

@ajafff
Copy link
Contributor

ajafff commented Jan 22, 2017

Didn't know there is this difference for parameters. So the checking would be limited to optional parameters only ...

Yes, that's what I had in mind for destructuring. All of the following examples should result in a failure:

let {foo = undefined} = someObject; // doesn't need initializer, always defaults to undefined
let [foo = undefined] = someArray; // same as above

let {foo = undefined} = {}; // is valid only if there is an initializer, but this is either by mistake or really bad practice to declare variables
let [foo = undefined] = []; // same as above 

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants