Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 15 additions & 33 deletions eslint.config.js
Copy link
Contributor

Choose a reason for hiding this comment

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

[Praise] Nice cleanup here on removing the ...s, etc. too 🙂

Original file line number Diff line number Diff line change
@@ -1,34 +1,24 @@
import eslintConfigESLint from "eslint-config-eslint";
import { defineConfig } from "@eslint/config-helpers";
import globals from "globals";
import { defineConfig, globalIgnores } from "@eslint/config-helpers";
import tseslint from "typescript-eslint";

const eslintPluginJSDoc = eslintConfigESLint.find(
config => config.plugins?.jsdoc,
).plugins.jsdoc;

export default defineConfig([
globalIgnores(
["**/tests/fixtures/", "**/dist/", "**/coverage/"],
"rewrite/global-ignores",
),
{
ignores: ["**/tests/fixtures/", "**/dist/", "**/coverage/"],
},

{
name: "rewrite/js",
files: ["**/*.js"],
extends: [eslintConfigESLint],
rules: {
// disable rules we don't want to use from eslint-config-eslint
"no-undefined": "off",

// TODO: re-enable eslint-plugin-jsdoc rules
...Object.fromEntries(
Object.keys(eslintPluginJSDoc.rules).map(name => [
`jsdoc/${name}`,
"off",
]),
),
},
},

// Tools and CLI
{
name: "rewrite/tools",
files: [
"scripts/**",
"tools/**",
Expand All @@ -39,30 +29,22 @@ export default defineConfig([
"n/no-process-exit": "off",
},
},

{
name: "rewrite/tests",
files: ["**/tests/**"],
languageOptions: {
globals: {
describe: "readonly",
xdescribe: "readonly",
it: "readonly",
xit: "readonly",
beforeEach: "readonly",
afterEach: "readonly",
before: "readonly",
after: "readonly",
...globals.mocha,
},
},
},

// TypeScript
...tseslint.config({
{
name: "rewrite/ts",
files: ["**/*.ts"],
ignores: ["**/tests/**/*.ts"],
extends: [...tseslint.configs.strict, ...tseslint.configs.stylistic],
extends: [tseslint.configs.strict, tseslint.configs.stylistic],
rules: {
"no-use-before-define": "off",
},
}),
},
]);
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"c8": "^10.1.3",
"eslint": "^9.35.0",
"eslint-config-eslint": "^13.0.0",
"globals": "^16.5.0",
"knip": "^5.62.0",
"lint-staged": "^16.0.0",
"mocha": "^11.5.0",
Expand Down
15 changes: 10 additions & 5 deletions packages/compat/src/fixup-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ const fixedUpPlugins = new WeakSet();

/**
* Determines if two nodes or tokens overlap.
* @param {object} first The first node or token to check.
* @param {object} second The second node or token to check.
* @param {Object} first The first node or token to check.
* @param {Object} second The second node or token to check.
* @returns {boolean} True if the two nodes or tokens overlap.
*/
function nodesOrTokensOverlap(first, second) {
Expand All @@ -90,7 +90,7 @@ function nodesOrTokensOverlap(first, second) {

/**
* Checks whether a node is an export declaration.
* @param {object} node An AST node.
* @param {Object} node An AST node.
* @returns {boolean} True if the node is an export declaration.
*/
function looksLikeExport(node) {
Expand All @@ -102,8 +102,8 @@ function looksLikeExport(node) {

/**
* Checks for the presence of a JSDoc comment for the given node and returns it.
* @param {object} node The AST node to get the comment for.
* @param {object} sourceCode A SourceCode instance to get comments.
* @param {Object} node The AST node to get the comment for.
* @param {Object} sourceCode A SourceCode instance to get comments.
* @returns {object|null} The Block comment token containing the JSDoc comment
* for the given node or null if not found.
*/
Expand Down Expand Up @@ -151,6 +151,11 @@ export function fixupRule(ruleDefinition) {
? ruleDefinition
: ruleDefinition.create.bind(ruleDefinition);

/**
* Compatibility rule creator that adds missing methods to context and sourceCode objects.
* @param {Object} context The rule context.
* @returns {Object} The rule visitor.
*/
function ruleCreate(context) {
const sourceCode = context.sourceCode;

Expand Down
14 changes: 14 additions & 0 deletions packages/compat/tests/fixup-rules.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ import {
} from "../src/fixup-rules.js";
import { Linter } from "eslint";

//-----------------------------------------------------------------------------
// Types
//-----------------------------------------------------------------------------

/** @typedef {import("@eslint/core").RuleDefinition["create"]} LegacyRuleDefinition */

//-----------------------------------------------------------------------------
// Data
//-----------------------------------------------------------------------------
Expand Down Expand Up @@ -170,6 +176,10 @@ describe("@eslint/compat", () => {
});

it("should return a rule object when a function-style rule is passed to fixupRule", () => {
/**
* Test fixture simulating a legacy function-style rule.
* @type {LegacyRuleDefinition}
*/
function rule(context) {
return {
Identifier(node) {
Expand Down Expand Up @@ -208,6 +218,10 @@ describe("@eslint/compat", () => {
});

it("should return a rule object with `meta.schema` when a function-style rule with schema is passed to fixupRule", () => {
/**
* Test fixture simulating a legacy function-style rule.
* @type {LegacyRuleDefinition}
*/
function rule(context) {
return {
Identifier(node) {
Expand Down
2 changes: 1 addition & 1 deletion packages/compat/tests/ignore-file.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* @filedescription Fixup tests
* @fileoverview Fixup tests
*/

//-----------------------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions packages/config-array/fix-std__path-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@

import { readFile, writeFile } from "node:fs/promises";

/**
* Replaces all occurrences of a pattern in a file with a replacement string.
* @param {string} file The file path to modify.
* @param {RegExp} search The regular expression to search for.
* @param {string} replacement The replacement string.
* @returns {Promise<void>} Resolves when the file has been written.
*/
async function replaceInFile(file, search, replacement) {
let text = await readFile(file, "utf-8");
text = text.replace(search, replacement);
Expand Down
38 changes: 26 additions & 12 deletions packages/config-array/src/config-array.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,10 @@ function getConfigName(config) {

/**
* Rethrows a config error with additional information about the config object.
* @param {object} config The config object to get the name of.
* @param {ConfigObject} config The config object to get the name of.
* @param {number} index The index of the config object in the array.
* @param {Error} error The error to rethrow.
* @returns {void}
* @throws {ConfigError} When the error is rethrown for a config.
*/
function rethrowConfigError(config, index, error) {
Expand All @@ -160,7 +161,7 @@ function rethrowConfigError(config, index, error) {
/**
* Shorthand for checking if a value is a string.
* @param {any} value The value to check.
* @returns {boolean} True if a string, false if not.
* @returns {value is string} True if a string, false if not.
Copy link
Contributor

Choose a reason for hiding this comment

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

[Praise] Nice "extra credit" to get the type predicate!

*/
function isString(value) {
return typeof value === "string";
Expand Down Expand Up @@ -220,8 +221,8 @@ function assertValidBaseConfig(config, index) {
* faster matching speed over multiple file path evaluations.
* @param {string} filepath The file path to match.
* @param {string} pattern The glob pattern to match against.
* @param {object} options The minimatch options to use.
* @returns
* @param {MinimatchOptions} options The minimatch options to use.
* @returns {boolean} True if the filepath matches the pattern.
*/
function doMatch(filepath, pattern, options = {}) {
let cache = minimatchCache;
Expand Down Expand Up @@ -266,7 +267,6 @@ function normalizePattern(pattern) {
* Checks if a given pattern requires normalization.
* @param {any} pattern The pattern to check.
* @returns {boolean} True if the pattern needs normalization, false otherwise.
*
*/
function needsPatternNormalization(pattern) {
return (
Expand Down Expand Up @@ -363,6 +363,12 @@ async function normalize(
const allowFunctions = extraConfigTypes.includes("function");
const allowArrays = extraConfigTypes.includes("array");

/**
* Recursively flattens and resolves items in the array into config objects.
* @param {Array} array The array to traverse.
* @returns {AsyncGenerator<Object, void, void>} Async generator yielding config objects.
* @throws {TypeError} If an unexpected function or array is encountered.
*/
async function* flatTraverse(array) {
for (let item of array) {
if (typeof item === "function") {
Expand Down Expand Up @@ -395,7 +401,7 @@ async function normalize(
* Async iterables cannot be used with the spread operator, so we need to manually
* create the array to return.
*/
const asyncIterable = await flatTraverse(items);
const asyncIterable = flatTraverse(items);
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically, this is a behavior change. It makes me a little nervous to include any in a chore:, even if it's a no-op. IMO this is fine as flatTraverse isn't async and doesn't return a Promise. Just calling out as something to keep an eye on.

const configs = [];

for await (const config of asyncIterable) {
Expand Down Expand Up @@ -427,6 +433,12 @@ function normalizeSync(
const allowFunctions = extraConfigTypes.includes("function");
const allowArrays = extraConfigTypes.includes("array");

/**
* Recursively flattens and resolves items in the array into config objects.
* @param {Array} array The array to traverse.
* @returns {Generator<Object, void, void>} Generator yielding config objects.
* @throws {TypeError} If an unexpected function, array, or async function is encountered.
*/
function* flatTraverse(array) {
for (let item of array) {
if (typeof item === "function") {
Expand Down Expand Up @@ -484,13 +496,13 @@ function toRelativePath(fileOrDirPath, namespacedBasePath, path) {
/**
* Determines if a given file path should be ignored based on the given
* matcher.
* @param {Array<{ basePath?: string, ignores: Array<string|((string) => boolean)>}>} configs Configuration objects containing `ignores`.
* @param {Array<{ basePath?: string, ignores: Array<string|((filePath: string) => boolean)>}>} configs Configuration objects containing `ignores`.
* @param {string} filePath The unprocessed file path to check.
* @param {string} relativeFilePath The path of the file to check relative to the base path,
* using forward slash (`"/"`) as a separator.
* @param {Object} [basePathData] Additional data needed to recalculate paths for configuration objects
* that have `basePath` property.
* @param {string} [basePathData.basePath] Namespaced path to witch `relativeFilePath` is relative.
* @param {string} [basePathData.basePath] Namespaced path to which `relativeFilePath` is relative.
* @param {PathImpl} [basePathData.path] Path-handling implementation.
* @returns {boolean} True if the path should be ignored and false if not.
*/
Expand Down Expand Up @@ -565,6 +577,12 @@ function shouldIgnorePath(
*/
function pathMatches(filePath, relativeFilePath, config) {
// match both strings and functions
/**
* Matches a pattern against the provided file path.
* @param {string|((filePath: string) => boolean)} pattern The matcher pattern or function.
* @returns {boolean} True if the pattern matches, false otherwise.
* @throws {TypeError} When the matcher type is unexpected.
*/
function match(pattern) {
if (isString(pattern)) {
return doMatch(relativeFilePath, pattern);
Expand Down Expand Up @@ -725,15 +743,13 @@ export class ConfigArray extends Array {

/**
* Tracks if the array has been normalized.
* @property isNormalized
* @type {boolean}
* @private
*/
this[ConfigArraySymbol.isNormalized] = normalized;

/**
* The schema used for validating and merging configs.
* @property schema
* @type {ObjectSchemaInstance}
* @private
*/
Expand All @@ -748,7 +764,6 @@ export class ConfigArray extends Array {
/**
* The path of the config file that this array was loaded from.
* This is used to calculate filename matches.
* @property basePath
* @type {string}
*/
this.basePath = basePath;
Expand All @@ -764,7 +779,6 @@ export class ConfigArray extends Array {

/**
* A cache to store calculated configs for faster repeat lookup.
* @property configCache
* @type {Map<string, Object>}
* @private
*/
Expand Down
6 changes: 3 additions & 3 deletions packages/config-array/src/files-and-ignores-schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/**
* Asserts that a given value is an array.
* @param {*} value The value to check.
* @param {any} value The value to check.
Copy link
Contributor

Choose a reason for hiding this comment

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

[Bug] This won't change anything specifically here, but preferring unknown is generally a better practice and leads to safer default behaviors:

Suggested change
* @param {any} value The value to check.
* @param {unknown} value The value to check.

(here and elsewhere that uses any for an unknown-typed value)

* @returns {void}
* @throws {TypeError} When the value is not an array.
*/
Expand All @@ -28,7 +28,7 @@ function assertIsArray(value) {

/**
* Asserts that a given value is an array containing only strings and functions.
* @param {*} value The value to check.
* @param {any} value The value to check.
* @returns {void}
* @throws {TypeError} When the value is not an array of strings and functions.
*/
Expand All @@ -48,7 +48,7 @@ function assertIsArrayOfStringsAndFunctions(value) {

/**
* Asserts that a given value is a non-empty array.
* @param {*} value The value to check.
* @param {any} value The value to check.
* @returns {void}
* @throws {TypeError} When the value is not an array or an empty array.
*/
Expand Down
14 changes: 14 additions & 0 deletions packages/config-array/tests/config-array.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ const CSSLanguage = class {};
const MarkdownLanguage = class {};
const JSONLanguage = class {};

/**
* Creates a `ConfigArray` pre-populated with test fixtures.
* @param {Object} [options] Options passed to the `ConfigArray` constructor.
* @returns {ConfigArray} The created instance.
*/
function createConfigArray(options) {
return new ConfigArray(
[
Expand Down Expand Up @@ -298,6 +303,15 @@ describe("ConfigArray", () => {
});

describe("Validation", () => {
/**
* Helper to assert validation errors in both async and sync normalize flows.
* @param {Object} options Test options.
* @param {boolean} [options.only=false] If true, run this test exclusively.
* @param {string} options.title Test title prefix.
* @param {Iterable|Function|Object} options.configs Configs to test with.
* @param {assert.AssertPredicate} options.expectedError Expected error matcher.
* @returns {void}
*/
function testValidationError({
only = false,
title,
Expand Down
Loading