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

syntactic_sugar rule now doesn't flag declarations that can't be fixed #1041

Merged
merged 3 commits into from
Dec 23, 2016
Merged

syntactic_sugar rule now doesn't flag declarations that can't be fixed #1041

merged 3 commits into from
Dec 23, 2016

Conversation

marcelofabri
Copy link
Collaborator

Fixes #928

@codecov-io
Copy link

codecov-io commented Dec 23, 2016

Current coverage is 82.90% (diff: 91.66%)

Merging #1041 into master will increase coverage by 0.01%

@@             master      #1041   diff @@
==========================================
  Files           144        144          
  Lines          6996       7014    +18   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           5799       5815    +16   
- Misses         1197       1199     +2   
  Partials          0          0          

Powered by Codecov. Last update 49eabb7...9253977

@@ -39,6 +39,10 @@
[Marcelo Fabri](https://github.com/marcelofabri)
[#1005](https://github.com/realm/SwiftLint/issues/1005)

* `syntactic_sugar` rule now doesn't flag declarations that can't be fixed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

more of a bug fix, wouldn't you say?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I think so. I'll update it.

@jpsim
Copy link
Collaborator

jpsim commented Dec 23, 2016

You could probably also fix the false positives reported from using Optional<T>.self, such as

unsafeBitCast(nonOptionalT, to: Optional<T>.self)

There are quite a few of these false positives in Realm: https://github.com/realm/realm-cocoa/blob/v2.1.2/RealmSwift/LinkingObjects.swift#L160


return file.matchPattern(pattern, excludingSyntaxKinds: kinds).flatMap { range in

// avoids trigering when referring to an associatedtype
Copy link
Collaborator

Choose a reason for hiding this comment

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

"avoids trigering" -> "avoid triggering"

@marcelofabri
Copy link
Collaborator Author

You could probably also fix the false positives reported from using Optional<T>.self

I've handled them in 0bd1647, but I don't know if .self is the only case that should be handled.

@jpsim
Copy link
Collaborator

jpsim commented Dec 23, 2016

There's one more false positive type in Realm: https://github.com/realm/realm-cocoa/blob/v2.1.2/RealmSwift/Object.swift#L331

if type is Optional<String>.Type

Can be fixed with:

-                if file.matchPattern("\\s*\\.self", withSyntaxKinds: [.keyword],
-                                     range: restOfFileRange).first?.location == start {
+                if let match = file.matchPattern("\\s*\\.(?:self|Type)", range: restOfFileRange).first,
+                    match.0.location == start, match.1 == [.keyword] || match.1 == [.identifier] {
                     return nil

@jpsim jpsim merged commit 55793d0 into realm:master Dec 23, 2016
@jpsim
Copy link
Collaborator

jpsim commented Dec 23, 2016

🎉

@marcelofabri marcelofabri deleted the syntactic-sugar-associatedtype branch December 23, 2016 20:06
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