Content-Length: 511851 | pFad | https://redirect.github.com/eslint/eslint/issues/19621

1BC feat: convert no-array-constructor suggestions to autofixes by sethamus · Pull Request #19621 · eslint/eslint · GitHub
Skip to content

feat: convert no-array-constructor suggestions to autofixes #19621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 7, 2025

Conversation

sethamus
Copy link
Contributor

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

  • Converted suggestions to automatic fixes, except for Array(...args) as we can't know if args might have only one element.
  • Ensured that the autofix correctly preserves comments.

Is there anything you'd like reviewers to focus on?

Closes #19608

@sethamus sethamus requested a review from a team as a code owner April 14, 2025 13:19
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Apr 14, 2025
@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Apr 14, 2025
@github-actions github-actions bot added the rule Relates to ESLint's core rules label Apr 14, 2025
Copy link

netlify bot commented Apr 14, 2025

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 74e4b16
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/681b00fdfe42520008a6087e

@mdjermanovic mdjermanovic moved this from Needs Triage to Evaluating in Triage Apr 15, 2025
@mdjermanovic mdjermanovic added the evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion label Apr 15, 2025
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion autofix This change is related to ESLint's autofixing capabilities and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Apr 17, 2025
@mdjermanovic mdjermanovic moved this from Evaluating to Implementing in Triage Apr 17, 2025
@mdjermanovic
Copy link
Member

I think we can simplify function hasCommentsInArrayConstructor and fix the above problems like this:

function hasCommentsInArrayConstructor(node) {
	const firstToken = sourceCode.getFirstToken(node);
	const lastToken = sourceCode.getLastToken(node);

	let lastRelevantToken = sourceCode.getLastToken(node.callee);

	while (
		lastRelevantToken !== lastToken &&
		!isOpeningParenToken(lastRelevantToken)
	) {
		lastRelevantToken = sourceCode.getTokenAfter(lastRelevantToken);
	}

	return sourceCode.commentsExistBetween(
		firstToken,
		lastRelevantToken
	);
}

mdjermanovic
mdjermanovic previously approved these changes May 2, 2025
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Leaving open for @nzakas and @snitin315 to verify their review comments.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

Thanks for all your work on this. I left one note about a situation where I don't think we should autofix.

snitin315
snitin315 previously approved these changes May 3, 2025
Copy link
Contributor

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@sethamus sethamus dismissed stale reviews from snitin315 and mdjermanovic via 221f34f May 6, 2025 14:19
nzakas
nzakas previously approved these changes May 6, 2025
Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. Leaving open for re-review by @mdjermanovic

@nzakas nzakas moved this from Implementing to Second Review Needed in Triage May 6, 2025
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit ee40364 into eslint:main May 7, 2025
30 checks passed
@github-project-automation github-project-automation bot moved this from Second Review Needed to Complete in Triage May 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion autofix This change is related to ESLint's autofixing capabilities contributor pool feature This change adds a new feature to ESLint rule Relates to ESLint's core rules
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

Rule Change: [no-array-constructor] Switch suggestions to full fixes
4 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://redirect.github.com/eslint/eslint/issues/19621

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy