feat: make cli expect array for rules dir

[ggarek]

PR checklist

• Addresses an existing issue: TsLint rule directory accepting single path for directory #3774
• bugfix
  • Includes tests
• Documentation update

Overview of change:

Is there anything you'd like reviewers to focus on?

It seems like the codebase is ready for both string and string[] for rulesDirectory, hence the change is just a cli option creation. (I have traced it back to Linter > getEnabledRules > arrayify)

⚠️ It seems, also, that there are no tests for this use case, at least i haven't found one.

✅ The current test suit passes.

CHANGELOG.md entry:

[palantirtech]

Thanks for your interest in palantir/tslint, @ggarek! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

[ggarek]

It seems the reason why CI fails is

yarn add typescript@next

TS (typescript@2.8.0-dev.20180321) now reports more errors 😉

[giladgray]

@ggarek isn't it string | string[] (per PR description)? can we support both types in the schema?

[ggarek]

@giladgray
Not sure i've understood correctly, but

These are cli option definitions. The type property is handled by the cli code basically adding collect coercion to commander. (and we can not actually use string | string[] here, these are typescript types, but we have just value of type type: "string" | "boolean" | "array";)

It seems like the codebase is ready for both string and string[] for rulesDirectory, hence the change is just a cli option creation. (I have traced it back to Linter > getEnabledRules > arrayify)

Considering this, CLI should be ready to handle both string and string[] values.

[mayankku]

Any updates on this?

[giladgray]

@ggarek there is a conflict with master, we just renamed the file to tslintCli.ts in #3978.

to confirm, how would one pass an array for this CLI option? I couldn't find specifics on the commander docs. is it like tslint -r one -r two -r three?

[mayankku]

@giladgray this PR seems to be stuck. Is there any way to get this merged?

Re your question: you're right about passing repeated args. It seems a collect function is provided to Commander to push args into an array:

tslint/src/tslintCli.ts

Line 298 in 1c31dc0

function collect(val: string, memo: string[]) {

[johnwiseheart]

@ggarek can you please fix the merge conflict and then we can merge this in?

[ggarek]

Repeated the patch on the latests master.

⚠️ I have change one line of code only, but the PR has many changes due to code formatting on pre-commit hook. It has happened probably, because there were no changes to the file after code formatting was introduced. Please, let me know if you want me to commit the patch in any other specific way.

Here is a check, i have made locally, to confirm if the cli works (both with rules directories defined in configuration file and passed via cli options)

the test project

➜ tree test-rule-dirs-array
test-rule-dirs-array
├── custom-rules
│   ├── 1
│   │   ├── oneTwoRule.d.ts
│   │   ├── oneTwoRule.js
│   │   └── oneTwoRule.ts
│   ├── 2
│   │   ├── threeFourRule.d.ts
│   │   ├── threeFourRule.js
│   │   └── threeFourRule.ts
│   └── tsconfig.json
├── index.ts
├── tsconfig.json
└── tslint.json

the index.ts

import t from 'typescript';

const a: number = 1;

tslint.json

{
    "extends": [],
    "x_rulesDirectory": ["./custom-rules/1", "./custom-rules/2"],
    "rules": {
        "one-two": true,
        "three-four": true
    }
}

test-case 1: pass two rule dirs

➜ ../bin/tslint -r ./custom-rules/1 -r ./custom-rules/2 index.ts

ERROR: index.ts[1, 1]: import statement forbidden
ERROR: index.ts[1, 8]: 'short identifier names are forbidden
ERROR: index.ts[3, 7]: 'short identifier names are forbidden

tests-case 2: pass only one rule dir

➜ ../bin/tslint -r ./custom-rules/1 index.ts

Could not find implementations for the following rules specified in the configuration:
    three-four
Try upgrading TSLint and/or ensuring that you have all necessary custom rules installed.
If TSLint was recently upgraded, you may have old rules configured which need to be cleaned up.


ERROR: index.ts[1, 1]: import statement forbidden

test-case 3: do not pass rule dirs

➜ ../bin/tslint index.ts

Could not find implementations for the following rules specified in the configuration:
    one-two
    three-four
Try upgrading TSLint and/or ensuring that you have all necessary custom rules installed.
If TSLint was recently upgraded, you may have old rules configured which need to be cleaned up.

No valid rules have been specified for TypeScript files

test-case 4: defined rule dirs in configuration (tslint.json is changed)

tslint.json

{
    "extends": [],
    "rulesDirectory": ["./custom-rules/1", "./custom-rules/2"],
    "rules": {
        "one-two": true,
        "three-four": true
    }
}

test case

➜ ../bin/tslint index.ts

ERROR: index.ts[1, 1]: import statement forbidden
ERROR: index.ts[1, 8]: 'short identifier names are forbidden
ERROR: index.ts[3, 7]: 'short identifier names are forbidden

@johnwiseheart @giladgray

I would be glad to have this fix merged, plz let me know if i have to do anything else to make it happen 🙏

[JoshuaKGoldberg]

Hey @ggarek, sorry for the delay! You're right that this is a good, simple fix plus a bunch of auto-added formatting. We can ignore that - the problem is external to this PR. #4229