Re-enable the unused variables tslint rule for the hygiene task fixes #42157

[egamma]

This Pull request enables the tslint rule for unused-variables. This is done by performing the tslint checks in the context of a Typescript program/project with the tslint rule for unused-variables enabled. Since the checks have to be done in the context of a TS project I currently define the src folder to be this project, this means extensions and tests are no longer tslinted as part of the hygiene.

The tslint rule had some false positive issues in the tslint version 5.8, so this PR also updates tslint to version 5.9.

[joaomoreno]

@egamma Looks good. Should we still keep the old tslint filter on the test/** and extension/** files?

[egamma]

@joaomoreno

Should we still keep the old tslint filter on the test/** and extension/** files?

The unused variable rule requires that linting is done in the context of a project. Currently the only project that hygiene knows about is the src project. It would be possible to add know how about the other projects in the test and extensions folder:

• in the test folder there is a single project for the smoke test
• in the extensions folder there is a project for each extension, that has a tsconfig.json (git, emmet, ...) about 10 of them.

Thinking about this again we should also run hygiene/tslint on the extensions. I´ll look into it.

[egamma]

@joaomoreno enabled linting of extensions and tests. Gave a heads-up to extension owners to enable the noUnusedLocals in the tsconfig.json.

[joaomoreno]

Looks good to me!

[bpasero]

@egamma @joaomoreno fyi after this change landed I feel that git-commit takes easily 3-5 times as long as before, could that be related?

[egamma]

@bpasero @joaomoreno the slow down is caused by tslint creating a project for the changed files, so that the no-unused-variables rule can be applied. Creating the program context depends on the dependencies of the source file. For files inside the base this takes below 1s. For files from an upper layer like the ones you used in commit 442903c this takes 12s. This is a one time cost and the cost is amortized for many files, but in a commit you usually do not have many files and hygiene runs over a small number of files only.

Since the goal is that the hygiene check is fast this setup takes too long and we should roll back and not create a program context for running tslint.

[bpasero]

@egamma as someone that commits very often right after making a change, I really cannot work with this new behaviour and would like to disable it.

[egamma]

@bpasero agreed I´ve disabled it.

[bpasero]

Thanks 👍

[joaomoreno]

Too bad... I think we're just going to have to bite unused variables as compiler errors, which we don't prevent from committing right now anyway.