Skip to content

[5.9] Make column and line non-optional #1562

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ class SourceManager {
return SourceLocation(
// NOTE: IUO because 'localLocation' is created by a location converter
// which guarantees non-nil line/column.
line: localLocation.line! + lineOffset,
column: localLocation.column! + columnOffset,
line: localLocation.line + lineOffset,
column: localLocation.column + columnOffset,
offset: localLocation.offset + positionOffset,
file: file
)
Expand Down
22 changes: 9 additions & 13 deletions Sources/SwiftDiagnostics/DiagnosticsFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,32 +112,28 @@ public struct DiagnosticsFormatter {
return nil
}

let startLoc = highlight.startLocation(converter: slc, afterLeadingTrivia: true);
guard let startLine = startLoc.line else {
return nil
}
let startLoc = highlight.startLocation(converter: slc, afterLeadingTrivia: true)
let startLine = startLoc.line

// Find the starting column.
let startColumn: Int
if startLine < lineNumber {
startColumn = 1
} else if startLine == lineNumber, let column = startLoc.column {
startColumn = column
} else if startLine == lineNumber {
startColumn = startLoc.column
} else {
return nil
}

// Find the ending column.
let endLoc = highlight.endLocation(converter: slc, afterTrailingTrivia: false)
guard let endLine = endLoc.line else {
return nil
}
let endLine = endLoc.line

let endColumn: Int
if endLine > lineNumber {
endColumn = annotatedLine.sourceString.count
} else if endLine == lineNumber, let column = endLoc.column {
endColumn = column
} else if endLine == lineNumber {
endColumn = endLoc.column
} else {
return nil
}
Expand Down Expand Up @@ -283,9 +279,9 @@ public struct DiagnosticsFormatter {
annotatedSource.append("\n")
}

let columnsWithDiagnostics = Set(annotatedLine.diagnostics.map { $0.location(converter: slc).column ?? 0 })
let columnsWithDiagnostics = Set(annotatedLine.diagnostics.map { $0.location(converter: slc).column })
let diagsPerColumn = Dictionary(grouping: annotatedLine.diagnostics) { diag in
diag.location(converter: slc).column ?? 0
diag.location(converter: slc).column
}.sorted { lhs, rhs in
lhs.key > rhs.key
}
Expand Down
67 changes: 8 additions & 59 deletions Sources/SwiftSyntax/SourceLocation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,83 +10,32 @@
//
//===----------------------------------------------------------------------===//

/// Represent the user-facing part of SourceLocation that can be calculated
/// on demand.
struct ComputedLocation: Hashable, Codable, CustomDebugStringConvertible {
/// The line in the file where this location resides. 1-based.
let line: Int

/// The UTF-8 byte offset from the beginning of the line where this location
/// resides. 1-based.
let column: Int

/// The file in which this location resides.
let file: String

var debugDescription: String {
// Print file name?
return "\(line):\(column)"
}

init(line: Int, column: Int, file: String) {
self.line = line
self.column = column
self.file = file
}
init(offset: Int, using converter: SourceLocationConverter) {
let loc = converter.location(for: AbsolutePosition(utf8Offset: offset))
precondition(loc.offset == offset)
self.line = loc.line!
self.column = loc.column!
self.file = loc.file!
}
}

/// Represents a source location in a Swift file.
public struct SourceLocation: Hashable, Codable, CustomDebugStringConvertible {

/// Line and column that can be computed on demand.
private var compLoc: ComputedLocation?

/// The UTF-8 byte offset into the file where this location resides.
public let offset: Int

/// The line in the file where this location resides. 1-based.
public var line: Int? {
return compLoc?.line
}
public var line: Int

/// The UTF-8 byte offset from the beginning of the line where this location
/// resides. 1-based.
public var column: Int? {
return compLoc?.column
}
public let column: Int

/// The file in which this location resides.
public var file: String? {
return compLoc?.file
}
public let file: String

public var debugDescription: String {
guard let compLoc = compLoc else {
return "\(offset)"
}
return compLoc.debugDescription
// Print file name?
return "\(line):\(column)"
}

public init(line: Int, column: Int, offset: Int, file: String) {
self.line = line
self.offset = offset
self.compLoc = ComputedLocation(line: line, column: column, file: file)
}

/// Initialize SourceLocation with a utf8 offset.
/// If a SourceLocationConverter is given, line, column and file will be populated;
/// otherwise they will be nil.
public init(offset: Int, converter: SourceLocationConverter? = nil) {
self.offset = offset
if let converter = converter {
self.compLoc = ComputedLocation(offset: offset, using: converter)
}
self.column = column
self.file = file
}
}

Expand Down
12 changes: 4 additions & 8 deletions Sources/SwiftSyntaxMacros/MacroExpansionContext.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,18 +134,14 @@ extension MacroExpansionContext {
at position: PositionInSyntaxNode,
filePathMode: SourceLocationFilePathMode
) -> AbstractSourceLocation? {
guard let sourceLoc: SourceLocation = location(of: node, at: position, filePathMode: filePathMode),
let file = sourceLoc.file,
let line = sourceLoc.line,
let column = sourceLoc.column
else {
guard let sourceLoc: SourceLocation = location(of: node, at: position, filePathMode: filePathMode) else {
return nil
}

return AbstractSourceLocation(
file: "\(literal: file)",
line: "\(literal: line)",
column: "\(literal: column)"
file: "\(literal: sourceLoc.file)",
line: "\(literal: sourceLoc.line)",
column: "\(literal: sourceLoc.column)"
)
}

Expand Down
12 changes: 2 additions & 10 deletions Tests/SwiftParserTest/Assertions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -336,16 +336,8 @@ func assertLocation<T: SyntaxProtocol>(
let locationConverter = SourceLocationConverter(file: "", source: tree.description)
let actualLocation = location
let expectedLocation = locationConverter.location(for: AbsolutePosition(utf8Offset: markerLoc))
if let actualLine = actualLocation.line,
let actualColumn = actualLocation.column,
let expectedLine = expectedLocation.line,
let expectedColumn = expectedLocation.column
{
if actualLine != expectedLine || actualColumn != expectedColumn {
XCTFail("Expected location \(expectedLine):\(expectedColumn) but got \(actualLine):\(actualColumn)", file: file, line: line)
}
} else {
XCTFail("Failed to resolve diagnostic location to line/column", file: file, line: line)
if actualLocation.line != expectedLocation.line || actualLocation.column != expectedLocation.column {
XCTFail("Expected location \(expectedLocation.line):\(expectedLocation.column) but got \(actualLocation.line):\(actualLocation.column)", file: file, line: line)
}
} else {
XCTFail("Did not find marker \(locationMarker) in the source code", file: file, line: line)
Expand Down
10 changes: 5 additions & 5 deletions Tests/SwiftSyntaxTest/AbsolutePositionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public class AbsolutePositionTests: XCTestCase {
XCTAssertEqual(startLoc.line, 8)
XCTAssertEqual(startLoc.column, 1)
XCTAssertEqual(
converter.position(ofLine: startLoc.line!, column: startLoc.column!),
converter.position(ofLine: startLoc.line, column: startLoc.column),
secondReturnStmt.positionAfterSkippingLeadingTrivia
)

Expand All @@ -167,7 +167,7 @@ public class AbsolutePositionTests: XCTestCase {
XCTAssertEqual(startLocBeforeTrivia.line, 6)
XCTAssertEqual(startLocBeforeTrivia.column, 1)
XCTAssertEqual(
converter.position(ofLine: startLocBeforeTrivia.line!, column: startLocBeforeTrivia.column!),
converter.position(ofLine: startLocBeforeTrivia.line, column: startLocBeforeTrivia.column),
secondReturnStmt.position
)

Expand All @@ -183,9 +183,9 @@ public class AbsolutePositionTests: XCTestCase {
XCTAssertEqual(endLocAfterTrivia.line, 11)
XCTAssertEqual(endLocAfterTrivia.column, 1)

XCTAssertTrue(converter.isValid(line: startLoc.line!, column: startLoc.column!))
XCTAssertFalse(converter.isValid(line: startLoc.line!, column: startLoc.column! + 50))
XCTAssertFalse(converter.isValid(line: 0, column: startLoc.column!))
XCTAssertTrue(converter.isValid(line: startLoc.line, column: startLoc.column))
XCTAssertFalse(converter.isValid(line: startLoc.line, column: startLoc.column + 50))
XCTAssertFalse(converter.isValid(line: 0, column: startLoc.column))
XCTAssertTrue(converter.isValid(position: secondReturnStmt.position))
XCTAssertFalse(converter.isValid(position: secondReturnStmt.position + SourceLength(utf8Length: 100)))
}
Expand Down