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

tslint test does not handle CRLF when lint failures span multiple lines #4171

Closed
jamcoupe opened this issue Sep 17, 2018 · 2 comments
Closed

Comments

@jamcoupe
Copy link

Bug Report

When using tslint test tool on a *.ts.lint file encoded in CRLF that has multi line lint warnings, it will return that the file has failed linting.

  • TSLint version: 5.1.1 & master
  • TypeScript version: 2.9.2
  • Running TSLint via: CLI & master via "yarn test:rules"

Actual behavior

After encoding test/rules/forin/test.ts.lint in CRLF and running yarn test:rules

$ node ./build/test/ruleTestRunner.js

Testing Lint Rules:
...
test/rules/forin/test.ts.lint: Failed!
Expected (from .lint file)
Actual (from TSLint)
  function a() {
      for (var i in obj) {
-     ~~~~~~~~~~~~~~~~~~~~~
      ~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~~~
          console.log("i");
- ~~~~~~~~~~~~~~~~~~~~~~~~~~
  ~~~~~~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~~~~
      }
  ~~~~~  [for (... in ...) statements must be filtered with an if statement]
  
      for (var j in obj) {
-     ~~~~~~~~~~~~~~~~~~~~~
      ~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~~~
          if (j === 3) {
- ~~~~~~~~~~~~~~~~~~~~~~~
  ~~~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~
              console.log("j");
- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
          }
- ~~~~~~~~~~
  ~~~~~~~~~
- ~~~~~~~~~~
          console.log("j");
- ~~~~~~~~~~~~~~~~~~~~~~~~~~
  ~~~~~~~~~~~~~~~~~~~~~~~~~
- ~~~~~~~~~~~~~~~~~~~~~~~~~~
      }
  ~~~~~  [for (... in ...) statements must be filtered with an if statement]
  
      for (var k in obj) {
          if (obj.hasOwnProperty(k)) {
              console.log("k");
          }
      }
      
      for (var m in obj) {
          if (!obj.hasOwnProperty(m)) {
              continue;
          }
          console.log("m");
      }
      
      for (var n in obj) {
          if (!obj.hasOwnProperty(n)) continue;
          console.log("m");
      }
  }

Expected behavior

Either a better warning message the inform the user that they can not use CRLF.

or

$ node ./build/test/ruleTestRunner.js

Testing Lint Rules:
...
test/rules/forin/test.ts.lint: Passed
...

Happy to create a PR for this but would need to know what the projects expected behaviour is.

@JoshuaKGoldberg
Copy link
Contributor

Gosh darn was this annoying: microsoft/tslint-microsoft-contrib#633

One issue here is the linebreak-style rule. How can the test infrastructure not accidentally trample intentionally different line endings in a file? Should there be some way of indicating linebreak style in a test file?

For now, you can use .gitattributes and force changes to linebreak-style-related files as needed:

*.csv eol=lf
*.js eol=lf
*.jsx eol=lf
*.lint eol=lf
*.ts eol=lf
*.tsx eol=lf

@JoshuaKGoldberg
Copy link
Contributor

Per #4534 this seems no longer a priority. The .gitattributes hack is recommended for now.

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

2 participants