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

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.25s vs 1.27s on main (1% faster)
📖 Linting Brave with this PR took 7.2s vs 7.19s on main (0% slower)
📖 Linting DuckDuckGo with this PR took 5.05s vs 5.09s on main (0% faster)
📖 Linting Firefox with this PR took 10.6s vs 10.57s on main (0% slower)
📖 Linting Kickstarter with this PR took 9.79s vs 9.83s on main (0% faster)
📖 Linting Moya with this PR took 0.53s vs 0.55s on main (3% faster)
📖 Linting NetNewsWire with this PR took 2.62s vs 2.64s on main (0% faster)
📖 Linting Nimble with this PR took 0.78s vs 0.79s on main (1% faster)
📖 Linting PocketCasts with this PR took 8.45s vs 8.53s on main (0% faster)
📖 Linting Quick with this PR took 0.46s vs 0.44s on main (4% slower)
📖 Linting Realm with this PR took 4.5s vs 4.47s on main (0% slower)
📖 Linting Sourcery with this PR took 2.31s vs 2.3s on main (0% slower)
📖 Linting Swift with this PR took 4.5s vs 4.48s on main (0% slower)
📖 Linting VLC with this PR took 1.27s vs 1.26s on main (0% slower)
📖 Linting Wire with this PR took 17.5s vs 17.46s on main (0% slower)
📖 Linting WordPress with this PR took 11.52s vs 11.5s 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 2 times, most recently from fbf54ad to ea398f2 Compare October 20, 2024 11:00
@mildm8nnered mildm8nnered force-pushed the mildm8nnered-add-lenient-option-to-config-file branch from ea398f2 to 1625eb0 Compare October 26, 2024 13:50
CHANGELOG.md Show resolved Hide resolved
Source/SwiftLintFramework/LintOrAnalyzeCommand.swift Outdated Show resolved Hide resolved
@mildm8nnered mildm8nnered merged commit 23d6a7c into realm:main Oct 30, 2024
14 checks passed
@mildm8nnered mildm8nnered deleted the mildm8nnered-add-lenient-option-to-config-file branch October 30, 2024 22:03
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
4 participants