Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESLINTJS-65 Remove decorated rules from ESLint plugin #4925

Merged
merged 26 commits into from
Nov 29, 2024

Conversation

vdiez
Copy link
Contributor

@vdiez vdiez commented Nov 25, 2024

Changes:

  • Decorated rules are not part of the ESLint plugin.
  • All external eslint plugins are not part of eslint-plugin-sonarjs any longer.
  • meta.ts can be modified and will never be overwritten by generate-meta
  • generate-meta will now write to generated-meta.ts, which is imported by meta.ts
  • A preliminary meta.ts will be autogenerated first time when creating a new rule. Done in JS-421 Fix new-rule script #4917
  • The new available metadata will point which rules are original, external, or decorated.
  • We generate rule indexes automatically for both the bridge and the ESLint plugin, no need to maintain the indexes manually
  • All rules now contain a folder inside the rules folder, as we need the metadata to be visible for all rules.
  • README.md lists not only the ESLint plugin rules, but also points to what rules the user should look for if they want to have the full SQ experience

Fixes JS-336 as well

@vdiez vdiez force-pushed the remove-decorated-eslint branch 3 times, most recently from 1287a53 to a34ca13 Compare November 25, 2024 15:29
@vdiez vdiez force-pushed the remove-decorated-eslint branch from a34ca13 to f980c2d Compare November 25, 2024 15:48
@vdiez vdiez force-pushed the remove-decorated-eslint branch from 238deb6 to cf8f4a5 Compare November 25, 2024 18:57
@vdiez vdiez force-pushed the remove-decorated-eslint branch from 1218b8e to f20a77a Compare November 25, 2024 21:17
vdiez added 14 commits November 25, 2024 23:08
# Conflicts:
#	package-lock.json
#	packages/jsts/src/rules/README.md
#	packages/jsts/src/rules/S100/meta.ts
#	packages/jsts/src/rules/S101/meta.ts
#	packages/jsts/src/rules/S104/meta.ts
#	packages/jsts/src/rules/S105/meta.ts
#	packages/jsts/src/rules/S1066/meta.ts
#	packages/jsts/src/rules/S1067/meta.ts
#	packages/jsts/src/rules/S1068/meta.ts
#	packages/jsts/src/rules/S107/meta.ts
#	packages/jsts/src/rules/S1077/meta.ts
#	packages/jsts/src/rules/S1082/meta.ts
#	packages/jsts/src/rules/S109/meta.ts
#	packages/jsts/src/rules/S1105/meta.ts
#	packages/jsts/src/rules/S1110/meta.ts
#	packages/jsts/src/rules/S1116/meta.ts
#	packages/jsts/src/rules/S1119/meta.ts
#	packages/jsts/src/rules/S1121/meta.ts
#	packages/jsts/src/rules/S1125/meta.ts
#	packages/jsts/src/rules/S1126/meta.ts
#	packages/jsts/src/rules/S1128/meta.ts
#	packages/jsts/src/rules/S1134/meta.ts
#	packages/jsts/src/rules/S1135/meta.ts
#	packages/jsts/src/rules/S1154/meta.ts
#	packages/jsts/src/rules/S117/meta.ts
#	packages/jsts/src/rules/S1172/meta.ts
#	packages/jsts/src/rules/S1186/meta.ts
#	packages/jsts/src/rules/S1192/meta.ts
#	packages/jsts/src/rules/S1219/meta.ts
#	packages/jsts/src/rules/S1226/meta.ts
#	packages/jsts/src/rules/S124/meta.ts
#	packages/jsts/src/rules/S125/meta.ts
#	packages/jsts/src/rules/S126/meta.ts
#	packages/jsts/src/rules/S1264/meta.ts
#	packages/jsts/src/rules/S128/meta.ts
#	packages/jsts/src/rules/S1301/meta.ts
#	packages/jsts/src/rules/S131/meta.ts
#	packages/jsts/src/rules/S1313/meta.ts
#	packages/jsts/src/rules/S134/meta.ts
#	packages/jsts/src/rules/S135/meta.ts
#	packages/jsts/src/rules/S138/meta.ts
#	packages/jsts/src/rules/S1438/meta.ts
#	packages/jsts/src/rules/S1439/meta.ts
#	packages/jsts/src/rules/S1444/meta.ts
#	packages/jsts/src/rules/S1451/meta.ts
#	packages/jsts/src/rules/S1472/meta.ts
#	packages/jsts/src/rules/S1479/meta.ts
#	packages/jsts/src/rules/S1481/meta.ts
#	packages/jsts/src/rules/S1488/meta.ts
#	packages/jsts/src/rules/S1515/meta.ts
#	packages/jsts/src/rules/S1523/meta.ts
#	packages/jsts/src/rules/S1526/meta.ts
#	packages/jsts/src/rules/S1527/meta.ts
#	packages/jsts/src/rules/S1528/meta.ts
#	packages/jsts/src/rules/S1529/meta.ts
#	packages/jsts/src/rules/S1530/meta.ts
#	packages/jsts/src/rules/S1533/meta.ts
#	packages/jsts/src/rules/S1534/meta.ts
#	packages/jsts/src/rules/S1535/meta.ts
#	packages/jsts/src/rules/S1541/meta.ts
#	packages/jsts/src/rules/S1607/meta.ts
#	packages/jsts/src/rules/S1751/meta.ts
#	packages/jsts/src/rules/S1763/meta.ts
#	packages/jsts/src/rules/S1764/meta.ts
#	packages/jsts/src/rules/S1788/meta.ts
#	packages/jsts/src/rules/S1821/meta.ts
#	packages/jsts/src/rules/S1848/meta.ts
#	packages/jsts/src/rules/S1854/meta.ts
#	packages/jsts/src/rules/S1862/meta.ts
#	packages/jsts/src/rules/S1871/meta.ts
#	packages/jsts/src/rules/S1874/meta.ts
#	packages/jsts/src/rules/S1940/meta.ts
#	packages/jsts/src/rules/S1994/meta.ts
#	packages/jsts/src/rules/S2004/meta.ts
#	packages/jsts/src/rules/S2068/meta.ts
#	packages/jsts/src/rules/S2077/meta.ts
#	packages/jsts/src/rules/S2092/meta.ts
#	packages/jsts/src/rules/S2123/meta.ts
#	packages/jsts/src/rules/S2137/meta.ts
#	packages/jsts/src/rules/S2138/meta.ts
#	packages/jsts/src/rules/S2187/meta.ts
#	packages/jsts/src/rules/S2189/meta.ts
#	packages/jsts/src/rules/S2201/meta.ts
#	packages/jsts/src/rules/S2208/meta.ts
#	packages/jsts/src/rules/S2234/meta.ts
#	packages/jsts/src/rules/S2245/meta.ts
#	packages/jsts/src/rules/S2251/meta.ts
#	packages/jsts/src/rules/S2255/meta.ts
#	packages/jsts/src/rules/S2259/meta.ts
#	packages/jsts/src/rules/S2301/meta.ts
#	packages/jsts/src/rules/S2310/meta.ts
#	packages/jsts/src/rules/S2376/meta.ts
#	packages/jsts/src/rules/S2392/meta.ts
#	packages/jsts/src/rules/S2424/meta.ts
#	packages/jsts/src/rules/S2428/meta.ts
#	packages/jsts/src/rules/S2430/meta.ts
#	packages/jsts/src/rules/S2486/meta.ts
#	packages/jsts/src/rules/S2589/meta.ts
#	packages/jsts/src/rules/S2598/meta.ts
#	packages/jsts/src/rules/S2612/meta.ts
#	packages/jsts/src/rules/S2639/meta.ts
#	packages/jsts/src/rules/S2681/meta.ts
#	packages/jsts/src/rules/S2688/meta.ts
#	packages/jsts/src/rules/S2692/meta.ts
#	packages/jsts/src/rules/S2699/meta.ts
#	packages/jsts/src/rules/S2703/meta.ts
#	packages/jsts/src/rules/S2737/meta.ts
#	packages/jsts/src/rules/S2755/meta.ts
#	packages/jsts/src/rules/S2757/meta.ts
#	packages/jsts/src/rules/S2814/meta.ts
#	packages/jsts/src/rules/S2817/meta.ts
#	packages/jsts/src/rules/S2819/meta.ts
#	packages/jsts/src/rules/S2870/meta.ts
#	packages/jsts/src/rules/S2871/meta.ts
#	packages/jsts/src/rules/S2970/meta.ts
#	packages/jsts/src/rules/S2990/meta.ts
#	packages/jsts/src/rules/S2999/meta.ts
#	packages/jsts/src/rules/S3001/meta.ts
#	packages/jsts/src/rules/S3003/meta.ts
#	packages/jsts/src/rules/S3317/meta.ts
#	packages/jsts/src/rules/S3330/meta.ts
#	packages/jsts/src/rules/S3358/meta.ts
#	packages/jsts/src/rules/S3402/meta.ts
#	packages/jsts/src/rules/S3403/meta.ts
#	packages/jsts/src/rules/S3415/meta.ts
#	packages/jsts/src/rules/S3498/meta.ts
#	packages/jsts/src/rules/S3499/meta.ts
#	packages/jsts/src/rules/S3500/meta.ts
#	packages/jsts/src/rules/S3504/meta.ts
#	packages/jsts/src/rules/S3512/meta.ts
#	packages/jsts/src/rules/S3513/meta.ts
#	packages/jsts/src/rules/S3514/meta.ts
#	packages/jsts/src/rules/S3516/meta.ts
#	packages/jsts/src/rules/S3524/meta.ts
#	packages/jsts/src/rules/S3525/meta.ts
#	packages/jsts/src/rules/S3531/meta.ts
#	packages/jsts/src/rules/S3533/meta.ts
#	packages/jsts/src/rules/S3579/meta.ts
#	packages/jsts/src/rules/S3616/meta.ts
#	packages/jsts/src/rules/S3626/meta.ts
#	packages/jsts/src/rules/S3686/meta.ts
#	packages/jsts/src/rules/S3696/meta.ts
#	packages/jsts/src/rules/S3699/meta.ts
#	packages/jsts/src/rules/S3723/meta.ts
#	packages/jsts/src/rules/S3735/meta.ts
#	packages/jsts/src/rules/S3757/meta.ts
#	packages/jsts/src/rules/S3758/meta.ts
#	packages/jsts/src/rules/S3760/meta.ts
#	packages/jsts/src/rules/S3776/meta.ts
#	packages/jsts/src/rules/S3782/meta.ts
#	packages/jsts/src/rules/S3785/meta.ts
#	packages/jsts/src/rules/S3796/meta.ts
#	packages/jsts/src/rules/S3798/meta.ts
#	packages/jsts/src/rules/S3800/meta.ts
#	packages/jsts/src/rules/S3801/meta.ts
#	packages/jsts/src/rules/S3827/meta.ts
#	packages/jsts/src/rules/S3854/meta.ts
#	packages/jsts/src/rules/S3923/meta.ts
#	packages/jsts/src/rules/S3972/meta.ts
#	packages/jsts/src/rules/S3973/meta.ts
#	packages/jsts/src/rules/S3981/meta.ts
#	packages/jsts/src/rules/S3984/meta.ts
#	packages/jsts/src/rules/S4023/meta.ts
#	packages/jsts/src/rules/S4030/meta.ts
#	packages/jsts/src/rules/S4036/meta.ts
#	packages/jsts/src/rules/S4043/meta.ts
#	packages/jsts/src/rules/S4084/meta.ts
#	packages/jsts/src/rules/S4123/meta.ts
#	packages/jsts/src/rules/S4138/meta.ts
#	packages/jsts/src/rules/S4139/meta.ts
#	packages/jsts/src/rules/S4143/meta.ts
#	packages/jsts/src/rules/S4144/meta.ts
#	packages/jsts/src/rules/S4156/meta.ts
#	packages/jsts/src/rules/S4158/meta.ts
#	packages/jsts/src/rules/S4165/meta.ts
#	packages/jsts/src/rules/S4275/meta.ts
#	packages/jsts/src/rules/S4322/meta.ts
#	packages/jsts/src/rules/S4323/meta.ts
#	packages/jsts/src/rules/S4324/meta.ts
#	packages/jsts/src/rules/S4327/meta.ts
#	packages/jsts/src/rules/S4328/meta.ts
#	packages/jsts/src/rules/S4335/meta.ts
#	packages/jsts/src/rules/S4423/meta.ts
#	packages/jsts/src/rules/S4426/meta.ts
#	packages/jsts/src/rules/S4502/meta.ts
#	packages/jsts/src/rules/S4507/meta.ts
#	packages/jsts/src/rules/S4524/meta.ts
#	packages/jsts/src/rules/S4619/meta.ts
#	packages/jsts/src/rules/S4619/rule.ts
#	packages/jsts/src/rules/S4621/meta.ts
#	packages/jsts/src/rules/S4622/meta.ts
#	packages/jsts/src/rules/S4623/meta.ts
#	packages/jsts/src/rules/S4624/meta.ts
#	packages/jsts/src/rules/S4634/meta.ts
#	packages/jsts/src/rules/S4721/meta.ts
#	packages/jsts/src/rules/S4782/meta.ts
#	packages/jsts/src/rules/S4784/meta.ts
#	packages/jsts/src/rules/S4787/meta.ts
#	packages/jsts/src/rules/S4790/meta.ts
#	packages/jsts/src/rules/S4798/meta.ts
#	packages/jsts/src/rules/S4817/meta.ts
#	packages/jsts/src/rules/S4818/meta.ts
#	packages/jsts/src/rules/S4822/meta.ts
#	packages/jsts/src/rules/S4823/meta.ts
#	packages/jsts/src/rules/S4829/meta.ts
#	packages/jsts/src/rules/S4830/meta.ts
#	packages/jsts/src/rules/S5042/meta.ts
#	packages/jsts/src/rules/S5122/meta.ts
#	packages/jsts/src/rules/S5148/meta.ts
#	packages/jsts/src/rules/S5247/meta.ts
#	packages/jsts/src/rules/S5254/meta.ts
#	packages/jsts/src/rules/S5256/meta.ts
#	packages/jsts/src/rules/S5257/meta.ts
#	packages/jsts/src/rules/S5260/meta.ts
#	packages/jsts/src/rules/S5264/meta.ts
#	packages/jsts/src/rules/S5332/meta.ts
#	packages/jsts/src/rules/S5443/meta.ts
#	packages/jsts/src/rules/S5527/meta.ts
#	packages/jsts/src/rules/S5542/meta.ts
#	packages/jsts/src/rules/S5547/meta.ts
#	packages/jsts/src/rules/S5604/meta.ts
#	packages/jsts/src/rules/S5659/meta.ts
#	packages/jsts/src/rules/S5689/meta.ts
#	packages/jsts/src/rules/S5691/meta.ts
#	packages/jsts/src/rules/S5693/meta.ts
#	packages/jsts/src/rules/S5725/meta.ts
#	packages/jsts/src/rules/S5728/meta.ts
#	packages/jsts/src/rules/S5730/meta.ts
#	packages/jsts/src/rules/S5732/meta.ts
#	packages/jsts/src/rules/S5734/meta.ts
#	packages/jsts/src/rules/S5736/meta.ts
#	packages/jsts/src/rules/S5739/meta.ts
#	packages/jsts/src/rules/S5742/meta.ts
#	packages/jsts/src/rules/S5743/meta.ts
#	packages/jsts/src/rules/S5757/meta.ts
#	packages/jsts/src/rules/S5759/meta.ts
#	packages/jsts/src/rules/S5842/meta.ts
#	packages/jsts/src/rules/S5843/meta.ts
#	packages/jsts/src/rules/S5850/meta.ts
#	packages/jsts/src/rules/S5852/meta.ts
#	packages/jsts/src/rules/S5856/meta.ts
#	packages/jsts/src/rules/S5860/meta.ts
#	packages/jsts/src/rules/S5863/meta.ts
#	packages/jsts/src/rules/S5867/meta.ts
#	packages/jsts/src/rules/S5868/meta.ts
#	packages/jsts/src/rules/S5869/meta.ts
#	packages/jsts/src/rules/S5876/meta.ts
#	packages/jsts/src/rules/S5958/meta.ts
#	packages/jsts/src/rules/S5973/meta.ts
#	packages/jsts/src/rules/S6019/meta.ts
#	packages/jsts/src/rules/S6035/meta.ts
#	packages/jsts/src/rules/S6079/meta.ts
#	packages/jsts/src/rules/S6080/meta.ts
#	packages/jsts/src/rules/S6092/meta.ts
#	packages/jsts/src/rules/S6245/meta.ts
#	packages/jsts/src/rules/S6249/meta.ts
#	packages/jsts/src/rules/S6252/meta.ts
#	packages/jsts/src/rules/S6265/meta.ts
#	packages/jsts/src/rules/S6268/meta.ts
#	packages/jsts/src/rules/S6270/meta.ts
#	packages/jsts/src/rules/S6275/meta.ts
#	packages/jsts/src/rules/S6281/meta.ts
#	packages/jsts/src/rules/S6299/meta.ts
#	packages/jsts/src/rules/S6302/meta.ts
#	packages/jsts/src/rules/S6303/meta.ts
#	packages/jsts/src/rules/S6304/meta.ts
#	packages/jsts/src/rules/S6308/meta.ts
#	packages/jsts/src/rules/S6317/meta.ts
#	packages/jsts/src/rules/S6319/meta.ts
#	packages/jsts/src/rules/S6321/meta.ts
#	packages/jsts/src/rules/S6323/meta.ts
#	packages/jsts/src/rules/S6324/meta.ts
#	packages/jsts/src/rules/S6326/meta.ts
#	packages/jsts/src/rules/S6327/meta.ts
#	packages/jsts/src/rules/S6328/meta.ts
#	packages/jsts/src/rules/S6329/meta.ts
#	packages/jsts/src/rules/S6330/meta.ts
#	packages/jsts/src/rules/S6331/meta.ts
#	packages/jsts/src/rules/S6332/meta.ts
#	packages/jsts/src/rules/S6333/meta.ts
#	packages/jsts/src/rules/S6351/meta.ts
#	packages/jsts/src/rules/S6353/meta.ts
#	packages/jsts/src/rules/S6397/meta.ts
#	packages/jsts/src/rules/S6426/meta.ts
#	packages/jsts/src/rules/S6439/meta.ts
#	packages/jsts/src/rules/S6440/meta.ts
#	packages/jsts/src/rules/S6441/meta.ts
#	packages/jsts/src/rules/S6442/meta.ts
#	packages/jsts/src/rules/S6443/meta.ts
#	packages/jsts/src/rules/S6477/meta.ts
#	packages/jsts/src/rules/S6478/meta.ts
#	packages/jsts/src/rules/S6479/meta.ts
#	packages/jsts/src/rules/S6481/meta.ts
#	packages/jsts/src/rules/S6486/meta.ts
#	packages/jsts/src/rules/S6535/meta.ts
#	packages/jsts/src/rules/S6544/meta.ts
#	packages/jsts/src/rules/S6551/meta.ts
#	packages/jsts/src/rules/S6557/meta.ts
#	packages/jsts/src/rules/S6564/meta.ts
#	packages/jsts/src/rules/S6571/meta.ts
#	packages/jsts/src/rules/S6572/meta.ts
#	packages/jsts/src/rules/S6582/meta.ts
#	packages/jsts/src/rules/S6594/meta.ts
#	packages/jsts/src/rules/S6598/meta.ts
#	packages/jsts/src/rules/S6606/meta.ts
#	packages/jsts/src/rules/S6627/meta.ts
#	packages/jsts/src/rules/S6643/meta.ts
#	packages/jsts/src/rules/S6647/meta.ts
#	packages/jsts/src/rules/S6660/meta.ts
#	packages/jsts/src/rules/S6661/meta.ts
#	packages/jsts/src/rules/S6666/meta.ts
#	packages/jsts/src/rules/S6676/meta.ts
#	packages/jsts/src/rules/S6679/meta.ts
#	packages/jsts/src/rules/S6747/meta.ts
#	packages/jsts/src/rules/S6749/meta.ts
#	packages/jsts/src/rules/S6754/meta.ts
#	packages/jsts/src/rules/S6759/meta.ts
#	packages/jsts/src/rules/S6788/meta.ts
#	packages/jsts/src/rules/S6791/meta.ts
#	packages/jsts/src/rules/S6827/meta.ts
#	packages/jsts/src/rules/S6844/meta.ts
#	packages/jsts/src/rules/S6853/meta.ts
#	packages/jsts/src/rules/S6957/meta.ts
#	packages/jsts/src/rules/S6958/meta.ts
#	packages/jsts/src/rules/S6959/meta.ts
#	packages/jsts/src/rules/S7059/meta.ts
#	packages/jsts/src/rules/S7060/meta.ts
#	packages/jsts/src/rules/S881/meta.ts
#	packages/jsts/src/rules/S888/meta.ts
#	packages/jsts/src/rules/S905/meta.ts
#	packages/jsts/src/rules/S930/meta.ts
#	packages/jsts/src/rules/core/index.ts
#	packages/jsts/src/rules/decorated.ts
#	packages/jsts/src/rules/external.ts
#	packages/jsts/src/rules/helpers/isHtmlElement.ts
#	packages/jsts/src/rules/helpers/table.ts
#	packages/jsts/src/rules/index.ts
#	packages/jsts/src/rules/original.ts
#	packages/jsts/src/rules/plugin.ts
#	packages/jsts/src/rules/typescript-eslint/index.ts
#	packages/jsts/tests/rules/index.test.ts
#	tools/check-distribution-filepath-length.cjs
#	tools/generate-meta.ts
# Conflicts:
#	packages/jsts/src/rules/README.md
#	packages/jsts/src/rules/S2068/meta.ts
#	packages/jsts/src/rules/original.ts
#	packages/jsts/src/rules/plugin.ts
# Conflicts:
#	packages/jsts/src/rules/README.md
#	packages/jsts/src/rules/S2068/meta.ts
#	packages/jsts/src/rules/original.ts
#	packages/jsts/src/rules/plugin.ts
Copy link
Contributor

