OrderedImports should create separate group for @_implementationOnly … (#1013)

* OrderedImports should create separate group for @_implementationOnly imports

Author: ebgraham

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

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,23 @@ 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 +125,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 +163,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 +173,8 @@ public final class OrderedImports: SyntaxFormatRule {
       switch lineType {
       case .declImport:
         declGroup = true
+      case .implementationOnlyImport:
+        implementationOnlyGroup = true
       case .testableImport:
         testableGroup = true
       case .codeBlock:
@@ -166,15 +184,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 +201,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 +237,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 +419,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 +431,8 @@ public enum LineType: CustomStringConvertible {
       return "regular"
     case .declImport:
       return "declaration"
+    case .implementationOnlyImport:
+      return "implementationOnly"
     case .testableImport:
       return "testable"
     case .codeBlock:
@@ -515,12 +547,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
     }

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"),
       ]
     )
   }
@@ -96,6 +104,50 @@ 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(
       OrderedImports.self,
@@ -146,6 +198,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
 
         import func Darwin.C.isatty
 
+        @_implementationOnly import InternalModuleA
+        @preconcurrency @_implementationOnly import InternalModuleB
+
         @testable import MyModuleUnderTest
         """,
       expected: """
@@ -156,6 +211,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
 
         import func Darwin.C.isatty
 
+        @_implementationOnly import InternalModuleA
+        @preconcurrency @_implementationOnly import InternalModuleB
+
         @testable import MyModuleUnderTest
         """,
       findings: []
@@ -324,7 +382,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 +408,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 +571,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 +589,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

api-breakages.txt

@@ -18,3 +18,4 @@ API breakage: func Configuration.MultilineStringReflowBehavior.hash(into:) has b
 API breakage: func Configuration.MultilineStringReflowBehavior.encode(to:) has been removed
 API breakage: var Configuration.MultilineStringReflowBehavior.hashValue has been removed
 API breakage: constructor Configuration.MultilineStringReflowBehavior.init(from:) has been removed
+API breakage: enumelement LineType.implementationOnlyImport has been added as a new enum case