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

chore: Introduce Knip #18005

Merged
merged 19 commits into from
Mar 11, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
100 changes: 51 additions & 49 deletions .github/workflows/ci.yml
Copy link
Contributor

Choose a reason for hiding this comment

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

[Style] Unrelated changes to whitespace all around?

Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,34 @@ jobs:
name: Verify Files
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: 'lts/*'
- name: Install Packages
run: npm install --force
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: "lts/*"
- name: Install Packages
run: npm install --force

- name: Install Docs Packages
working-directory: docs
run: npm install
- name: Install Docs Packages
working-directory: docs
run: npm install

- name: Lint Files
run: node Makefile lint
- name: Lint Files
run: node Makefile lint

- name: Check Rule Files
run: node Makefile checkRuleFiles
- name: Check Rule Files
run: node Makefile checkRuleFiles

- name: Check Licenses
run: node Makefile checkLicenses
- name: Check Licenses
run: node Makefile checkLicenses

- name: Lint Docs JS Files
run: node Makefile lintDocsJS
- name: Lint Docs JS Files
run: node Makefile lintDocsJS

- name: Check Rule Examples
run: node Makefile checkRuleExamples
- name: Check Rule Examples
run: node Makefile checkRuleExamples

- name: Lint Files, Dependencies & Exports
run: npm run lint:imports

test_on_node:
name: Test
Expand All @@ -47,40 +49,40 @@ jobs:
os: [ubuntu-latest]
node: [21.x, 20.x, 18.x, "18.18.0"]
include:
- os: windows-latest
node: "lts/*"
- os: macOS-latest
node: "lts/*"
- os: windows-latest
node: "lts/*"
- os: macOS-latest
node: "lts/*"
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Install Packages
run: npm install --force
- name: Test
run: node Makefile mocha
- name: Fuzz Test
run: node Makefile fuzz
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- name: Install Packages
run: npm install --force
- name: Test
run: node Makefile mocha
- name: Fuzz Test
run: node Makefile fuzz

test_on_browser:
name: Browser Test
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: '20' # Should be the same as the version used on Netlify to build the ESLint Playground
- name: Install Packages
run: npm install --force
- name: Test
run: node Makefile wdio
- name: Fuzz Test
run: node Makefile fuzz
- uses: actions/upload-artifact@v3
if: failure()
with:
name: logs
path: |
wdio-logs/*.log
- uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: "20" # Should be the same as the version used on Netlify to build the ESLint Playground
- name: Install Packages
run: npm install --force
- name: Test
run: node Makefile wdio
- name: Fuzz Test
run: node Makefile fuzz
- uses: actions/upload-artifact@v3
if: failure()
with:
name: logs
path: |
wdio-logs/*.log
4 changes: 2 additions & 2 deletions Makefile.js
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,8 @@ target.lintDocsJS = function([fix = false] = []) {
};

target.fuzz = function({ amount = 1000, fuzzBrokenAutofixes = false } = {}) {
const fuzzerRunner = require("./tools/fuzzer-runner");
const fuzzResults = fuzzerRunner.run({ amount, fuzzBrokenAutofixes });
const { run } = require("./tools/fuzzer-runner");
const fuzzResults = run({ amount, fuzzBrokenAutofixes });
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

if (fuzzResults.length) {

Expand Down
3 changes: 2 additions & 1 deletion bin/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ ${getErrorMessage(error)}`;
}

// Otherwise, call the CLI.
const exitCode = await require("../lib/cli").execute(
const cli = require("../lib/cli");
const exitCode = await cli.execute(
process.argv,
process.argv.includes("--stdin") ? await readStdin() : null,
true
Expand Down
5 changes: 0 additions & 5 deletions docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,15 @@
"autoprefixer": "^10.4.13",
"cross-env": "^7.0.3",
"cssnano": "^5.1.14",
"dom-parser": "^0.1.6",
"eleventy-plugin-nesting-toc": "^1.3.0",
"eleventy-plugin-page-assets": "^0.3.0",
"eleventy-plugin-reading-time": "^0.0.1",
"github-slugger": "^1.5.0",
"hyperlink": "^5.0.4",
"imagemin": "^8.0.1",
"imagemin-cli": "^7.0.0",
"js-yaml": "^3.14.1",
"luxon": "^2.4.0",
"markdown-it": "^12.2.0",
"markdown-it-anchor": "^8.1.2",
"markdown-it-container": "^3.0.0",
"netlify-cli": "^10.3.1",
"npm-run-all2": "^5.0.0",
"postcss-cli": "^10.0.0",
"postcss-html": "^1.5.0",
Expand Down
46 changes: 46 additions & 0 deletions knip.jsonc
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

It would be helpful if you could explain what all of this does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the configuration comes down to providing entry files in order for Knip to do its thing.

This repository is much like a monorepo, but without explicitly defined workspaces as package managers like npm and pnpm recognize them. Knip normally hooks into package.json#workspaces, but those folders with a package.json manifest can also be manually added to the Knip configuration file. So the keys of the workspaces object are the paths to those folders.

"workspaces": {
".": {
// These entries are complementary to the ones found in package.json
"entry": [
"lib/rules/index.js",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"tools/internal-rules/index.js",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"tools/update-rule-types.js",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/webpro/knip/issues/464
// Remove when Knip has a wdio plugin
"wdio.conf.js"
],
"project": ["{conf,lib,tools}/**/*.js"],
"mocha": {
"entry": [
"tests/{bin,conf,lib,tools}/**/*.js", // see Makefile.js
"tests/_utils/test-lazy-loading-rules.js"
],
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
"project": ["tests/**/*.js"]
},
"ignore": [
// If Knip would consider exports as named, their usage is too dynamic: globals[`es${ecmaVersion}`]
// An alternative is to add `__esModule: true` to the export and we can remove it here from the ignores:
"conf/globals.js",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
// These contain unresolved imports and other oddities:
"tests/bench/large.js",
"tests/lib/rule-tester/rule-tester.js",
"tests/performance/jshint.js",
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved
// Many are required using dynamic paths such as `fs.readFileSync(path.join())`:
"tests/fixtures/**"
],
"ignoreDependencies": [
"c8",
// Ignore until Knip has a wdio plugin:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Ignore until Knip has a wdio plugin:
// Ignore until Knip has a wdio plugin:
// https://github.com/webpro/knip/issues/483#issuecomment-1925751516

"@wdio/*",
"rollup-plugin-node-polyfills"
]
},
"docs": {
"ignore": ["src/assets/js/search.js", "_examples/**"]
},
// Workspaces with default configs:
"packages/*": {},
"tools/internal-rules": {}
}
}
34 changes: 0 additions & 34 deletions lib/cli-engine/xml-escape.js

This file was deleted.

3 changes: 1 addition & 2 deletions lib/config/flat-config-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,5 @@ const flatConfigSchema = {

module.exports = {
flatConfigSchema,
assertIsRuleSeverity,
assertIsRuleOptions
assertIsRuleSeverity
};
1 change: 0 additions & 1 deletion lib/eslint/eslint-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,6 @@ function getCacheFile(cacheFile, cwd) {
//-----------------------------------------------------------------------------

module.exports = {
isGlobPattern,
findFiles,

isNonEmptyString,
Expand Down
4 changes: 1 addition & 3 deletions lib/linter/index.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
"use strict";

const { Linter } = require("./linter");
const { interpolate } = require("./interpolate");
const SourceCodeFixer = require("./source-code-fixer");

module.exports = {
Linter,

// For testers.
SourceCodeFixer,
interpolate
SourceCodeFixer
};
2 changes: 0 additions & 2 deletions lib/linter/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ const { LATEST_ECMA_VERSION } = require("../../conf/ecma-version");
// Typedefs
//------------------------------------------------------------------------------

/** @typedef {InstanceType<import("../cli-engine/config-array").ConfigArray>} ConfigArray */
/** @typedef {InstanceType<import("../cli-engine/config-array").ExtractedConfig>} ExtractedConfig */
/** @typedef {import("../shared/types").ConfigData} ConfigData */
/** @typedef {import("../shared/types").Environment} Environment */
/** @typedef {import("../shared/types").GlobalConf} GlobalConf */
Expand Down
4 changes: 3 additions & 1 deletion lib/rule-tester/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
"use strict";

const RuleTester = require("./rule-tester");

module.exports = {
RuleTester: require("./rule-tester")
RuleTester
};
2 changes: 0 additions & 2 deletions lib/rules/utils/lazy-loading-rule-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

const debug = require("debug")("eslint:rules");

/** @typedef {import("./types").Rule} Rule */
JoshuaKGoldberg marked this conversation as resolved.
Show resolved Hide resolved

/**
* The `Map` object that loads each rule when it's accessed.
* @example
Expand Down
13 changes: 9 additions & 4 deletions lib/rules/utils/unicode/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,14 @@
*/
"use strict";

const isCombiningCharacter = require("./is-combining-character");
const isEmojiModifier = require("./is-emoji-modifier");
const isRegionalIndicatorSymbol = require("./is-regional-indicator-symbol");
const isSurrogatePair = require("./is-surrogate-pair");

module.exports = {
isCombiningCharacter: require("./is-combining-character"),
isEmojiModifier: require("./is-emoji-modifier"),
isRegionalIndicatorSymbol: require("./is-regional-indicator-symbol"),
isSurrogatePair: require("./is-surrogate-pair")
isCombiningCharacter,
isEmojiModifier,
isRegionalIndicatorSymbol,
isSurrogatePair
};
58 changes: 0 additions & 58 deletions lib/shared/deprecation-warnings.js

This file was deleted.

1 change: 1 addition & 0 deletions lib/shared/runtime-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ function version() {
//------------------------------------------------------------------------------

module.exports = {
__esModule: true, // Indicate intent for imports, remove ambiguity for Knip (see: https://github.com/eslint/eslint/pull/18005#discussion_r1484422616)
environment,
version
};