ronantakizawa/codereview-bench huggingface dataset:
Repository: eslint/eslint
File: lib/rules/no-return-await.js (JavaScript)

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

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

/** @type {import('../types').Rule.RuleModule} */
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,

		deprecated: {
			message:
				"The original assumption of the rule no longer holds true because of engine optimization.",
			url: "https://github.com/eslint/eslint/issues/17345",
			deprecatedSince: "8.46.0",
			availableUntil: null,
			replacedBy: [],
		},

		schema: [],

		messages: {
			removeAwait: "Remove redundant `await`.",
			redundantUseOfAwait: "Redundant use of `await` on a return value.",
		},
	},

	create(context) {
		/**
		 * Reports a found unnecessary `await` expression.
		 * @param {ASTNode} node The node representing the `await` expression to report
		 * @returns {void}
		 */
		function reportUnnecessaryAwait(node) {
			context.report({
				node: context.sourceCode.getFirstToken(node),
				loc: node.loc,
				messageId: "redundantUseOfAwait",
				suggest: [

Review comment (suggestion):
This was a link to the docs page for this rule because the explanation of the deprecation is on that page. It just looks weird when the URL is used from the page itself. Maybe we could add logic to hide the "Learn more" link in these cases?

(the same for `prefer-reflect`).

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

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

/** @type {import('../types').Rule.RuleModule} */
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,

		deprecated: {
			message:
				"The original assumption of the rule no longer holds true because of engine optimization.",
			deprecatedSince: "8.46.0",
			availableUntil: null,
			replacedBy: [],
		},

		schema: [],

		messages: {
			removeAwait: "Remove redundant `await`.",
			redundantUseOfAwait: "Redundant use of `await` on a return value.",
		},
	},

	create(context) {
		/**
		 * Reports a found unnecessary `await` expression.
		 * @param {ASTNode} node The node representing the `await` expression to report
		 * @returns {void}
		 */
		function reportUnnecessaryAwait(node) {
			context.report({
				node: context.sourceCode.getFirstToken(node),
				loc: node.loc,
				messageId: "redundantUseOfAwait",
				suggest: [
					{
Repository: eslint/eslint
File: lib/linter/rule-fixer.js (JavaScript)

Code before:
	}

	/**
	 * Creates a fix command that removes the node or token from the source.
	 * The fix is not applied until applyFixes() is called.
	 * @param {ASTNode|Token} nodeOrToken The node or token to remove.
	 * @returns {Object} The fix command.
	 */
	remove(nodeOrToken) {
		const range = this.#sourceCode.getRange(nodeOrToken);

		return this.removeRange(range);
	}

	/**
	 * Creates a fix command that removes the specified range of text from the source.
	 * The fix is not applied until applyFixes() is called.
	 * @param {number[]} range The range to remove, first item is start of range, second
	 *      is end of range.
	 * @returns {Object} The fix command.
	 */
	removeRange(range) {
		return {
			range,
			text: "",
		};
	}
}

module.exports = { RuleFixer };


Review comment (suggestion):
```suggestion
	 * @param {SourceRange} range The range to remove, first item is start of range, second
```

Code after:
	 * @param {string} text The text to insert.
	 * @returns {Object} The fix command.
	 */
	replaceTextRange(range, text) {
		return {
			range,
			text,
		};
	}

	/**
	 * Creates a fix command that removes the node or token from the source.
	 * The fix is not applied until applyFixes() is called.
	 * @param {ASTNode|Token} nodeOrToken The node or token to remove.
	 * @returns {Object} The fix command.
	 */
	remove(nodeOrToken) {
		const range = this.#sourceCode.getRange(nodeOrToken);

		return this.removeRange(range);
	}

	/**
	 * Creates a fix command that removes the specified range of text from the source.
	 * The fix is not applied until applyFixes() is called.
	 * @param {SourceRange} range The range to remove, first item is start of range, second
	 *      is end of range.
	 * @returns {Object} The fix command.
	 */
	removeRange(range) {
		return {
			range,
			text: "",
		};
	}
}

module.exports = { RuleFixer };
Repository: eslint/eslint
File: lib/rules/linebreak-style.js (JavaScript)

Code before:
			url: "https://eslint.org/docs/latest/rules/linebreak-style",
		},

		fixable: "whitespace",

		schema: [
			{
				enum: ["unix", "windows"],
			},
		],
		messages: {
			expectedLF: "Expected linebreaks to be 'LF' but found 'CRLF'.",
			expectedCRLF: "Expected linebreaks to be 'CRLF' but found 'LF'.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

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

		/**
		 * Builds a fix function that replaces text at the specified range in the source text.
		 * @param {number[]} range The range to replace
		 * @param {string} text The text to insert.
		 * @returns {Function} Fixer function
		 * @private
		 */
		function createFix(range, text) {
			return function (fixer) {
				return fixer.replaceTextRange(range, text);
			};
		}

		//--------------------------------------------------------------------------
		// Public
		//--------------------------------------------------------------------------

		return {
			Program: function checkForLinebreakStyle(node) {
				const linebreakStyle = context.options[0] || "unix",
					expectedLF = linebreakStyle === "unix",
					expectedLFChars = expectedLF ? "\n" : "\r\n",
					source = sourceCode.getText(),
					pattern = astUtils.createGlobalLinebreakMatcher();
				let match;

				let i = 0;


Review comment (suggestion):
```suggestion
		 * @param {SourceRange} range The range to replace
```

This is the type `SourceRange` from `@eslint/core`.

Code after:
				},
			],
		},
		type: "layout",

		docs: {
			description: "Enforce consistent linebreak style",
			recommended: false,
			url: "https://eslint.org/docs/latest/rules/linebreak-style",
		},

		fixable: "whitespace",

		schema: [
			{
				enum: ["unix", "windows"],
			},
		],
		messages: {
			expectedLF: "Expected linebreaks to be 'LF' but found 'CRLF'.",
			expectedCRLF: "Expected linebreaks to be 'CRLF' but found 'LF'.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

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

		/**
		 * Builds a fix function that replaces text at the specified range in the source text.
		 * @param {SourceRange} range The range to replace
		 * @param {string} text The text to insert.
		 * @returns {Function} Fixer function
		 * @private
		 */
		function createFix(range, text) {
			return function (fixer) {
				return fixer.replaceTextRange(range, text);
			};
		}

		//--------------------------------------------------------------------------
		// Public
		//--------------------------------------------------------------------------

		return {
			Program: function checkForLinebreakStyle(node) {
				const linebreakStyle = context.options[0] || "unix",
Repository: eslint/eslint
File: lib/rules/no-nonoctal-decimal-escape.js (JavaScript)

Code before:
			recommended: true,
			url: "https://eslint.org/docs/latest/rules/no-nonoctal-decimal-escape",
		},

		hasSuggestions: true,

		schema: [],

		messages: {
			decimalEscape: "Don't use '{{decimalEscape}}' escape sequence.",

			// suggestions
			refactor:
				"Replace '{{original}}' with '{{replacement}}'. This maintains the current functionality.",
			escapeBackslash:
				"Replace '{{original}}' with '{{replacement}}' to include the actual backslash character.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		/**
		 * Creates a new Suggestion object.
		 * @param {string} messageId "refactor" or "escapeBackslash".
		 * @param {number[]} range The range to replace.
		 * @param {string} replacement New text for the range.
		 * @returns {Object} Suggestion
		 */
		function createSuggestion(messageId, range, replacement) {
			return {
				messageId,
				data: {
					original: sourceCode.getText().slice(...range),
					replacement,
				},
				fix(fixer) {
					return fixer.replaceTextRange(range, replacement);
				},
			};
		}

		return {
			Literal(node) {
				if (typeof node.value !== "string") {
					return;
				}

				if (!QUICK_TEST_REGEX.test(node.raw)) {
					return;
				}

Review comment (suggestion):
```suggestion
		 * @param {SourceRange} range The range to replace.
```

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

		docs: {
			description:
				"Disallow `\\8` and `\\9` escape sequences in string literals",
			recommended: true,
			url: "https://eslint.org/docs/latest/rules/no-nonoctal-decimal-escape",
		},

		hasSuggestions: true,

		schema: [],

		messages: {
			decimalEscape: "Don't use '{{decimalEscape}}' escape sequence.",

			// suggestions
			refactor:
				"Replace '{{original}}' with '{{replacement}}'. This maintains the current functionality.",
			escapeBackslash:
				"Replace '{{original}}' with '{{replacement}}' to include the actual backslash character.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		/**
		 * Creates a new Suggestion object.
		 * @param {string} messageId "refactor" or "escapeBackslash".
		 * @param {SourceRange} range The range to replace.
		 * @param {string} replacement New text for the range.
		 * @returns {Object} Suggestion
		 */
		function createSuggestion(messageId, range, replacement) {
			return {
				messageId,
				data: {
					original: sourceCode.getText().slice(...range),
					replacement,
				},
				fix(fixer) {
					return fixer.replaceTextRange(range, replacement);
				},
			};
		}

		return {
Repository: eslint/eslint
File: lib/rules/no-trailing-spaces.js (JavaScript)

Code before:
				},
				additionalProperties: false,
			},
		],

		messages: {
			trailingSpace: "Trailing spaces not allowed.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		const BLANK_CLASS = "[ \t\u00a0\u2000-\u200b\u3000]",
			SKIP_BLANK = `^${BLANK_CLASS}*$`,
			NONBLANK = `${BLANK_CLASS}+$`;

		const options = context.options[0] || {},
			skipBlankLines = options.skipBlankLines || false,
			ignoreComments = options.ignoreComments || false;

		/**
		 * Report the error message
		 * @param {ASTNode} node node to report
		 * @param {number[]} location range information
		 * @param {number[]} fixRange Range based on the whole program
		 * @returns {void}
		 */
		function report(node, location, fixRange) {
			/*
			 * Passing node is a bit dirty, because message data will contain big
			 * text in `source`. But... who cares :) ?
			 * One more kludge will not make worse the bloody wizardry of this
			 * plugin.
			 */
			context.report({
				node,
				loc: location,
				messageId: "trailingSpace",
				fix(fixer) {
					return fixer.removeRange(fixRange);
				},
			});
		}

		/**
		 * Given a list of comment nodes, return the line numbers for those comments.
		 * @param {Array} comments An array of comment nodes.
		 * @returns {number[]} An array of line numbers containing comments.
		 */
		function getCommentLineNumbers(comments) {

Review comment (suggestion):
```suggestion
		 * @param {SourceLocation} location range information
		 * @param {SourceRange} fixRange Range based on the whole program
```

Both types are from `@eslint/core`.

Code after:
					skipBlankLines: {
						type: "boolean",
						default: false,
					},
					ignoreComments: {
						type: "boolean",
						default: false,
					},
				},
				additionalProperties: false,
			},
		],

		messages: {
			trailingSpace: "Trailing spaces not allowed.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		const BLANK_CLASS = "[ \t\u00a0\u2000-\u200b\u3000]",
			SKIP_BLANK = `^${BLANK_CLASS}*$`,
			NONBLANK = `${BLANK_CLASS}+$`;

		const options = context.options[0] || {},
			skipBlankLines = options.skipBlankLines || false,
			ignoreComments = options.ignoreComments || false;

		/**
		 * Report the error message
		 * @param {ASTNode} node node to report
		 * @param {SourceLocation} location range information
		 * @param {SourceRange} fixRange Range based on the whole program
		 * @returns {void}
		 */
		function report(node, location, fixRange) {
			/*
			 * Passing node is a bit dirty, because message data will contain big
			 * text in `source`. But... who cares :) ?
			 * One more kludge will not make worse the bloody wizardry of this
			 * plugin.
			 */
			context.report({
				node,
				loc: location,
				messageId: "trailingSpace",
				fix(fixer) {
					return fixer.removeRange(fixRange);
				},
			});
Repository: eslint/eslint
File: tests/lib/types/types.test.ts (TypeScript)

Code before:
				const expected: StaticBlock & Rule.NodeParentExtension = node;
				expected.parent;
			},
		};
	},
};

type DeprecatedRuleContextKeys =
	| "getAncestors"
	| "getDeclaredVariables"
	| "getScope"
	| "markVariableAsUsed";
(): RuleDefinition => ({
	create(context) {
		// Ensure that deprecated RuleContext methods are not defined when using RuleDefinition
		context satisfies {
			[Key in keyof typeof context]: Key extends DeprecatedRuleContextKeys
				? never
				: (typeof context)[Key];
		};
		return {};
	},
});

// All options optional - JSRuleDefinition, JSRuleDefinition<{}> and RuleModule
// should be the same type.
(
	rule1: Rule.JSRuleDefinition,
	rule2: Rule.JSRuleDefinition<{}>,
	rule3: Rule.RuleModule,
) => {
	rule1 satisfies typeof rule2 satisfies typeof rule3;
	rule2 satisfies typeof rule1 satisfies typeof rule3;
	rule3 satisfies typeof rule1 satisfies typeof rule2;
};

// Type restrictions should be enforced
(): Rule.JSRuleDefinition<{
	RuleOptions: [string, number];
	MessageIds: "foo" | "bar";
	ExtRuleDocs: { foo: string; bar: number };
}> => ({
	meta: {
		messages: {
			foo: "FOO",

			// @ts-expect-error Wrong type for message ID
			bar: 42,
		},
		docs: {
			foo: "FOO",

Review comment (suggestion):
Note that `JSRuleDefinition` isn't exactly the same type as `Rule.RuleModule`. In `Rule.RuleModule`, the `RuleOptions` property must be `any[]` for compatibility with legacy code.

Code after:
			StaticBlock(node) {
				const expected: StaticBlock & Rule.NodeParentExtension = node;
				expected.parent;
			},
		};
	},
};

type DeprecatedRuleContextKeys =
	| "getAncestors"
	| "getDeclaredVariables"
	| "getScope"
	| "markVariableAsUsed";
(): RuleDefinition => ({
	create(context) {
		// Ensure that deprecated RuleContext methods are not defined when using RuleDefinition
		context satisfies {
			[Key in keyof typeof context]: Key extends DeprecatedRuleContextKeys
				? never
				: (typeof context)[Key];
		};
		return {};
	},
});

// All options optional - JSRuleDefinition and JSRuleDefinition<{}>
// should be the same type.
(rule1: JSRuleDefinition, rule2: JSRuleDefinition<{}>) => {
	rule1 satisfies typeof rule2;
	rule2 satisfies typeof rule1;
};

// Type restrictions should be enforced
(): JSRuleDefinition<{
	RuleOptions: [string, number];
	MessageIds: "foo" | "bar";
	ExtRuleDocs: { foo: string; bar: number };
}> => ({
	meta: {
		messages: {
			foo: "FOO",

			// @ts-expect-error Wrong type for message ID
			bar: 42,
		},
		docs: {
			foo: "FOO",

			// @ts-expect-error Wrong type for declared property
			bar: "BAR",
Repository: eslint/eslint
File: tests/lib/rules/no-restricted-properties.js (JavaScript)

Code before:
			code: "const [{ bad }] = foo;",
			options: [{ property: "bad" }],
			languageOptions: { ecmaVersion: 6 },
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						propertyName: "bad",
						message: "",
					},
				},
			],
		},
		{
			code: "someObject.disallowedProperty",
			options: [
				{
					property: "disallowedProperty",
					allowObjects: ["anotherObject"],
				},
			],
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						objectName: "someObject",
						propertyName: "disallowedProperty",
						message: "",
					},
					type: "MemberExpression",
				},
			],
		},
		{
			code: "someObject.disallowedProperty",
			options: [
				{
					property: "disallowedProperty",
					allowObjects: ["anotherObject"],
					message: "Please use someObject.allowedProperty instead.",
				},
			],
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						objectName: "someObject",
						propertyName: "disallowedProperty",
						message:
							" Please use someObject.allowedProperty instead.",
					},

Review comment (suggestion):
```suggestion
```

These messages don't have `objectName`.

Code after:
			code: "const [{ bad }] = foo;",
			options: [{ property: "bad" }],
			languageOptions: { ecmaVersion: 6 },
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						propertyName: "bad",
						message: "",
					},
				},
			],
		},
		{
			code: "someObject.disallowedProperty",
			options: [
				{
					property: "disallowedProperty",
					allowObjects: ["anotherObject"],
				},
			],
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						propertyName: "disallowedProperty",
						message: "",
					},
					type: "MemberExpression",
				},
			],
		},
		{
			code: "someObject.disallowedProperty",
			options: [
				{
					property: "disallowedProperty",
					allowObjects: ["anotherObject"],
					message: "Please use someObject.allowedProperty instead.",
				},
			],
			errors: [
				{
					messageId: "restrictedProperty",
					data: {
						propertyName: "disallowedProperty",
						message:
							" Please use someObject.allowedProperty instead.",
					},
					type: "MemberExpression",
				},
Repository: eslint/eslint
File: lib/rules/no-unassigned-vars.js (JavaScript)

Code before:
/**
 * @fileoverview Rule to flag variables that are never assigned
 * @author Jacob Bandes-Storch <https://github.com/jtbandes>
 */
"use strict";

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

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

		defaultOptions: [
			{
				typeof: false,
			},
		],

		docs: {
			description:
				"Disallow variable declarations that use `let` or `var` but have no intitial value and are never assigned",
			recommended: true,
			url: "https://eslint.org/docs/latest/rules/no-unassigned-vars",
		},

		schema: [
			{
				type: "object",
				properties: {
					typeof: {
						type: "boolean",
					},
				},
				additionalProperties: false,
			},
		],
		messages: {
			unassigned:
				"'{{name}}' is not initialized and never assigned, so it will always be undefined.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		return {
			[`VariableDeclaration:matches([kind=let], [kind=var]):not([declare=true]) > VariableDeclarator[id.type=Identifier]:not([init])`](

Review comment (suggestion):
```suggestion
			recommended: false,
```

Per our [semver policies](https://github.com/eslint/eslint?tab=readme-ov-file#semantic-versioning-policy), we can add rules to the recommended config only in major versions.

Code after:
/**
 * @fileoverview Rule to flag variables that are never assigned
 * @author Jacob Bandes-Storch <https://github.com/jtbandes>
 */
"use strict";

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

/** @type {import('../types').Rule.RuleModule} */
module.exports = {
	meta: {
		type: "problem",
		dialects: ["typescript", "javascript"],
		language: "javascript",

		docs: {
			description:
				"Disallow `let` or `var` variables that are read but never assigned",
			recommended: false,
			url: "https://eslint.org/docs/latest/rules/no-unassigned-vars",
		},

		schema: [],
		messages: {
			unassigned:
				"'{{name}}' is always 'undefined' because it's never assigned.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		return {
			VariableDeclarator(node) {
				/** @type {import('estree').VariableDeclaration} */
				const declaration = node.parent;
				const shouldCheck =
					!node.init &&
					node.id.type === "Identifier" &&
					declaration.kind !== "const" &&
					!declaration.declare;
				if (!shouldCheck) {
					return;
				}
				const [variable] = sourceCode.getDeclaredVariables(node);
				if (!variable) {
					return;
				}
Repository: eslint/eslint
File: lib/rules/no-unassigned-vars.js (JavaScript)

Code before:
		schema: [
			{
				type: "object",
				properties: {
					typeof: {
						type: "boolean",
					},
				},
				additionalProperties: false,
			},
		],
		messages: {
			unassigned:
				"'{{name}}' is not initialized and never assigned, so it will always be undefined.",
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		return {
			[`VariableDeclaration:matches([kind=let], [kind=var]):not([declare=true]) > VariableDeclarator[id.type=Identifier]:not([init])`](
				node,
			) {
				let hasRead = false;
				for (const variable of sourceCode.getDeclaredVariables(node)) {
					for (const reference of variable.references) {
						if (reference.isWrite()) {
							return;
						}
						if (reference.isRead()) {
							hasRead = true;
						}
					}
				}
				if (!hasRead) {
					// Variables that are never read should be flagged by no-unused-vars instead
					return;
				}
				context.report({
					node,
					messageId: "unassigned",
					data: { name: node.id.name },
				});
			},
		};
	},
};


Review comment (refactor):
As in this case there's exactly one variable, maybe we can simplify this:

```js
const [variable] = sourceCode.getDeclaredVariables(node);
```

Code after:
		},
	},

	create(context) {
		const sourceCode = context.sourceCode;

		return {
			VariableDeclarator(node) {
				/** @type {import('estree').VariableDeclaration} */
				const declaration = node.parent;
				const shouldCheck =
					!node.init &&
					node.id.type === "Identifier" &&
					declaration.kind !== "const" &&
					!declaration.declare;
				if (!shouldCheck) {
					return;
				}
				const [variable] = sourceCode.getDeclaredVariables(node);
				if (!variable) {
					return;
				}
				let hasRead = false;
				for (const reference of variable.references) {
					if (reference.isWrite()) {
						return;
					}
					if (reference.isRead()) {
						hasRead = true;
					}
				}
				if (!hasRead) {
					// Variables that are never read should be flagged by no-unused-vars instead
					return;
				}
				context.report({
					node,
					messageId: "unassigned",
					data: { name: node.id.name },
				});
			},
		};
	},
};