Skip to content

Commit

Permalink
Merge remote-tracking branch 'swiftlang/main' into release/6.1
Browse files Browse the repository at this point in the history
  • Loading branch information
ahoppen committed Dec 10, 2024
2 parents 6e4c968 + 3b27ab5 commit 2abf666
Show file tree
Hide file tree
Showing 9 changed files with 226 additions and 38 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/pull_request.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@ jobs:
tests:
name: Test
uses: swiftlang/github-workflows/.github/workflows/swift_package_test.yml@main
with:
enable_windows_docker: false # Dockerless Windows is 5-10 minutes faster than Docker on Windows
soundness:
name: Soundness
uses: swiftlang/github-workflows/.github/workflows/soundness.yml@main
with:
license_header_check_enabled: false
license_header_check_project_name: "Swift.org"
api_breakage_check_allowlist_path: "api-breakages.txt"
14 changes: 0 additions & 14 deletions Sources/SwiftFormat/API/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -486,17 +486,3 @@ public struct NoAssignmentInExpressionsConfiguration: Codable, Equatable {

public init() {}
}

fileprivate extension URL {
var isRoot: Bool {
#if os(Windows)
// 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.
// https://github.com/swiftlang/swift-format/issues/844
return self.pathComponents.count <= 1
#else
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
return self.path == "/" || self.path == ""
#endif
}
}
3 changes: 2 additions & 1 deletion Sources/SwiftFormat/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ add_library(SwiftFormat
Rules/UseTripleSlashForDocumentationComments.swift
Rules/UseWhereClausesInForLoops.swift
Rules/ValidateDocumentationComments.swift
Utilities/FileIterator.swift)
Utilities/FileIterator.swift
Utilities/URL+isRoot.swift)
target_link_libraries(SwiftFormat PUBLIC
SwiftMarkdown::Markdown
SwiftSyntax::SwiftSyntax
Expand Down
15 changes: 14 additions & 1 deletion Sources/SwiftFormat/PrettyPrint/PrettyPrintBuffer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,20 @@ struct PrettyPrintBuffer {
writeRaw(text)
consecutiveNewlineCount = 0
pendingSpaces = 0
column += text.count

// In case of comments, we may get a multi-line string.
// To account for that case, we need to correct the lineNumber count.
// The new column is only the position within the last line.
let lines = text.split(separator: "\n")
lineNumber += lines.count - 1
if lines.count > 1 {
// in case we have inserted new lines, we need to reset the column
column = lines.last?.count ?? 0
} else {
// in case it is an end of line comment or a single line comment,
// we just add to the current column
column += lines.last?.count ?? 0
}
}

/// Request that the given number of spaces be printed out before the next text token.
Expand Down
28 changes: 19 additions & 9 deletions Sources/SwiftFormat/Utilities/FileIterator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@

import Foundation

#if os(Windows)
import WinSDK
#endif

/// Iterator for looping over lists of files and directories. Directories are automatically
/// traversed recursively, and we check for files with a ".swift" extension.
@_spi(Internal)
Expand All @@ -32,7 +36,7 @@ public struct FileIterator: Sequence, IteratorProtocol {

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

/// Keep track of the current directory we're recursing through.
private var currentDirectory = URL(fileURLWithPath: "")
Expand All @@ -46,8 +50,13 @@ public struct FileIterator: Sequence, IteratorProtocol {
/// Create a new file iterator over the given list of file URLs.
///
/// The given URLs may be files or directories. If they are directories, the iterator will recurse
/// into them.
public init(urls: [URL], followSymlinks: Bool) {
/// into them. Symlinks are never followed on Windows platforms as Foundation doesn't support it.
/// - Parameters:
/// - urls: `Array` of files or directories to iterate.
/// - followSymlinks: `Bool` to indicate if symbolic links should be followed when iterating.
/// - workingDirectory: `URL` that indicates the current working directory. Used for testing.
public init(urls: [URL], followSymlinks: Bool, workingDirectory: URL = URL(fileURLWithPath: ".")) {
self.workingDirectory = workingDirectory
self.urls = urls
self.urlIterator = self.urls.makeIterator()
self.followSymlinks = followSymlinks
Expand Down Expand Up @@ -144,12 +153,13 @@ public struct FileIterator: Sequence, IteratorProtocol {
// if the user passes paths that are relative to the current working directory, they will
// be displayed as relative paths. Otherwise, they will still be displayed as absolute
// paths.
let relativePath =
path.hasPrefix(workingDirectory.path)
? String(path.dropFirst(workingDirectory.path.count + 1))
: path
output =
URL(fileURLWithPath: relativePath, isDirectory: false, relativeTo: workingDirectory)
let relativePath: String
if !workingDirectory.isRoot, path.hasPrefix(workingDirectory.path) {
relativePath = String(path.dropFirst(workingDirectory.path.count).drop(while: { $0 == "/" || $0 == #"\"# }))
} else {
relativePath = path
}
output = URL(fileURLWithPath: relativePath, isDirectory: false, relativeTo: workingDirectory)

default:
break
Expand Down
32 changes: 32 additions & 0 deletions Sources/SwiftFormat/Utilities/URL+isRoot.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
//
//===----------------------------------------------------------------------===//

import Foundation

extension URL {
@_spi(Testing) public var isRoot: Bool {
#if os(Windows)
// 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.
// https://github.com/swiftlang/swift-format/issues/844
var pathComponents = self.pathComponents
if pathComponents.first == "/" {
// Canonicalize `/C:/` to `C:/`.
pathComponents = Array(pathComponents.dropFirst())
}
return pathComponents.count <= 1
#else
// On Linux, we may end up with an string for the path due to https://github.com/swiftlang/swift-foundation/issues/980
// TODO: Remove the check for "" once https://github.com/swiftlang/swift-foundation/issues/980 is fixed.
return self.path == "/" || self.path == ""
#endif
}
}
84 changes: 84 additions & 0 deletions Tests/SwiftFormatTests/PrettyPrint/LineNumbersTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import SwiftFormat
import _SwiftFormatTestSupport

final class LineNumbersTests: PrettyPrintTestCase {
func testLineNumbers() {
let input =
"""
final class A {
@Test func b() throws {
doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long
}
}
"""

let expected =
"""
final class A {
@Test func b() throws {
doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long
}
}
"""

assertPrettyPrintEqual(
input: input,
expected: expected,
linelength: 120,
whitespaceOnly: true,
findings: [
FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length")
]
)
}

func testLineNumbersWithComments() {
let input =
"""
// Copyright (C) 2024 My Coorp. All rights reserved.
//
// This document is the property of My Coorp.
// It is considered confidential and proprietary.
//
// This document may not be reproduced or transmitted in any form,
// in whole or in part, without the express written permission of
// My Coorp.
final class A {
@Test func b() throws {
doSomethingInAFunctionWithAVeryLongName() 1️⃣// Here we have a very long comment that should not be here because it is far too long
}
}
"""

let expected =
"""
// Copyright (C) 2024 My Coorp. All rights reserved.
//
// This document is the property of My Coorp.
// It is considered confidential and proprietary.
//
// This document may not be reproduced or transmitted in any form,
// in whole or in part, without the express written permission of
// My Coorp.
final class A {
@Test func b() throws {
doSomethingInAFunctionWithAVeryLongName() // Here we have a very long comment that should not be here because it is far too long
}
}
"""

assertPrettyPrintEqual(
input: input,
expected: expected,
linelength: 120,
whitespaceOnly: true,
findings: [
FindingSpec("1️⃣", message: "move end-of-line comment that exceeds the line length")
]
)
}
}
81 changes: 68 additions & 13 deletions Tests/SwiftFormatTests/Utilities/FileIteratorTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,31 @@
@_spi(Internal) import SwiftFormat
@_spi(Internal) @_spi(Testing) import SwiftFormat
import XCTest

extension URL {
/// Assuming this is a file URL, resolves all symlinks in the path.
///
/// - Note: We need this because `URL.resolvingSymlinksInPath()` not only resolves symlinks but also standardizes the
/// path by stripping away `private` prefixes. Since sourcekitd is not performing this standardization, using
/// `resolvingSymlinksInPath` can lead to slightly mismatched URLs between the sourcekit-lsp response and the test
/// assertion.
fileprivate var realpath: URL {
#if canImport(Darwin)
return self.path.withCString { path in
guard let realpath = Darwin.realpath(path, nil) else {
return self
}
let result = URL(fileURLWithPath: String(cString: realpath))
free(realpath)
return result
}
#else
// Non-Darwin platforms don't have the `/private` stripping issue, so we can just use `self.resolvingSymlinksInPath`
// here.
return self.resolvingSymlinksInPath()
#endif
}
}

final class FileIteratorTests: XCTestCase {
private var tmpdir: URL!

Expand All @@ -10,7 +35,7 @@ final class FileIteratorTests: XCTestCase {
in: .userDomainMask,
appropriateFor: FileManager.default.temporaryDirectory,
create: true
)
).realpath

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

func testFollowSymlinks() throws {
Expand All @@ -41,10 +66,10 @@ final class FileIteratorTests: XCTestCase {
#endif
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: true)
XCTAssertEqual(seen.count, 3)
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.hasSuffix("project/real2.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real1.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/real2.swift") })
// Hidden but found through the visible symlink project/link.swift
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
}

func testTraversesHiddenFilesIfExplicitlySpecified() throws {
Expand All @@ -56,8 +81,8 @@ final class FileIteratorTests: XCTestCase {
followSymlinks: false
)
XCTAssertEqual(seen.count, 2)
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.build/generated.swift") })
XCTAssertTrue(seen.contains { $0.hasSuffix("project/.hidden.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.build/generated.swift") })
XCTAssertTrue(seen.contains { $0.path.hasSuffix("project/.hidden.swift") })
}

func testDoesNotFollowSymlinksIfFollowSymlinksIsFalseEvenIfExplicitlySpecified() {
Expand All @@ -71,6 +96,32 @@ final class FileIteratorTests: XCTestCase {
)
XCTAssertTrue(seen.isEmpty)
}

func testDoesNotTrimFirstCharacterOfPathIfRunningInRoot() throws {
// Find the root of tmpdir. On Unix systems, this is always `/`. On Windows it is the drive.
var root = tmpdir!
while !root.isRoot {
root.deleteLastPathComponent()
}
var rootPath = root.path
#if os(Windows) && compiler(<6.1)
if rootPath.hasPrefix("/") {
// Canonicalize /C: to C:
rootPath = String(rootPath.dropFirst())
}
#endif
// Make sure that we don't drop the beginning of the path if we are running in root.
// https://github.com/swiftlang/swift-format/issues/862
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: root).map(\.relativePath)
XCTAssertTrue(seen.allSatisfy { $0.hasPrefix(rootPath) }, "\(seen) does not contain root directory '\(rootPath)'")
}

func testShowsRelativePaths() throws {
// Make sure that we still show the relative path if using them.
// https://github.com/swiftlang/swift-format/issues/862
let seen = allFilesSeen(iteratingOver: [tmpdir], followSymlinks: false, workingDirectory: tmpdir)
XCTAssertEqual(Set(seen.map(\.relativePath)), ["project/real1.swift", "project/real2.swift"])
}
}

extension FileIteratorTests {
Expand Down Expand Up @@ -111,11 +162,15 @@ extension FileIteratorTests {
}

/// Computes the list of all files seen by using `FileIterator` to iterate over the given URLs.
private func allFilesSeen(iteratingOver urls: [URL], followSymlinks: Bool) -> [String] {
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks)
var seen: [String] = []
private func allFilesSeen(
iteratingOver urls: [URL],
followSymlinks: Bool,
workingDirectory: URL = URL(fileURLWithPath: ".")
) -> [URL] {
let iterator = FileIterator(urls: urls, followSymlinks: followSymlinks, workingDirectory: workingDirectory)
var seen: [URL] = []
for next in iterator {
seen.append(next.path)
seen.append(next)
}
return seen
}
Expand Down
4 changes: 4 additions & 0 deletions api-breakages.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
6.2
---

API breakage: constructor FileIterator.init(urls:followSymlinks:) has been removed

0 comments on commit 2abf666

Please sign in to comment.