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

This reverts commit ae522f2. Adding
separate groups can impact a lot of code, let's skip for 6.2 for now.

Author: bnbarham

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, 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.
+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.
 
 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, 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.
+/// 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.
 ///
 /// 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,7 +34,6 @@ public final class OrderedImports: SyntaxFormatRule {
 
     var regularImports: [Line] = []
     var declImports: [Line] = []
-    var implementationOnlyImports: [Line] = []
     var testableImports: [Line] = []
     var codeBlocks: [Line] = []
     var fileHeader: [Line] = []
@@ -53,23 +52,14 @@ 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,
-        implementationOnlyImports,
-        testableImports,
-        codeBlocks
-      )
+      let joined = joinLines(fileHeader, regularImports, declImports, testableImports, codeBlocks)
       formattedLines.append(contentsOf: joined)
 
       regularImports = []
       declImports = []
-      implementationOnlyImports = []
       testableImports = []
       codeBlocks = []
       fileHeader = []
@@ -125,11 +115,6 @@ 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)
@@ -163,7 +148,6 @@ 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
 
@@ -173,8 +157,6 @@ public final class OrderedImports: SyntaxFormatRule {
       switch lineType {
       case .declImport:
         declGroup = true
-      case .implementationOnlyImport:
-        implementationOnlyGroup = true
       case .testableImport:
         testableGroup = true
       case .codeBlock:
@@ -184,28 +166,17 @@ public final class OrderedImports: SyntaxFormatRule {
 
       if codeGroup {
         switch lineType {
-        case .regularImport, .declImport, .implementationOnlyImport, .testableImport:
+        case .regularImport, .declImport, .testableImport:
           diagnose(.placeAtTopOfFile, on: line.firstToken)
         default: ()
         }
       }
 
       if testableGroup {
-        switch lineType {
-        case .regularImport, .declImport, .implementationOnlyImport:
-          diagnose(
-            .groupImports(before: lineType, after: LineType.testableImport),
-            on: line.firstToken
-          )
-        default: ()
-        }
-      }
-
-      if implementationOnlyGroup {
         switch lineType {
         case .regularImport, .declImport:
           diagnose(
-            .groupImports(before: lineType, after: LineType.implementationOnlyImport),
+            .groupImports(before: lineType, after: LineType.testableImport),
             on: line.firstToken
           )
         default: ()
@@ -237,7 +208,7 @@ public final class OrderedImports: SyntaxFormatRule {
 
     for line in imports {
       switch line.type {
-      case .regularImport, .declImport, .implementationOnlyImport, .testableImport:
+      case .regularImport, .declImport, .testableImport:
         let fullyQualifiedImport = line.fullyQualifiedImport
         // Check for duplicate imports and potentially remove them.
         if let previousMatchingImportIndex = visitedImports[fullyQualifiedImport] {
@@ -419,7 +390,6 @@ fileprivate func convertToCodeBlockItems(lines: [Line]) -> [CodeBlockItemSyntax]
 public enum LineType: CustomStringConvertible {
   case regularImport
   case declImport
-  case implementationOnlyImport
   case testableImport
   case codeBlock
   case comment
@@ -431,8 +401,6 @@ public enum LineType: CustomStringConvertible {
       return "regular"
     case .declImport:
       return "declaration"
-    case .implementationOnlyImport:
-      return "implementationOnly"
     case .testableImport:
       return "testable"
     case .codeBlock:
@@ -547,16 +515,12 @@ fileprivate class Line {
 
   /// Returns a `LineType` the represents the type of import from the given import decl.
   private func importType(of importDecl: ImportDeclSyntax) -> LineType {
-
-    let importIdentifierTypes = importDecl.attributes.compactMap { $0.as(AttributeSyntax.self)?.attributeName }
-    let importAttributeNames = importIdentifierTypes.compactMap { $0.as(IdentifierTypeSyntax.self)?.name.text }
-
-    if importAttributeNames.contains("testable") {
+    if let attr = importDecl.attributes.firstToken(viewMode: .sourceAccurate),
+      attr.tokenKind == .atSign,
+      attr.nextToken(viewMode: .sourceAccurate)?.text == "testable"
+    {
       return .testableImport
     }
-    if importAttributeNames.contains("_implementationOnly") {
-      return .implementationOnlyImport
-    }
     if importDecl.importKindSpecifier != nil {
       return .declImport
     }

Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift

@@ -27,17 +27,14 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         import UIKit
 
         @testable import SwiftFormat
-        🔟import enum Darwin.D.isatty
+        8️⃣import enum Darwin.D.isatty
         // Starts Test
         3️⃣@testable import MyModuleUnderTest
         // Starts Ind
-        2️⃣8️⃣import func Darwin.C.isatty
-
-        // Starts ImplementationOnly
-        9️⃣@_implementationOnly import InternalModule
+        2️⃣7️⃣import func Darwin.C.isatty
 
         let a = 3
-        4️⃣5️⃣6️⃣7️⃣import SwiftSyntax
+        4️⃣5️⃣6️⃣import SwiftSyntax
         """,
       expected: """
         // Starts Imports
@@ -51,9 +48,6 @@ 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
@@ -66,11 +60,9 @@ 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 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"),
+        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"),
       ]
     )
   }
@@ -104,50 +96,6 @@ 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,
@@ -198,9 +146,6 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
 
         import func Darwin.C.isatty
 
-        @_implementationOnly import InternalModuleA
-        @preconcurrency @_implementationOnly import InternalModuleB
-
         @testable import MyModuleUnderTest
         """,
       expected: """
@@ -211,9 +156,6 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
 
         import func Darwin.C.isatty
 
-        @_implementationOnly import InternalModuleA
-        @preconcurrency @_implementationOnly import InternalModuleB
-
         @testable import MyModuleUnderTest
         """,
       findings: []
@@ -382,7 +324,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         @testable import testZ  // trailing comment about testZ
         3️⃣@testable import testC
         // swift-format-ignore
-        @_implementationOnly import testB
+        @testable import testB
         // Comment about Bar
         import enum Bar
 
@@ -408,7 +350,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
         @testable import testZ  // trailing comment about testZ
 
         // swift-format-ignore
-        @_implementationOnly import testB
+        @testable import testB
 
         // Comment about Bar
         import enum Bar
@@ -571,7 +513,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
       input: """
         // exported import of bar
         @_exported import bar
-        @preconcurrency import bar
+        @_implementationOnly import bar
         import bar
         import foo
         // second import of foo
@@ -589,7 +531,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
       expected: """
         // exported import of bar
         @_exported import bar
-        @preconcurrency import bar
+        @_implementationOnly import bar
         import bar
         // second import of foo
         import foo

api-breakages.txt

@@ -18,4 +18,3 @@ 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