ronantakizawa/codereview-bench huggingface dataset:
Repository: eslint/eslint
File: lib/config/flat-config-schema.js (JavaScript)

Code before:
    } else {
        secondMergeMap = new Map();
        mergeMap.set(first, secondMergeMap);
    }

    /*
     * First create a result object where properties from the second object
     * overwrite properties from the first. This sets up a baseline to use
     * later rather than needing to inspect and change every property
     * individually.
     */
    let result = {
        ...first,
        ...second
    };

    delete result.__proto__; // eslint-disable-line no-proto -- don't merge own property "__proto__"
    if (Array.isArray(second)) {
        result = Object.assign([], result);
    }

    // Store the pending result for this combination of first and second arguments.
    secondMergeMap.set(second, result);

    for (const key of Object.keys(second)) {

        // avoid hairy edge case
        if (key === "__proto__") {
            continue;
        }

        const firstValue = first[key];
        const secondValue = second[key];

        if (isNonNullObject(firstValue) && isNonNullObject(secondValue)) {
            result[key] = deepMerge(firstValue, secondValue, mergeMap);
        } else if (isUndefined(secondValue)) {
            result[key] = firstValue;
        }
    }

    return result;

}

/**
 * Normalizes the rule options config for a given rule by ensuring that
 * it is an array and that the first item is 0, 1, or 2.
 * @param {Array|string|number} ruleOptions The rule options config.
 * @returns {Array} An array of rule options.
 */

Review comment (suggestion):
If we don't want the `__proto__`, then I'd suggest using `Object.create(null)` to create `result`.

Code after:

    if (secondMergeMap) {
        const result = secondMergeMap.get(second);

        if (result) {

            // If this combination of first and second arguments has been already visited, return the previously created result.
            return result;
        }
    } else {
        secondMergeMap = new Map();
        mergeMap.set(first, secondMergeMap);
    }

    /*
     * First create a result object where properties from the second object
     * overwrite properties from the first. This sets up a baseline to use
     * later rather than needing to inspect and change every property
     * individually.
     */
    const result = {
        ...first,
        ...second
    };

    delete result.__proto__; // eslint-disable-line no-proto -- don't merge own property "__proto__"

    // Store the pending result for this combination of first and second arguments.
    secondMergeMap.set(second, result);

    for (const key of Object.keys(second)) {

        // avoid hairy edge case
        if (key === "__proto__" || !Object.prototype.propertyIsEnumerable.call(first, key)) {
            continue;
        }

        const firstValue = first[key];
        const secondValue = second[key];

        if (isNonArrayObject(firstValue) && isNonArrayObject(secondValue)) {
            result[key] = deepMerge(firstValue, secondValue, mergeMap);
        } else if (isUndefined(secondValue)) {
            result[key] = firstValue;
        }
    }

    return result;

}
Repository: eslint/eslint
File: lib/shared/serialization.js (JavaScript)

Code before:
 * @returns {boolean} true if so
 * @private
 */
function isSerializablePrimitiveOrPlainObject(val) {
    return (
        val === null ||
        typeof val === "string" ||
         typeof val === "boolean" ||
         typeof val === "number" ||
         (typeof val === "object" && val.constructor === Object) ||
         Array.isArray(val)
    );
}

/**
 * Check if an object is serializable.
 * Functions or objects like RegExp cannot be serialized by JSON.stringify().
 * Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
 * @param {any} obj the object
 * @returns {boolean} true if the object is serializable
 */
function isSerializable(obj) {
    if (!isSerializablePrimitiveOrPlainObject(obj)) {
        return false;
    }
    for (const property in obj) {
        if (Object.hasOwn(obj, property)) {
            if (!isSerializablePrimitiveOrPlainObject(obj[property])) {
                return false;
            }
            if (typeof obj[property] === "object") {
                if (!isSerializable(obj[property])) {
                    return false;
                }
            }
        }
    }
    return true;
}

module.exports = {
    isSerializable
};


Review comment (suggestion):
nits: it's better to use `for...of` to iter arrays - `for...in` is more commonly used for just objects.

Code after:
 * @returns {boolean} true if so
 * @private
 */
function isSerializablePrimitiveOrPlainObject(val) {
    return (
        val === null ||
        typeof val === "string" ||
         typeof val === "boolean" ||
         typeof val === "number" ||
         (typeof val === "object" && val.constructor === Object) ||
         Array.isArray(val)
    );
}

/**
 * Check if a value is serializable.
 * Functions or objects like RegExp cannot be serialized by JSON.stringify().
 * Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
 * @param {any} val the value
 * @returns {boolean} true if the value is serializable
 */
function isSerializable(val) {
    if (!isSerializablePrimitiveOrPlainObject(val)) {
        return false;
    }
    if (typeof val === "object") {
        for (const property in val) {
            if (Object.hasOwn(val, property)) {
                if (!isSerializablePrimitiveOrPlainObject(val[property])) {
                    return false;
                }
                if (typeof val[property] === "object") {
                    if (!isSerializable(val[property])) {
                        return false;
                    }
                }
            }
        }
    }
    return true;
}

module.exports = {
    isSerializable
};
Repository: eslint/eslint
File: lib/shared/serialization.js (JavaScript)

Code before:
 * @returns {boolean} true if so
 * @private
 */
function isSerializablePrimitiveOrPlainObject(val) {
    return (
        val === null ||
        typeof val === "string" ||
         typeof val === "boolean" ||
         typeof val === "number" ||
         (typeof val === "object" && val.constructor === Object) ||
         Array.isArray(val)
    );
}

/**
 * Check if an object is serializable.
 * Functions or objects like RegExp cannot be serialized by JSON.stringify().
 * Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
 * @param {any} obj the object
 * @returns {boolean} true if the object is serializable
 */
function isSerializable(obj) {
    if (!isSerializablePrimitiveOrPlainObject(obj)) {
        return false;
    }
    for (const property in obj) {
        if (Object.hasOwn(obj, property)) {
            if (!isSerializablePrimitiveOrPlainObject(obj[property])) {
                return false;
            }
            if (typeof obj[property] === "object") {
                if (!isSerializable(obj[property])) {
                    return false;
                }
            }
        }
    }
    return true;
}

module.exports = {
    isSerializable
};


Review comment (suggestion):
Does "object" mean that this function expects only objects? If that's not the case, then it might be better to rename `obj` to `val` to clarify that it accepts any value, and also do for-in only on objects so that it doesn't, for example, iterate over string characters (currently, for string test cases, it calls `isSerializablePrimitiveOrPlainObject` for each character of the string).

Code after:
 * @returns {boolean} true if so
 * @private
 */
function isSerializablePrimitiveOrPlainObject(val) {
    return (
        val === null ||
        typeof val === "string" ||
         typeof val === "boolean" ||
         typeof val === "number" ||
         (typeof val === "object" && val.constructor === Object) ||
         Array.isArray(val)
    );
}

/**
 * Check if a value is serializable.
 * Functions or objects like RegExp cannot be serialized by JSON.stringify().
 * Inspired by: https://stackoverflow.com/questions/30579940/reliable-way-to-check-if-objects-is-serializable-in-javascript
 * @param {any} val the value
 * @returns {boolean} true if the value is serializable
 */
function isSerializable(val) {
    if (!isSerializablePrimitiveOrPlainObject(val)) {
        return false;
    }
    if (typeof val === "object") {
        for (const property in val) {
            if (Object.hasOwn(val, property)) {
                if (!isSerializablePrimitiveOrPlainObject(val[property])) {
                    return false;
                }
                if (typeof val[property] === "object") {
                    if (!isSerializable(val[property])) {
                        return false;
                    }
                }
            }
        }
    }
    return true;
}

module.exports = {
    isSerializable
};
Repository: eslint/eslint
File: tests/tools/config-rule.js (JavaScript)

Code before:
/**
 * @fileoverview Tests for ConfigOps
 * @author Ian VanSchooten
 */

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const assert = require("chai").assert,
    { generateConfigsFromSchema } = require("../../tools/config-rule"),
    builtInRules = require("../../lib/rules"),
    schema = require("../fixtures/config-rule/schemas");

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const SEVERITY = 2;

describe("ConfigRule", () => {

    describe("generateConfigsFromSchema()", () => {
        let actualConfigs;

        it("should create a config with only severity for an empty schema", () => {
            actualConfigs = generateConfigsFromSchema([]);
            assert.deepStrictEqual(actualConfigs, [SEVERITY]);
        });

        it("should create a config with only severity with no arguments", () => {
            actualConfigs = generateConfigsFromSchema();
            assert.deepStrictEqual(actualConfigs, [SEVERITY]);
        });

        describe("for a single enum schema", () => {

Review comment (suggestion):
The `ConfigRule.createCoreRuleConfigs` below also needs to be updated:

```suggestion
    { createCoreRuleConfigs, generateConfigsFromSchema } = require("../../tools/config-rule"),
```

Code after:
/**
 * @fileoverview Tests for ConfigOps
 * @author Ian VanSchooten
 */

"use strict";

//------------------------------------------------------------------------------
// Requirements
//------------------------------------------------------------------------------

const assert = require("chai").assert,
    { createCoreRuleConfigs, generateConfigsFromSchema } = require("../../tools/config-rule"),
    builtInRules = require("../../lib/rules"),
    schema = require("../fixtures/config-rule/schemas");

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const SEVERITY = 2;

describe("ConfigRule", () => {

    describe("generateConfigsFromSchema()", () => {
        let actualConfigs;

        it("should create a config with only severity for an empty schema", () => {
            actualConfigs = generateConfigsFromSchema([]);
            assert.deepStrictEqual(actualConfigs, [SEVERITY]);
        });

        it("should create a config with only severity with no arguments", () => {
            actualConfigs = generateConfigsFromSchema();
            assert.deepStrictEqual(actualConfigs, [SEVERITY]);
        });

        describe("for a single enum schema", () => {
Repository: eslint/eslint
File: lib/rules/utils/lazy-loading-rule-map.js (JavaScript)

Code before:
/**
 * @fileoverview `Map` to load rules lazily.
 * @author Toru Nagashima <https://github.com/mysticatea>
 */
"use strict";

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

/**
 * The `Map` object that loads each rule when it's accessed.
 * @example
 * const rules = new LazyLoadingRuleMap([
 *     ["eqeqeq", () => require("eqeqeq")],
 *     ["semi", () => require("semi")],
 *     ["no-unused-vars", () => require("no-unused-vars")]
 * ]);
 *
 * rules.get("semi"); // call `() => require("semi")` here.
 *
 * @extends {Map<string, () => Rule>}
 */
class LazyLoadingRuleMap extends Map {

    /**
     * Initialize this map.
     * @param {Array<[string, function(): Rule]>} loaders The rule loaders.
     */
    constructor(loaders) {
        let remaining = loaders.length;

        super(
            debug.enabled
                ? loaders.map(([ruleId, load]) => {
                    let cache = null;

Review comment (suggestion):
This might work:

```js
/** @typedef {import("../../shared/types").Rule} Rule */
```

Code after:
/**
 * @fileoverview `Map` to load rules lazily.
 * @author Toru Nagashima <https://github.com/mysticatea>
 */
"use strict";

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

/** @typedef {import("../../shared/types").Rule} Rule */

/**
 * The `Map` object that loads each rule when it's accessed.
 * @example
 * const rules = new LazyLoadingRuleMap([
 *     ["eqeqeq", () => require("eqeqeq")],
 *     ["semi", () => require("semi")],
 *     ["no-unused-vars", () => require("no-unused-vars")]
 * ]);
 *
 * rules.get("semi"); // call `() => require("semi")` here.
 *
 * @extends {Map<string, () => Rule>}
 */
class LazyLoadingRuleMap extends Map {

    /**
     * Initialize this map.
     * @param {Array<[string, function(): Rule]>} loaders The rule loaders.
     */
    constructor(loaders) {
        let remaining = loaders.length;

        super(
            debug.enabled
Repository: eslint/eslint
File: tests/lib/rules/no-restricted-imports.js (JavaScript)

Code before:
            line: 1,
            column: 10,
            endColumn: 11
        }, {
            message: "'a' import from 'mod' is restricted.",
            type: "ImportDeclaration",
            line: 1,
            column: 13,
            endColumn: 19
        }]
    }, {
        code: "export { x as y, x as z } from 'mod';",
        options: [{
            paths: [{
                name: "mod",
                importNames: ["x"]
            }]
        }],
        errors: [{
            message: "'x' import from 'mod' is restricted.",
            type: "ExportNamedDeclaration",
            line: 1,
            column: 10,
            endColumn: 16
        }, {
            message: "'x' import from 'mod' is restricted.",
            type: "ExportNamedDeclaration",
            line: 1,
            column: 18,
            endColumn: 24
        }]
    }, {
        code: "import foo, { default as bar } from 'mod';",
        options: [{
            paths: [{
                name: "mod",
                importNames: ["default"]
            }]
        }],
        errors: [{
            message: "'default' import from 'mod' is restricted.",
            type: "ImportDeclaration",
            line: 1,
            column: 8,
            endColumn: 11
        }, {
            message: "'default' import from 'mod' is restricted.",
            type: "ImportDeclaration",
            line: 1,
            column: 15,
            endColumn: 29

Review comment (suggestion):
This produces two errors: one for the whole import, and another specifically for the imported name `bar`. I think it's fine and technically correct.

Code after:

                // message for a specific import name
                {
                    name: "mod",
                    importNames: ["bar"],
                    message: "Import bar from qux instead."
                }
            ]
        }],
        errors: [
            {
                message: "'mod' import is restricted from being used.",
                type: "ImportDeclaration",
                line: 1,
                column: 1,
                endColumn: 26
            },
            {
                message: "'bar' import from 'mod' is restricted. Import bar from qux instead.",
                type: "ImportDeclaration",
                line: 1,
                column: 10,
                endColumn: 13
            }
        ]
    },

    {
        code: "import foo, { bar } from 'mod';",
        options: [{
            paths: [{
                name: "mod",
                importNames: ["default"]
            }]
        }],
        errors: [{
            message: "'default' import from 'mod' is restricted.",
            type: "ImportDeclaration",
            line: 1,
            column: 8,
            endColumn: 11
        }]
    },
    {
        code: "import foo, * as bar from 'mod';",
        options: [{
            paths: [{
                name: "mod",
                importNames: ["default"]
            }]
        }],
Repository: eslint/eslint
File: lib/linter/interpolate.js (JavaScript)

Code before:
/**
 * @fileoverview Interpolate keys from an object into a string with {{ }} markers.
 * @author Jed Fox
 */

"use strict";

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

module.exports = {
    getPlaceholderMatcher() {
        return /\{\{([^{}]+?)\}\}/gu;
    },
    interpolate(text, data) {
        if (!data) {
            return text;
        }

        const matcher = module.exports.getPlaceholderMatcher();

        // Substitution content for any {{ }} markers.
        return text.replace(matcher, (fullMatch, termWithWhitespace) => {
            const term = termWithWhitespace.trim();

            if (term in data) {
                return data[term];
            }

            // Preserve old behavior: If parameter name not provided, don't replace it.
            return fullMatch;
        });
    }
};


Review comment (suggestion):
To avoid confusion.
```suggestion
        const matcher = this.getPlaceholderMatcher();
```

Code after:
/**
 * @fileoverview Interpolate keys from an object into a string with {{ }} markers.
 * @author Jed Fox
 */

"use strict";

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------

/**
 * Returns a global expression matching placeholders in messages.
 * @returns {RegExp} Global regular expression matching placeholders
 */
function getPlaceholderMatcher() {
    return /\{\{([^{}]+?)\}\}/gu;
}

/**
 * Replaces {{ placeholders }} in the message with the provided data.
 * Does not replace placeholders not available in the data.
 * @param {string} text Original message with potential placeholders
 * @param {Record<string, string>} data Map of placeholder name to its value
 * @returns {string} Message with replaced placeholders
 */
function interpolate(text, data) {
    if (!data) {
        return text;
    }

    const matcher = getPlaceholderMatcher();

    // Substitution content for any {{ }} markers.
    return text.replace(matcher, (fullMatch, termWithWhitespace) => {
        const term = termWithWhitespace.trim();

        if (term in data) {
            return data[term];
        }

        // Preserve old behavior: If parameter name not provided, don't replace it.
        return fullMatch;
    });
}
Repository: eslint/eslint
File: tests/lib/api.js (JavaScript)

Code before:
/**
 * @fileoverview Tests for api.
 * @author Gyandeep Singh
 */

"use strict";

//-----------------------------------------------------------------------------
// Requirements
//-----------------------------------------------------------------------------

const assert = require("chai").assert,
    api = require("../../lib/api"),
    { FlatESLint } = require("../../lib/eslint");

//-----------------------------------------------------------------------------
// Tests
//-----------------------------------------------------------------------------

describe("api", () => {

    it("should have ESLint exposed", () => {
        assert.isFunction(api.ESLint);
    });

    it("should have RuleTester exposed", () => {
        assert.isFunction(api.RuleTester);
    });

    it("should not have CLIEngine exposed", () => {
        assert.isUndefined(api.CLIEngine);
    });

    it("should not have linter exposed", () => {
        assert.isUndefined(api.linter);
    });

    it("should have Linter exposed", () => {
        assert.isFunction(api.Linter);

Review comment (suggestion):
```suggestion
    { FlatESLint } = require("../../lib/eslint"),
    os = require("os");
```

Code after:
/**
 * @fileoverview Tests for api.
 * @author Gyandeep Singh
 */

"use strict";

//-----------------------------------------------------------------------------
// Requirements
//-----------------------------------------------------------------------------

const assert = require("chai").assert,
    api = require("../../lib/api"),
    { FlatESLint } = require("../../lib/eslint"),
    os = require("os");

//-----------------------------------------------------------------------------
// Tests
//-----------------------------------------------------------------------------

describe("api", () => {

    it("should have ESLint exposed", () => {
        assert.isFunction(api.ESLint);
    });

    it("should have RuleTester exposed", () => {
        assert.isFunction(api.RuleTester);
    });

    it("should not have CLIEngine exposed", () => {
        assert.isUndefined(api.CLIEngine);
    });

    it("should not have linter exposed", () => {
        assert.isUndefined(api.linter);
    });

    it("should have Linter exposed", () => {
Repository: eslint/eslint
File: docs/src/_data/eslintNextVersion.js (JavaScript)

Code before:
/**
 * @fileoverview
 * @author Nicholas C. Zakas
 */

//-----------------------------------------------------------------------------
// Requirements
//-----------------------------------------------------------------------------

const eleventyFetch  = require("@11ty/eleventy-fetch");

//-----------------------------------------------------------------------------
// Exports
//-----------------------------------------------------------------------------

module.exports = async function() {

    // if we're on the next branch, we can just read the package.json file
    if (true && process.env.BRANCH === "next") {
        return require("../../package.json").version;
    }

    // otherwise, we need to fetch the latest version from the GitHub API
    const url = "https://raw.githubusercontent.com/eslint/eslint/next/docs/package.json";

    const response = await eleventyFetch(url, {
        duration: "1d",
        type: "json"
    });

    return response.version;
}


Review comment (suggestion):
```suggestion
    if (process.env.BRANCH === "next") {
```

Code after:
/**
 * @fileoverview
 * @author Nicholas C. Zakas
 */

//-----------------------------------------------------------------------------
// Requirements
//-----------------------------------------------------------------------------

const eleventyFetch  = require("@11ty/eleventy-fetch");

//-----------------------------------------------------------------------------
// Exports
//-----------------------------------------------------------------------------

module.exports = async function() {

    // if we're on the next branch, we can just read the package.json file
    if (process.env.BRANCH === "next") {
        return require("../../package.json").version;
    }

    // otherwise, we need to fetch the latest version from the GitHub API
    const url = "https://raw.githubusercontent.com/eslint/eslint/next/docs/package.json";

    const response = await eleventyFetch(url, {
        duration: "1d",
        type: "json"
    });

    return response.version;
}