@zglicz zglicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few comments. It's hard to correctly review such a big PR

I hope I looked at all of the non-automated files. Also, I'd say it's a nice touch to add a bit more comprehensive PR summary, where you give some reasoning why you make certain decisions.

package.json Show resolved Hide resolved
package.json Show resolved Hide resolved

## SonarJS additional rules

There are some rules which are not shipped in this ESLint plugin to avoid duplication with already existing ESLint rules.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does it mean "they are not shipped"? In the end, if you use our plugin, you will have these rules running, no? Perhaps lets be more explicit, what it means in the plugin and in SonarJS.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, I have removed all decorated rules from our plugin, so if they use our plugin those rules, listed in the readme file at the very bottom, will not be available and the user needs to install those plugins if they want the same results as in SonarJS

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the wording on the README, pls let me know if it's better now

packages/jsts/tests/rules/index.test.ts Outdated Show resolved Hide resolved
packages/jsts/tests/rules/index.test.ts Show resolved Hide resolved
tools/generate-rule-indexes.ts Show resolved Hide resolved
tools/generate-rule-indexes.ts Show resolved Hide resolved
tools/generate-external-rules-docs.ts Show resolved Hide resolved
tools/generate-external-rules-docs.ts Show resolved Hide resolved
return `[${key}](${sonarURL(key)})`;
}

function externalURL(plugin: string, key: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this could be extracted into 'external/{library}' files? that way it's extracted a bit in the correct places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand your point

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the modules in external/plugin.ts be exporting this method/constant? So in external/core.js you would have a method like: getExternalURL(key: string) and similarly in the other files like external/a11y.ts. As in, remove the switches and have these declared in the plugin modules themselves. But I'm fine with this as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

I did a first pass and left some comments.
I might add more about the plugin's README.

.eslint-doc-generatorrc.cjs Show resolved Hide resolved
packages/jsts/src/rules/README.md Outdated Show resolved Hide resolved
packages/jsts/src/rules/README.md Outdated Show resolved Hide resolved
packages/jsts/src/rules/README.md Outdated Show resolved Hide resolved
packages/jsts/tests/rules/index.test.ts Show resolved Hide resolved
tools/generate-meta.ts Show resolved Hide resolved
tools/generate-rule-indexes.ts Show resolved Hide resolved
packages/jsts/src/rules/S6325/meta.ts Show resolved Hide resolved
@vdiez
Copy link
Contributor Author

vdiez commented Nov 28, 2024

added a list of changes and goals of this PR

@vdiez vdiez force-pushed the remove-decorated-eslint branch from e4c4f53 to f785d58 Compare November 28, 2024 11:38
@yassin-kammoun-sonarsource
Copy link
Contributor

yassin-kammoun-sonarsource commented Nov 28, 2024

Here, I am proposing a new version of the plugin README. You are free to discard my suggestions or adapt them as you wish. Nevertheless, I would like to share my rationale behind this proposition:

  1. The section discussing ESLint and Sonar appears too late in the README. I believe this introduction should be moved to the top of the document. I have reformulated it to emphasize what users can expect from the rules offered by this ESLint plugin, such as accessibility and security vulnerabilities.
  2. While GitHub README files do include a built-in table of contents, I'm not sure that many people are aware of it. The fact that we only realized that recently highlights that point. Therefore, I recommend including an explicit table of contents, as the GitHub version does not display by default. Additionally, for reference, ESLint also features its own table of contents.
  3. I created a specific subsection about the TypeScript ESLint Parser. However, I'm not entirely sure of some details and would appreciate your clarification on this matter. If I understand correctly, the typescript-eslint/parser dependency is mandatory. Without it, users of this plugin may encounter runtime failures with rules that use the type-checker.
  4. I added a new section titled "Feedback" to guide users to the Sonar community forum. While there is a link to the forum at the root of the repository, I believe users of this plugin will typically access the plugin's README rather than the root one. As a result, they may not know how to reach us. While handling community last week, I came across a user that was complaining about this.
  5. While I really like it, I am starting to have mixed feelings about the "Rules" section of the README. I am not sure we should mix internal documentation intended for us developers with public documentation meant for users. We reference "ESLint rules" and "Improved ESLint rules" that are not part of eslint-plugin-sonarjs, which is irrelevant from the user's perspective. Unless I am mistaken, this README eventually ends up on the landing page of our plugin on npm.

eslint-plugin-sonarjs (new README)

eslint-plugin-sonarjs is an ESLint plugin maintained by Sonar, designed to help developers write Clean Code. It provides a subset of rules from SonarJS, an analyzer for JavaScript and TypeScript within the Sonar ecosystem. This plugin offers general-purpose rules for detecting code smells and bugs, as well as rules for other aspects of code quality, including testing, accessibility, and more. Additionally, it enhances code security by providing rules to report potential security vulnerabilities.

Table of Contents

  1. Prerequisites
  2. Installation
  3. Usage
  4. Configuration
  5. SonarLint
  6. Feedback
  7. Rules

Prerequisites

The prerequisites for using this plugin depend on the ESLint version you are using:

  • For ESLint 8, you need Node.js version >= 16.
  • For ESLint 9, you need Node.js version that complies with (^18.18.0 || ^20.9.0 || >=21).

Installation

First, ensure that your project is configured with ESLint. If it is not, please follow the ESLint instructions to set it up.

To install the eslint-plugin-sonarjs, use the following npm command:

npm install eslint-plugin-sonarjs --save-dev # locally
npm install eslint-plugin-sonarjs -g         # globally

Usage

The usage of the eslint-plugin-sonarjs dependency depends on the ESLint version used by your project.

For ESLint 8

Add sonarjs to your .eslintrc file:

{
  "plugins": [
    "sonarjs"
  ],
  "extends": [
    "plugin:sonarjs/recommended-legacy"
  ]
}

For ESLint 9

Add sonarjs to your eslint.config.js file:

import sonarjs from "eslint-plugin-sonarjs";

export default [
  {
    plugins: {
      sonarjs
    },
    extends: [
      "plugin:sonarjs/recommended"
    ]
  }
];

TypeScript ESLint Parser

Several rules are designed for linting both JavaScript and TypeScript code, and some even rely on type checking through TypeScript. Therefore, you will need to install the @typescript-eslint/parser dependency and instruct ESLint to use this parser through the parserOptions property.

Configuration

This ESLint plugin provides a single configuration named recommended. This configuration enables most of the rules except for a few exceptions, and the rules are enabled with the error severity. If those exceptions make sense for your project, they can be enabled manually as follows:

{
  "rules": {
    "sonarjs/rule-name": "error"
  }
}

There is also another configuration named recommended-legacy that aims to be backward-compatible with ESLint 8, using the same rule configuration:

{
  "extends": ["plugin:sonarjs/recommended-legacy"]
}

SonarLint

As an alternative to using this ESLint plugin, you can use SonarLint. SonarLint is an IDE extension that helps you detect and fix quality issues as you write code. It provides a broader set of rules compared to the ESLint plugin, improved versions of ESLint rules, and additional features that enhance your linting experience.

Feedback

If you have any questions, encounter any bugs, or have feature requests, please reach out to us through the Sonar Community Forum.

Your messages will reach the maintainers of this GitHub repository.

Rules

Depending on what you decide, either the current contents of the "Rules" section or only the rules that are actually provided by our ESLint plugin.

@vdiez
Copy link
Contributor Author

vdiez commented Nov 28, 2024

I think mentioning the rules that are missing is relevant for users looking for an experience equivalent to SonarJS but using ESLint, so they can install and enable the listed rules.

Thanks for the proposal, I'll do that tomorrow

@yassin-kammoun-sonarsource
Copy link
Contributor

I think mentioning the rules that are missing is relevant for users looking for an experience equivalent to SonarJS but using ESLint, so they can install and enable the listed rules.

Ah ok, I think I misunderstood then. So this means that even ESLint rules or decorated rules can manually be enabled, right?

@vdiez
Copy link
Contributor Author

vdiez commented Nov 28, 2024

yes, they can install other plugins and enable as many rules as they want, indeed

@vdiez
Copy link
Contributor Author

vdiez commented Nov 29, 2024

I will handle the README on another PR, I don't want to have this branch for so long

# Conflicts:
#	package-lock.json
#	package.json
# Conflicts:
#	package-lock.json
#	package.json
),
);

//sort once;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need this comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!
Thank you for your efforts.

@vdiez vdiez merged commit df5e45a into master Nov 29, 2024
19 of 20 checks passed
@vdiez vdiez deleted the remove-decorated-eslint branch November 29, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants