ronantakizawa/codereview-bench huggingface dataset:
Repository: eslint/eslint
File: lib/rules/require-unicode-regexp.js (JavaScript)

Code before:
"use strict";

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

const {
    CALL,
    CONSTRUCT,
    ReferenceTracker,
    getStringIfConstant
} = require("@eslint-community/eslint-utils");
const astUtils = require("./utils/ast-utils.js");
const { isValidWithUnicodeFlag } = require("./utils/regular-expressions");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "Enforce the use of `u` and `v` flag on RegExp",
            recommended: false,
            url: "https://eslint.org/docs/latest/rules/require-unicode-regexp"
        },

        hasSuggestions: true,

        messages: {
            addUFlag: "Add the 'u' flag.",
            requireUFlag: "Use the 'u' flag."
        },

        schema: []
    },

    create(context) {

        const sourceCode = context.sourceCode;

        return {
            "Literal[regex]"(node) {
                const flags = node.regex.flags || "";

                if (!flags.includes("u") && !flags.includes("v")) {
                    context.report({
                        messageId: "requireUFlag",

Review comment (suggestion):
```suggestion
            description: "Enforce the use of `u` or `v` flag on RegExp",
```

I think "or" is correct here.

Code after:
"use strict";

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

const {
    CALL,
    CONSTRUCT,
    ReferenceTracker,
    getStringIfConstant
} = require("@eslint-community/eslint-utils");
const astUtils = require("./utils/ast-utils.js");
const { isValidWithUnicodeFlag } = require("./utils/regular-expressions");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "Enforce the use of `u` or `v` flag on RegExp",
            recommended: false,
            url: "https://eslint.org/docs/latest/rules/require-unicode-regexp"
        },

        hasSuggestions: true,

        messages: {
            addUFlag: "Add the 'u' flag.",
            requireUFlag: "Use the 'u' flag."
        },

        schema: []
    },

    create(context) {

        const sourceCode = context.sourceCode;

        return {
            "Literal[regex]"(node) {
                const flags = node.regex.flags || "";

                if (!flags.includes("u") && !flags.includes("v")) {
                    context.report({
                        messageId: "requireUFlag",
Repository: eslint/eslint
File: tests/lib/rules/no-regex-spaces.js (JavaScript)

Code before:
            code: "var foo = RegExp('\\u0041   ')",
            output: null,
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "3" },
                    type: "CallExpression"
                }
            ]
        },
        {
            code: "var foo = new RegExp('\\\\[  \\\\]');",
            output: null,
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "2" },
                    type: "NewExpression"
                }
            ]
        },

        // ES2024
        {
            code: "var foo = new RegExp('[[    ]    ]    ', 'v');",
            output: "var foo = new RegExp('[[    ]    ] {4}', 'v');",
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "4" },
                    type: "NewExpression"
                }
            ]
        }
    ]
});


Review comment (suggestion):
```suggestion
        // ES2024
        {
            code: "var foo = /[[    ]    ]    /v;",
            output: "var foo = /[[    ]    ] {4}/v;",
            parserOptions: {
                ecmaVersion: 2024
            },
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "4" },
                    type: "Literal"
                }
            ]
        },
```

One test case with regex literals.

Code after:
            ]
        },
        {
            code: "var foo = RegExp('\\u0041   ')",
            output: null,
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "3" },
                    type: "CallExpression"
                }
            ]
        },
        {
            code: "var foo = new RegExp('\\\\[  \\\\]');",
            output: null,
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "2" },
                    type: "NewExpression"
                }
            ]
        },

        // ES2024
        {
            code: "var foo = /[[    ]    ]    /v;",
            output: "var foo = /[[    ]    ] {4}/v;",
            parserOptions: {
                ecmaVersion: 2024
            },
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "4" },
                    type: "Literal"
                }
            ]
        },
        {
            code: "var foo = new RegExp('[[    ]    ]    ', 'v');",
            output: "var foo = new RegExp('[[    ]    ] {4}', 'v');",
            errors: [
                {
                    messageId: "multipleSpaces",
                    data: { length: "4" },
                    type: "NewExpression"
                }
            ]
        }
Repository: eslint/eslint
File: lib/rules/no-return-await.js (JavaScript)

Code before:
/**
 * @fileoverview Disallows unnecessary `return await`
 * @author Jordan Harband
 * @deprecated
 */
"use strict";

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        hasSuggestions: true,
        type: "suggestion",

        docs: {
            description: "Disallow unnecessary `return await`",

            recommended: false,

            url: "https://eslint.org/docs/latest/rules/no-return-await"
        },

        fixable: null,


Review comment (suggestion):
```suggestion
 * @deprecated in ESLint v8.46.0
```

Code after:
/**
 * @fileoverview Disallows unnecessary `return await`
 * @author Jordan Harband
 * @deprecated in ESLint v8.46.0
 */
"use strict";

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        hasSuggestions: true,
        type: "suggestion",

        docs: {
            description: "Disallow unnecessary `return await`",

            recommended: false,

            url: "https://eslint.org/docs/latest/rules/no-return-await"
        },

        fixable: null,
Repository: eslint/eslint
File: tests/lib/rule-tester/flat-rule-tester.js (JavaScript)

Code before:
            sinon.assert.calledWith(spyRuleTesterDescribe, "valid");
        });

        it("should not create a valid test suite if there are no valid test cases", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: [],
                invalid: []
            });
            sinon.assert.neverCalledWith(spyRuleTesterDescribe, "valid");
        });

        it("should create a valid test suite if there is an invalid test cases", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: ["value = 0;"],
                invalid: [
                    {
                        code: "var value = 0;",
                        errors: [/^Bad var/u],
                        output: " value = 0;"
                    }
                ]
            });
            sinon.assert.calledWith(spyRuleTesterDescribe, "invalid");
        });

        it("should not create a invalid test suite if there are no invalid test cases", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: ["value = 0;"],
                invalid: []
            });
            sinon.assert.neverCalledWith(spyRuleTesterDescribe, "invalid");
        });
    });
});


Review comment (suggestion):
```suggestion
        it("should not create an invalid test suite if there are no invalid test cases", () => {
```

Code after:
            sinon.assert.calledWith(spyRuleTesterDescribe, "valid");
        });

        it("should not create a valid test suite if there are no valid test cases", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: [],
                invalid: [
                    {
                        code: "var value = 0;",
                        errors: [/^Bad var/u],
                        output: " value = 0;"
                    }
                ]
            });
            sinon.assert.neverCalledWith(spyRuleTesterDescribe, "valid");
        });

        it("should create an invalid test suite if there is an invalid test case", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: [],
                invalid: [
                    {
                        code: "var value = 0;",
                        errors: [/^Bad var/u],
                        output: " value = 0;"
                    }
                ]
            });
            sinon.assert.calledWith(spyRuleTesterDescribe, "invalid");
        });

        it("should not create an invalid test suite if there are no invalid test cases", () => {
            ruleTester.run("no-var", require("../../fixtures/testers/rule-tester/no-var"), {
                valid: ["value = 0;"],
                invalid: []
            });
            sinon.assert.neverCalledWith(spyRuleTesterDescribe, "invalid");
        });
    });
});
Repository: eslint/eslint
File: lib/options.js (JavaScript)

Code before:
    if (!usingFlatConfig) {
        rulesDirFlag = {
            option: "rulesdir",
            type: "[path::String]",
            description: "Load additional rules from this directory. Deprecated: Use rules from plugins"
        };
    }

    let ignorePathFlag;

    if (!usingFlatConfig) {
        ignorePathFlag = {
            option: "ignore-path",
            type: "path::String",
            description: "Specify path of ignore file"
        };
    }

    let warnIgnoredFlag;

    if (usingFlatConfig) {
        warnIgnoredFlag = {
            option: "warn-ignored",
            type: "Boolean",
            default: "true",
            description: "Suppress warning when the file list includes ignored files"
        };
    }

    return optionator({
        prepend: "eslint [options] file.js [file.js] [dir]",
        defaults: {
            concatRepeatedArrays: true,
            mergeRepeatedObjects: true
        },
        options: [
            {
                heading: "Basic configuration"
            },
            lookupFlag,
            {
                option: "config",
                alias: "c",
                type: "path::String",
                description: usingFlatConfig
                    ? "Use this configuration instead of eslint.config.js"
                    : "Use this configuration, overriding .eslintrc.* config options if present"
            },
            envFlag,
            extFlag,
            {

Review comment (suggestion):
```suggestion
            description: "Suppress warnings when the file list includes ignored files"
```

Code after:
    if (!usingFlatConfig) {
        rulesDirFlag = {
            option: "rulesdir",
            type: "[path::String]",
            description: "Load additional rules from this directory. Deprecated: Use rules from plugins"
        };
    }

    let ignorePathFlag;

    if (!usingFlatConfig) {
        ignorePathFlag = {
            option: "ignore-path",
            type: "path::String",
            description: "Specify path of ignore file"
        };
    }

    let warnIgnoredFlag;

    if (usingFlatConfig) {
        warnIgnoredFlag = {
            option: "warn-ignored",
            type: "Boolean",
            default: "true",
            description: "Suppress warnings when the file list includes ignored files"
        };
    }

    return optionator({
        prepend: "eslint [options] file.js [file.js] [dir]",
        defaults: {
            concatRepeatedArrays: true,
            mergeRepeatedObjects: true
        },
        options: [
            {
                heading: "Basic configuration"
            },
            lookupFlag,
            {
                option: "config",
                alias: "c",
                type: "path::String",
                description: usingFlatConfig
                    ? "Use this configuration instead of eslint.config.js"
                    : "Use this configuration, overriding .eslintrc.* config options if present"
            },
            envFlag,
            extFlag,
            {
Repository: eslint/eslint
File: lib/rules/no-new-object.js (JavaScript)

Code before:
/**
 * @fileoverview A rule to disallow calls to the Object constructor
 * @author Matt DuVall <http://www.mattduvall.com/>
 * @deprecated in ESLint v8.XX.0
 */

"use strict";

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

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "Disallow `Object` constructors",
            recommended: false,
            url: "https://eslint.org/docs/latest/rules/no-new-object"
        },


Review comment (suggestion):
```suggestion
 * @deprecated in ESLint v8.50.0
```

v8.50.0 or later

Code after:
/**
 * @fileoverview A rule to disallow calls to the Object constructor
 * @author Matt DuVall <http://www.mattduvall.com/>
 * @deprecated in ESLint v8.50.0
 */

"use strict";

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

const astUtils = require("./utils/ast-utils");

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/** @type {import('../shared/types').Rule} */
module.exports = {
    meta: {
        type: "suggestion",

        docs: {
            description: "Disallow `Object` constructors",
            recommended: false,
            url: "https://eslint.org/docs/latest/rules/no-new-object"
        },
Repository: eslint/eslint
File: lib/linter/config-comment-parser.js (JavaScript)

Code before:
                    line: location.start.line,
                    column: location.start.column + 1,
                    nodeType: null
                }
            };

        }

        return {
            success: true,
            config: items
        };
    }

    /**
     * Parses a config of values separated by comma.
     * @param {string} string The string to parse.
     * @returns {Object} Result map of values and true values
     */
    parseListConfig(string) {
        debug("Parsing list config");

        const items = {};

        string.split(",").forEach(name => {
            const trimmedName = name.replace(/^\s*(?<quote>['"]?)(?<ruleId>\S*)\k<quote>\s*$/u, "$<ruleId>");

            if (trimmedName) {
                items[trimmedName] = true;
            }
        });
        return items;
    }

};


Review comment (suggestion):
If this regex doesn't match, which can happen when there's whitespace between non-whitespace (e.g., when the comma is accidentally omitted), it won't trim the list element. For example:

```js
// Definition for rule ' no-undef no-unused-vars ' was not found
/* eslint-disable no-undef no-unused-vars , no-alert */
```

Perhaps we could first `.trim()`, and then just remove quotes?

```suggestion
            const trimmedName = name.trim().replace(/^(?<quote>['"]?)(?<ruleId>.*)\k<quote>$/us, "$<ruleId>");
```



Code after:
                    line: location.start.line,
                    column: location.start.column + 1,
                    nodeType: null
                }
            };

        }

        return {
            success: true,
            config: items
        };
    }

    /**
     * Parses a config of values separated by comma.
     * @param {string} string The string to parse.
     * @returns {Object} Result map of values and true values
     */
    parseListConfig(string) {
        debug("Parsing list config");

        const items = {};

        string.split(",").forEach(name => {
            const trimmedName = name.trim().replace(/^(?<quote>['"]?)(?<ruleId>.*)\k<quote>$/us, "$<ruleId>");

            if (trimmedName) {
                items[trimmedName] = true;
            }
        });
        return items;
    }

};
Repository: eslint/eslint
File: tests/lib/rules/no-object-constructor.js (JavaScript)

Code before:
            {
                code: `
                foo = this.return
                Object()
                `
            },
            {
                code: `
                var yield = bar.yield
                Object()
                `
            },
            {
                code: `
                var foo = { bar: baz }
                Object()
                `
            }
        ].map(props => ({
            ...props,
            errors: [{
                messageId: "preferLiteral",
                suggestions: [{
                    desc: "Replace with '({})', add preceding semicolon.",
                    messageId: "useLiteralAfterSemicolon"
                }]
            }]
        })),

        ...[

            // No semicolon required before `({})` because ASI does not occur
            { code: "Object()" },
            {
                code: `
                {}
                Object()
                `
            },
            {
                code: `
                function foo() {}
                Object()
                `
            },
            {
                code: `
                class Foo {}
                Object()
                `
            },

Review comment (suggestion):
Suggestion test objects should generally have the `output` property (and it will become mandatory after we implement https://github.com/eslint/rfcs/pull/84).

Code after:
            {
                code: `
                foo = this.return
                Object()
                `
            },
            {
                code: `
                var yield = bar.yield
                Object()
                `
            },
            {
                code: `
                var foo = { bar: baz }
                Object()
                `
            }
        ].map(props => ({
            ...props,
            errors: [{
                messageId: "preferLiteral",
                suggestions: [{
                    desc: "Replace with '({})', add preceding semicolon.",
                    messageId: "useLiteralAfterSemicolon",
                    output: props.code.replace(/(new )?Object\(\)/u, ";({})")
                }]
            }]
        })),

        ...[

            // No semicolon required before `({})` because ASI does not occur
            { code: "Object()" },
            {
                code: `
                {}
                Object()
                `
            },
            {
                code: `
                function foo() {}
                Object()
                `
            },
            {
                code: `
                class Foo {}
                Object()
                `
Repository: eslint/eslint
File: tests/lib/config/flat-config-array.js (JavaScript)

Code before:
                    camelcase: ruleConfig,
                    "default-case": ruleConfig
                }
            }]);

            configs.normalizeSync();

            const config = configs.getConfig("foo.js");

            assert.deepStrictEqual(config.rules, {
                camelcase: [2, {
                    ignoreDestructuring: false,
                    ignoreGlobals: false,
                    ignoreImports: false
                }],
                "default-case": [2, {}]
            });

        });


        it("should throw rule validation error for camelcase", async () => {


            /* eslint-disable-next-line prefer-const -- need to allow mutation inside FlatConfigArray */
            let ruleConfig = ["error", {}];

            const configs = new FlatConfigArray([{
                rules: {
                    camelcase: ruleConfig,
                    "default-case": ruleConfig,

                    /* eslint-disable-next-line no-dupe-keys -- needed for testing */
                    camelcase: [
                        "error",
                        {
                            ignoreDestructuring: Date
                        }

                    ]
                }
            }]);

            configs.normalizeSync();

            // exact error may differ based on structuredClone implementation so just test prefix
            assert.throws(() => {
                configs.getConfig("foo.js");
            }, /Key "rules": Key "camelcase":/u);

        });

Review comment (suggestion):
```suggestion
            const ruleConfig = ["error", {}];
```

Code after:
                    "default-case": ruleConfig
                }
            }]);

            configs.normalizeSync();

            const config = configs.getConfig("foo.js");

            assert.deepStrictEqual(config.rules, {
                camelcase: [2, {
                    ignoreDestructuring: false,
                    ignoreGlobals: false,
                    ignoreImports: false
                }],
                "default-case": [2, {}]
            });

        });


        it("should throw rule validation error for camelcase", async () => {

            const ruleConfig = ["error", {}];

            const configs = new FlatConfigArray([
                {
                    rules: {
                        camelcase: ruleConfig
                    }
                },
                {
                    rules: {
                        "default-case": ruleConfig,


                        camelcase: [
                            "error",
                            {
                                ignoreDestructuring: Date
                            }

                        ]
                    }
                }
            ]);

            configs.normalizeSync();

            // exact error may differ based on structuredClone implementation so just test prefix
            assert.throws(() => {
                configs.getConfig("foo.js");