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

no-shadowed-variable: ignore declaration files and ambient modules #3387

Merged
merged 1 commit into from
Nov 28, 2017
Merged

no-shadowed-variable: ignore declaration files and ambient modules #3387

merged 1 commit into from
Nov 28, 2017

Conversation

ajafff
Copy link
Contributor

@ajafff ajafff commented Oct 24, 2017

PR checklist

Overview of change:

[bugfix] no-shadowed-variable now excludes declaration files and ambient modules
Fixes: #3088

Is there anything you'd like reviewers to focus on?

CHANGELOG.md entry:

[bugfix] `no-shadowed-variable` now excludes declaration files and ambient modules
Fixes: #3088
@adidahiya adidahiya self-assigned this Oct 31, 2017
@@ -0,0 +1,7 @@
// no errors in declaration files
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm not sure about this -- why should we skip declaration files entirely? Is the idea that declaration files have to adhere to all the idiosyncrasies of their corresponding JS source and therefore can't help it if it has shadowed names, so it's impossible to avoid the shadowing unless the JS is fixed as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly. We skip declartation files for the same reason we ignore module augmentations: they model something that already exists. Like you said:

declaration files have to adhere to all the idiosyncrasies of their corresponding JS source and therefore can't help it if it has shadowed names, so it's impossible to avoid the shadowing unless the JS is fixed as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

cool, we're on the same page.

@adidahiya
Copy link
Contributor

merging because this passes the old status checks (before we split into multiple builds)

@adidahiya adidahiya merged commit 22ec110 into palantir:master Nov 28, 2017
HyphnKnight pushed a commit to HyphnKnight/tslint that referenced this pull request Apr 9, 2018
…alantir#3387)

[bugfix] `no-shadowed-variable` now excludes declaration files and ambient modules
Fixes: palantir#3088
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.

no-shadowed-variable: issue with interface extension
2 participants