ronantakizawa/codereview-bench huggingface dataset:
Repository: eslint/eslint
File: tests/lib/rules/no-useless-assignment.js (JavaScript)

Code before:
            b = (a = 2)
        } = obj;
        console.log(a, b);`,
        `let { a, b: {c = a} = {} } = obj;
        console.log(c);`,
        {
            code: `/*eslint test/jsx:1*/
                function App() {
                    const A = "";
                    return <A/>;
                }
            `,
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        jsx: true
                    }
                }
            }
        },
        {
            code: `/*eslint test/jsx:1*/
                function App() {
                    let A = "";
                    foo(A);
                    a = "A";
                    return <A/>;
                }
            `,
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        jsx: true
                    }
                }
            }
        }
    ],
    invalid: [
        {
            code:
            `let v = 'used';
            console.log(v);
            v = 'unused'`,
            errors: [
                {
                    messageId: "unnecessaryAssignment",
                    line: 3,
                    column: 13
                }
            ]

Review comment (suggestion):
This test is passing without the code change. Should this line maybe be this:

```suggestion
                    A = "a";
```

Code after:
            b = (a = 2)
        } = obj;
        console.log(a, b);`,
        `let { a, b: {c = a} = {} } = obj;
        console.log(c);`,
        {
            code: `/*eslint test/jsx:1*/
                function App() {
                    const A = "";
                    return <A/>;
                }
            `,
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        jsx: true
                    }
                }
            }
        },
        {
            code: `/*eslint test/jsx:1*/
                function App() {
                    let A = "";
                    foo(A);
                    A = "A";
                    return <A/>;
                }
            `,
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        jsx: true
                    }
                }
            }
        },
        {
            code: `/*eslint test/jsx:1*/
                function App() {
                    let A = "a";
                    A = "b";
                    A = "c";
                    foo(A);
                    return <A/>;
                }
            `,
            languageOptions: {
                parserOptions: {
                    ecmaFeatures: {
                        jsx: true
Repository: eslint/eslint
File: messages/config-serialize-function.js (JavaScript)

Code before:
"use strict";

module.exports = function({ key, objectKey }) {

    // special case for parsers
    const isParser = objectKey === "parser" && key === "parse";
    const parserMessage = `
    This typically happens when you're using a custom parser that does not
provide a "meta" property, which is how ESLint determines the serialized
representation. Please open an issue with the maintainer of the custom parser
and share this link:

https://eslint.org/docs/latest/extend/custom-parsers#meta-data-in-custom-parsers
`.trimStart();

    return `
The requested operation requires ESLint to serialize configuration data,
but the configuration key "${objectKey}.${key}" contains a function value,
which cannot be serialized.

${
    isParser ? parserMessage : "Please double-check your configuration for errors."
}

If you still have problems, please stop by https://eslint.org/chat/help to chat
with the team.
`.trimStart();
};


Review comment (suggestion):
```suggestion
    const isParser = objectKey === "parser" && (key === "parse" || key === "parseForESLint");
```

Custom parsers more often provide `parseForESLint()` instead of `parse()`.

Code after:
"use strict";

module.exports = function({ key, objectKey }) {

    // special case for parsers
    const isParser = objectKey === "parser" && (key === "parse" || key === "parseForESLint");
    const parserMessage = `
    This typically happens when you're using a custom parser that does not
provide a "meta" property, which is how ESLint determines the serialized
representation. Please open an issue with the maintainer of the custom parser
and share this link:

https://eslint.org/docs/latest/extend/custom-parsers#meta-data-in-custom-parsers
`.trim();

    return `
The requested operation requires ESLint to serialize configuration data,
but the configuration key "${objectKey}.${key}" contains a function value,
which cannot be serialized.

${
    isParser ? parserMessage : "Please double-check your configuration for errors."
}

If you still have problems, please stop by https://eslint.org/chat/help to chat
with the team.
`.trimStart();
};
Repository: eslint/eslint
File: messages/config-serialize-function.js (JavaScript)

Code before:
"use strict";

module.exports = function({ key, objectKey }) {

    // special case for parsers
    const isParser = objectKey === "parser" && key === "parse";
    const parserMessage = `
    This typically happens when you're using a custom parser that does not
provide a "meta" property, which is how ESLint determines the serialized
representation. Please open an issue with the maintainer of the custom parser
and share this link:

https://eslint.org/docs/latest/extend/custom-parsers#meta-data-in-custom-parsers
`.trimStart();

    return `
The requested operation requires ESLint to serialize configuration data,
but the configuration key "${objectKey}.${key}" contains a function value,
which cannot be serialized.

${
    isParser ? parserMessage : "Please double-check your configuration for errors."
}

If you still have problems, please stop by https://eslint.org/chat/help to chat
with the team.
`.trimStart();
};


Review comment (suggestion):
```suggestion
`.trim();
```

To avoid having two blank lines between this link and "If you still have problems...".

Code after:
"use strict";

module.exports = function({ key, objectKey }) {

    // special case for parsers
    const isParser = objectKey === "parser" && (key === "parse" || key === "parseForESLint");
    const parserMessage = `
    This typically happens when you're using a custom parser that does not
provide a "meta" property, which is how ESLint determines the serialized
representation. Please open an issue with the maintainer of the custom parser
and share this link:

https://eslint.org/docs/latest/extend/custom-parsers#meta-data-in-custom-parsers
`.trim();

    return `
The requested operation requires ESLint to serialize configuration data,
but the configuration key "${objectKey}.${key}" contains a function value,
which cannot be serialized.

${
    isParser ? parserMessage : "Please double-check your configuration for errors."
}

If you still have problems, please stop by https://eslint.org/chat/help to chat
with the team.
`.trimStart();
};
Repository: eslint/eslint
File: lib/rules/no-useless-assignment.js (JavaScript)

Code before:
/**
 * @fileoverview A rule to disallow unnecessary assignments`.
 * @author Yosuke Ota
 */

"use strict";

const { findVariable } = require("@eslint-community/eslint-utils");
const { cat } = require("shelljs");

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

/** @typedef {import("estree").Node} ASTNode */
/** @typedef {import("estree").Pattern} Pattern */
/** @typedef {import("estree").Identifier} Identifier */
/** @typedef {import("estree").VariableDeclarator} VariableDeclarator */
/** @typedef {import("estree").AssignmentExpression} AssignmentExpression */
/** @typedef {import("estree").UpdateExpression} UpdateExpression */
/** @typedef {import("estree").Expression} Expression */
/** @typedef {import("eslint-scope").Scope} Scope */
/** @typedef {import("eslint-scope").Variable} Variable */
/** @typedef {import("../linter/code-path-analysis/code-path")} CodePath */
/** @typedef {import("../linter/code-path-analysis/code-path-segment")} CodePathSegment */

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
 * Extract identifier from the given pattern node used on the left-hand side of the assignment.
 * @param {Pattern} pattern The pattern node to extract identifier
 * @returns {Iterable<Identifier>} The extracted identifier

Review comment (suggestion):
```suggestion
```

This seems accidentally added.

Code after:
/**
 * @fileoverview A rule to disallow unnecessary assignments`.
 * @author Yosuke Ota
 */

"use strict";

const { findVariable } = require("@eslint-community/eslint-utils");

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

/** @typedef {import("estree").Node} ASTNode */
/** @typedef {import("estree").Pattern} Pattern */
/** @typedef {import("estree").Identifier} Identifier */
/** @typedef {import("estree").VariableDeclarator} VariableDeclarator */
/** @typedef {import("estree").AssignmentExpression} AssignmentExpression */
/** @typedef {import("estree").UpdateExpression} UpdateExpression */
/** @typedef {import("estree").Expression} Expression */
/** @typedef {import("eslint-scope").Scope} Scope */
/** @typedef {import("eslint-scope").Variable} Variable */
/** @typedef {import("../linter/code-path-analysis/code-path")} CodePath */
/** @typedef {import("../linter/code-path-analysis/code-path-segment")} CodePathSegment */

//------------------------------------------------------------------------------
// Helpers
//------------------------------------------------------------------------------

/**
 * Extract identifier from the given pattern node used on the left-hand side of the assignment.
 * @param {Pattern} pattern The pattern node to extract identifier
 * @returns {Iterable<Identifier>} The extracted identifier
 */
Repository: eslint/eslint
File: docs/src/_plugins/pre-wrapper.js (JavaScript)

Code before:
module.exports = (md) => {
    const defaultFenceRenderer = md.renderer.rules.fence;

    md.renderer.rules.fence = (...args) => {
        const [tokens, index] = args;

        if (/^\s*(?:in)?correct(?!\S)/u.test(tokens[index - 1].info)) {
            return defaultFenceRenderer(...args);
        }

        return `
<div class="code-wrapper">
    ${defaultFenceRenderer(...args)}
    <button class="copy-btn" id="copyBtn" aria-labelledby="copy-button-label">
        <span hidden id="copy-button-label">Copy code to clipboard</span>
        <svg width="20" height="20" viewBox="0 0 20 20" role="img" aria-label="copy" fill="none" focusable="false">
            <path d="M4.16667 12.5H3.33333C2.89131 12.5 2.46738 12.3244 2.15482 12.0118C1.84226 11.6993 1.66667 11.2754 1.66667 10.8333V3.33332C1.66667 2.8913 1.84226 2.46737 2.15482 2.15481C2.46738 1.84225 2.89131 1.66666 3.33333 1.66666H10.8333C11.2754 1.66666 11.6993 1.84225 12.0118 2.15481C12.3244 2.46737 12.5 2.8913 12.5 3.33332V4.16666M9.16667 7.49999H16.6667C17.5871 7.49999 18.3333 8.24618 18.3333 9.16666V16.6667C18.3333 17.5871 17.5871 18.3333 16.6667 18.3333H9.16667C8.24619 18.3333 7.5 17.5871 7.5 16.6667V9.16666C7.5 8.24618 8.24619 7.49999 9.16667 7.49999Z" stroke="currentColor" stroke-width="1.66667" stroke-linecap="round" stroke-linejoin="round" />
        </svg>
    </button>
</div>
`;
    };
};


Review comment (suggestion):
```suggestion
    <button class="copy-btn" aria-labelledby="copy-button-label">
```
Let's avoid giving the same ID to all buttons, as it's not recommended since IDs should be unique.

Code after:
module.exports = (md) => {
    const defaultFenceRenderer = md.renderer.rules.fence;

    md.renderer.rules.fence = (...args) => {
        const [tokens, index] = args;

        if (/^\s*(?:in)?correct(?!\S)/u.test(tokens[index - 1].info)) {
            return defaultFenceRenderer(...args);
        }

        return `
<div class="code-wrapper">
    ${defaultFenceRenderer(...args)}
    <button class="copy-btn" aria-labelledby="copy-button-label">
        <span hidden id="copy-button-label">Copy code to clipboard</span>
        <svg width="20" height="20" viewBox="0 0 20 20" role="img" aria-label="copy" fill="none" focusable="false">
            <path d="M4.16667 12.5H3.33333C2.89131 12.5 2.46738 12.3244 2.15482 12.0118C1.84226 11.6993 1.66667 11.2754 1.66667 10.8333V3.33332C1.66667 2.8913 1.84226 2.46737 2.15482 2.15481C2.46738 1.84225 2.89131 1.66666 3.33333 1.66666H10.8333C11.2754 1.66666 11.6993 1.84225 12.0118 2.15481C12.3244 2.46737 12.5 2.8913 12.5 3.33332V4.16666M9.16667 7.49999H16.6667C17.5871 7.49999 18.3333 8.24618 18.3333 9.16666V16.6667C18.3333 17.5871 17.5871 18.3333 16.6667 18.3333H9.16667C8.24619 18.3333 7.5 17.5871 7.5 16.6667V9.16666C7.5 8.24618 8.24619 7.49999 9.16667 7.49999Z" stroke="currentColor" stroke-width="1.66667" stroke-linecap="round" stroke-linejoin="round" />
        </svg>
    </button>
</div>
`;
    };
};
Repository: eslint/eslint
File: lib/rules/init-declarations.js (JavaScript)

Code before:
	},

	create(context) {
		const MODE_ALWAYS = "always",
			MODE_NEVER = "never";

		const mode = context.options[0] || MODE_ALWAYS;
		const params = context.options[1] || {};

		// Track whether we're inside a declared namespace
		let insideDeclaredNamespace = false;

		//--------------------------------------------------------------------------
		// Public API
		//--------------------------------------------------------------------------

		return {
			TSModuleDeclaration(node) {
				if (node.declare) {
					insideDeclaredNamespace = true;
				}
			},

			"TSModuleDeclaration:exit"(node) {
				if (node.declare) {
					insideDeclaredNamespace = false;
				}
			},

			"VariableDeclaration:exit"(node) {
				const kind = node.kind,
					declarations = node.declarations;

				if (mode === MODE_ALWAYS) {
					if (node.declare) {
						return;
					}
					if (insideDeclaredNamespace) {
						return;
					}
				}

				for (let i = 0; i < declarations.length; ++i) {
					const declaration = declarations[i],
						id = declaration.id,
						initialized = isInitialized(declaration),
						isIgnoredForLoop =
							params.ignoreForLoopInit && isForLoop(node.parent);
					let messageId = "";

					if (mode === MODE_ALWAYS && !initialized) {

Review comment (bug):
[Testing] Removing this line doesn't fail any tests right now. But it's definitely an important line to have for cases of variables after a declared namespace:

```ts
declare namespace myLib {
    let valueInside: number;
}
let valueOutside: number;
```

Code after:
	},

	create(context) {
		const MODE_ALWAYS = "always",
			MODE_NEVER = "never";

		const mode = context.options[0] || MODE_ALWAYS;
		const params = context.options[1] || {};

		// Track whether we're inside a declared namespace
		let insideDeclaredNamespace = false;

		//--------------------------------------------------------------------------
		// Public API
		//--------------------------------------------------------------------------

		return {
			TSModuleDeclaration(node) {
				if (node.declare) {
					insideDeclaredNamespace = true;
				}
			},

			"TSModuleDeclaration:exit"(node) {
				if (node.declare) {
					insideDeclaredNamespace = false;
				}
			},

			"VariableDeclaration:exit"(node) {
				const kind = node.kind,
					declarations = node.declarations;

				if (node.declare || insideDeclaredNamespace) {
					return;
				}

				for (let i = 0; i < declarations.length; ++i) {
					const declaration = declarations[i],
						id = declaration.id,
						initialized = isInitialized(declaration),
						isIgnoredForLoop =
							params.ignoreForLoopInit && isForLoop(node.parent);
					let messageId = "";

					if (mode === MODE_ALWAYS && !initialized) {
						messageId = "initialized";
					} else if (
						mode === MODE_NEVER &&
						kind !== "const" &&
						initialized &&
Repository: eslint/eslint
File: lib/rules/init-declarations.js (JavaScript)

Code before:

		return {
			TSModuleDeclaration(node) {
				if (node.declare) {
					insideDeclaredNamespace = true;
				}
			},

			"TSModuleDeclaration:exit"(node) {
				if (node.declare) {
					insideDeclaredNamespace = false;
				}
			},

			"VariableDeclaration:exit"(node) {
				const kind = node.kind,
					declarations = node.declarations;

				if (mode === MODE_ALWAYS) {
					if (node.declare) {
						return;
					}
					if (insideDeclaredNamespace) {
						return;
					}
				}

				for (let i = 0; i < declarations.length; ++i) {
					const declaration = declarations[i],
						id = declaration.id,
						initialized = isInitialized(declaration),
						isIgnoredForLoop =
							params.ignoreForLoopInit && isForLoop(node.parent);
					let messageId = "";

					if (mode === MODE_ALWAYS && !initialized) {
						messageId = "initialized";
					} else if (
						mode === MODE_NEVER &&
						kind !== "const" &&
						initialized &&
						!isIgnoredForLoop
					) {
						messageId = "notInitialized";
					}

					if (id.type === "Identifier" && messageId) {
						context.report({
							node: declaration,
							messageId,
							data: {

Review comment (bug):
`declare`d variables circumvent the rule anyway, so I don't think we need to check the mode here. Tests don't fail if we remove the `if`:

```suggestion
				if (node.declare || insideDeclaredNamespace) {
					return;
				}
```

Code after:

		return {
			TSModuleDeclaration(node) {
				if (node.declare) {
					insideDeclaredNamespace = true;
				}
			},

			"TSModuleDeclaration:exit"(node) {
				if (node.declare) {
					insideDeclaredNamespace = false;
				}
			},

			"VariableDeclaration:exit"(node) {
				const kind = node.kind,
					declarations = node.declarations;

				if (node.declare || insideDeclaredNamespace) {
					return;
				}

				for (let i = 0; i < declarations.length; ++i) {
					const declaration = declarations[i],
						id = declaration.id,
						initialized = isInitialized(declaration),
						isIgnoredForLoop =
							params.ignoreForLoopInit && isForLoop(node.parent);
					let messageId = "";

					if (mode === MODE_ALWAYS && !initialized) {
						messageId = "initialized";
					} else if (
						mode === MODE_NEVER &&
						kind !== "const" &&
						initialized &&
						!isIgnoredForLoop
					) {
						messageId = "notInitialized";
					}

					if (id.type === "Identifier" && messageId) {
						context.report({
							node: declaration,
							messageId,
							data: {
								idName: id.name,
							},
						});
					}
				}
Repository: eslint/eslint
File: lib/rules/max-params.js (JavaScript)

Code before:
						additionalProperties: false,
					},
				],
			},
		],
		messages: {
			exceed: "{{name}} has too many parameters ({{count}}). Maximum allowed is {{max}}.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;
		const option = context.options[0];
		let numParams = 3;
		let countVoidThis = false;

		if (typeof option === "object") {
			if (
				Object.hasOwn(option, "maximum") ||
				Object.hasOwn(option, "max")
			) {
				numParams = option.maximum || option.max;
			}
			if (Object.hasOwn(option, "countVoidThis")) {
				countVoidThis = option.countVoidThis;
			}
		}
		if (typeof option === "number") {
			numParams = option;
		}

		/**
		 * Removes 'this' parameter with void type annotation from parameter count
		 * @param {ASTNode} node The node to process.
		 * @returns {ASTNode} The processed node.
		 * @private
		 */
		function removeVoidThisParam(node) {
			if (
				node.params.length === 0 ||
				node.params[0].type !== "Identifier" ||
				node.params[0].name !== "this" ||
				node.params[0].typeAnnotation?.typeAnnotation.type !==
					"TSVoidKeyword"
			) {
				return node;
			}

			return {
				...node,
				params: node.params.slice(1),

Review comment (bug):
My understanding of the existing `Object.hasOwn`s is to let `0` act as truthy in the logical checks. But I don't think that's necessary for `countVoidThis`? No tests fail if we skip it:

```suggestion
			countVoidThis = option.countVoidThis;
```

Code after:
						additionalProperties: false,
					},
				],
			},
		],
		messages: {
			exceed: "{{name}} has too many parameters ({{count}}). Maximum allowed is {{max}}.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;
		const option = context.options[0];
		let numParams = 3;
		let countVoidThis = false;

		if (typeof option === "object") {
			if (
				Object.hasOwn(option, "maximum") ||
				Object.hasOwn(option, "max")
			) {
				numParams = option.maximum || option.max;
			}
			countVoidThis = option.countVoidThis;
		}
		if (typeof option === "number") {
			numParams = option;
		}

		/**
		 * Checks a function to see if it has too many parameters.
		 * @param {ASTNode} node The node to check.
		 * @returns {void}
		 * @private
		 */
		function checkFunction(node) {
			const hasVoidThisParam =
				node.params.length > 0 &&
				node.params[0].type === "Identifier" &&
				node.params[0].name === "this" &&
				node.params[0].typeAnnotation?.typeAnnotation.type ===
					"TSVoidKeyword";

			const effectiveParamCount =
				hasVoidThisParam && !countVoidThis
					? node.params.length - 1
					: node.params.length;

			if (effectiveParamCount > numParams) {
				context.report({
					loc: astUtils.getFunctionHeadLoc(node, sourceCode),
Repository: eslint/eslint
File: lib/rules/max-params.js (JavaScript)

Code before:
				node.params[0].type !== "Identifier" ||
				node.params[0].name !== "this" ||
				node.params[0].typeAnnotation?.typeAnnotation.type !==
					"TSVoidKeyword"
			) {
				return node;
			}

			return {
				...node,
				params: node.params.slice(1),
			};
		}

		/**
		 * Checks a function to see if it has too many parameters.
		 * @param {ASTNode} node The node to check.
		 * @returns {void}
		 * @private
		 */
		function checkFunction(node) {
			const processedNode = !countVoidThis
				? removeVoidThisParam(node)
				: node;

			if (processedNode.params.length > numParams) {
				context.report({
					loc: astUtils.getFunctionHeadLoc(node, sourceCode),
					node,
					messageId: "exceed",
					data: {
						name: upperCaseFirst(
							astUtils.getFunctionNameWithKind(node),
						),
						count: processedNode.params.length,
						max: numParams,
					},
				});
			}
		}

		return {
			FunctionDeclaration: checkFunction,
			ArrowFunctionExpression: checkFunction,
			FunctionExpression: checkFunction,
			TSDeclareFunction: checkFunction,
			TSFunctionType: checkFunction,
		};
	},
};


Review comment (refactor):
Can we simplify this? Creating a new node with different params seemed suitable for an extension rule to reuse the logic from the base rule. Here, if the function has a void this param, we could just compare its `params.length -1` instead of `params.length`.

Code after:
				node.params[0].type === "Identifier" &&
				node.params[0].name === "this" &&
				node.params[0].typeAnnotation?.typeAnnotation.type ===
					"TSVoidKeyword";

			const effectiveParamCount =
				hasVoidThisParam && !countVoidThis
					? node.params.length - 1
					: node.params.length;

			if (effectiveParamCount > numParams) {
				context.report({
					loc: astUtils.getFunctionHeadLoc(node, sourceCode),
					node,
					messageId: "exceed",
					data: {
						name: upperCaseFirst(
							astUtils.getFunctionNameWithKind(node),
						),
						count: effectiveParamCount,
						max: numParams,
					},
				});
			}
		}

		return {
			FunctionDeclaration: checkFunction,
			ArrowFunctionExpression: checkFunction,
			FunctionExpression: checkFunction,
			TSDeclareFunction: checkFunction,
			TSFunctionType: checkFunction,
		};
	},
};