Skip to content

OrderedImports should create separate group for @_implementationOnly … #1013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Documentation/RuleDocumentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
58 changes: 47 additions & 11 deletions Sources/SwiftFormat/Rules/OrderedImports.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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] = []
Expand All @@ -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 = []
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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:
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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] {
Expand Down Expand Up @@ -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
Expand All @@ -401,6 +431,8 @@ public enum LineType: CustomStringConvertible {
return "regular"
case .declImport:
return "declaration"
case .implementationOnlyImport:
return "implementationOnly"
case .testableImport:
return "testable"
case .codeBlock:
Expand Down Expand Up @@ -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
}
Expand Down
78 changes: 68 additions & 10 deletions Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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"),
]
)
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -146,6 +198,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
import func Darwin.C.isatty
@_implementationOnly import InternalModuleA
@preconcurrency @_implementationOnly import InternalModuleB
@testable import MyModuleUnderTest
""",
expected: """
Expand All @@ -156,6 +211,9 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase {
import func Darwin.C.isatty
@_implementationOnly import InternalModuleA
@preconcurrency @_implementationOnly import InternalModuleB
@testable import MyModuleUnderTest
""",
findings: []
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions api-breakages.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading