From 7c46e82fabe2de0f5f27b01ac4a5a337b34bc291 Mon Sep 17 00:00:00 2001 From: Erich Graham Date: Tue, 6 May 2025 17:01:59 -0400 Subject: [PATCH 1/3] OrderedImports should create separate group for @_implementationOnly imports --- Documentation/RuleDocumentation.md | 6 +- .../SwiftFormat/Rules/OrderedImports.swift | 51 +++++++++--- .../Rules/OrderedImportsTests.swift | 79 ++++++++++++++++--- 3 files changed, 112 insertions(+), 24 deletions(-) diff --git a/Documentation/RuleDocumentation.md b/Documentation/RuleDocumentation.md index a6d2d0bac..510cc0704 100644 --- 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 index 16ace9be9..dfcf45c06 100644 --- 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(_ 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,7 +177,7 @@ public final class OrderedImports: SyntaxFormatRule { if codeGroup { switch lineType { - case .regularImport, .declImport, .testableImport: + case .regularImport, .declImport, .implementationOnlyImport, .testableImport: diagnose(.placeAtTopOfFile, on: line.firstToken) default: () } @@ -174,7 +185,7 @@ public final class OrderedImports: SyntaxFormatRule { 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 index 673dca6d8..41c050f69 100644 --- 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 From 021d31af27e61681dc7825e7fcbd0c9dc5347ba9 Mon Sep 17 00:00:00 2001 From: Erich Graham Date: Tue, 6 May 2025 17:55:44 -0400 Subject: [PATCH 2/3] Code review: format and api-breakages --- .../SwiftFormat/Rules/OrderedImports.swift | 9 +- .../Rules/OrderedImportsTests.swift | 91 +++++++++---------- api-breakages.txt | 1 + 3 files changed, 54 insertions(+), 47 deletions(-) diff --git a/Sources/SwiftFormat/Rules/OrderedImports.swift b/Sources/SwiftFormat/Rules/OrderedImports.swift index dfcf45c06..57afc04d9 100644 --- a/Sources/SwiftFormat/Rules/OrderedImports.swift +++ b/Sources/SwiftFormat/Rules/OrderedImports.swift @@ -57,7 +57,14 @@ public final class OrderedImports: SyntaxFormatRule { testableImports = formatImports(testableImports) formatCodeblocks(&codeBlocks) - let joined = joinLines(fileHeader, regularImports, declImports, implementationOnlyImports, testableImports, codeBlocks) + let joined = joinLines( + fileHeader, + regularImports, + declImports, + implementationOnlyImports, + testableImports, + codeBlocks + ) formattedLines.append(contentsOf: joined) regularImports = [] diff --git a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift index 41c050f69..9a3c51d66 100644 --- a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift +++ b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift @@ -103,51 +103,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 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( @@ -198,7 +197,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { import UIKit import func Darwin.C.isatty - + @_implementationOnly import InternalModuleA @preconcurrency @_implementationOnly import InternalModuleB @@ -211,7 +210,7 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { import UIKit import func Darwin.C.isatty - + @_implementationOnly import InternalModuleA @preconcurrency @_implementationOnly import InternalModuleB diff --git a/api-breakages.txt b/api-breakages.txt index 519c9a091..8b1fa77e2 100644 --- a/api-breakages.txt +++ b/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: enum LineType.implementationOnlyImport has been added as a new enum case From 73ec5a1bb6798c6c8d4ef688a392a6cd8a308853 Mon Sep 17 00:00:00 2001 From: Erich Graham Date: Wed, 7 May 2025 07:46:55 -0400 Subject: [PATCH 3/3] Fix API breakage message --- api-breakages.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api-breakages.txt b/api-breakages.txt index 8b1fa77e2..d9d73bc24 100644 --- a/api-breakages.txt +++ b/api-breakages.txt @@ -18,4 +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: enum LineType.implementationOnlyImport has been added as a new enum case +API breakage: enumelement LineType.implementationOnlyImport has been added as a new enum case