Skip to content

Support for formatting a selection #708

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.

Sign up for GitHub

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
Jun 14, 2024
Merged

Support for formatting a selection #708

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6640067
Support for formatting a selection (given as an array of ranges) <htt…
DaveEwing Mar 21, 2024
88beb85
Some small refectorings and updates from review feedback. Add a few m…
DaveEwing Jun 3, 2024
0c0977d
Change the `--offsets` argument to take a single pair of offsets, and…
DaveEwing Jun 4, 2024
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
63 changes: 63 additions & 0 deletions Sources/SwiftFormat/API/Selection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2024 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
import SwiftSyntax

/// The selection as given on the command line - an array of offets and lengths
public enum Selection {
case infinite
case ranges([Range<AbsolutePosition>])

/// Create a selection from an array of utf8 ranges. An empty array means an infinite selection.
public init(offsetPairs: [Range<Int>]) {
if offsetPairs.isEmpty {
self = .infinite
} else {
let ranges = offsetPairs.map {
AbsolutePosition(utf8Offset: $0.lowerBound) ..< AbsolutePosition(utf8Offset: $0.upperBound)
}
self = .ranges(ranges)
}
}

public func contains(_ position: AbsolutePosition) -> Bool {
switch self {
case .infinite:
return true
case .ranges(let ranges):
return ranges.contains { $0.contains(position) }
}
}

public func overlapsOrTouches(_ range: Range<AbsolutePosition>) -> Bool {
switch self {
case .infinite:
return true
case .ranges(let ranges):
return ranges.contains { $0.overlapsOrTouches(range) }
}
}
}


public extension Syntax {
/// return true if the node is _completely_ inside any range in the selection
func isInsideSelection(_ selection: Selection) -> Bool {
switch selection {
case .infinite:
return true
case .ranges(let ranges):
return ranges.contains { return $0.lowerBound <= position && endPosition <= $0.upperBound }
}
}
}
31 changes: 16 additions & 15 deletions Sources/SwiftFormat/API/SwiftFormatter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
// Copyright (c) 2014 - 2024 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
Expand All @@ -21,6 +21,9 @@ public final class SwiftFormatter {
/// The configuration settings that control the formatter's behavior.
public let configuration: Configuration

/// the ranges of text to format
public var selection: Selection = .infinite

/// An optional callback that will be notified with any findings encountered during formatting.
public let findingConsumer: ((Finding) -> Void)?

Expand Down Expand Up @@ -70,6 +73,7 @@ public final class SwiftFormatter {
try format(
source: String(contentsOf: url, encoding: .utf8),
assumingFileURL: url,
selection: .infinite,
to: &outputStream,
parsingDiagnosticHandler: parsingDiagnosticHandler)
}
Expand All @@ -86,6 +90,7 @@ public final class SwiftFormatter {
/// - url: A file URL denoting the filename/path that should be assumed for this syntax tree,
/// which is associated with any diagnostics emitted during formatting. If this is nil, a
/// dummy value will be used.
/// - selection: The ranges to format
/// - outputStream: A value conforming to `TextOutputStream` to which the formatted output will
/// be written.
/// - parsingDiagnosticHandler: An optional callback that will be notified if there are any
Expand All @@ -94,6 +99,7 @@ public final class SwiftFormatter {
public func format<Output: TextOutputStream>(
source: String,
assumingFileURL url: URL?,
selection: Selection,
to outputStream: inout Output,
parsingDiagnosticHandler: ((Diagnostic, SourceLocation) -> Void)? = nil
) throws {
Expand All @@ -108,8 +114,8 @@ public final class SwiftFormatter {
assumingFileURL: url,
parsingDiagnosticHandler: parsingDiagnosticHandler)
try format(
syntax: sourceFile, operatorTable: .standardOperators, assumingFileURL: url, source: source,
to: &outputStream)
syntax: sourceFile, source: source, operatorTable: .standardOperators, assumingFileURL: url,
selection: selection, to: &outputStream)
}

/// Formats the given Swift syntax tree and writes the result to an output stream.
Expand All @@ -122,32 +128,26 @@ public final class SwiftFormatter {
///
/// - Parameters:
/// - syntax: The Swift syntax tree to be converted to source code and formatted.
/// - source: The original Swift source code used to build the syntax tree.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I’m not a fan of having a public API that requires the user to provide both syntax and source because that seems like redundant information. I think we should continue to have a public function that takes either syntax or source.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the syntax tree hold on to the source (I didn't see that when I did this two months ago 🙃, but I can look...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, syntax.description gives you the source code. It does need to stitch the syntax tree back together from the individual tokens, so there is a use case for a function that takes both the syntax tree and the source as a String, but I think that should not be a public function.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having to regenerate the original source text if we already have it to start (which, of course, we do). And eventually we'll have a transformed syntax tree, and we still need to use the original source along with that. I think passing both makes the most sense.

/// - operatorTable: The table that defines the operators and their precedence relationships.
/// This must be the same operator table that was used to fold the expressions in the `syntax`
/// argument.
/// - url: A file URL denoting the filename/path that should be assumed for this syntax tree,
/// which is associated with any diagnostics emitted during formatting. If this is nil, a
/// dummy value will be used.
/// - selection: The ranges to format
/// - outputStream: A value conforming to `TextOutputStream` to which the formatted output will
/// be written.
/// - Throws: If an unrecoverable error occurs when formatting the code.
public func format<Output: TextOutputStream>(
syntax: SourceFileSyntax, operatorTable: OperatorTable, assumingFileURL url: URL?,
to outputStream: inout Output
) throws {
try format(
syntax: syntax, operatorTable: operatorTable, assumingFileURL: url, source: nil,
to: &outputStream)
}

private func format<Output: TextOutputStream>(
syntax: SourceFileSyntax, operatorTable: OperatorTable,
assumingFileURL url: URL?, source: String?, to outputStream: inout Output
syntax: SourceFileSyntax, source: String, operatorTable: OperatorTable,
assumingFileURL url: URL?, selection: Selection, to outputStream: inout Output
) throws {
let assumedURL = url ?? URL(fileURLWithPath: "source")
let context = Context(
configuration: configuration, operatorTable: operatorTable, findingConsumer: findingConsumer,
fileURL: assumedURL, sourceFileSyntax: syntax, source: source, ruleNameCache: ruleNameCache)
fileURL: assumedURL, selection: selection, sourceFileSyntax: syntax, source: source,
ruleNameCache: ruleNameCache)
let pipeline = FormatPipeline(context: context)
let transformedSyntax = pipeline.rewrite(Syntax(syntax))

Expand All @@ -158,6 +158,7 @@ public final class SwiftFormatter {

let printer = PrettyPrinter(
context: context,
source: source,
node: transformedSyntax,
printTokenStream: debugOptions.contains(.dumpTokenStream),
whitespaceOnly: false)
Expand Down
6 changes: 4 additions & 2 deletions Sources/SwiftFormat/API/SwiftLinter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,17 +119,18 @@ public final class SwiftLinter {
/// - Throws: If an unrecoverable error occurs when formatting the code.
public func lint(
syntax: SourceFileSyntax,
source: String,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar here. It feels redundant to pass both syntax and source to this function (also error prone if the two are not in sync).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here. Since the whitespace linting uses PrettyPrinter to compare to the original source, we need to pass it along too.

operatorTable: OperatorTable,
assumingFileURL url: URL
) throws {
try lint(syntax: syntax, operatorTable: operatorTable, assumingFileURL: url, source: nil)
try lint(syntax: syntax, operatorTable: operatorTable, assumingFileURL: url, source: source)
}

private func lint(
syntax: SourceFileSyntax,
operatorTable: OperatorTable,
assumingFileURL url: URL,
source: String?
source: String
) throws {
let context = Context(
configuration: configuration, operatorTable: operatorTable, findingConsumer: findingConsumer,
Expand All @@ -145,6 +146,7 @@ public final class SwiftLinter {
// pretty-printer.
let printer = PrettyPrinter(
context: context,
source: source,
node: Syntax(syntax),
printTokenStream: debugOptions.contains(.dumpTokenStream),
whitespaceOnly: true)
Expand Down
11 changes: 9 additions & 2 deletions Sources/SwiftFormat/Core/Context.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2019 Apple Inc. and the Swift project authors
// Copyright (c) 2014 - 2024 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
Expand Down Expand Up @@ -39,6 +39,9 @@ public final class Context {
/// The configuration for this run of the pipeline, provided by a configuration JSON file.
let configuration: Configuration

/// The optional ranges to process
let selection: Selection

/// Defines the operators and their precedence relationships that were used during parsing.
let operatorTable: OperatorTable

Expand Down Expand Up @@ -66,6 +69,7 @@ public final class Context {
operatorTable: OperatorTable,
findingConsumer: ((Finding) -> Void)?,
fileURL: URL,
selection: Selection = .infinite,
sourceFileSyntax: SourceFileSyntax,
source: String? = nil,
ruleNameCache: [ObjectIdentifier: String]
Expand All @@ -74,6 +78,7 @@ public final class Context {
self.operatorTable = operatorTable
self.findingEmitter = FindingEmitter(consumer: findingConsumer)
self.fileURL = fileURL
self.selection = selection
self.importsXCTest = .notDetermined
let tree = source.map { Parser.parse(source: $0) } ?? sourceFileSyntax
self.sourceLocationConverter =
Expand All @@ -86,8 +91,10 @@ public final class Context {
}

/// Given a rule's name and the node it is examining, determine if the rule is disabled at this
/// location or not.
/// location or not. Also makes sure the entire node is contained inside any selection.
func isRuleEnabled<R: Rule>(_ rule: R.Type, node: Syntax) -> Bool {
guard node.isInsideSelection(selection) else { return false }

let loc = node.startLocation(converter: self.sourceLocationConverter)

assert(
Expand Down
85 changes: 78 additions & 7 deletions Sources/SwiftFormat/PrettyPrint/PrettyPrint.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//

import SwiftSyntax
import Foundation

/// PrettyPrinter takes a Syntax node and outputs a well-formatted, re-indented reproduction of the
/// code as a String.
Expand Down Expand Up @@ -66,6 +67,19 @@ public class PrettyPrinter {
private var configuration: Configuration { return context.configuration }
private let maxLineLength: Int
private var tokens: [Token]
private var source: String

/// keep track of where formatting was disabled in the original source
///
/// To format a selection, we insert `enableFormatting`/`disableFormatting` tokens into the
/// stream when entering/exiting a selection range. Those tokens include utf8 offsets into the
/// original source. When enabling formatting, we copy the text between `disabledPosition` and the
/// current position to `outputBuffer`. From then on, we continue to format until the next
/// `disableFormatting` token.
private var disabledPosition: AbsolutePosition? = nil
/// true if we're currently formatting
private var writingIsEnabled: Bool { disabledPosition == nil }

private var outputBuffer: String = ""

/// The number of spaces remaining on the current line.
Expand Down Expand Up @@ -172,11 +186,14 @@ public class PrettyPrinter {
/// - printTokenStream: Indicates whether debug information about the token stream should be
/// printed to standard output.
/// - whitespaceOnly: Whether only whitespace changes should be made.
public init(context: Context, node: Syntax, printTokenStream: Bool, whitespaceOnly: Bool) {
public init(context: Context, source: String, node: Syntax, printTokenStream: Bool, whitespaceOnly: Bool) {
self.context = context
self.source = source
let configuration = context.configuration
self.tokens = node.makeTokenStream(
configuration: configuration, operatorTable: context.operatorTable)
configuration: configuration,
selection: context.selection,
operatorTable: context.operatorTable)
self.maxLineLength = configuration.lineLength
self.spaceRemaining = self.maxLineLength
self.printTokenStream = printTokenStream
Expand Down Expand Up @@ -216,7 +233,9 @@ public class PrettyPrinter {
}

guard numberToPrint > 0 else { return }
writeRaw(String(repeating: "\n", count: numberToPrint))
if writingIsEnabled {
writeRaw(String(repeating: "\n", count: numberToPrint))
}
lineNumber += numberToPrint
isAtStartOfLine = true
consecutiveNewlineCount += numberToPrint
Expand All @@ -238,13 +257,17 @@ public class PrettyPrinter {
/// leading spaces that are required before the text itself.
private func write(_ text: String) {
if isAtStartOfLine {
writeRaw(currentIndentation.indentation())
if writingIsEnabled {
writeRaw(currentIndentation.indentation())
}
spaceRemaining = maxLineLength - currentIndentation.length(in: configuration)
isAtStartOfLine = false
} else if pendingSpaces > 0 {
} else if pendingSpaces > 0 && writingIsEnabled {
writeRaw(String(repeating: " ", count: pendingSpaces))
}
writeRaw(text)
if writingIsEnabled {
writeRaw(text)
}
consecutiveNewlineCount = 0
pendingSpaces = 0
}
Expand Down Expand Up @@ -523,7 +546,9 @@ public class PrettyPrinter {
}

case .verbatim(let verbatim):
writeRaw(verbatim.print(indent: currentIndentation))
if writingIsEnabled {
writeRaw(verbatim.print(indent: currentIndentation))
}
consecutiveNewlineCount = 0
pendingSpaces = 0
lastBreak = false
Expand Down Expand Up @@ -569,6 +594,40 @@ public class PrettyPrinter {
write(",")
spaceRemaining -= 1
}

case .enableFormatting(let enabledPosition):
// if we're not disabled, we ignore the token
if let disabledPosition {
let start = source.utf8.index(source.utf8.startIndex, offsetBy: disabledPosition.utf8Offset)
let end: String.Index
if let enabledPosition {
end = source.utf8.index(source.utf8.startIndex, offsetBy: enabledPosition.utf8Offset)
} else {
end = source.endIndex
}
var text = String(source[start..<end])
// strip trailing whitespace so that the next formatting can add the right amount
if let nonWhitespace = text.rangeOfCharacter(
from: CharacterSet.whitespaces.inverted, options: .backwards) {
text = String(text[..<nonWhitespace.upperBound])
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the printer pick up the whitespace characters that were dropped here? Ie. how does it know to format those but not anything in text? Seems like I’m missing something here.

Copy link
Contributor Author

@DaveEwing DaveEwing May 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we have to make sure our state variables are set correctly so that it does that correctly. That's what the next few lines do. 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m still missing how the whitespace characters that we’re shaving off with this are being printed.

If you have the following

func foo()   ⏩{}⏪

Then I think this only adds func foo() to the output stream, right (no whitespace)? But the next token that we print is the {, which doesn’t have any leading trivia (spaces are trailing trivia to )).

If those examples work because of some trivia re-attribution rule, how about the following?

func foo() /**/  ⏩{}⏪
func foo() /**/
  ⏩{}⏪


writeRaw(text)
if text.hasSuffix("\n") {
isAtStartOfLine = true
consecutiveNewlineCount = 1
} else {
isAtStartOfLine = false
consecutiveNewlineCount = 0
}
self.disabledPosition = nil
}

case .disableFormatting(let newPosition):
// a second disable is ignored
if writingIsEnabled {
disabledPosition = newPosition
}
}
}

Expand Down Expand Up @@ -673,6 +732,10 @@ public class PrettyPrinter {
let length = isSingleElement ? 0 : 1
total += length
lengths.append(length)

case .enableFormatting, .disableFormatting:
// no effect on length calculations
lengths.append(0)
}
}

Expand Down Expand Up @@ -775,6 +838,14 @@ public class PrettyPrinter {
case .contextualBreakingEnd:
printDebugIndent()
print("[END BREAKING CONTEXT Idx: \(idx)]")

case .enableFormatting(let pos):
printDebugIndent()
print("[ENABLE FORMATTING utf8 offset: \(String(describing: pos))]")

case .disableFormatting(let pos):
printDebugIndent()
print("[DISABLE FORMATTING utf8 offset: \(pos)]")
}
}

Expand Down
Loading