SwiftLint easing conflict

April 30th, 2017
#swiftlint #teamwork

Anyone who has ever worked in a team knows only too well that there is no such thing as one true way of formatting code. Everyone has their own way of formatting and provided it compiles, all of those formatting choices are valid. So when it comes to formatting, it really is a personal choice based on some aesthetic.

Often one of the first questions I ask when joining a new team is:

"Can I see the coding style guide?"

However, even when there is a style guide and I've read and followed it, I will still get this most frustrating of PR comments:

"That's not how we do things here"

because a style guide (much like comments) is as only good as it is up-to-date. It can feel like playing a game that you can't win as a new joiner, and if the team you join is large enough with multiple developers reviewing your PRs it can really hit your confidence. Having your confidence knocked for something trivial like leaving a space around + isn't great when you trying hard to make a good impression on your new teammates. While having an up-to-date style guide can help reduce stress for new joiners, it also has longer-term benefits for existing team members:

  • Quicker to review code
  • Easier to spot bugs
  • Improved intra-team communication
  • Improved team morale
  • Quicker on-boarding of new joiners

Getting to those benefits however can be a painful process, potentially involving hours of discussion about mundane formatting points like: "where should the opening { go" or "if there should be a space after (". And once a coding style guide has been agreed, the team then needs to enforce the rules 😮. Remembering those rules can be repetitive and error-prone - much better to hand that responsibility off to the computer. For the past few months, I've been experimenting with SwiftLint and have found it to be a very low impact way of enforcing a coding style guide.

SwiftLint 🏎️

On it's GitHub readme SwiftLint is described as:

"A tool to enforce Swift style and conventions, loosely based on GitHub's Swift Style Guide."

It can be installed via Homebrew, CocoaPods or directly from source. As a team when we were trying out SwiftLint, there wasn't a pod to be installed so we used Homebrew but have recently switched over to using the pod to ensure that everyone has the same version installed. Due to the way SwiftLint integrates into Xcode, feedback on non-conformance to the rules is shown in the Issue Navigator. This feedback can take the form or warning or errors depending on how the rule is defined. When we first introduced SwiftLint we left the default rules on and compiled the project only to be faced with having to resolve hundreds of issues - we quickly rowed back on this and decided to disable most rules and then gradually re-introduce them one at time so that we could ensure that each rule we switched on was correct for us. We eventually ended up with our .swiftlint.yml file looking like:

excluded: ### paths to ignore during linting. Takes precedence over `included`.
  - Pods

  - force_cast
  - type_name
  - force_try
  - function_body_length
  - nesting
  - variable_name
  - control_statement
  - line_length
  - trailing_whitespace
  - statement_position
  - type_body_length
  - todo
  - valid_docs
  - missing_docs
  - file_length
  - function_parameter_count
  - cyclomatic_complexity
  - unused_closure_parameter
  - for_where
  - unused_optional_binding
  - redundant_void_return
  - void_return
  - vertical_parameter_alignment
  - unused_enumerated
  - redundant_discardable_let
  - empty_parentheses_with_trailing_closure

opt_in_rules: ### some rules are only opt-in
  - force_https
  - empty_count
  - conditional_binding_cascade
  - explicit_failure_calls
  - explicit_init
  - redundant_nil_coalescing
  - syntactic_sugar

line_length: 120

    name: "Avoid asserting 'false'"
    regex: '((assert|precondition)\(false)'
    message: "Use assertionFailure() or preconditionFailure() instead."
    severity: warning

If a rule isn't present in the disabled_rules list it is considered on and it's not in the opt_in_rules list it is considered off.

As you can see with SwiftLint you are even able to define custom rules. In the above snippet, we have decided to discourage the use of assert and precondition so that it shows up as a warning when they are used.

Now, whenever we stray off course with our formatting SwiftLint is there to correct it.

As a side note, it's possible for SwiftLint to produce false-positives however it's straight forward to disable rule enforcement for a section of code by using one of the following comment instructions:

// swiftlint:disable:next [insert_rule_name]
// swiftlint:disable:this [insert_rule_name]
// swiftlint:disable:previous [insert_rule_name]

Leaving everyone's ego alone 😈

While Swiftlint is a useful formatting tool I also discovered that it was a useful team harmony tool 🕊️. In a team dynamic when enforcing a style guide manually via PR comments, these comments can begin to break down the bonds between team members as some people get annoyed at others nit-picking while those nit-pickers can't understand why the other party doesn't just follow the agreed convention and they need to waste their time leaving formatting comments. Happily enough, I've found that most people can be surprisingly robust when facing criticism that they feel is valid or significant enough; but criticise something that they think is trivial and this is a cocktail for annoyance and hurt feelings. Not so happily enough, I've also found that most people are not great at talking about what annoys them while that annoyance is small enough to be easily dealt with. Instead, they let that annoyance grow and become resentment and before you know it you have people arguing and trying to settle scores. Now there are many ways to overcome this and I found to my surprise that SwiftLint was one of those ways.

Peace among team members

With SwiftLint it's the tool rather than a person that is providing feedback during development (when it's just you and the compiler that knows how you just tried to format that closure 😉). This frees up the PRs to be about the bigger issues in the code rather than the positioning of a bracket. This ability to provide regular feedback throughout the development process without damaging anyone's ego is, I think, the main strength of SwiftLint. And if you follow the Swift community driven rules rather than create your own, it results in the conversations about which rules to opt into (or as the case may be - opt out of) being much more impersonal. Rather than two developers debating for or against each other's rule/style preference instead each developer is debating a rule/style which no one party "owns". This change in rule/style ownership more easily allows one party to change their mind without their ego being bruised as it's not their idea which is being rejected. While we like to think that as developers we are driven by logic and reason, it's been my experience that a good percentage of what we do is in fact driven by "how it feels" and that instinctive sense that "our way is the best way". All of which is tied to our sense of self-worth/ego - once we are able to reduce the role of our ego in a debate/conversation we are able to more quickly reach a position which most people can accept.

I went into my SwiftLint experiment focused on the different rules it had and how enforcing those rules would produce a better code base. In the end, I got that but I also got a more unified team that speaks more 🏄.

If you haven't used SwiftLint or are still unconvinced of its benefits, there is a great talk on it by JP Simard.

What do you think? Let me know by getting in touch on Twitter - @wibosco