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

Fix CommaRule mismatch between violations and corrections #542

Merged
merged 16 commits into from Feb 20, 2016
Merged

Conversation

norio-nomura
Copy link
Collaborator

@norio-nomura norio-nomura changed the title Fix CommaRule did not match between violations and corrections [WIP] Fix CommaRule did not match between violations and corrections Feb 16, 2016
@norio-nomura
Copy link
Collaborator Author

It seems more than two spaces after comma does not generate violation:

let a = [1,       2]

But correction happens for that.
I think correction's behavior may be correct.

@@ -14,6 +14,10 @@
[Norio Nomura](https://github.com/norio-nomura)
[#535](https://github.com/realm/SwiftLint/issues/535)

* Fix `CommaRule` did not match between violations and corrections.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Fix CommaRule mismatch between violations and corrections.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks 🙏

@norio-nomura
Copy link
Collaborator Author

@jpsim I got a question while I'm implementing following won't generate violation and correction.
A:

func f(
  a: Int,
  b: Int // comma+newline+spaces
) {
}

Should we generate violation and correction for following?
B:

func f(a: Int
     , b: Int // newline+spaces+comma+space
) {
}

In other word, should we exclude line ending character on checking?
B is weird for me. But I can't determine whether it should do or not.

@norio-nomura
Copy link
Collaborator Author

Updated.

Should we generate violation and correction for following?
B:

I leave it generates on case B.

By applying this PR, The duration CommaRule of linting Carthage 0.14 increased from 154ms to 457ms.

@norio-nomura norio-nomura changed the title [WIP] Fix CommaRule did not match between violations and corrections Fix CommaRule did not match between violations and corrections Feb 17, 2016
@norio-nomura norio-nomura changed the title Fix CommaRule did not match between violations and corrections Fix CommaRule mismatch between violations and corrections Feb 17, 2016
That returns array of SyntaxTokens intersecting with byte range.
On linting Carthage 0.14, the duration of `SyntaxMap.tokensIn(_:)` reduced from 145ms to 51ms.
```
 SwiftLintFrameworkTests.IntegrationTests
  testSwiftLintLints, failed - Documented declarations should be valid.
```
That has been fixed on locally by jpsim/SourceKitten#175
@jpsim
Copy link
Collaborator

jpsim commented Feb 20, 2016

Travis is backlogged at the moment, but the only commit not tested is the changelog one, so this is good to merge.

These changes all look good to me. Thanks @norio-nomura!

jpsim added a commit that referenced this pull request Feb 20, 2016
Fix `CommaRule` mismatch between violations and corrections
@jpsim jpsim merged commit 0de9155 into master Feb 20, 2016
@jpsim jpsim deleted the nn-fix-466 branch February 20, 2016 22:00
@norio-nomura
Copy link
Collaborator Author

Thanks! 🙏

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