Skip to content

Commit a978bf3

Browse files
ahoppenahoppen
authored
Merge pull request #893 from ahoppen/merge-main-6.1-2024-12-10
Merge `main` into `release/6.1`
2 parents 6e4c968 + 2abf666 commit a978bf3

File tree

9 files changed

+226
-38
lines changed

9 files changed

+226
-38
lines changed

‎.github/workflows/pull_request.yml

+3
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,12 @@ jobs:
88
tests:
99
name: Test
1010
uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main
11+
with:
12+
enable_windows_docker: false # Dockerless Windows is 5-10 minutes faster than Docker on Windows
1113
soundness:
1214
name: Soundness
1315
uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main
1416
with:
1517
license_header_check_enabled: false
1618
license_header_check_project_name: "Swift.org"
19+
api_breakage_check_allowlist_path: "api-breakages.txt"

‎Sources/SwiftFormat/API/Configuration.swift

-14
Original file line numberDiff line numberDiff line change
@@ -486,17 +486,3 @@ public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable {
486486

487487
public init() {}
488488
}
489-
490-
fileprivate extension URL {
491-
var isRoot: Bool {
492-
#if os(Windows)
493-
// FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed.
494-
// https://github.com/swiftlang/swift-format/issues/844
495-
return self.pathComponents.count <= 1
496-
#else
497-
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
498-
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
499-
return self.path == "/" || self.path == ""
500-
#endif
501-
}
502-
}

‎Sources/SwiftFormat/CMakeLists.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ add_library(SwiftFormat
9797
Rules/UseTripleSlashForDocumentationComments.swift
9898
Rules/UseWhereClausesInForLoops.swift
9999
Rules/ValidateDocumentationComments.swift
100-
Utilities/FileIterator.swift)
100+
Utilities/FileIterator.swift
101+
Utilities/URL+isRoot.swift)
101102
target_link_libraries(SwiftFormat PUBLIC
102103
SwiftMarkdown::Markdown
103104
SwiftSyntax::SwiftSyntax

‎Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift

+14-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,20 @@ struct PrettyPrintBuffer {
118118
writeRaw(text)
119119
consecutiveNewlineCount = 0
120120
pendingSpaces = 0
121-
column += text.count
121+
122+
// In case of comments, we may get a multi-line string.
123+
// To account for that case, we need to correct the lineNumber count.
124+
// The new column is only the position within the last line.
125+
let lines = text.split(separator: "\n")
126+
lineNumber += lines.count - 1
127+
if lines.count > 1 {
128+
// in case we have inserted new lines, we need to reset the column
129+
column = lines.last?.count ?? 0
130+
} else {
131+
// in case it is an end of line comment or a single line comment,
132+
// we just add to the current column
133+
column += lines.last?.count ?? 0
134+
}
122135
}
123136

124137
/// Request that the given number of spaces be printed out before the next text token.

‎Sources/SwiftFormat/Utilities/FileIterator.swift

+19-9
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212

1313
import Foundation
1414

15+
#if os(Windows)
16+
import WinSDK
17+
#endif
18+
1519
/// Iterator for looping over lists of files and directories. Directories are automatically
1620
/// traversed recursively, and we check for files with a ".swift" extension.
1721
@_spi(Internal)
@@ -32,7 +36,7 @@ public struct FileIterator: Sequence, IteratorProtocol {
3236

3337
/// The current working directory of the process, which is used to relativize URLs of files found
3438
/// during iteration.
35-
private let workingDirectory = URL(fileURLWithPath: ".")
39+
private let workingDirectory: URL
3640

3741
/// Keep track of the current directory we're recursing through.
3842
private var currentDirectory = URL(fileURLWithPath: "")
@@ -46,8 +50,13 @@ public struct FileIterator: Sequence, IteratorProtocol {
4650
/// Create a new file iterator over the given list of file URLs.
4751
///
4852
/// The given URLs may be files or directories. If they are directories, the iterator will recurse
49-
/// into them.
50-
public init(urls: [URL], followSymlinks: Bool) {
53+
/// into them. Symlinks are never followed on Windows platforms as Foundation doesn't support it.
54+
/// - Parameters:
55+
/// - urls: `Array` of files or directories to iterate.
56+
/// - followSymlinks: `Bool` to indicate if symbolic links should be followed when iterating.
57+
/// - workingDirectory: `URL` that indicates the current working directory. Used for testing.
58+
public init(urls: [URL], followSymlinks: Bool, workingDirectory: URL = URL(fileURLWithPath: ".")) {
59+
self.workingDirectory = workingDirectory
5160
self.urls = urls
5261
self.urlIterator = self.urls.makeIterator()
5362
self.followSymlinks = followSymlinks
@@ -144,12 +153,13 @@ public struct FileIterator: Sequence, IteratorProtocol {
144153
// if the user passes paths that are relative to the current working directory, they will
145154
// be displayed as relative paths. Otherwise, they will still be displayed as absolute
146155
// paths.
147-
let relativePath =
148-
path.hasPrefix(workingDirectory.path)
149-
? String(path.dropFirst(workingDirectory.path.count + 1))
150-
: path
151-
output =
152-
URL(fileURLWithPath: relativePath, isDirectory: false, relativeTo: workingDirectory)
156+
let relativePath: String
157+
if !workingDirectory.isRoot, path.hasPrefix(workingDirectory.path) {
158+
relativePath = String(path.dropFirst(workingDirectory.path.count).drop(while: { $0 == "/" || $0 == #"\"# }))
159+
} else {
160+
relativePath = path
161+
}
162+
output = URL(fileURLWithPath: relativePath, isDirectory: false, relativeTo: workingDirectory)
153163

154164
default:
155165
break

‎Sources/SwiftFormat/Utilities/URL+isRoot.swift

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import Foundation
14+
15+
extension URL {
16+
@_spi(Testing) public var isRoot: Bool {
17+
#if os(Windows)
18+
// FIXME: We should call into Windows' native check to check if this path is a root once https://github.com/swiftlang/swift-foundation/issues/976 is fixed.
19+
// https://github.com/swiftlang/swift-format/issues/844
20+
var pathComponents = self.pathComponents
21+
if pathComponents.first == "/" {
22+
// Canonicalize `/C:/` to `C:/`.
23+
pathComponents = Array(pathComponents.dropFirst())
24+
}
25+
return pathComponents.count <= 1
26+
#else
27+
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
28+
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
29+
return self.path == "/" || self.path == ""
30+
#endif
31+
}
32+
}

‎Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift

+84
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
import SwiftFormat
2+
import _SwiftFormatTestSupport
3+
4+
final class LineNumbersTests: PrettyPrintTestCase {
5+
func testLineNumbers() {
6+
let input =
7+
"""
8+
final class A {
9+
@Test func b() throws {
10+
doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long
11+
}
12+
}
13+
"""
14+
15+
let expected =
16+
"""
17+
final class A {
18+
@Test func b() throws {
19+
doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long
20+
}
21+
}
22+
23+
"""
24+
25+
assertPrettyPrintEqual(
26+
input: input,
27+
expected: expected,
28+
linelength: 120,
29+
whitespaceOnly: true,
30+
findings: [
31+
FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length")
32+
]
33+
)
34+
}
35+
36+
func testLineNumbersWithComments() {
37+
let input =
38+
"""
39+
// Copyright (C) 2024 My Coorp. All rights reserved.
40+
//
41+
// This document is the property of My Coorp.
42+
// It is considered confidential and proprietary.
43+
//
44+
// This document may not be reproduced or transmitted in any form,
45+
// in whole or in part, without the express written permission of
46+
// My Coorp.
47+
48+
final class A {
49+
@Test func b() throws {
50+
doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long
51+
}
52+
}
53+
"""
54+
55+
let expected =
56+
"""
57+
// Copyright (C) 2024 My Coorp. All rights reserved.
58+
//
59+
// This document is the property of My Coorp.
60+
// It is considered confidential and proprietary.
61+
//
62+
// This document may not be reproduced or transmitted in any form,
63+
// in whole or in part, without the express written permission of
64+
// My Coorp.
65+
66+
final class A {
67+
@Test func b() throws {
68+
doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long
69+
}
70+
}
71+
72+
"""
73+
74+
assertPrettyPrintEqual(
75+
input: input,
76+
expected: expected,
77+
linelength: 120,
78+
whitespaceOnly: true,
79+
findings: [
80+
FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length")
81+
]
82+
)
83+
}
84+
}

‎Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift

+68-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,31 @@
1-
@_spi(Internal) import SwiftFormat
1+
@_spi(Internal) @_spi(Testing) import SwiftFormat
22
import XCTest
33

4+
extension URL {
5+
/// Assuming this is a file URL, resolves all symlinks in the path.
6+
///
7+
/// - Note: We need this because `URL.resolvingSymlinksInPath()` not only resolves symlinks but also standardizes the
8+
/// path by stripping away `private` prefixes. Since sourcekitd is not performing this standardization, using
9+
/// `resolvingSymlinksInPath` can lead to slightly mismatched URLs between the sourcekit-lsp response and the test
10+
/// assertion.
11+
fileprivate var realpath: URL {
12+
#if canImport(Darwin)
13+
return self.path.withCString { path in
14+
guard let realpath = Darwin.realpath(path, nil) else {
15+
return self
16+
}
17+
let result = URL(fileURLWithPath: String(cString: realpath))
18+
free(realpath)
19+
return result
20+
}
21+
#else
22+
// Non-Darwin platforms don't have the `/private` stripping issue, so we can just use `self.resolvingSymlinksInPath`
23+
// here.
24+
return self.resolvingSymlinksInPath()
25+
#endif
26+
}
27+
}
28+
429
final class FileIteratorTests: XCTestCase {
530
private var tmpdir: URL!
631

@@ -10,7 +35,7 @@ final class FileIteratorTests: XCTestCase {
1035
in: .userDomainMask,
1136
appropriateFor: FileManager.default.temporaryDirectory,
1237
create: true
13-
)
38+
).realpath
1439

1540
// Create a simple file tree used by the tests below.
1641
try touch("project/real1.swift")
@@ -31,8 +56,8 @@ final class FileIteratorTests: XCTestCase {
3156
#endif
3257
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false)
3358
XCTAssertEqual(seen.count, 2)
34-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real1.swift") })
35-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real2.swift") })
59+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real1.swift") })
60+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real2.swift") })
3661
}
3762

3863
func testFollowSymlinks() throws {
@@ -41,10 +66,10 @@ final class FileIteratorTests: XCTestCase {
4166
#endif
4267
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: true)
4368
XCTAssertEqual(seen.count, 3)
44-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real1.swift") })
45-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real2.swift") })
69+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real1.swift") })
70+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real2.swift") })
4671
// Hidden but found through the visible symlink project/link.swift
47-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
72+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
4873
}
4974

5075
func testTraversesHiddenFilesIfExplicitlySpecified() throws {
@@ -56,8 +81,8 @@ final class FileIteratorTests: XCTestCase {
5681
followSymlinks: false
5782
)
5883
XCTAssertEqual(seen.count, 2)
59-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.build/generated.swift") })
60-
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
84+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.build/generated.swift") })
85+
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
6186
}
6287

6388
func testDoesNotFollowSymlinksIfFollowSymlinksIsFalseEvenIfExplicitlySpecified() {
@@ -71,6 +96,32 @@ final class FileIteratorTests: XCTestCase {
7196
)
7297
XCTAssertTrue(seen.isEmpty)
7398
}
99+
100+
func testDoesNotTrimFirstCharacterOfPathIfRunningInRoot() throws {
101+
// Find the root of tmpdir. On Unix systems, this is always `/`. On Windows it is the drive.
102+
var root = tmpdir!
103+
while !root.isRoot {
104+
root.deleteLastPathComponent()
105+
}
106+
var rootPath = root.path
107+
#if os(Windows) && compiler(<6.1)
108+
if rootPath.hasPrefix("/") {
109+
// Canonicalize /C: to C:
110+
rootPath = String(rootPath.dropFirst())
111+
}
112+
#endif
113+
// Make sure that we don't drop the beginning of the path if we are running in root.
114+
// https://github.com/swiftlang/swift-format/issues/862
115+
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: root).map(\.relativePath)
116+
XCTAssertTrue(seen.allSatisfy { $0.hasPrefix(rootPath) }, "\(seen) does not contain root directory '\(rootPath)'")
117+
}
118+
119+
func testShowsRelativePaths() throws {
120+
// Make sure that we still show the relative path if using them.
121+
// https://github.com/swiftlang/swift-format/issues/862
122+
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: tmpdir)
123+
XCTAssertEqual(Set(seen.map(\.relativePath)), ["project/real1.swift", "project/real2.swift"])
124+
}
74125
}
75126

76127
extension FileIteratorTests {
@@ -111,11 +162,15 @@ extension FileIteratorTests {
111162
}
112163

113164
/// Computes the list of all files seen by using `FileIterator` to iterate over the given URLs.
114-
private func allFilesSeen(iteratingOver urls: [URL], followSymlinks: Bool) -> [String] {
115-
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks)
116-
var seen: [String] = []
165+
private func allFilesSeen(
166+
iteratingOver urls: [URL],
167+
followSymlinks: Bool,
168+
workingDirectory: URL = URL(fileURLWithPath: ".")
169+
) -> [URL] {
170+
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks, workingDirectory: workingDirectory)
171+
var seen: [URL] = []
117172
for next in iterator {
118-
seen.append(next.path)
173+
seen.append(next)
119174
}
120175
return seen
121176
}

‎api-breakages.txt

+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
6.2
2+
---
3+
4+
API breakage: constructor FileIterator.init(urls:followSymlinks:) has been removed

0 commit comments

Comments
 (0)