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

Colon rules acting differently for same situation. (+ custom question) #730

Closed
freak4pc opened this issue Jul 25, 2016 · 4 comments
Closed
Labels

Comments

@freak4pc
Copy link
Collaborator

freak4pc commented Jul 25, 2016

Hey There :)

In many places in my code I prefer having my colons attached to the identifier, but not with a single space to the type. I think this requires a custom rule (Something like identifier(no spaces):(multiple spaces)type), but I'm still not fluent enough in Swiftlint to write this rule

The odd part is the current rule seems picky on this and sometimes lets these kind of statements pass... I'm not sure what's the difference between the first line that wasn't detected as a "warning" to the other two, which are.

screen shot 2016-07-25 at 11 38 05 am

Also, if you could guide me on how to write a rule that will allow the following detection (meaning disabled colon_rules and writing something custom that allows multiple spaces after the colon) I'd be happy to read up on it.

Thank you!

@jpsim jpsim added the bug label Jul 25, 2016
@jpsim
Copy link
Collaborator

jpsim commented Jul 25, 2016

It's odd that only 2 of the 3 warnings are rendered for you.... 🤔

I can't reproduce that:

$ swiftlint version
0.11.0
$ cat main.swift
public class Location {
  /// Store Location Address.
  dynamic public var address:    String!

  /// Store Location City.
  dynamic public var city:       String!

  /// Store Location Country.
  dynamic public var country:    String!
}
$ swiftlint
Linting Swift files in current working directory
Linting 'main.swift' (1/1)
main.swift:3:22: warning: Colon Violation: Colons should be next to the identifier when specifying a type. (colon)
main.swift:6:22: warning: Colon Violation: Colons should be next to the identifier when specifying a type. (colon)
main.swift:9:22: warning: Colon Violation: Colons should be next to the identifier when specifying a type. (colon)
Done linting! Found 3 violations, 0 serious in 1 file.

As for making changes to rules to support this kind of style, I'm all for it!

The way I'd recommend this be done is to make ColonRule configurable, maybe with a flexible_right_spacing boolean flag that allows one or more spaces to the right of the colon.

@freak4pc
Copy link
Collaborator Author

freak4pc commented Jul 25, 2016

I'm down for trying to write that but I'm still not very acquainted with how the rule mechanism works.

Gonna read a bit into it and see how well my brain works today 🤓

freak4pc added a commit to freak4pc/SwiftLint that referenced this issue Jul 26, 2016
freak4pc added a commit to freak4pc/SwiftLint that referenced this issue Jul 26, 2016
@freak4pc
Copy link
Collaborator Author

freak4pc commented Jul 26, 2016

Well that was easier than I thought: #734.

Tested on my pretty massive code base and resolved the issue as expected. Let me know if you're cool with merging this solution or there are any modifications you'd like me to make.

My .swiftlint.yml now includes

colon:
    flexible_right_spacing: true

Cheers :)
Shai.

P.S. I wanted to add another unit test for testing this specific case but wasn't completely sure on how to make that work, since nonTriggeringExamples and triggeingExamples should be different based on the configuration. Any info on that would be appreciated :)

freak4pc added a commit to freak4pc/SwiftLint that referenced this issue Jul 26, 2016
@freak4pc
Copy link
Collaborator Author

Not sure why, the Travis build for SPM fails (it can't clone Commandant for some odd reason).
The tests are running fine locally.

freak4pc added a commit to freak4pc/SwiftLint that referenced this issue Jul 26, 2016
freak4pc added a commit to freak4pc/SwiftLint that referenced this issue Jul 27, 2016
@freak4pc freak4pc closed this as completed Aug 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants