From ae522f25489d3939d13b1b1e59c009af4d92d107 Mon Sep 17 00:00:00 2001 From: ebgraham Date: Wed, 7 May 2025 08:10:30 -0400 Subject: [PATCH 1/8] =?UTF-8?q?OrderedImports=20should=20create=20separate?= =?UTF-8?q?=20group=20for=20@=5FimplementationOnly=20=E2=80=A6=20(#1013)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * OrderedImports should create separate group for @_implementationOnly imports --- Documentation/RuleDocumentation.md | 6 +- .../SwiftFormat/Rules/OrderedImports.swift | 58 +++++++++++--- .../Rules/OrderedImportsTests.swift | 78 ++++++++++++++++--- api-breakages.txt | 1 + 4 files changed, 119 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..57afc04d9 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,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(_ 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,7 +184,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 +192,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 +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 } diff --git a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift index 673dca6d8..9a3c51d66 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"), ] ) } @@ -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 diff --git a/api-breakages.txt b/api-breakages.txt index 519c9a091..d9d73bc24 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: enumelement LineType.implementationOnlyImport has been added as a new enum case From e70ab31798706840377146e01b74743af72be426 Mon Sep 17 00:00:00 2001 From: Vladimir Gusev <53124465+antigluten@users.noreply.github.com> Date: Fri, 9 May 2025 20:47:20 +0300 Subject: [PATCH 2/8] Allow comments in `.swift-format` (#1014) * Allow comments in `.swift-format` --- Sources/SwiftFormat/API/Configuration.swift | 6 +++- .../API/ConfigurationTests.swift | 35 +++++++++++++++++-- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftFormat/API/Configuration.swift b/Sources/SwiftFormat/API/Configuration.swift index cb5dfb025..98a1b9b70 100644 --- a/Sources/SwiftFormat/API/Configuration.swift +++ b/Sources/SwiftFormat/API/Configuration.swift @@ -292,7 +292,11 @@ public struct Configuration: Codable, Equatable { /// Creates a new `Configuration` by decoding it from the UTF-8 representation in the given data. public init(data: Data) throws { - self = try JSONDecoder().decode(Configuration.self, from: data) + let jsonDecoder = JSONDecoder() + #if canImport(Darwin) || compiler(>=6) + jsonDecoder.allowsJSON5 = true + #endif + self = try jsonDecoder.decode(Configuration.self, from: data) } public init(from decoder: Decoder) throws { diff --git a/Tests/SwiftFormatTests/API/ConfigurationTests.swift b/Tests/SwiftFormatTests/API/ConfigurationTests.swift index 9c6977db8..d983e9cb9 100644 --- a/Tests/SwiftFormatTests/API/ConfigurationTests.swift +++ b/Tests/SwiftFormatTests/API/ConfigurationTests.swift @@ -23,6 +23,9 @@ final class ConfigurationTests: XCTestCase { let emptyDictionaryData = "{}\n".data(using: .utf8)! let jsonDecoder = JSONDecoder() + #if canImport(Darwin) || compiler(>=6) + jsonDecoder.allowsJSON5 = true + #endif let emptyJSONConfig = try! jsonDecoder.decode(Configuration.self, from: emptyDictionaryData) @@ -79,7 +82,11 @@ final class ConfigurationTests: XCTestCase { } """.data(using: .utf8)! - let config = try JSONDecoder().decode(Configuration.self, from: jsonData) + let jsonDecoder = JSONDecoder() + #if canImport(Darwin) || compiler(>=6) + jsonDecoder.allowsJSON5 = true + #endif + let config = try jsonDecoder.decode(Configuration.self, from: jsonData) XCTAssertEqual(config.reflowMultilineStringLiterals, expectedBehavior) } } @@ -99,9 +106,33 @@ final class ConfigurationTests: XCTestCase { } """.data(using: .utf8)! - let config = try JSONDecoder().decode(Configuration.self, from: jsonData) + let jsonDecoder = JSONDecoder() + #if canImport(Darwin) || compiler(>=6) + jsonDecoder.allowsJSON5 = true + #endif + let config = try jsonDecoder.decode(Configuration.self, from: jsonData) XCTAssertEqual(config.reflowMultilineStringLiterals, expectedBehavior) } } + func testConfigurationWithComments() throws { + #if !canImport(Darwin) && compiler(<6) + try XCTSkipIf(true, "JSONDecoder does not support JSON5") + #else + let expected = Configuration() + + let jsonData = """ + { + // Indicates the configuration schema version. + "version": 1, + } + """.data(using: .utf8)! + + let jsonDecoder = JSONDecoder() + + jsonDecoder.allowsJSON5 = true + let config = try jsonDecoder.decode(Configuration.self, from: jsonData) + XCTAssertEqual(config, expected) + #endif + } } From 1618de90e6125cddc8cb8676bd2acd74c5a7b53d Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Fri, 9 May 2025 12:43:53 -0700 Subject: [PATCH 3/8] Update the allowed user for publishing releases to bnbarham --- .github/workflows/publish_release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/publish_release.yml b/.github/workflows/publish_release.yml index 0a6d51ae0..6995e8ce1 100644 --- a/.github/workflows/publish_release.yml +++ b/.github/workflows/publish_release.yml @@ -30,7 +30,7 @@ jobs: runs-on: ubuntu-latest steps: - run: | - if [[ "${{ github.triggering_actor }}" != "ahoppen" ]]; then + if [[ "${{ github.triggering_actor }}" != "bnbarham" ]]; then echo "${{ github.triggering_actor }} is not allowed to create a release" exit 1 fi From 0d998b303457a8de8b6598ce8db4c48f60fa3cfb Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Mon, 12 May 2025 13:43:29 -0700 Subject: [PATCH 4/8] [CI] Remove skip for ready for review This unfortunately ends up causing CI checks to reset and always skipped after marking a PR ready for review. --- .github/workflows/pull_request.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index f1d7840ca..777a2d19d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -1,5 +1,8 @@ name: Pull request +# PRs created by GitHub Actions don't kick off further actions (https://github.com/peter-evans/create-pull-request/blob/d57e551ebc1a16dee0b8c9ea6d24dba7627a6e35/docs/concepts-guidelines.md#triggering-further-workflow-runs). +# As a workaround, we mark automerge PRs that are created by GitHub actions as draft and trigger the GitHub actions by marking the PR as ready for review. We'd prefer not re-triggering testing on a normal user's PR in this case, but skipping them causes the checks to reset. + on: pull_request: types: [opened, reopened, synchronize, ready_for_review] @@ -12,17 +15,11 @@ jobs: tests: name: Test uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main - # PRs created by GitHub Actions don't kick off further actions (https://github.com/peter-evans/create-pull-request/blob/d57e551ebc1a16dee0b8c9ea6d24dba7627a6e35/docs/concepts-guidelines.md#triggering-further-workflow-runs). - # As a workaround, we mark automerge PRs that are created by GitHub actions as draft and trigger the GitHub actions by marking the PR as ready for review. But we don't want to re-trigger testing this when a normal user's PR is marked as ready for review. - if: (github.event.action != 'ready_for_review') || (github.event.action == 'ready_for_review' && github.event.pull_request.user.login == 'github-actions[bot]') with: linux_exclude_swift_versions: "[{\"swift_version\": \"5.8\"}]" soundness: name: Soundness uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main - # PRs created by GitHub Actions don't kick off further actions (https://github.com/peter-evans/create-pull-request/blob/d57e551ebc1a16dee0b8c9ea6d24dba7627a6e35/docs/concepts-guidelines.md#triggering-further-workflow-runs). - # As a workaround, we mark automerge PRs that are created by GitHub actions as draft and trigger the GitHub actions by marking the PR as ready for review. But we don't want to re-trigger testing this when a normal user's PR is marked as ready for review. - if: (github.event.action != 'ready_for_review') || (github.event.action == 'ready_for_review' && github.event.pull_request.user.login == 'github-actions[bot]') with: license_header_check_project_name: "Swift.org" api_breakage_check_allowlist_path: "api-breakages.txt" From 512103ca09568be2400dc83af6ec5c078ba3bcf3 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Thu, 15 May 2025 11:18:41 +0200 Subject: [PATCH 5/8] Remove Alex Hoppen as code owner --- CODEOWNERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index 28d802dc0..a8142cf6a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -8,7 +8,7 @@ # See https://swift.org/CONTRIBUTORS.txt for Swift project authors # -* @ahoppen @allevato @bnbarham @shawnhyam +* @allevato @bnbarham @shawnhyam -.github/ @ahoppen @bnbarham @shahmishal -.swiftci/ @ahoppen @bnbarham @shahmishal +.github/ @bnbarham @shahmishal +.swiftci/ @bnbarham @shahmishal From adcbed1267abe757624a05bf5d3d498f2d746233 Mon Sep 17 00:00:00 2001 From: Kenta Kubo <601636+kkebo@users.noreply.github.com> Date: Fri, 16 May 2025 02:16:26 +0900 Subject: [PATCH 6/8] Fix `unsafe` expressions in `NoAssignmentInExpressions` (#1021) fixes #977 --- Sources/SwiftFormat/Rules/NoAssignmentInExpressions.swift | 6 +++--- .../Rules/NoAssignmentInExpressionsTests.swift | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftFormat/Rules/NoAssignmentInExpressions.swift b/Sources/SwiftFormat/Rules/NoAssignmentInExpressions.swift index bc10f3a31..194ed43cb 100644 --- a/Sources/SwiftFormat/Rules/NoAssignmentInExpressions.swift +++ b/Sources/SwiftFormat/Rules/NoAssignmentInExpressions.swift @@ -113,15 +113,15 @@ public final class NoAssignmentInExpressions: SyntaxFormatRule { /// Returns a value indicating whether the given node is a standalone assignment statement. /// - /// This function considers try/await expressions and automatically walks up through them as - /// needed. This is because `try f().x = y` should still be a standalone assignment for our + /// This function considers try/await/unsafe expressions and automatically walks up through them + /// as needed. This is because `try f().x = y` should still be a standalone assignment for our /// purposes, even though a `TryExpr` will wrap the `InfixOperatorExpr` and thus would not be /// considered a standalone assignment if we only checked the infix expression for a /// `CodeBlockItem` parent. private func isStandaloneAssignmentStatement(_ node: InfixOperatorExprSyntax) -> Bool { var node = Syntax(node) while let parent = node.parent, - parent.is(TryExprSyntax.self) || parent.is(AwaitExprSyntax.self) + parent.is(TryExprSyntax.self) || parent.is(AwaitExprSyntax.self) || parent.is(UnsafeExprSyntax.self) { node = parent } diff --git a/Tests/SwiftFormatTests/Rules/NoAssignmentInExpressionsTests.swift b/Tests/SwiftFormatTests/Rules/NoAssignmentInExpressionsTests.swift index 928296c59..7a9093d2c 100644 --- a/Tests/SwiftFormatTests/Rules/NoAssignmentInExpressionsTests.swift +++ b/Tests/SwiftFormatTests/Rules/NoAssignmentInExpressionsTests.swift @@ -187,19 +187,21 @@ final class NoAssignmentInExpressionsTests: LintOrFormatRuleTestCase { ) } - func testTryAndAwaitAssignmentExpressionsAreUnchanged() { + func testTryAndAwaitAndUnsafeAssignmentExpressionsAreUnchanged() { assertFormatting( NoAssignmentInExpressions.self, input: """ func foo() { try a.b = c await a.b = c + unsafe a.b = c } """, expected: """ func foo() { try a.b = c await a.b = c + unsafe a.b = c } """, findings: [] From c2d7ede28b415fca14c70d0d4f16c0aacc86bf58 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Mon, 19 May 2025 15:34:42 -0700 Subject: [PATCH 7/8] =?UTF-8?q?Revert=20"OrderedImports=20should=20create?= =?UTF-8?q?=20separate=20group=20for=20@=5FimplementationOnly=20=E2=80=A6?= =?UTF-8?q?=20(#1013)"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit ae522f25489d3939d13b1b1e59c009af4d92d107. Adding separate groups can impact a lot of code, let's skip for 6.2 for now. --- Documentation/RuleDocumentation.md | 6 +- .../SwiftFormat/Rules/OrderedImports.swift | 58 +++----------- .../Rules/OrderedImportsTests.swift | 78 +++---------------- api-breakages.txt | 1 - 4 files changed, 24 insertions(+), 119 deletions(-) diff --git a/Documentation/RuleDocumentation.md b/Documentation/RuleDocumentation.md index 510cc0704..a6d2d0bac 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, 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 diff --git a/Sources/SwiftFormat/Rules/OrderedImports.swift b/Sources/SwiftFormat/Rules/OrderedImports.swift index 57afc04d9..16ace9be9 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, 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(_ 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 } diff --git a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift index 9a3c51d66..673dca6d8 100644 --- a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift +++ b/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 diff --git a/api-breakages.txt b/api-breakages.txt index d9d73bc24..519c9a091 100644 --- a/api-breakages.txt +++ b/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 From 009925c463e73aecb326bb47718a4727dc130a61 Mon Sep 17 00:00:00 2001 From: Ben Barham Date: Mon, 19 May 2025 15:35:31 -0700 Subject: [PATCH 8/8] Revert "Merge pull request #1020 from ahoppen/remove-code-owner" This reverts commit 42c98264a10cdfd10bdd70b753fb0e218cd22e88, reversing changes made to 7156f95b3bc52cc8be68be9da514bcd8ecc07b67. Codeowner is branch managers on 6.2. --- CODEOWNERS | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CODEOWNERS b/CODEOWNERS index a8142cf6a..28d802dc0 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -8,7 +8,7 @@ # See https://swift.org/CONTRIBUTORS.txt for Swift project authors # -* @allevato @bnbarham @shawnhyam +* @ahoppen @allevato @bnbarham @shawnhyam -.github/ @bnbarham @shahmishal -.swiftci/ @bnbarham @shahmishal +.github/ @ahoppen @bnbarham @shahmishal +.swiftci/ @ahoppen @bnbarham @shahmishal