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

tslint saying "Fixes caused errors in ..." even though no fixes are asked #2400

Closed
victornoel opened this issue Mar 24, 2017 · 7 comments
Closed

Comments

@victornoel
Copy link

Bug Report

  • TSLint version: 4.5.1
  • TypeScript version: 2.2.1
  • Running TSLint via: CLI

TypeScript code being linted

I can't find a way to make the problem smaller, but basically:

  1. clone https://[email protected]/linagora/petals-cockpit.git (at 3175a1e4 for reproducibility
  2. go into the frontend directory
  3. run yarn or npm install (the first being better, also for reproducibility).
  4. edit a file and add an unused import (for example change line 18 of src/app/core/core.module.ts to be import { NgModule, Component } from '@angular/core';

Actual behavior

[victor@lasagna frontend]$ ./node_modules/.bin/tslint --project src/tsconfig.app.json --type-check --exclude "**/node_modules/**" -c tslint.json 
Fixes caused errors in /home/victor/code/eclipse/petals-cockpit/frontend/src/app/core/core.module.ts
[victor@lasagna frontend]$ echo $?
0
[victor@lasagna frontend]$ 

Note that I don't even asked for fixes to be applied and that the return code is not a failure as it should.

Expected behavior

The error being properly detected, printed and the return code being 1.

@ajafff
Copy link
Contributor

ajafff commented Mar 24, 2017

That's a leftover debug message coming from no-unused-variable.

Some background:
The rule searches for unused imports and internally applies fixes for each one to see if it results in a compiler error. When the compilation fails with the applied fix, there will be no warning for that import. That is done to work around microsoft/TypeScript#5711

The rule got refactored recently which removed the log message. This change will be shipped with [email protected]

@victornoel
Copy link
Author

@ajafff I'm not sure I understand. Does it mean that tslint will be buggy with type checking until v5 is released?

@ajafff
Copy link
Contributor

ajafff commented Mar 24, 2017

The rule works quite well right now. The message you are seeing is just printed when a certain condition is met (that is an import is not used, but needed by typescript's declaration emitter microsoft/TypeScript#5711). That doesn't mean there is anything wrong, it's just an unnecessary log message.

@victornoel
Copy link
Author

@ajafff it does not, tslint does not return an error as it should: there is a linting error in the file and when I disable type checking, tslint does return an error!

@ajafff
Copy link
Contributor

ajafff commented Mar 24, 2017

Ah, now I get it.
Yes, that's a limitation we cannot work around. We need the type checker to tell if the import is used implicitly.
The new implementation that will be released with v5 - as it is right now - does not work without type checking. At least it will not show inconsistent error messages.
I hope to find the time to make it work without the type checker again, but then it will not complain about unused imports unless you enable type checking

@victornoel
Copy link
Author

We will see how it is with v5, but until then, for people like me using v4, I don't really understand why we can't do something about the buggy behaviour...

If there is a problem during the checking of the rule, the rule should fail, not be skipped and the linting error ignored.

@adidahiya
Copy link
Contributor

closing as fixed in 5.x

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

No branches or pull requests

3 participants