From dc580e8377fcebaae478e4dd15a2eb43b27db756 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Thu, 26 Feb 2026 21:28:13 +0100 Subject: [PATCH] Sort imports by access level (#2358) Co-authored-by: Cal Stephens --- Rules.md | 4 +- Sources/OptionDescriptor.swift | 16 +- Sources/Options.swift | 32 ++- Sources/ParsingHelpers.swift | 20 +- Sources/Rules/SortImports.swift | 45 +++-- Tests/OptionDescriptorTests.swift | 44 ++++ Tests/Rules/BlankLineAfterImportsTests.swift | 2 +- Tests/Rules/SortImportsTests.swift | 200 ++++++++++++++++++- 8 files changed, 331 insertions(+), 32 deletions(-) diff --git a/Rules.md b/Rules.md index 0a38fc7e..b5c5835c 100644 --- a/Rules.md +++ b/Rules.md @@ -3371,11 +3371,11 @@ Option | Description ## sortImports -Sort import statements alphabetically. +Sort and group import statements. Option | Description --- | --- -`--import-grouping` | Import statement grouping: "alpha" (default), "length", "testable-first" or "testable-last" +`--import-grouping` | Comma-delimited list of import sorting/grouping options: "alpha", "access-control", "length", "testable-first", "testable-last". Defaults to "access-control,alpha"
Examples diff --git a/Sources/OptionDescriptor.swift b/Sources/OptionDescriptor.swift index 312ea2e9..14af6d19 100644 --- a/Sources/OptionDescriptor.swift +++ b/Sources/OptionDescriptor.swift @@ -856,14 +856,16 @@ struct _Descriptors { let importGrouping = OptionDescriptor( argumentName: "import-grouping", displayName: "Import Grouping", - help: "Import statement grouping:", + help: "Comma-delimited list of import sorting/grouping options: \"alpha\", \"access-control\", \"length\", \"testable-first\", \"testable-last\". Defaults to \"access-control,alpha\"", keyPath: \FormatOptions.importGrouping, - altOptions: [ - "alphabetized": .alpha, - "alphabetical": .alpha, - "testable-top": .testableFirst, - "testable-bottom": .testableLast, - ] + type: .text, + fromArgument: { arg in + Set(parseCommaDelimitedList(arg).compactMap { ImportGrouping(rawValue: $0) }) + }, + toArgument: { options in + let order = ImportGrouping.allCases + return order.filter { options.contains($0) }.map(\.rawValue).joined(separator: ",") + } ) let trailingClosures = OptionDescriptor( argumentName: "trailing-closures", diff --git a/Sources/Options.swift b/Sources/Options.swift index 7be76f1e..3f548caa 100644 --- a/Sources/Options.swift +++ b/Sources/Options.swift @@ -446,12 +446,34 @@ public enum Grouping: Equatable, RawRepresentable, CustomStringConvertible { } } -/// Grouping for sorting imports -public enum ImportGrouping: String, CaseIterable { +/// Individual import sorting/grouping options, combined as a Set +public enum ImportGrouping: String, CaseIterable, Hashable { case alpha case length + case accessControl = "access-control" case testableFirst = "testable-first" case testableLast = "testable-last" + + public init?(rawValue: String) { + switch rawValue { + case "alphabetized", + "alphabetical", + "alpha": + self = .alpha + case "length": + self = .length + case "access-control": + self = .accessControl + case "testable-first", + "testable-top": + self = .testableFirst + case "testable-last", + "testable-bottom": + self = .testableLast + default: + return nil + } + } } /// Self insertion mode @@ -803,7 +825,7 @@ public struct FormatOptions: CustomStringConvertible { public var throwCapturing: Set public var asyncCapturing: Set public var experimentalRules: Bool - public var importGrouping: ImportGrouping + public var importGrouping: Set public var trailingClosures: Set public var neverTrailing: Set public var xcodeIndentation: Bool @@ -950,7 +972,7 @@ public struct FormatOptions: CustomStringConvertible { throwCapturing: Set = [], asyncCapturing: Set = [], experimentalRules: Bool = false, - importGrouping: ImportGrouping = .alpha, + importGrouping: Set = [.accessControl, .alpha], trailingClosures: Set = [], neverTrailing: Set = [], xcodeIndentation: Bool = false, @@ -1207,6 +1229,8 @@ public struct FormatOptions: CustomStringConvertible { value = array.joined(separator: ",") case let set as Set: value = set.sorted().joined(separator: ",") + case let set as Set: + value = ImportGrouping.allCases.filter { set.contains($0) }.map(\.rawValue).joined(separator: ",") default: break } diff --git a/Sources/ParsingHelpers.swift b/Sources/ParsingHelpers.swift index 0c8e5594..e8fdcda3 100644 --- a/Sources/ParsingHelpers.swift +++ b/Sources/ParsingHelpers.swift @@ -2241,6 +2241,7 @@ extension Formatter { var module: String var range: Range var attributes: [String] + var accessLevel: String? var isTestable: Bool { attributes.contains("@testable") @@ -2400,8 +2401,12 @@ extension Formatter { } previousKeywordIndex = index(of: .keywordOrAttribute, before: previousIndex) startIndex = nextStart ?? startIndex + } else if case let .keyword(kw) = tokens[previousIndex], + _FormatRules.aclModifiers.contains(kw) + { + // Allow import access modifiers (Swift 6 SE-0409) + previousKeywordIndex = index(of: .keywordOrAttribute, before: previousIndex) } else if previousIndex >= startIndex { - // Can't handle another keyword on same line as import return } else { break @@ -2450,10 +2455,15 @@ extension Formatter { partIndex = nextPartIndex } let range = startIndex ..< endIndex as Range + let accessLevel: String? = tokens[range].lazy.compactMap { token -> String? in + guard case let .keyword(kw) = token, _FormatRules.aclModifiers.contains(kw) else { return nil } + return kw + }.first importRanges.append(ImportRange( module: name, range: range, - attributes: tokens[range].compactMap { $0.isAttribute ? $0.string : nil } + attributes: tokens[range].compactMap { $0.isAttribute ? $0.string : nil }, + accessLevel: accessLevel )) } else { // Error @@ -2474,7 +2484,11 @@ extension Formatter { } nextTokenIndex = nextIndex } - if tokens[nextTokenIndex] != .keyword("import") { + let nextToken = tokens[nextTokenIndex] + let isImportKeyword = nextToken == .keyword("import") + // Access modifier (e.g. "public") can precede "import" on the same line (Swift 6) + let isAccessModifierBeforeImport = nextToken.isKeyword && _FormatRules.aclModifiers.contains(nextToken.string) + if !isImportKeyword, !isAccessModifierBeforeImport { // End of imports pushStack() return diff --git a/Sources/Rules/SortImports.swift b/Sources/Rules/SortImports.swift index c31f2b2b..44b70ef7 100644 --- a/Sources/Rules/SortImports.swift +++ b/Sources/Rules/SortImports.swift @@ -11,7 +11,7 @@ import Foundation public extension FormatRule { /// Sort import statements static let sortImports = FormatRule( - help: "Sort import statements alphabetically.", + help: "Sort and group import statements.", options: ["import-grouping"], sharedOptions: ["linebreaks"] ) { formatter in @@ -62,19 +62,40 @@ public extension FormatRule { extension Formatter { func sortRanges(_ ranges: [Formatter.ImportRange]) -> [Formatter.ImportRange] { - if case .alpha = options.importGrouping { - return ranges.sorted(by: <) - } else if case .length = options.importGrouping { - return ranges.sorted { $0.module.count < $1.module.count } + let grouping = options.importGrouping + + let partitions: [[Formatter.ImportRange]] + if grouping.contains(.testableFirst) { + partitions = [ranges.filter(\.isTestable), ranges.filter { !$0.isTestable }] + } else if grouping.contains(.testableLast) { + partitions = [ranges.filter { !$0.isTestable }, ranges.filter(\.isTestable)] + } else { + partitions = [ranges] } - // Group @testable imports at the top or bottom - // TODO: need more general solution for handling other import attributes - return ranges.sorted { - // If both have a @testable keyword, or neither has one, just sort alphabetically - guard $0.isTestable != $1.isTestable else { - return $0 < $1 + + return partitions.flatMap { partition in + partition.sorted { lhs, rhs in + if grouping.contains(.accessControl) { + let lhsAccessOrder = accessLevelSortOrder(for: lhs) + let rhsAccessOrder = accessLevelSortOrder(for: rhs) + if lhsAccessOrder != rhsAccessOrder { + return lhsAccessOrder > rhsAccessOrder + } + } + + if grouping.contains(.length) { + return lhs.module.count < rhs.module.count + } + // Default to alphabetical + return lhs < rhs } - return options.importGrouping == .testableFirst ? $0.isTestable : $1.isTestable } } + + /// Sort order for import access level using aclModifiers (higher index = more visible). + /// Unlabeled imports return -1 (sorted last). + func accessLevelSortOrder(for range: Formatter.ImportRange) -> Int { + guard let level = range.accessLevel else { return -1 } + return _FormatRules.aclModifiers.firstIndex(of: level) ?? -1 + } } diff --git a/Tests/OptionDescriptorTests.swift b/Tests/OptionDescriptorTests.swift index 37fe93f9..02720529 100644 --- a/Tests/OptionDescriptorTests.swift +++ b/Tests/OptionDescriptorTests.swift @@ -355,4 +355,48 @@ final class OptionDescriptorTests: XCTestCase { var options: FormatOptions = .default XCTAssertNoThrow(try Descriptors.preferFileMacro.toOptions(argument, &options)) } + + // MARK: - importGrouping + + func testImportGroupingAcceptsCommaDelimitedList() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("access-control,alpha,testable-last", &options)) + XCTAssertEqual(options.importGrouping, [.accessControl, .alpha, .testableLast]) + } + + func testImportGroupingAcceptsAlphabetical() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("alphabetical", &options)) + XCTAssertTrue(options.importGrouping.contains(.alpha)) + } + + func testImportGroupingAcceptsAlphabetized() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("alphabetized", &options)) + XCTAssertTrue(options.importGrouping.contains(.alpha)) + } + + func testImportGroupingAcceptsTestableLast() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("testable-last", &options)) + XCTAssertEqual(options.importGrouping, [.testableLast]) + } + + func testImportGroupingAcceptsTestableBottom() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("testable-bottom", &options)) + XCTAssertEqual(options.importGrouping, [.testableLast]) + } + + func testImportGroupingAcceptsTestableFirst() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("testable-first", &options)) + XCTAssertEqual(options.importGrouping, [.testableFirst]) + } + + func testImportGroupingAcceptsTestableTop() { + var options: FormatOptions = .default + XCTAssertNoThrow(try Descriptors.importGrouping.toOptions("testable-top", &options)) + XCTAssertEqual(options.importGrouping, [.testableFirst]) + } } diff --git a/Tests/Rules/BlankLineAfterImportsTests.swift b/Tests/Rules/BlankLineAfterImportsTests.swift index 7ea8b4c3..5f908f29 100644 --- a/Tests/Rules/BlankLineAfterImportsTests.swift +++ b/Tests/Rules/BlankLineAfterImportsTests.swift @@ -105,6 +105,6 @@ final class BlankLineAfterImportsTests: XCTestCase { public class Foo {} """ - testFormatting(for: input, output, rule: .blankLineAfterImports) + testFormatting(for: input, output, rule: .blankLineAfterImports, exclude: [.sortImports]) } } diff --git a/Tests/Rules/SortImportsTests.swift b/Tests/Rules/SortImportsTests.swift index 1fa2ebe2..e7cdc68b 100644 --- a/Tests/Rules/SortImportsTests.swift +++ b/Tests/Rules/SortImportsTests.swift @@ -22,6 +22,26 @@ final class SortImportsTests: XCTestCase { testFormatting(for: input, output, rule: .sortImports) } + func testDefaultGroupingBehaviorIsAccessControlThenAlpha() { + let input = """ + @testable import Foo + import Zed + public import Alpha + import Bar + public import Beta + @testable import Ace + """ + let output = """ + public import Alpha + public import Beta + @testable import Ace + import Bar + @testable import Foo + import Zed + """ + testFormatting(for: input, output, rule: .sortImports) + } + func testSortImportsKeepsPreviousCommentWithImport() { let input = """ import Foo @@ -198,7 +218,7 @@ final class SortImportsTests: XCTestCase { import Bar3 import Module """ - let options = FormatOptions(importGrouping: .length) + let options = FormatOptions(importGrouping: [.length]) testFormatting(for: input, output, rule: .sortImports, options: options) } @@ -227,7 +247,7 @@ final class SortImportsTests: XCTestCase { @testable import Bar @testable import UIKit """ - let options = FormatOptions(importGrouping: .testableLast) + let options = FormatOptions(importGrouping: [.alpha, .testableLast]) testFormatting(for: input, output, rule: .sortImports, options: options) } @@ -242,7 +262,7 @@ final class SortImportsTests: XCTestCase { @testable import UIKit import Foo """ - let options = FormatOptions(importGrouping: .testableFirst) + let options = FormatOptions(importGrouping: [.alpha, .testableFirst]) testFormatting(for: input, output, rule: .sortImports, options: options) } @@ -402,4 +422,178 @@ final class SortImportsTests: XCTestCase { """ testFormatting(for: input, output, rule: .sortImports) } + + // MARK: - Access control sorting + + func testAccessControlSortImports() { + let input = """ + import Foo + private import Bar + public import Baz + """ + let output = """ + public import Baz + private import Bar + import Foo + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testAccessControlSortAlphaWithinLevel() { + let input = """ + public import Zebra + public import Alpha + public import Middle + """ + let output = """ + public import Alpha + public import Middle + public import Zebra + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testAccessControlSortLengthWithinLevel() { + let input = """ + public import Zebra + public import Al + public import Middle + """ + let output = """ + public import Al + public import Zebra + public import Middle + """ + let options = FormatOptions(importGrouping: [.length, .accessControl]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testAccessControlSortLengthWithMultipleACLs() { + let input = """ + private import LongPrivate + public import Baz + private import Al + public import LongPublic + import Foo + """ + let output = """ + public import Baz + public import LongPublic + private import Al + private import LongPrivate + import Foo + """ + let options = FormatOptions(importGrouping: [.length, .accessControl]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testAccessControlWithTestableFirst() { + let input = """ + import Foo + @testable import Bar + public import Baz + """ + let output = """ + @testable import Bar + public import Baz + import Foo + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl, .testableFirst]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testAccessControlWithTestableLast() { + let input = """ + public import Baz + @testable import Bar + import Foo + """ + let output = """ + public import Baz + import Foo + @testable import Bar + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl, .testableLast]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testUnlabeledImportsSortLast() { + let input = """ + import Foo + public import Bar + internal import Baz + import Qux + """ + let output = """ + public import Bar + internal import Baz + import Foo + import Qux + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testTestableImportsSortedByACLAndAlpha() { + let input = """ + @testable import DModule + @testable public import CModule + @testable import AModule + @testable public import BModule + import ZModule + public import UModule + import YModule + public import TModule + """ + let output = """ + public import TModule + public import UModule + import YModule + import ZModule + @testable public import BModule + @testable public import CModule + @testable import AModule + @testable import DModule + """ + let options = FormatOptions(importGrouping: [.alpha, .accessControl, .testableLast]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + // MARK: - Length + testable combinations + + func testLengthSortWithTestableTop() { + let input = """ + import Foo + @testable import LongModule + import Ba + @testable import Az + """ + let output = """ + @testable import Az + @testable import LongModule + import Ba + import Foo + """ + let options = FormatOptions(importGrouping: [.length, .testableFirst]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } + + func testLengthSortWithTestableBottom() { + let input = """ + @testable import LongModule + import Foo + import Ba + @testable import Az + """ + let output = """ + import Ba + import Foo + @testable import Az + @testable import LongModule + """ + let options = FormatOptions(importGrouping: [.length, .testableLast]) + testFormatting(for: input, output, rule: .sortImports, options: options) + } }