OrderedImports should create separate group for @_implementationOnly imports

ebgraham · ebgraham · commit 7c46e82fabe2 · 2025-05-06T17:01:59.000-04:00
diff --git a/Documentation/RuleDocumentation.md b/Documentation/RuleDocumentation.md
@@ -407,9 +407,9 @@ Lint: If a function call with a trailing closure also contains a non-trailing cl
 ### OrderedImports
 
 Imports must be lexicographically ordered and logically grouped at the top of each source file.
-The order of the import groups is 1) regular imports, 2) declaration imports, and 3) @testable
-imports. These groups are separated by a single blank line. Blank lines in between the import
-declarations are removed.
+The order of the import groups is 1) regular imports, 2) declaration imports, 3) @_implementationOnly
+imports, and 4) @testable imports. These groups are separated by a single blank line. Blank lines in
+between the import declarations are removed.
 
 Lint: If an import appears anywhere other than the beginning of the file it resides in,
       not lexicographically ordered, or  not in the appropriate import group, a lint error is
diff --git a/Sources/SwiftFormat/Rules/OrderedImports.swift b/Sources/SwiftFormat/Rules/OrderedImports.swift
@@ -13,9 +13,9 @@
 import SwiftSyntax
 
 /// Imports must be lexicographically ordered and logically grouped at the top of each source file.
-/// The order of the import groups is 1) regular imports, 2) declaration imports, and 3) @testable
-/// imports. These groups are separated by a single blank line. Blank lines in between the import
-/// declarations are removed.
+/// The order of the import groups is 1) regular imports, 2) declaration imports, 3) @_implementationOnly
+/// imports, and 4) @testable imports. These groups are separated by a single blank line. Blank lines in
+/// between the import declarations are removed.
 ///
 /// Lint: If an import appears anywhere other than the beginning of the file it resides in,
 ///       not lexicographically ordered, or  not in the appropriate import group, a lint error is
@@ -34,6 +34,7 @@ public final class OrderedImports: SyntaxFormatRule {
 
     var regularImports: [Line] = []
     var declImports: [Line] = []
+    var implementationOnlyImports: [Line] = []
     var testableImports: [Line] = []
     var codeBlocks: [Line] = []
     var fileHeader: [Line] = []
@@ -52,14 +53,16 @@ public final class OrderedImports: SyntaxFormatRule {
 
       regularImports = formatImports(regularImports)
       declImports = formatImports(declImports)
+      implementationOnlyImports = formatImports(implementationOnlyImports)
       testableImports = formatImports(testableImports)
       formatCodeblocks(&codeBlocks)
 
-      let joined = joinLines(fileHeader, regularImports, declImports, testableImports, codeBlocks)
+      let joined = joinLines(fileHeader, regularImports, declImports, implementationOnlyImports, testableImports, codeBlocks)
       formattedLines.append(contentsOf: joined)
 
       regularImports = []
       declImports = []
+      implementationOnlyImports = []
       testableImports = []
       codeBlocks = []
       fileHeader = []
@@ -115,6 +118,11 @@ public final class OrderedImports: SyntaxFormatRule {
         regularImports.append(line)
         commentBuffer = []
 
+      case .implementationOnlyImport:
+        implementationOnlyImports.append(contentsOf: commentBuffer)
+        implementationOnlyImports.append(line)
+        commentBuffer = []
+
       case .testableImport:
         testableImports.append(contentsOf: commentBuffer)
         testableImports.append(line)
@@ -148,6 +156,7 @@ public final class OrderedImports: SyntaxFormatRule {
   /// statements do not appear at the top of the file.
   private func checkGrouping<C: Collection>(_ lines: C) where C.Element == Line {
     var declGroup = false
+    var implementationOnlyGroup = false
     var testableGroup = false
     var codeGroup = false
 
@@ -157,6 +166,8 @@ public final class OrderedImports: SyntaxFormatRule {
       switch lineType {
       case .declImport:
         declGroup = true
+      case .implementationOnlyImport:
+        implementationOnlyGroup = true
       case .testableImport:
         testableGroup = true
       case .codeBlock:
@@ -166,15 +177,15 @@ public final class OrderedImports: SyntaxFormatRule {
 
       if codeGroup {
         switch lineType {
-        case .regularImport, .declImport, .testableImport:
+        case .regularImport, .declImport, .implementationOnlyImport, .testableImport:
           diagnose(.placeAtTopOfFile, on: line.firstToken)
         default: ()
         }
       }
 
       if testableGroup {
         switch lineType {
-        case .regularImport, .declImport:
+        case .regularImport, .declImport, .implementationOnlyImport:
           diagnose(
             .groupImports(before: lineType, after: LineType.testableImport),
             on: line.firstToken
@@ -183,6 +194,17 @@ public final class OrderedImports: SyntaxFormatRule {
         }
       }
 
+      if implementationOnlyGroup {
+        switch lineType {
+        case .regularImport, .declImport:
+          diagnose(
+            .groupImports(before: lineType, after: LineType.implementationOnlyImport),
+            on: line.firstToken
+          )
+        default: ()
+        }
+      }
+
       if declGroup {
         switch lineType {
         case .regularImport:
@@ -208,7 +230,7 @@ public final class OrderedImports: SyntaxFormatRule {
 
     for line in imports {
       switch line.type {
-      case .regularImport, .declImport, .testableImport:
+      case .regularImport, .declImport, .implementationOnlyImport, .testableImport:
         let fullyQualifiedImport = line.fullyQualifiedImport
         // Check for duplicate imports and potentially remove them.
         if let previousMatchingImportIndex = visitedImports[fullyQualifiedImport] {
@@ -390,6 +412,7 @@ fileprivate func convertToCodeBlockItems(lines: [Line]) -> [CodeBlockItemSyntax]
 public enum LineType: CustomStringConvertible {
   case regularImport
   case declImport
+  case implementationOnlyImport
   case testableImport
   case codeBlock
   case comment
@@ -401,6 +424,8 @@ public enum LineType: CustomStringConvertible {
       return "regular"
     case .declImport:
       return "declaration"
+    case .implementationOnlyImport:
+      return "implementationOnly"
     case .testableImport:
       return "testable"
     case .codeBlock:
@@ -515,12 +540,16 @@ fileprivate class Line {
 
   /// Returns a `LineType` the represents the type of import from the given import decl.
   private func importType(of importDecl: ImportDeclSyntax) -> LineType {
-    if let attr = importDecl.attributes.firstToken(viewMode: .sourceAccurate),
-      attr.tokenKind == .atSign,
-      attr.nextToken(viewMode: .sourceAccurate)?.text == "testable"
-    {
+
+    let importIdentifierTypes = importDecl.attributes.compactMap { $0.as(AttributeSyntax.self)?.attributeName }
+    let importAttributeNames = importIdentifierTypes.compactMap { $0.as(IdentifierTypeSyntax.self)?.name.text }
+
+    if importAttributeNames.contains("testable") {
       return .testableImport
     }
+    if importAttributeNames.contains("_implementationOnly") {
+      return .implementationOnlyImport
+    }
     if importDecl.importKindSpecifier != nil {
       return .declImport
     }
diff --git a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift
@@ -27,14 +27,17 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         import UIKit
 
         @testable import SwiftFormat
-        8️⃣import enum Darwin.D.isatty
+        🔟import enum Darwin.D.isatty
         // Starts Test
         3️⃣@testable import MyModuleUnderTest
         // Starts Ind
-        2️⃣7️⃣import func Darwin.C.isatty
+        2️⃣8️⃣import func Darwin.C.isatty
+
+        // Starts ImplementationOnly
+        9️⃣@_implementationOnly import InternalModule
 
         let a = 3
-        4️⃣5️⃣6️⃣import SwiftSyntax
+        4️⃣5️⃣6️⃣7️⃣import SwiftSyntax
         """,
       expected: """
         // Starts Imports
@@ -48,6 +51,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         import func Darwin.C.isatty
         import enum Darwin.D.isatty
 
+        // Starts ImplementationOnly
+        @_implementationOnly import InternalModule
+
         // Starts Test
         @testable import MyModuleUnderTest
         @testable import SwiftFormat
@@ -60,9 +66,11 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         FindingSpec("3️⃣", message: "sort import statements lexicographically"),
         FindingSpec("4️⃣", message: "place imports at the top of the file"),
         FindingSpec("5️⃣", message: "place regular imports before testable imports"),
-        FindingSpec("6️⃣", message: "place regular imports before declaration imports"),
-        FindingSpec("7️⃣", message: "sort import statements lexicographically"),
-        FindingSpec("8️⃣", message: "place declaration imports before testable imports"),
+        FindingSpec("6️⃣", message: "place regular imports before implementationOnly imports"),
+        FindingSpec("7️⃣", message: "place regular imports before declaration imports"),
+        FindingSpec("8️⃣", message: "sort import statements lexicographically"),
+        FindingSpec("9️⃣", message: "place implementationOnly imports before testable imports"),
+        FindingSpec("🔟", message: "place declaration imports before testable imports"),
       ]
     )
   }
@@ -95,6 +103,51 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
       ]
     )
   }
+    
+    func testImportsWithAttributes() {
+        assertFormatting(
+          OrderedImports.self,
+          input: """
+            import Foundation
+            1️⃣@preconcurrency import AVFoundation
+            
+            @preconcurrency @_implementationOnly import InternalModuleC
+            
+            2️⃣@_implementationOnly import InternalModuleA
+            
+            3️⃣import Core
+            
+            @testable @preconcurrency import TestServiceB
+            4️⃣@preconcurrency @testable import TestServiceA
+            
+            5️⃣@_implementationOnly @preconcurrency import InternalModuleB
+            
+            let a = 3
+            """,
+          expected: """
+            @preconcurrency import AVFoundation
+            import Core
+            import Foundation
+            
+            @_implementationOnly import InternalModuleA
+            @_implementationOnly @preconcurrency import InternalModuleB
+            @preconcurrency @_implementationOnly import InternalModuleC
+
+            @preconcurrency @testable import TestServiceA
+            @testable @preconcurrency import TestServiceB
+            
+            let a = 3
+            """,
+          findings: [
+            FindingSpec("1️⃣", message: "sort import statements lexicographically"),
+            FindingSpec("2️⃣", message: "sort import statements lexicographically"),
+            FindingSpec("3️⃣", message: "place regular imports before implementationOnly imports"),
+            FindingSpec("4️⃣", message: "sort import statements lexicographically"),
+            FindingSpec("5️⃣", message: "place implementationOnly imports before testable imports")
+          ]
+        )
+      }
+
 
   func testImportsOrderWithDocComment() {
     assertFormatting(
@@ -145,6 +198,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         import UIKit
 
         import func Darwin.C.isatty
+        
+        @_implementationOnly import InternalModuleA
+        @preconcurrency @_implementationOnly import InternalModuleB
 
         @testable import MyModuleUnderTest
         """,
@@ -155,6 +211,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         import UIKit
 
         import func Darwin.C.isatty
+        
+        @_implementationOnly import InternalModuleA
+        @preconcurrency @_implementationOnly import InternalModuleB
 
         @testable import MyModuleUnderTest
         """,
@@ -324,7 +383,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         @testable import testZ  // trailing comment about testZ
         3️⃣@testable import testC
         // swift-format-ignore
-        @testable import testB
+        @_implementationOnly import testB
         // Comment about Bar
         import enum Bar
 
@@ -350,7 +409,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         @testable import testZ  // trailing comment about testZ
 
         // swift-format-ignore
-        @testable import testB
+        @_implementationOnly import testB
 
         // Comment about Bar
         import enum Bar
@@ -513,7 +572,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
       input: """
         // exported import of bar
         @_exported import bar
-        @_implementationOnly import bar
+        @preconcurrency import bar
         import bar
         import foo
         // second import of foo
@@ -531,7 +590,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
       expected: """
         // exported import of bar
         @_exported import bar
-        @_implementationOnly import bar
+        @preconcurrency import bar
         import bar
         // second import of foo
         import foo
