Support disabling/enabling rules from source code comments

[jpsim]

For example:

// swiftlint:disable_rule:line_length
... some long line
// swiftlint:enable_rule:line_length

[aarondaub]

@jpsim do we want to make this purely additive or subtractive (additive would be having no rules on by default, subtractive would be having all rules on by default)? I don't think we should support both - that would complicate things unnecessarily.

How do you feel about using introspection to map YAML properties to rules? Another option would be to add a property to each Rule for a YAML identifier.

[segiddins]

I like rubocops approach -- sensible defaults, can override in either direction.

[regexident]

I would vote for an approach based on a context-stack akin to clang analyzer:

Instead of simple serial switches one would be able to have elaborate nested configurations.

And instead of binary enable_rule/disable_rule switches one would be offered to choose from the same options that clang offers: ignored, note, remark, warning, error, fatal.

// swiftlint: push
// swiftlint: ignored some_rule

... some code

// swiftlint: pop

Or a more complex scenario:

// swiftlint: push
// swiftlint: warning rule_1
// swiftlint: error rule_2
// swiftlint: error rule_3

... some code

// swiftlint: push
// swiftlint: ignored rule_3

... some code

// swiftlint: pop

... some code

// swiftlint: pop

With sensible defaults of course.

People should be fairly familiar with this approach by now and can just apply their knowledge of clang-analyzer to swiftlint, making swiftlint feel kinda like a community-based extension to clang-analyzer. With Apple being more than likely to come up with a stack-based equivalent for #pragma clang diagnostic for Swift in the future, migrating from one stack-based system to another would be much easier than from a stack-less one.

[aarondaub]

👍 stack approach. Not sure if we should add the non-binary options in this issue or open another.

[regexident]

I'll add a separate issue for non-binary warning types if desired.

[aarondaub]

@regexident I have an implementation of the stack approach in #24. It'll be pretty easy to add options for enabling/disabling rules within a LinterContext (but I'll open a new PR for that if/when #24 is merged)

[regexident]

@aarondaub that's great! Looking forward to it!

[aarondaub]

@jpsim curious about your thoughts on something: if we decide to go with the stack approach should commands in a stack apply to that context regardless of order. E.g.

// swift-lint:begin-context
// swift-lint:disable-rule:line-length
... some code
// swift-lint:disable-rule:force-cast
... some more code
// swift-lint:end-context

Should both rules be disabled for both ... some code and ... some more code? Doing it that way might be a bit counter intuitive but if we don't we'll need to jump through some more hoops. Thoughts?

[jpsim]

No, I don't think it makes sense to apply commands retro-actively across the same stack. The more I think of it, I really dislike the stack-based approach, both from a user's perspective and in terms of how it would add additional complexity to the parsing implementation.

[alexanderjarvis]

Can we also provide a global configuration file where users of SwiftLint can specify their own coding style to enable/disable certain rules as they see fit? This is what is stopping me from keeping SwiftLint in my project because I either didn't agree with one of the rules, or one of the warnings was too verbose to warrant having it in my build.

[jpsim]

@alexanderjarvis we're tracking that in #3 😄

[alexanderjarvis]

Amazing, thank you!

[keith]

Any thoughts on implementing this on a per file basis, without the push/pop first? Meaning that you could disable one or more rules for the entire file? I've implemented this on a fork by checking that the comment at the top of the file contains a string matching the rules identifier to remove it.

[keith]

Ah I see that something similar has been implemented in #76

[LeonidKokhnovich]

Does it somehow address the issue when you have the code like this:

    /// Function used to test swiftlint.
    ///
    /// - Returns: Some bool.
    // swiftlint:disable:next function_body_length
    func test() -> Bool {
      // very long function, 100+ lines of code, triggers "function_body_length" rule violation
      ...
    }

If the "swiftlint disable" statement is added as shown above, the documentation won't be displayed in Xcode quick documentation (Option + Click).