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

Added lenient command line option #5801

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mildm8nnered
Copy link
Collaborator

@mildm8nnered mildm8nnered commented Sep 20, 2024

Addresses #5779

So I basically just copied #5226 (which I wrote).

If you have strict and lenient as true in the config file, a fatalError will be thrown, but either can be overridden by the --strict or --lenient command line options, and they will also override the opposite setting in the config file.

For example, if you have

lenient: true in your config file, but you specify --strict on the command line, you will get the strict behaviour, and your config file setting will be ignored.

I tested this manually - it was hard to do a unit test, because we fatalError instead of throwing an error right now.

As before, --strict and --lenient on the command line will also fatal error.

@SwiftLintBot
Copy link

SwiftLintBot commented Sep 20, 2024

17 Messages
📖 Linting Aerial with this PR took 0.92s vs 0.94s on main (2% faster)
📖 Linting Alamofire with this PR took 1.27s vs 1.27s on main (0% slower)
📖 Linting Brave with this PR took 7.2s vs 7.17s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 5.02s vs 4.99s on main (0% slower)
📖 Linting Firefox with this PR took 10.46s vs 10.47s on main (0% faster)
📖 Linting Kickstarter with this PR took 9.82s vs 9.77s on main (0% slower)
📖 Linting Moya with this PR took 0.53s vs 0.54s on main (1% faster)
📖 Linting NetNewsWire with this PR took 2.63s vs 2.62s on main (0% slower)
📖 Linting Nimble with this PR took 0.78s vs 0.77s on main (1% slower)
📖 Linting PocketCasts with this PR took 8.5s vs 8.38s on main (1% slower)
📖 Linting Quick with this PR took 0.44s vs 0.46s on main (4% faster)
📖 Linting Realm with this PR took 4.49s vs 4.5s on main (0% faster)
📖 Linting Sourcery with this PR took 2.31s vs 2.29s on main (0% slower)
📖 Linting Swift with this PR took 4.49s vs 4.47s on main (0% slower)
📖 Linting VLC with this PR took 1.25s vs 1.26s on main (0% faster)
📖 Linting Wire with this PR took 17.33s vs 17.28s on main (0% slower)
📖 Linting WordPress with this PR took 11.54s vs 11.48s on main (0% slower)

Generated by 🚫 Danger

@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch from 4c5f7d0 to fc3c378 Compare September 21, 2024 18:46
@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch from fc3c378 to 5a9b47c Compare September 29, 2024 22:15
@mildm8nnered mildm8nnered marked this pull request as ready for review October 2, 2024 03:19
@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch from 5a9b47c to a399c0b Compare October 5, 2024 17:22
@SimplyDanny SimplyDanny linked an issue Oct 7, 2024 that may be closed by this pull request
2 tasks
@SimplyDanny
Copy link
Collaborator

The options turn more and more complex. It'd be really useful to have the combinations tested.

@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch 2 times, most recently from f06b9f3 to 90d3837 Compare October 11, 2024 18:25
@mildm8nnered
Copy link
Collaborator Author

The options turn more and more complex. It'd be really useful to have the combinations tested.

Did you have anything particular in mind - it would be quite easy to expose the logic in applyLeniency for the interactions between command line and config options, or are you thinking of something more.

@SimplyDanny
Copy link
Collaborator

The options turn more and more complex. It'd be really useful to have the combinations tested.

Did you have anything particular in mind - it would be quite easy to expose the logic in applyLeniency for the interactions between command line and config options, or are you thinking of something more.

Not sure if it's clean to move applyLeniency into LintOrAnalyzeOptions. In there, it would naturally be internal and can thus be tested.

@mildm8nnered
Copy link
Collaborator Author

The options turn more and more complex. It'd be really useful to have the combinations tested.

Did you have anything particular in mind - it would be quite easy to expose the logic in applyLeniency for the interactions between command line and config options, or are you thinking of something more.

Not sure if it's clean to move applyLeniency into LintOrAnalyzeOptions. In there, it would naturally be internal and can thus be tested.

Something like this perhaps, which tests the logic, if not the actual violations mapping?

extension LintOrAnalyzeOptions {
    // config file settings can be overridden by either `--strict` or `--lenient` command line options
    func leniency(strict: Bool, lenient: Bool) -> (Bool , Bool) {
        let strict = (strict && !self.lenient) || self.strict
        let lenient = (lenient && !self.strict) || self.lenient
        return (strict, lenient)
    }
}

@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch from 90d3837 to fbf54ad Compare October 15, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for configuring lenient: true in .swiftlint.yml
3 participants