Add version pinning à la Podfile.lock

[ZevEisenberg]

To enforce multiple team members running Swiftlint, the .swiftlint.yml file should include a specifier for the version of SwiftLint that it is expecting to be used with. Then the swiftlint build script (or versions of swiftlint after the first to support such a feature) could check the version as the first step when running, and throw a warning or error if there is a mismatch.

(Aside: Safari/OS X keeps wanting to autocorrect swiftlint to swxftlint, which as far as I can tell is not a word, and only appears on the Internet here: #169. Anyone else have this problem?)

[jpsim]

Thanks for the request! We'd probably go with a minimum supported version rather than requiring an exact version match.

[marcelofabri]

Do you guys think it's worth to add a dependency to deal with comparing versions?

Here are some that I found:
https://github.com/mrackwitz/Version
https://github.com/AlexanderNey/SemanticVersioning
https://github.com/trifia/LXSemVer

Or using NSString is enough for us? This SO thread lists some of the problems we might have, such as 1.4.5 would be considered greater than 1.5.

if version.compare(currentVersion, options: .NumericSearch) == .OrderedDescending {

}

[jpsim]

If we need a dependency, I have no trouble adding it.

But that's far from the most challenging decision to make to enable this feature.

Notably, what should be pinned?

The SwiftLint version? Homebrew only allows installing the latest version of a formula, unless resorting to its 'versions' tap. And it's cumbersome for a user to just be told "you don't have the right version of SwiftLint, run this command and try again".

So if not SwiftLint, should we version the rules and keep all previous versions of a rule in our code base, only applying the pinned ones? That would complicate the project significantly, bloat the resulting binary and I'm not sure how useful it would be to end users.

The more I think of this, the more whitelist_rules seems like the right design to accomplish the goals that motivated this feature request.

[scottrhoyt]

I agree with all of that, @jpsim .

[marcelofabri]

I don't think whitelist_rules is the right call to solve this. Particularly, it needs that people actively add new rules (and discover them!). Although new rules may trigger warnings on an existing codebase, I'd like to see these violations first and the choose whether I'd embrace the rule and fix the violations or just ignore the rule.

I like how CocoaPods deals with the problem: https://github.com/CocoaPods/CocoaPods/blob/1711e76c599f37ab9402ee44652ea667c010a11c/lib/cocoapods/installer/analyzer.rb#L172

I definitely agree that we shouldn't version rules.

[jpsim]

@marcelofabri the only thing CocoaPods seems to be doing here is logging a warning whenever the current version exceeds the version last written to in the .lock file.

This means that if your team is typically on the latest version of SwiftLint, you'll constantly have these noisy warnings about your .lock file being out of date, or committing changes to your lockfile to update to the version you're currently using.

Given SwiftLint's current pace of development and rate of new releases, I get the impression that users will get "warning blindness" because of this, since they'll often expect these warnings, they won't pay attention to the ones that actually matter (like incorrect configurations).

[marcelofabri]

@jpsim Yeah, but (ideally) I'd like that everyone on my team would use the same version. I'm OK with specifying a minimum supported version too on the configuration file.

I just think (as an user) that it's too much maintenance work to keep whitelist_rules updated and that it'll harder to know about new rules if whitelist_rules is the recommended approach to pinning versions.

[jpsim]

I understand the motivation and agree that the current solutions fall short of the ideal, but I also think that pinning the entire SwiftLint version is not the answer.

I have kind of a wacky idea. From a user's standpoint, it'd be great to be able to make the latest version of SwiftLint emulate a previous version. So if you had a rules_version: 0.7.0 in your configuration file, SwiftLint would only run the rules that were enabled in that version of SwiftLint. We just add a versionIntroduced member to the Rule protocol and if rules_version is specified in the configuration file, later rules are excluded. If the current version of SwiftLint is older than what's specified in rules_version, we prompt the user like CocoaPods does in the link you shared above.

This approach avoids the difficulties with Homebrew to downgrade formulas. It's also pretty easy for SwiftLint developers to incorporate this into their workflow.

[ZevEisenberg]

Many of these are still sounding pretty burdensome to users. They ultimately shouldn't have to care about what version of SwiftLint they're using, only what rules they're enforcing. This is why I would advocate using the two-tiered approach I mentioned elsewhere:

1. By default, all rules are disabled, and you can be conservative and enable them one by one.
2. If you like knowing about the new rules when they come out, you flip a switch that says "enable everything by default", and then have a blacklist to re-disable the ones you didn't want. That way, you can evaluate every new rule as it's added, and see if it matches your team's desires. It also means that your .swiftlint.yml will likely stay pretty small, since you're maintaining a blacklist, which I imagine would be smaller than a whitelist for most people (could be wrong, though).

[scottrhoyt]

I'm don't know. Sounds like the only benefit to the existing system is not needing to pay attention to when new opt_in_rules are released. I'm not sure that's a strong enough justification to add more complexity to the system.

[ZevEisenberg]

I read release notes because I'm fastidious about such things, but in my experience, I'm in a minority. In general, I prioritize ease of use over ease of development, although of course it's a free project and there is only so much time you can spend making it perfect. But I think anything that can be done to make it hard to miss new things (i.e. proactively introducing new rules in a non-ignorable way) would at least be worth considering.

[jpsim]

By default, all rules are disabled, and you can be conservative and enable them one by one.

IMHO this would lead to a terrible user experience, especially for first-time users. None of the style linters for other languages I've seen out there take this approach, and for good reason I think.

If you like knowing about the new rules when they come out, you flip a switch that says "enable everything by default", and then have a blacklist to re-disable the ones you didn't want. That way, you can evaluate every new rule as it's added, and see if it matches your team's desires. It also means that your .swiftlint.yml will likely stay pretty small, since you're maintaining a blacklist, which I imagine would be smaller than a whitelist for most people (could be wrong, though).

All of this can be done by using whitelist_rules and just commenting it out when you want to test the waters by having "everything enabled by default".

[daniel-beard]

I like the idea of a rules_version configuration, and maybe an equivalent of -Weverything. Seems like with those, we could cover the above use cases. But, it does introduce further complexity around which rules are enabled or not.

[ZevEisenberg]

All of this can be done by using whitelist_rules and just commenting it out when you want to test the waters by having "everything enabled by default".

But then you still have to

1. Know that this is the expectation of how to use the tool, and
2. Remember to do it periodically.

This is at odds with how I would want to use such a tool. If I'm going to run brew update occasionally, why not have that trigger new warnings as well? But that's just my opinion.. Definitely should optimize for how the most people would want to use the tool.

[scottrhoyt]

@ZevEisenberg If you are utilizing disabled_rules, you will only miss out on automatic update of opt_in_rules which, by their nature, have been identified as potentially hazardous to enable by default. On the flip side, such a use case would be potentially disruptive to any collaborator or CI service that also relied on the same .swiftlint.yml and recently updated swiftlint. IMO, this would only increase the need for version pinning as per the original issue, not decrease it. All in all, it seems like a small win for the users interested in staying up to date with the latest opt_in_rules at the expense of complexity and risk to the others. Checking in periodically is exactly how potentially-breaking updates should be done, and opt_in_rules run the highest risk of that.

[ZevEisenberg]

Checking in periodically is exactly how potentially-breaking updates should be done

If that's the intent (and I'm not saying it isn't - just acknowledging that it's been clearly stated), then the more conservative, opt-in approach does sound like the right choice. Good talk :)

[jonah-williams]

Hi folks, I ran into a case where I wish this feature existed. Maybe it can serve as a useful example since there seems to be some debate over how version pinning should work to best serve users.

1. My team runs SwiftLint locally and in CI. Initially we were running 0.9.1 everywhere.
2. I wrote some code which ran into cases where Unwanted closing_brace violation warning #592 was triggering unwanted warnings.
3. I updated to 0.10.0 which changes this rules behavior to fix this issue. I haven't made any changes to the set of rules we're whitelisting.

Now when I push my changes I'm going to trigger new build warnings on CI when it runs using 0.9.1. I can update our CI system but then my coworkers will discover new issues when they build locally. I have to warn them that I've changed SwiftLint versions.

If we're whitelisting rules I can ask everyone to update SwiftLint before I merge these changes and we're fine. If we're blacklisting rules or if there are other changes in rules I needed to account for then the team will see lots of warnings if they update SwiftLint today before I merge these changes. In that case we really want to update SwiftLint when and only when they pull code changes which include the update (a thing we have no mechanism to express today).

This will become even more painful when I have multiple apps, each of which conforming to a different version of SwiftLint's rules. A SwiftLint update on a CI box or developer machine for one project could trigger hundreds of warnings on other projects.

Ideally I'd like to be able to treat SwiftLint like our other dependencies; capturing a known version of the tool and it's rules as part of the project repository. For example I can include a Gemfile and Gemfile.lock in our repositories to allow each project to specify the version of CocoaPods, Fastlane, or other tools currently in use and I can install multiple versions of those tools side by side to support multiple projects.

[ZevEisenberg]

+1 the ability to use a Gemfile à la CocoaPods, or include the library itself as a dependency of the project in the Podfile or Cartfile à la SwiftGen, would be a great solution to this. I didn't really understand either of those when I opened this issue, but now that I do, that seems like it would be the way to go.

Is there a Gemfile equivalent for Homebrew?

[ZevEisenberg]

Answering my own question: https://robots.thoughtbot.com/brewfile-a-gemfile-but-for-homebrew

However, I think supporting Gemfile and/or Podfile would make more sense, because those are both more common in iOS development, at least from what I've seen.

[norio-nomura]

Since #669, make portable_zip can create zip file package that contains swiftlint executable. That can be execute in any location of the file system that is different from swiftlint installed by .pkg.
It seems SwiftGen's Podfile support downloads zip file from github release and place it under working copy of repositories. I guess it would be possible that SwiftLint would supports install by Podfile as same as SwiftGen doing.

[Ben-G]

Since we've been running into issues with teammates that had outdated versions of SwiftLint a few teams before, we're now using this crude workaround: https://gist.github.com/Ben-G/ec6fb4455c849bceb4af831d157dab50

Posting this in case anyone else finds it useful until actual solution is implemented.

[marcelofabri]

An example podspec would be:

Pod::Spec.new do |s|

  s.name         = "SwiftLint"
  s.version      = "0.12.0"
  s.summary      = "A tool to enforce Swift style and conventions."

  s.description  = <<-DESC
                   A tool to enforce Swift style and conventions, loosely based on GitHub's Swift Style Guide.
                   SwiftLint hooks into Clang and SourceKit to use the AST representation of your source files
                   for more accurate results.
                   DESC

  s.homepage     = "https://github.com/realm/SwiftLint"
  s.license      = "MIT"
  s.author       = { "Realm" => "help@realm.io" }

  s.source       = { git: 'https://github.com/realm/SwiftLint.git', submodules: true, commit: 'ec953e28d58e1f0356cbeb889b135e9573ec618e'}
  s.prepare_command = <<-CMD
       make portable_zip &&
       find . ! -name portable_swiftlint.zip -delete &&
       unzip portable_swiftlint.zip &&
       rm portable_swiftlint.zip
  CMD

  s.preserve_paths = '*'
end

Note that it's pinned to a commit (it should pin a release, but since there's no release compatible with Xcode 8 yet...). Also, the binary is being compiled on pod install. Ideally, we could provide a zip on GitHub and the just change the source to use that zip instead.

[jonah-williams]

@marcelofabri that looks fantastic. I was able to use this immediately with the 0.13.0 release in Xcode 8.1 by adding a podspec to my project:

pod 'SwiftLint', podspec: './SwiftLint/SwiftLint.podspec'

Pod::Spec.new do |s|

  s.name         = "SwiftLint"
  s.version      = "0.13.0"
  s.summary      = "A tool to enforce Swift style and conventions."

  s.description  = <<-DESC
                   A tool to enforce Swift style and conventions, loosely based on GitHub's Swift Style Guide.
                   SwiftLint hooks into Clang and SourceKit to use the AST representation of your source files
                   for more accurate results.
                   DESC

  s.homepage     = "https://github.com/realm/SwiftLint"
  s.license      = "MIT"
  s.author       = { "Realm" => "help@realm.io" }

  s.source       = { git: 'https://github.com/realm/SwiftLint.git', submodules: true, tag: '0.13.0'}
  s.prepare_command = <<-CMD
       make portable_zip &&
       find . ! -name portable_swiftlint.zip -delete &&
       unzip portable_swiftlint.zip &&
       rm portable_swiftlint.zip
  CMD

  s.preserve_paths = '*'
end

Compilation takes a couple of minutes but once built I checked the resulting pod into my repo (along with the rest of our dependencies) so installation on other machines (new developers, running CI builds) is fast and only requires a single git operation. This eliminates both a brew install swiftlint setup step for new team members and some version checking scripts we used to catch mismatches between machines 😃

Would it be reasonable to publish SwiftLint as a pod? Can I help with that somehow?

[marcelofabri]

@jonah-williams Before publishing the podspec, I think it'd be great if the portable_swiftlint.zip was published under https://github.com/realm/SwiftLint/releases on each release, so we could skip building, making the podspec more useful for people who don't commit their Pods (myself for example 😅)

[marcelofabri]

Just as an update, SwiftLint 0.14 and higher supports usage through CocoaPods 🎉

[jpsim]

I've re-read this thread again, and have come to the conclusion that several mechanisms now exist to support nearly all the use cases shared here.

There are now four ways to specify and automatically install a specific version of SwiftLint to be used for a project:

1. In a project's Podfile and running pod install
2. In a project's Package.swift and running swift build [-c release]
3. By downloading a portable_swiftlint.zip archive from GitHub releases
4. By downloading a SwiftLint.pkg and running installer -pkg SwiftLint.pkg -target ...
5. There are more ways if you're creative.

Additionally, it's possible to pin rules (rather than a SwiftLint version) by using the whitelist_rules configuration key.

I think the only things that aren't covered are:

1. A canonical way to specify which SwiftLint version should be used
2. A way to inform users that they're using a different version of SwiftLint than the project intended

Both of these could be addressed by having a swiftlint_version configuration key as suggested here which would just log if using a different version. This is easy to do, so maybe I'll just make a quick PR for this.

[jpsim]

Done in #1134.