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

TSLint won't pick up additional rules. #909

Closed
cerebrl opened this issue Jan 13, 2016 · 20 comments
Closed

TSLint won't pick up additional rules. #909

cerebrl opened this issue Jan 13, 2016 · 20 comments

Comments

@cerebrl
Copy link

cerebrl commented Jan 13, 2016

I originally reported this issue at Microsoft's additional TSLint rules, but maybe it's better reported here:

TSLint works fine, but it won't pick up the rules supplied by this module. Here's what I've done:

  1. npm install tslint-microsoft-contrib --save-dev
  2. Tried multiple ways of adding the rules directory:
    1. In tslint.json added "rulesDirectory": "node_modules/tslint-microsoft-contrib" as a root property.
    2. In package.json, where I'm running this as a NPM script to the script property, I wrote tslint --rules-dir node_modules/tslint-microsoft-contrib index.js
  3. Added the flags to the tslint.json file that this module provides within the rules property.

The bottom line, I could not get this to work for the life of me using this within an NPM script. Is that what the problem is? Can you just not pick these rules up from with the context of running this within NPM? Any help would be appreciated. Thanks.

@jkillian
Copy link
Contributor

@cerebrl What version of TSLint are you using? I'd recommend trying v3.2.1 if you're not on it already.

The --rules-dir or rulesDirectoy property should point to the directory with the built .js rules files. I know this can be non-obvious at first.

@u840903
Copy link

u840903 commented Jan 14, 2016

I have the same problem and I have created a repo where it manifests. I an using tslint 3.2.1.

Here is my package.json:
https://github.com/janjarfalk/ionic2-angular2-typescript-boilerplate/blob/master/package.json

Here is my tslint.json:
https://github.com/janjarfalk/ionic2-angular2-typescript-boilerplate/blob/master/tslint.json

The "export-name-rule is not picked up. When I run npm run lint

@jkillian
Copy link
Contributor

I think this is a bug with how TSLint handles paths - working on fixing it now. In the meantime, things will work correctly if you install TSLint globally and just run tslint -s tslint/formatters -t color --config tslint.json app/{pages,services}/**/*.ts from the command line you'll get the correct results.

@adidahiya
Copy link
Contributor

I think the reason it isn't working for folks using npm scripts is that the tslint command in an npm script references node_modules/.bin/tslint, so it tries to look for node_modules/.bin/tslint/node_modules/tslint-microsoft-contrib and doesn't find any custom rules.

I filed #910 and #911 to improve this experience.

@HamletDRC
Copy link
Contributor

I got this to work with no issues on @janjarfalk 's repo.

I had to change his script to :

  "scripts": {
    "lint": "tslint -s tslint/formatters -t color --config tslint.json app/**/*.ts -r node_modules/tslint-microsoft-contrib/",
    "validate": "npm ls"
  },

The -r option picked up the extra rules and the ** glob (instead of the {pages,services} glob) picked up the .ts files.

@HamletDRC
Copy link
Contributor

Two people now have tried to put tslint options within the rules.json file... which will not work. Perhaps clarifying this in the readme is a good idea.

@jkillian
Copy link
Contributor

Actually @HamletDRC, it theoretically should work. There are some strange bugs right now with how relative paths are handled though. I've hopefully fixed that all in #912 :)

@cerebrl
Copy link
Author

cerebrl commented Jan 16, 2016

I'd like to update this thread with the info that everything is now working.

I removed the "rulesDirectory": "node_modules/tslint-microsoft-contrib" property from the tslint.json and just use the -r command to point to node_modules/tslint-microsoft-contrib.

I can confirm that, at least for me, adding the rulesDirectory property back to the tslint.json does reliably produce the original issue reported here.

Thanks to everyone for the assistance!

@jkillian
Copy link
Contributor

@cerebrl @janjarfalk: We just released TSLint v3.2.2 which should fix this bug. If you specify a custom rules directory on the command line, the path is treated as relative to the current working directory (as it was before). However, if you specify a path in a tslint.json file, the path is now relative to the location of the tslint.json file (which makes things much more intuitive in my opinion.)

I went through and tested out your repo with v3.2.2 @janjarfalk and found that running npm run lint now works as expected. Try things out yourself though and let me know if you encounter any bugs or issues!

@u840903
Copy link

u840903 commented Jan 19, 2016

@jkillian: Thank you!

But I think you introduced a bug.
Array of paths doesn't work as expected.

"rulesDirectory": ["node_modules/tslint-microsoft-contrib"]
// Works as expected.
"rulesDirectory": ["tslint/rules"]
// Works as expected.
"rulesDirectory": ["node_modules/tslint-microsoft-contrib", "tslint/rules"]
// Warnings from `tslint-microsoft-contrib` ONLY.
"rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"]
// Warnings from `tslint/rules` ONLY.

@jkillian
Copy link
Contributor

@janjarfalk Interesting, although I'm not convinced this is a bug. As soon as TSLint finds an invalid path, it throws an error and aborts. I'm not sure it seems worth it to me to do anything fancier than that... Do you think different behavior would be better?

@u840903
Copy link

u840903 commented Jan 19, 2016

I think you misunderstood. Both tslint/rules and node_modules/tslint-microsoft-contrib are valid paths. Right now it only picks up additional rules from the first path in the array.

"rulesDirectory": ["node_modules/tslint-microsoft-contrib"]
// If this result in warnings...
"rulesDirectory": ["tslint/rules"]
// ...and this doesn't result in any warnings.
"rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"]
// I still expect this to result in warnings, but it doesn't.

I have updated this repo https://github.com/janjarfalk/ionic2-angular2-typescript-boilerplate.git with a setup that reproduces the problem running npm run lint.

Case A - tslint-microsoft-contrib first:

// tslint.json
{
  "rulesDirectory": ["node_modules/tslint-microsoft-contrib", "tslint/rules"],
  "rules": {
    "missing-jsdoc": true, // A rule form Microsoft
    "export-class-name": true // A rule from me (tslint/rules folder)
  }
}
// ACUTAL output

   app/MyApp.ts 
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts
// EXPECTED output

   app/MyApp.ts 
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts

   app/pages/page1/page1.ts 
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1

Case B - tslint/rules first:

// tslint.json
{
  "rulesDirectory": ["tslint/rules", "node_modules/tslint-microsoft-contrib"],
  "rules": {
    "missing-jsdoc": true, // A rule form Microsoft
    "export-class-name": true // A rule from me (tslint/rules folder)
  }
}
// ACTUAL output

   app/pages/page1/page1.ts 
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1
// EXPECTED output

   app/MyApp.ts 
-- [1, 1] - File missing JSDoc comment at the top-level: app/MyApp.ts

   app/pages/page1/page1.ts 
-- [1, 1] - The exported module name must match the file name. Found: app/pages/page1/page1.ts and Page1

@jkillian
Copy link
Contributor

Thanks for the detailed explanation @janjarfalk, I indeed did misunderstand you. This definitely seems like a bug, I'll look into it!

@jkillian jkillian reopened this Jan 21, 2016
@adidahiya
Copy link
Contributor

That's a separate issue, I've filed #928 for it.

@jkillian
Copy link
Contributor

@janjarfalk This should be fixed now in v3.3.0. As always, let us know if you come across any other issues 😃

@u840903
Copy link

u840903 commented Jan 23, 2016

@jkillian Awesome! Will do! Happy to help!

@alexanderbird
Copy link

It may help someone down the road: after reading the source, I learned that custom rules are loaded in the following way:

  1. Take the setting name in tslint.json
  2. remove leading and trailing punctuation
  3. camelize the name
  4. append 'Rule'
  5. append '.js'
  6. Attempt to require that file from each rulesDirectory

So "foo-bar": true in tslint.json will result in a require('yourRulesDirectory/fooBarRule.js')

@adidahiya
Copy link
Contributor

@alexanderbird that should mostly be covered by the "Important Conventions" section in the README and docs site. open to PRs if you think the docs could be improved.

@ghost
Copy link

ghost commented Sep 5, 2017

@alexanderbird Thank you! I started by looking here https://github.com/mgechev/codelyzer and didn't see any instructions for naming conventions. As soon as I changed the name of my rule, it started working! Probably there is a doc update on both projects that are necessary.

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Sep 15, 2020
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

7 participants