Skip to content

Apply the 'no-unused-variable' tslint rule to our codebase #6140

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

Merged
merged 20 commits into from
Dec 18, 2015

Conversation

DanielRosenwasser
Copy link
Member

There is also a check-parameters option, but that would've been a much bigger, less reviewable change.

@yuit
Copy link
Contributor

yuit commented Dec 18, 2015

That is lots of unused stuffs

@@ -583,7 +555,9 @@ namespace ts.server {
}

handleProjectFilelistChanges(project: Project) {
const { succeeded, projectOptions, error } = this.configFileToProjectOptions(project.projectFilename);
// TODO: Ignoring potentially returned 'error' and 'succeeded' condition
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this Todo for? you already ignore error and succeeded?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess if there's a problem, it'll get fixed at that point. No sense leaving the comment.

@yuit
Copy link
Contributor

yuit commented Dec 18, 2015

👍

DanielRosenwasser added a commit that referenced this pull request Dec 18, 2015
Apply the 'no-unused-variable' tslint rule to our codebase
@DanielRosenwasser DanielRosenwasser merged commit 22856de into master Dec 18, 2015
@DanielRosenwasser DanielRosenwasser deleted the noUnused branch December 18, 2015 07:56
@weswigham
Copy link
Member

👍

if (defaultLibFileName) {
for (const current of declarations) {
const sourceFile = current.getSourceFile();
// TODO (drosen): When is there no source file?
Copy link
Member

Choose a reason for hiding this comment

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

To answer the comment: When you've fooled the language service by injecting a declaration node which is not parented to anything, yet is associated with a symbol returned by getSymbolAtLocation - which could conceivably happen if you wrapped the language service and injected declarations for templated components, but have not rewritten the getSourceFile implementation to do something other than walk up the tree? TBH, it probably never happens under normal conditions and is likely exceptional - every node should be nested within a source file in a well-behaved program. Maybe it would be more correct to toss in a Debug.fail here, despite that not being the old behavior?

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
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.

4 participants