Sort imports by access level (#2358)

Co-authored-by: Cal Stephens <cal@calstephens.tech>

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"
 
 <details>
 <summary>Examples</summary>

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",

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<String>
     public var asyncCapturing: Set<String>
     public var experimentalRules: Bool
-    public var importGrouping: ImportGrouping
+    public var importGrouping: Set<ImportGrouping>
     public var trailingClosures: Set<String>
     public var neverTrailing: Set<String>
     public var xcodeIndentation: Bool
@@ -950,7 +972,7 @@ public struct FormatOptions: CustomStringConvertible {
                 throwCapturing: Set<String> = [],
                 asyncCapturing: Set<String> = [],
                 experimentalRules: Bool = false,
-                importGrouping: ImportGrouping = .alpha,
+                importGrouping: Set<ImportGrouping> = [.accessControl, .alpha],
                 trailingClosures: Set<String> = [],
                 neverTrailing: Set<String> = [],
                 xcodeIndentation: Bool = false,
@@ -1207,6 +1229,8 @@ public struct FormatOptions: CustomStringConvertible {
                 value = array.joined(separator: ",")
             case let set as Set<String>:
                 value = set.sorted().joined(separator: ",")
+            case let set as Set<ImportGrouping>:
+                value = ImportGrouping.allCases.filter { set.contains($0) }.map(\.rawValue).joined(separator: ",")
             default:
                 break
             }

Sources/ParsingHelpers.swift

@@ -2241,6 +2241,7 @@ extension Formatter {
         var module: String
         var range: Range<Int>
         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

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
+    }
 }

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])
+    }
 }

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])
     }
 }

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)
+    }
 }