Swiftlint: Setting goals for a 1.0.0 milestone

[scottrhoyt]

/cc @realm/swiftlint

I think it might be good to start a discussion about creating a 1.0.0 release milestone. We could nominate outstanding issues, create new issues, and add to this milestone where appropriate. That way collaborators can have clear direction on how to get the project to that point, and users could vote for what features are most important to them. Obviously stabilizing the public interface of both swiftlint and SwiftLintFramework should be primary focuses.

Another consideration should be what a breaking change is? Do we consider introducing a new rule that defaults to on or changing the default parameters of an old rule to be breaking changes? How about removing or combining old rules? These changes should not break the CLI on one hand, however they have the potential to cause failing build scripts both locally and on CI machines via a swiftlint upgrade.

What do you guys think, or is this too early to discuss?

If we want to start this now, I would nominate the following issues to be part of a 1.0.0 release milestone:

• #409
• #386
• #425
• #392

[daniel-beard]

Something I've brought up in the past is that using swiftlint in an enterprise setting usually means that we want only a subset of rules to be enabled (i.e. a whitelist single source of truth that overrides opt-in and disabled rules lists).

The lack of such support has made upgrading swiftlint a real pain, as the rules often change and cause new unexpected violations. I would be happy to contribute something like this, and would hate to have to maintain a fork to achieve the same behavior.

Note, this was completed in #256

[jpsim]

Thanks for taking the initiative to bring this up, @scottrhoyt! This is something I've thought of occasionally but never really pinned down what it would mean for SwiftLint to be 1.0.

A few ideas:

1. Trimming the public API. There are probably many public declarations that don't necessarily need to be. Removing those, or reducing their ACL will make it easier to maintain API stability post-1.0.
2. Reviewing & finalizing the YAML configuration format. The nice thing about YAML is that you can add support for new keys without making breaking changes, but it'd be nice to make sure the config options that work today will continue to work for a little while after 1.0.
3. I feel like #221 might be a good candidate for 1.0 as well, although the need for that is somewhat lessened by #436.
4. #376 would also be a good candidate for 1.0.

[ZevEisenberg]

Is there an easy way to specify the version of SwiftLint to use per-project, like one might specify the version of a Pod? If so, that could be a good way to sidestep the issue of breaking changes, because you could just pin the version of SwiftLint per-project. Guessing that would have to be external, i.e. specifying a version of SwiftLint for brew to activate, but it could also be a feature of the tool itself: stick rules_version = 0.7.2 at the top of .swiftlint.yml, and any version of the tool released after that feature can activate and deactivate rules. Obviously, though, that would introduce more overhead in terms of maintaining rules perpetually, so it might be overkill.

[jpsim]

@ZevEisenberg yeah, I think that should be considered, which is why I linked to the issue you opened about this in my comment above (#221).

[tamarnachmany]

I, like @daniel-beard would be interested in supporting a configuration that explicitly enables rules, rather than explicitly disabling rules, for exactly the same reason -- violations which are introduced in new versions of SwiftLint cause unanticipated errors, and may also reflect rules that we don't want implemented on our team.

I'd be happy to work on this if I know there is interest in this kind of change, or just to participate in the conversation.

[daniel-beard]

@tamarnachmany there is now a new configuration in the .swiftlint.yml files called whitelist_rules that performs this behavior. See #256

[ZevEisenberg]

@what about a model like GCC/Clang warnings and -Weverything: by default, everything is off and you switch them on one by one. But if you want, you can enable -Weverything and then disable the ones you _don't_ want, so that you get a chance to evaluate new rules when they are introduced. If you're into that sort of thing.

[tamarnachmany]

When I tried to use that functionality - I just whitelisted the rules I want to lint for - some of the rules I did not explicitly disable but did not whitelist were still linted.

[daniel-beard]

@tamarnachmany this is only present in the latest master builds, there has not been a release yet with this functionality. If this was happening from a source build from master, can you raise a ticket for it?

[tamarnachmany]

Hm. So it sounds like I saw that PR, added a whitelist to my config file, and then my config file must have been ignored since it was invalid (the configuration was invalid, since the whitelist functionality is unreleased). Cool. Looking forward to that change.

[marcelofabri]

Almost one year later, I'd like to share my thoughts on what should be done to reach 1.0 IMO:

Performance

These already have pending PRs, but should improve drastically SwiftLint performance

• [x] #1077: Parallel linting
• [x] #868: Cache between lints

Documentation/Experience

• [x] #1002: I think we should remove configurations from swiftlint rules
• [x] #1078: We should have a page with all rules, configurations and examples. Something like rubocop does. This can be autogenerated. Since it's hard to import a framework on a script, my idea here would be adding a swiftlint export_rules (please come up with a better name) that would generate a JSON file with all rules informations. Then, a script would create a Markdown file in the repo. People should run this script before opening PRs that change or add rules.
• [ ] #532: swiftlint config command. We should at least implement the help and init. We could move the rule configurations to a subcommand here.
• [ ] #533: swiftlint config --check to check the syntax of the .swiftlint.yml
• [ ] #1088 and #531: We should support UNIX-style arguments (--help for example)
• [x] #1102: If we can warn if no violations were produced in a disabled region, that'd cool

Configuration handling

We have several opened issues on how configurations are handled: #1075, #987, #736, #676, #606, #551, #550, #554.

Stability

• [x] #973: Rule aliases would enable us to deprecate rules. There's already a PR implementing this.
• [x] #221: Version pinning. I'm still not totally convinced that the tool should handle this. However, the only way to pin it currently is using CocoaPods, which is not ideal as it's not commonly used for distributing binaries.
• [ ] #479: Trim public API - the less public APIs we have, easier it is to make changes.

Bugs

• [x] #1045: Getting version is broken with SPM.
• [x] #1006: A crash when using emoji
• [x] #949: There's already a PR for this
• [x] #921: Segmentation fault on trailing_comma with Swift 2.3. If we can't find a way to fix it, we should at least disable the rule when linting Swift 2.3 code.

There are lots of issues linked here but IMO most of the hardest ones are being tackled already. The ones that probably deserved an special attention are the ones related to configuration as no progress has been done on that area for a while.

[jpsim]

If API stability is a goal for 1.0 (which it should be IMHO), then conforming to the Swift 3 API Guidelines is also a must.

[jpsim]

The migration to conform to the Swift 3 API Guidelines was done in #1139, #1140, #1141, #1142, #1143, #1144 and #1145. Thanks @marcelofabri for the quick reviews.

[jpsim]

Cache implemented by @marcelofabri in #1073

[jpsim]

Checked off a few items from https://github.com/realm/SwiftLint/issues/427#issuecomment-269885667.

Big pieces left to do are (loosely):

• Change cli arguments to be unix-compliant
• Add config command with init and check options
• Trim public API

I propose we push to get these done in the coming weeks. It'd be nice to finally mark SwiftLint as 1.0.

[marcelofabri]

I'd love to get some feedback about https://github.com/realm/SwiftLint/pull/1843 to see if it's something we want to do or not. If it is, IMO we should do it before 1.0.

[jpsim]

Never really took a good luck at that to be honest. Was a bit scared off by the size.

[marcelofabri]

I don't particularly care about the implementation, it's more about its goals and (breaking) changes.

[stale[bot]]

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!