feat(eslint-plugin): add config all.json

[ldrick]

Fixes: #298
Fixes: #353

This PR fixes possibility to run recreation of shared configs with ts-node and adds all.json as new shared config.

[ldrick]

Don't know, why it fails 🔨
Please can someone have a look and help me finding the issue. My commits didn't change anything, which should lead to:

[command]/bin/bash --noprofile --norc /home/vsts/work/_temp/3c03136b-737b-4892-bb7e-ac0cee6b5046.sh
yarn run v1.13.0
$ eslint . --ext .js,.ts
Error: Error while loading rule '@typescript-eslint/promise-function-async': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.
    at Object.getParserServices (/home/vsts/work/1/s/packages/eslint-plugin/dist/util/getParserServices.js:14:15)
    at create (/home/vsts/work/1/s/packages/eslint-plugin/dist/rules/promise-function-async.js:65:37)
    at Object.create (/home/vsts/work/1/s/packages/eslint-plugin/dist/util/createRule.js:15:20)
    at createRuleListeners (/home/vsts/work/1/s/node_modules/eslint/lib/linter.js:577:21)
    at Object.keys.forEach.ruleId (/home/vsts/work/1/s/node_modules/eslint/lib/linter.js:731:31)
    at Array.forEach (<anonymous>)
    at runRules (/home/vsts/work/1/s/node_modules/eslint/lib/linter.js:689:34)
    at Linter._verifyWithoutProcessors (/home/vsts/work/1/s/node_modules/eslint/lib/linter.js:891:31)
    at preprocess.map.textBlock (/home/vsts/work/1/s/node_modules/eslint/lib/linter.js:940:35)
    at Array.map (<anonymous>)
error Command failed with exit code 2.

Resolved 😅

[codecov]

Codecov Report

Merging #313 into master will increase coverage by 0.29%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #313      +/-   ##
==========================================
+ Coverage   95.52%   95.82%   +0.29%     
==========================================
  Files          89       90       +1     
  Lines        4112     4164      +52     
  Branches     1153     1153              
==========================================
+ Hits         3928     3990      +62     
+ Misses         82       72      -10     
  Partials      102      102

Impacted Files	Coverage Δ
packages/eslint-plugin/src/rules/ban-ts-ignore.ts	100% <ø> (ø)	⬆️
.../eslint-plugin/src/rules/promise-function-async.ts	100% <ø> (ø)	⬆️
packages/eslint-plugin/src/rules/unbound-method.ts	96.87% <ø> (ø)	⬆️
...ages/eslint-plugin/src/rules/no-require-imports.ts	100% <ø> (ø)	⬆️
packages/eslint-plugin/src/index.ts	100% <100%> (+100%)	⬆️
packages/eslint-plugin/src/rules/index.ts	100% <100%> (ø)
...es/eslint-plugin/src/configs/eslint-recommended.ts	100% <0%> (+100%)	⬆️

[bradzacher]

A few things:

• How come you dropped requireindex in favour of using a manually written barrel file? I like that you have a test which will help prevent people from forgetting to do it when adding new rules - just wondering why.

• IMO instead of hard coding the list of disabled rules in base.json, we should add it as a config into the createRule function. This way it's explicitly defined by the rule itself instead of being an implicit connection we need to remember to maintain.
  @typescript-eslint/core-team let me know if you agree here.

createRule, and its return type is RuleModule

would suggest specifically adding a new property to RuleMetaDataDocs, say

interface RuleMetaDataDocs {
  // ....

  /**
   * Set to true if the rule overrides (i.e. replaces) the base eslint rule of the same name. This will cause the associated base rule to be disabled when generating the shared configs.
   */
  overridesBaseRule?: true
}

• finally, I would move base.json into the generated configs themselves. If we were writing these configs by hand - I would agree that it's good to follow DRY, but considering that we're generating the configs, IMO this extra layer of indirection just makes it harder to figure out exactly what each of the configs does.

[ldrick]

Hi @bradzacher , thanks for reviewing.

How come you dropped requireindex in favour of using a manually written barrel file? I like that you have a test which will help prevent people from forgetting to do it when adding new rules - just wondering why.

I tried to shortly explain on @armano2 's review, but I see it collapsed due to resolving the request.
When I came over to write tests for all.json, I noticed after looking into requireindex, that it requires the dist/rules not src/rules and I was not happy testing on dist. So I tried to load src/rules, which didn't work, due to requireindex doesn't work with .ts files. Later following that path of dist/rules, I noticed, that requireindex doesn't recognize export.default - which would have made me comment more things. So finally I came to the decision to come up with a way which doesn't do anything implicit (or with lots of comments) and had a look into eslint, which also provides all rules with an index file. I still hope the index.ts is for the best for future maintainers and requires not so much knowledge on how requireindex works. I even tried to provide a requireindexts, which I've written locally, but ended up with using import(), which would have been async and might have broken some things in how dist/rules work. But please let me know, if this is not the intended way, I am willing to change it for the best of maintainers.

IMO instead of hard coding the list of disabled rules in base.json, we should add it as a config into the createRule function. This way it's explicitly defined by the rule itself instead of being an implicit connection we need to remember to maintain.
@typescript-eslint/core-team let me know if you agree here.

createRule, and its return type is RuleModule

would suggest specifically adding a new property to RuleMetaDataDocs, say

interface RuleMetaDataDocs {
  // ....

  /**
   * Set to true if the rule overrides (i.e. replaces) the base eslint rule of the same name. This will cause the associated base rule to be disabled when generating the shared configs.
   */
  overridesBaseRule?: true
}

I'll wait for an answer here and sure I'm willing to change it.
🔜

finally, I would move base.json into the generated configs themselves. If we were writing these configs by hand - I would agree that it's good to follow DRY, but considering that we're generating the configs, IMO this extra layer of indirection just makes it harder to figure out exactly what each of the configs does.

Sure, I'll do that, so every config comes from generate-configs.ts
✔️

[VincentLanglet]

@ldrick Hi ! I'd love to use this all config. Keep the good work !
You actually have conflict.

stale

[bradzacher]

code LGTM.
one required change with the base config. few nitpicks.

Great work.

[VincentLanglet]

What is missing to use the all config ? There is nothing in the doc: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/README.md

[bradzacher]

We just never added documentation.
It's in there and up for use. It's by no means a perfect configuration because there are some overlaps in the rules, but it works.