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

[SR-11798] Scrunching of Range Operators Breaks Code #341

Closed
swift-ci opened this issue Nov 18, 2019 · 3 comments
Closed

[SR-11798] Scrunching of Range Operators Breaks Code #341

swift-ci opened this issue Nov 18, 2019 · 3 comments
Assignees
Labels
bug Something isn't working swift-format

Comments

@swift-ci
Copy link

Previous ID SR-11798
Radar None
Original Reporter SDGGiesbrecht (JIRA User)
Type Bug
Status Resolved
Resolution Done
Environment

swift-5.1-branch as of 2019‐11‐17

Additional Detail from JIRA
Votes 0
Component/s swift-format
Labels Bug
Assignee @allevato
Priority Medium

md5: 0ba030d617ad8d07c00151cd32d99a3e

Issue Description:

Start with this code:

for _ in 1 ... |x| { /* ... */ }
Int.random(in: 10 ... 1)

Run SwiftFormat once and the operators become conjoined, failing to compile:

for _ in 1...|x| { /* ... */ }
Int.random(in: 10...−1)

Run SwiftFormat again and the spaces are restored, but in the wrong places—it’s still broken:

for _ in 1 ...| x| { /* ... */ }
Int.random(in: 10 ...− 1)

I would expect range operators to be spaced with the same uniformity as every other infix operator, but if the scrunched variant is desired, then it should at least be restricted to the situations where it is actually valid code.

@swift-ci
Copy link
Author

Comment by Jeremy David Giesbrecht (JIRA)

I think pull request #91 aimed to solve this? Attn: @allevato

I just discovered it can also break if the upperBound starts with a dot, such as an enumeration case:

let range: ClosedRange<SomeEnumeration> = .caseFive ... .caseTen

Did your fix happen to handle that too or not?

@allevato
Copy link
Collaborator

allevato commented Dec 5, 2019

The original PR didn't handle that case, because it only considered following tokens that were `prefixOperator`s, whereas the case above is `prefixPeriod` because it's not "technically" an operator in the case of implicit member references.

#98 addresses this case.

Arguably, a safer way to check this would be to simply check the previous and next characters to see if they're operator code points, but I don't really want to hardcode that list into the formatter—after all, the whole point of using the compiler's syntax parser is to have it do the work so we don't have to duplicate it.

@akyrtzi, maybe the syntax parser library could be extended to have generally useful utility functions, like "is this character an identifier character or operator character"?

@allevato
Copy link
Collaborator

allevato commented Dec 5, 2019

#98 has been merged so this should be fixed now. (It, among other commits, still needs to be cherry-picked into the 5.1 branch.)

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 9, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working swift-format
Projects
None yet
Development

No branches or pull requests

2 participants