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.

Sign up for GitHub

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

Jump to bottom

Converted npm-naming from TSLint to ESLint #681

Merged
merged 19 commits into from
Dec 12, 2023
Merged

Converted npm-naming from TSLint to ESLint #681

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fb4ca4b
Converted npm-naming from TSLint to ESLint
JoshuaKGoldberg Dec 8, 2023
0bfae60
Converted npm-naming from TSLint to ESLint
JoshuaKGoldberg Dec 8, 2023
338b006
disable rule in fixtures dir
JoshuaKGoldberg Dec 8, 2023
5142471
Merge branch 'main'
JoshuaKGoldberg Dec 11, 2023
574cc71
Add I think equivalent logic to expectOnly
JoshuaKGoldberg Dec 11, 2023
6e50f20
fix: .eslintrc.json, not tslint.json
JoshuaKGoldberg Dec 11, 2023
d1c8033
docs: mention npmNamingOnly in .md files too
JoshuaKGoldberg Dec 11, 2023
f7d7c30
Drop new flag
jakebailey Dec 11, 2023
15a91e8
Baseline plugin exports
jakebailey Dec 11, 2023
e692e20
Remove npm-naming from preset
jakebailey Dec 11, 2023
f2950e0
Formatting
jakebailey Dec 11, 2023
351b5e7
knip
jakebailey Dec 11, 2023
441aeee
Make tslint instance undefined
jakebailey Dec 11, 2023
24cc08f
Undo changes in fixture eslintrc
jakebailey Dec 11, 2023
159dc92
Changeset
jakebailey Dec 11, 2023
151f33b
Forgot to actually enable the rule, oops
jakebailey Dec 11, 2023
81eae93
npm apparently returns a single string when there's one version
jakebailey Dec 11, 2023
4c991ae
Set npm-naming in baseConfig so it can be overridden
jakebailey Dec 11, 2023
a0b842d
Fix message for disabling lint
jakebailey Dec 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/eighty-jobs-wave.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@definitelytyped/eslint-plugin": patch
"@definitelytyped/dts-critic": patch
"@definitelytyped/dtslint": patch
---

Move npm-naming lint rule from tslint to eslint
28 changes: 14 additions & 14 deletions packages/dts-critic/dt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,34 @@ import { dtsCritic as critic, ErrorKind } from "./index";
import fs = require("fs");
import stripJsonComments = require("strip-json-comments");

function hasNpmNamingLintRule(tslintPath: string): boolean {
if (fs.existsSync(tslintPath)) {
const tslint = JSON.parse(stripJsonComments(fs.readFileSync(tslintPath, "utf-8")));
if (tslint.rules && tslint.rules["npm-naming"] !== undefined) {
return !!tslint.rules["npm-naming"];
function hasNpmNamingLintRule(eslintPath: string): boolean {
if (fs.existsSync(eslintPath)) {
const eslint = JSON.parse(stripJsonComments(fs.readFileSync(eslintPath, "utf-8")));
if (eslint.rules?.["@definitelytyped/npm-naming"] !== undefined) {
return !!eslint.rules["@definitelytyped/npm-naming"];
}
return true;
}
return false;
}

function addNpmNamingLintRule(tslintPath: string): void {
if (fs.existsSync(tslintPath)) {
const tslint = JSON.parse(stripJsonComments(fs.readFileSync(tslintPath, "utf-8")));
if (tslint.rules) {
tslint.rules["npm-naming"] = false;
function addNpmNamingLintRule(eslintPath: string): void {
if (fs.existsSync(eslintPath)) {
const eslint = JSON.parse(stripJsonComments(fs.readFileSync(eslintPath, "utf-8")));
if (eslint.rules) {
eslint.rules["@definitelytyped/npm-naming"] = false;
} else {
tslint.rules = { "npm-naming": false };
eslint.rules = { "@definitelytyped/npm-naming": false };
}
fs.writeFileSync(tslintPath, JSON.stringify(tslint, undefined, 4), "utf-8");
fs.writeFileSync(eslintPath, JSON.stringify(eslint, undefined, 4), "utf-8");
}
}

function main() {
for (const item of fs.readdirSync("../DefinitelyTyped/types")) {
const entry = "../DefinitelyTyped/types/" + item;
try {
if (hasNpmNamingLintRule(entry + "/tslint.json")) {
if (hasNpmNamingLintRule(entry + "/.eslintrc.json")) {
const errors = critic(entry + "/index.d.ts");
for (const error of errors) {
switch (error.kind) {
Expand Down Expand Up @@ -57,7 +57,7 @@ function main() {
const npmvers = m[2].split(",").map((s: string) => parseFloat(s.trim()));
const fixto = npmvers.every((v: number) => headerver > v) ? -1.0 : Math.max(...npmvers);
console.log(`npm-version:${item}:${m[1]}:${m[2]}:${fixto}`);
addNpmNamingLintRule(entry + "/tslint.json");
addNpmNamingLintRule(entry + "/.eslintrc.json");
} else {
console.log("could not parse error message: ", error.message);
}
Expand Down
5 changes: 1 addition & 4 deletions packages/dtslint/dt.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
{
"extends": "./dtslint.json",
"rules": {
"npm-naming": [true, { "mode": "code" }]
}
"extends": "./dtslint.json"
}
4 changes: 1 addition & 3 deletions packages/dtslint/dtslint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
{
"rulesDirectory": "./dist/rules",
"rules": {
"npm-naming": true
}
"rules": {}
}
1 change: 0 additions & 1 deletion packages/dtslint/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
"test": "../../node_modules/.bin/jest --config ../../jest.config.js packages/dtslint"
},
"dependencies": {
"@definitelytyped/dts-critic": "workspace:*",
"@definitelytyped/header-parser": "workspace:*",
"@definitelytyped/typescript-versions": "workspace:*",
"@definitelytyped/utils": "workspace:*",
Expand Down
4 changes: 2 additions & 2 deletions packages/dtslint/src/lint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ export async function lint(
// TODO: To remove tslint, replace this with a ts.createProgram (probably)
const lintProgram = Linter.createProgram(tsconfigPath);
Copy link
Member

Choose a reason for hiding this comment

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

We only use this program to build a list of files; not for this PR, but we're going to have to remove this when we remove tslint so I expect that we will just ask eslint to run on all non-ignored files (which is a lot better than having to find a list of all files here).

Copy link
Member

Choose a reason for hiding this comment

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

That being said, since we run type-using rules and also use project=true, it's likely that people will have random files that aren't in the program that may break via ts-eslint complaining about not being in a program, or via new lints that were previously uncaught.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cost reduction: Linter.createProgram is a standalone public static, so an equivalent could always be re-written.

Copy link
Member

Choose a reason for hiding this comment

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

Probably, yeah, though it seems a bit overkill to build a Program to only use it to list files!


// tslint no longer checks ExpectType; skip linting entirely if we're only checking ExpectType.
const linter = !expectOnly ? new Linter({ fix: false, formatter: "stylish" }, lintProgram) : undefined;
// TODO: remove tslint entirely
const linter = undefined as Linter | undefined;
const configPath = getConfigPath(dirPath);
// TODO: To port expect-rule, eslint's config will also need to include [minVersion, maxVersion]
// Also: expect-rule should be renamed to expect-type or check-type or something
Expand Down