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 false positive in operator_usage_whitespace #1030

Merged
merged 1 commit into from
Dec 22, 2016
Merged

Fix false positive in operator_usage_whitespace #1030

merged 1 commit into from
Dec 22, 2016

Conversation

marcelofabri
Copy link
Collaborator

Fixes #1028

@codecov-io
Copy link

codecov-io commented Dec 22, 2016

Current coverage is 82.68% (diff: 100%)

Merging #1030 into master will not change coverage

@@             master      #1030   diff @@
==========================================
  Files           140        140          
  Lines          6832       6832          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           5649       5649          
  Misses         1183       1183          
  Partials          0          0          

Powered by Codecov. Last update ced652e...37771fd

Copy link
Collaborator

@jpsim jpsim 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 the fix. This will have the intended effect. But I'd like to see if other rules might be affected.

@@ -105,7 +107,7 @@ public struct OperatorUsageWhitespaceRule: OptInRule, CorrectableRule, Configura
zeroSpaces + trailingVariableOrNumber
let excludingPattern = "(?:\(genericPattern)|\(validRangePattern))"

let kinds = SyntaxKind.commentAndStringKinds()
let kinds = SyntaxKind.commentAndStringKinds() + [.objectLiteral]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would suspect that other rules that exclude comments and strings would also need to exclude object literals.

This is why checking for explicit matching kinds is always preferable to excluding.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, but I'm not sure what we should do. Maybe rename commentAndStringKinds and make it return .objectLiteral as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes, basically "non-code kinds", and check to see if there are even more kinds that fall into that category.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've looked at all the other rules using commentAndStringKinds() and I don't think any of them would benefit from also including image literals. So let's keep it just here for now.

@jpsim jpsim merged commit 07bae49 into realm:master Dec 22, 2016
@marcelofabri marcelofabri deleted the bugfix_1028 branch December 22, 2016 17:36
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.

3 participants