diff --git a/.swiftlint.yml b/.swiftlint.yml index 0f6d61786c..17319cdaf1 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -44,6 +44,7 @@ disabled_rules: - prefer_nimble - prefixed_toplevel_constant - required_deinit + - sorted_collection_members - sorted_enum_cases - strict_fileprivate - switch_case_on_newline diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 51b2a4eee4..99780db068 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -199,6 +199,7 @@ public let builtInRules: [any Rule.Type] = [ ShorthandOperatorRule.self, ShorthandOptionalBindingRule.self, SingleTestClassRule.self, + SortedCollectionMembersRule.self, SortedEnumCasesRule.self, SortedFirstLastRule.self, SortedImportsRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/SortedCollectionMembersConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/SortedCollectionMembersConfiguration.swift new file mode 100644 index 0000000000..a85ed0bb09 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/SortedCollectionMembersConfiguration.swift @@ -0,0 +1,10 @@ +import SwiftLintCore + +@AutoConfigParser +struct SortedCollectionMembersConfiguration: SeverityBasedRuleConfiguration { + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + + @ConfigurationElement(key: "reverse") + private(set) var reverse = false +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Style/SortedCollectionMembersRule.swift b/Source/SwiftLintBuiltInRules/Rules/Style/SortedCollectionMembersRule.swift new file mode 100644 index 0000000000..898d8b0d00 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Style/SortedCollectionMembersRule.swift @@ -0,0 +1,265 @@ +import Foundation +import SwiftLintCore +import SwiftSyntax + +@SwiftSyntaxRule(optIn: true) +struct SortedCollectionMembersRule: Rule { + var configuration = SortedCollectionMembersConfiguration() + + static let description = RuleDescription( + identifier: "sorted_collection_members", + name: "Sorted Collection Members", + description: "Please keep the elements of this collection literal sorted", + kind: .style, + nonTriggeringExamples: #examples([ + // Arrays + + "[1, 2, 3]", + "[a, b, c]", + "[c, b, a]".asExample(configuration: reverseSort), + "[]", + "[1]", + "[a]", + """ + ["a", "b", "c"] + """, + """ + ["c", "b", "a"] + """.asExample(configuration: reverseSort), + """ + ["a"] + """, + """ + [ + .thingA, + .thingB, + // comments are ignored in sort checking + .thingC, + ] + """, + """ + [ + .thingC, + .thingB, + // comments are ignored in sort checking + .thingA, + ] + """.asExample(configuration: reverseSort), + + // Dictionaries + + "[1: 1, 2: 2, 3: 3]", + "[a: 200, b: 10, c: 0]", + "[c: 0, b: 0, a: 0]".asExample(configuration: reverseSort), + "[:]", + "[1: 1]", + "[a: 1]", + """ + ["a": "A", "b": "B", "c": "C"] + """, + """ + ["c": "C", "b": "B", "a": "A"] + """.asExample(configuration: reverseSort), + """ + ["a": "A"] + """, + """ + [ + .thingA: "A", + .thingB: "B", + // comments are ignored in sort checking + .thingC: "C", + ] + """, + """ + [ + .thingC: "C", + .thingB: "B", + // comments are ignored in sort checking + .thingA: "A", + ] + """.asExample(configuration: reverseSort), + ]), + triggeringExamples: #examples([ + // Arrays + + "[1, ↓3, 2]", + "[↓1, 3, 2]".asExample(configuration: reverseSort), + "[↓1, 2, 3]".asExample(configuration: reverseSort), + """ + [↓"b", "c", "a"] + """, + """ + [↓"b", "c", "a"] + """.asExample(configuration: reverseSort), + """ + [ + ↓.thingBNoComment, + .thingCNoComment, + .thingANoComment, + ] + """, + """ + [ + ↓.thingANoComment, + .thingBNoComment, + .thingCNoComment, + ] + """.asExample(configuration: reverseSort), + """ + [ + ↓.thingBWithComment, + // Comments are not counted when evaluating sort order + .thingCWithComment, + .thingAWithComment, + ] + """, + """ + [ + ↓.thingAWithComment, + .thingBWithComment, + // Comments are not counted when evaluating sort order + .thingCWithComment, + ] + """.asExample(configuration: reverseSort), + // integration-style test + """ + let package = Package( + name: "Packages", + platforms: [ + ↓.macOS(.v10_15), + .iOS(.v26), + ], + products: [ + ↓.library(name: "Library2", type: type, targets: ["Library2"]), + .library(name: "Library1", type: type, targets: ["Library1"]), + ], + dependencies: [ + ↓.package( + url: "https://github.com/qux/quiz", + exact: "4.5.6" + ), + .package( + url: "https://github.com/foo/bar", + exact: "1.2.3" + ), + ], + targets: [ + ↓.target( + name: "CoolFeatureB", + dependencies: [ + "SomeDependency", + .product(name: "AnotherDependency", package: "another-dependency"), + ] + ), + .target( + name: "CoolFeatureA", + dependencies: [ + ↓.dependencyB, + .dependencyA, + ] + ), + ] + ) + """, + + // Dictionaries are unordered, but you may still want to sort dictionary + // literals for code style reasons, or if you're using them to construct + // an instance of KeyValuePairs or some other ordered dictionary-like + // structure. + + "[1: 1, ↓3: 3, 2: 2]", + "[↓1: 1, 3: 3, 2: 2]".asExample(configuration: reverseSort), + "[↓1: 1, 2: 2, 3: 3]".asExample(configuration: reverseSort), + """ + [↓"b": 0, "c": 0, "a": 0] + """, + """ + [↓"b": 0, "c": 0, "a": 0] + """.asExample(configuration: reverseSort), + """ + [ + ↓.thingBNoComment: 0, + .thingCNoComment: 0, + .thingANoComment: 0, + ] + """, + """ + [ + ↓.thingANoComment: 0, + .thingBNoComment: 0, + .thingCNoComment: 0, + ] + """.asExample(configuration: reverseSort), + """ + [ + ↓.thingBWithComment: 0, + // Comments are not counted when evaluating sort order + .thingCWithComment: 0, + .thingAWithComment: 0, + ] + """, + """ + [ + ↓.thingAWithComment: 0, + .thingBWithComment: 0, + // Comments are not counted when evaluating sort order + .thingCWithComment: 0, + ] + """.asExample(configuration: reverseSort), + ]) + ) +} + +private let reverseSort: [String: any Sendable] = ["reverse": true] + +// TODO: seems like /*>*/ syntax has an off-by-one compared with ↓ syntax +// TODO: find a way to opt out of visiting nodes that do not apply to us. +// TODO: test that inline override works, so a file where this is enabled can opt-out for a single array + +private extension SortedCollectionMembersRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visit(_ node: ArrayExprSyntax) -> SyntaxVisitorContinueKind { + let sortedNames = node.elements + .map(\.expression.trimmedDescription) + .sorted(by: configuration.reverse ? (>) : (<)) + + let originalAndSorted = zip(zip(node.elements.indices, node.elements), sortedNames) + for ((originalIndex, originalElement), sortedName) in originalAndSorted + where originalElement.expression.trimmedDescription != sortedName { + violations.append(node.elements[originalIndex].positionAfterSkippingLeadingTrivia) + // break on the first sorting violation because everything after it is necessarily not sorted + break + } + + return .visitChildren + } + + override func visit(_ node: DictionaryExprSyntax) -> SyntaxVisitorContinueKind { + let content = node.content + + let elements: DictionaryElementListSyntax + switch content { + case .colon: + // empty dictionary, so there's nothing to sort + return .visitChildren + case .elements(let dictionaryElementListSyntax): + elements = dictionaryElementListSyntax + } + + let sortedNames = elements + .map(\.key.trimmedDescription) + .sorted(by: configuration.reverse ? (>) : (<)) + + let originalAndSorted = zip(zip(elements.indices, elements), sortedNames) + for ((originalIndex, originalElement), sortedName) in originalAndSorted + where originalElement.key.trimmedDescription != sortedName { + violations.append(elements[originalIndex].positionAfterSkippingLeadingTrivia) + // break on the first sorting violation because everything after it is necessarily not sorted + break + } + + return .visitChildren + } + } +} diff --git a/Tests/BuiltInRulesTests/SortedCollectionMembersRuleTests.swift b/Tests/BuiltInRulesTests/SortedCollectionMembersRuleTests.swift new file mode 100644 index 0000000000..671ccd4857 --- /dev/null +++ b/Tests/BuiltInRulesTests/SortedCollectionMembersRuleTests.swift @@ -0,0 +1,12 @@ +import TestHelpers +import Testing + +@testable import SwiftLintBuiltInRules + +@Suite(.rulesRegistered) +struct SortedCollectionMembersRuleTests { + @Test + func verify() { + verifyRule(SortedCollectionMembersRule.description, ruleConfiguration: []) + } +} diff --git a/Tests/GeneratedTests/GeneratedTests_08.swift b/Tests/GeneratedTests/GeneratedTests_08.swift index 9a9e613da4..cfb31ecf08 100644 --- a/Tests/GeneratedTests/GeneratedTests_08.swift +++ b/Tests/GeneratedTests/GeneratedTests_08.swift @@ -186,25 +186,25 @@ struct SingleTestClassRuleGeneratedTests { } @Suite(.rulesRegistered) -struct SortedEnumCasesRuleGeneratedTests { +struct SortedCollectionMembersRuleGeneratedTests { @Test func withDefaultConfiguration() { - verifyRule(SortedEnumCasesRule.description) + verifyRule(SortedCollectionMembersRule.description) } } @Suite(.rulesRegistered) -struct SortedFirstLastRuleGeneratedTests { +struct SortedEnumCasesRuleGeneratedTests { @Test func withDefaultConfiguration() { - verifyRule(SortedFirstLastRule.description) + verifyRule(SortedEnumCasesRule.description) } } @Suite(.rulesRegistered) -struct SortedImportsRuleGeneratedTests { +struct SortedFirstLastRuleGeneratedTests { @Test func withDefaultConfiguration() { - verifyRule(SortedImportsRule.description) + verifyRule(SortedFirstLastRule.description) } } diff --git a/Tests/GeneratedTests/GeneratedTests_09.swift b/Tests/GeneratedTests/GeneratedTests_09.swift index 63724b0191..f891cb0f10 100644 --- a/Tests/GeneratedTests/GeneratedTests_09.swift +++ b/Tests/GeneratedTests/GeneratedTests_09.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct SortedImportsRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(SortedImportsRule.description) + } +} + @Suite(.rulesRegistered) struct StatementPositionRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct UnhandledThrowingTaskRuleGeneratedTests { verifyRule(UnhandledThrowingTaskRule.description) } } - -@Suite(.rulesRegistered) -struct UnneededBreakInSwitchRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(UnneededBreakInSwitchRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index 6e8193c399..6b51b6d07c 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct UnneededBreakInSwitchRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(UnneededBreakInSwitchRule.description) + } +} + @Suite(.rulesRegistered) struct UnneededEscapingRuleGeneratedTests { @Test @@ -200,11 +208,3 @@ struct VoidFunctionInTernaryConditionRuleGeneratedTests { verifyRule(VoidFunctionInTernaryConditionRule.description) } } - -@Suite(.rulesRegistered) -struct VoidReturnRuleGeneratedTests { - @Test - func withDefaultConfiguration() { - verifyRule(VoidReturnRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_11.swift b/Tests/GeneratedTests/GeneratedTests_11.swift index 61a9b2b37e..8d40b52c40 100644 --- a/Tests/GeneratedTests/GeneratedTests_11.swift +++ b/Tests/GeneratedTests/GeneratedTests_11.swift @@ -9,6 +9,14 @@ import Testing @testable import SwiftLintBuiltInRules @testable import SwiftLintCore +@Suite(.rulesRegistered) +struct VoidReturnRuleGeneratedTests { + @Test + func withDefaultConfiguration() { + verifyRule(VoidReturnRule.description) + } +} + @Suite(.rulesRegistered) struct WeakDelegateRuleGeneratedTests { @Test diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index cc2f59c8b2..3b1db7c8f0 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -1141,6 +1141,12 @@ single_test_class: meta: opt-in: true correctable: false +sorted_collection_members: + severity: warning + reverse: false + meta: + opt-in: true + correctable: false sorted_enum_cases: severity: warning meta: