Skip to content
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

[Lint] Add a rule to replace .forEach with a for-in loop #603

Merged
merged 1 commit into from Aug 30, 2023

Conversation

xedin
Copy link
Member

@xedin xedin commented Aug 29, 2023

If there is no chaining using for-in loop more understandable and requires less chaining/nesting.

If there is no chaining using for-in loop more understandable
and requires less chaining/nesting.
Copy link
Collaborator

@allevato allevato left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

I tend to be a bit wary about introducing syntactic rules that technically require semantic information to prevent false positives. To be 100% accurate we'd want to check that the forEach reference is a Sequence.forEach witness; it's easy to think of a type that's not a Sequence that introduces its own forEach method that this could diagnose on.

But that might be rare enough that we shouldn't worry about it, unless it becomes a problem in practice. And since this is only linting and not rewriting the tree, the worst-case outcome is that someone in that situation would have to add a swift-format-ignore comment.

@allevato allevato merged commit 2d5461e into apple:main Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants