realm
Adds invalid swiftlint command rule
Rule-based approach to https://github.com/realm/SwiftLint/pull/4543
Adds an invalid_swiftlint_command rule to detect malformed swiftlint commands. Bungled enable's in particular, after a valid disable can lead to stretches of code going unintentionally un-linted.
nonTriggeringExamples:
// swiftlint:disable unused_import
// swiftlint:enable unused_import
// swiftlint:disable:next unused_import
// swiftlint:disable:previous unused_import
// swiftlint:disable:this unused_import
triggeringExamples:
// swiftlint:
// swiftlint:
// swiftlint::
// swiftlint::
// swiftlint:disable
// swiftlint:dissable unused_import
// swiftlint:enaaaable unused_import
// swiftlint:disable:nxt unused_import
// swiftlint:enable:prevus unused_import
// swiftlint:enable:ths unused_import
// swiftlint:enable
// swiftlint:enable:
// swiftlint:enable:
// swiftlint:disable: unused_import
Command Parsing
This PR also slightly changes the parsing of commands. Previously cases like
// swiftlint:disable: unused_import
// swiftlint:disable:nxt unused_import
// swiftlint:enable:prevus unused_import
// swiftlint:enable:ths unused_import
Would have been parsed, incorrectly, as
// swiftlint:disable unused_import
// swiftlint:disable unused_import
// swiftlint:enable unused_import
// swiftlint:enable unused_import
These cases will now be ignored, and will be warned about by this rule.
| 1 Warning | |
|---|---|
| :warning: | This PR introduced a violation in WordPress: /WordPress/UITestsFoundation/Screens/ReaderScreen.swift:15:53: warning: Invalid SwiftLint Command Violation: swiftlint command does not have a valid action or modifier (invalid_swiftlint_command) |
| 18 Messages | |
|---|---|
| :book: | Linting Aerial with this PR took 1.03s vs 1.03s on main (0% slower) |
| :book: | Linting Alamofire with this PR took 1.34s vs 1.35s on main (0% faster) |
| :book: | Linting Brave with this PR took 7.2s vs 7.17s on main (0% slower) |
| :book: | Linting DuckDuckGo with this PR took 3.02s vs 3.01s on main (0% slower) |
| :book: | Linting Firefox with this PR took 9.15s vs 9.09s on main (0% slower) |
| :book: | Linting Kickstarter with this PR took 10.0s vs 10.05s on main (0% faster) |
| :book: | Linting Moya with this PR took 0.54s vs 0.53s on main (1% slower) |
| :book: | Linting NetNewsWire with this PR took 2.99s vs 2.98s on main (0% slower) |
| :book: | Linting Nimble with this PR took 0.58s vs 0.58s on main (0% slower) |
| :book: | Linting PocketCasts with this PR took 7.03s vs 7.0s on main (0% slower) |
| :book: | Linting Quick with this PR took 0.22s vs 0.22s on main (0% slower) |
| :book: | Linting Realm with this PR took 11.29s vs 11.32s on main (0% faster) |
| :book: | Linting SourceKitten with this PR took 0.43s vs 0.43s on main (0% slower) |
| :book: | Linting Sourcery with this PR took 2.19s vs 2.16s on main (1% slower) |
| :book: | Linting Swift with this PR took 4.44s vs 4.41s on main (0% slower) |
| :book: | Linting VLC with this PR took 1.32s vs 1.32s on main (0% slower) |
| :book: | Linting Wire with this PR took 8.2s vs 8.15s on main (0% slower) |
| :book: | Linting WordPress with this PR took 10.91s vs 10.88s on main (0% slower) |
Generated by :no_entry_sign: Danger
So I managed to work out make sourcery to get the new rule into PrimaryRuleList, but I'm failing the generated tests because of the nature of the rule, it needs to be validated with
func testExamples() {
verifyRule(InvalidSwiftLintCommandRule.description, skipCommentTests: true, skipDisableCommandTests: true)
}
as I've done in InvalidSwiftLintCommandRuleTests, but there doesn't seem to be any way to pass these parameters into the generated tests, or to exclude that rule from the generated tests.
Not sure what the best way is to go from here ...
Not sure what the best way is to go from here ...
See for example ExpiringTodoRule. You can pass the options to skip certain tests to every example or an array of examples.
So I guess I could special case this rule in .sourcery/GeneratedTests.stencil, something like
@_spi(TestHelper)
@testable import SwiftLintFramework
import SwiftLintTestHelpers
import XCTest
// swiftlint:disable file_length single_test_class type_name
{% for rule in types.structs %}
{% if rule.name|hasSuffix:"Rule" and rule.name != "InvalidSwiftLintCommandRule" %}
class {{ rule.name }}GeneratedTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule({{ rule.name }}.description)
}
}
{% if not forloop.last %}
{% endif %}
{% endif %}
{% endfor %}
and rely on the handwritten test.
Or I could further edit the stencil to generate the appropriate code for this case. That doesn't feel very graceful. I will also face a similar problem for https://github.com/realm/SwiftLint/issues/4684, so ideally any mechanism here would be easily extensible.
See also my comment at https://github.com/realm/SwiftLint/issues/3769#issuecomment-1445466113 - it looks like all kinds of odd strings, like // hjkhjkkswiftlint:disable all will be treated as valid, which seems off.