Skip to content

Use presumed file/line number when printing grouped diagnostics #2501

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 1 commit into from
Feb 19, 2024
Merged

Use presumed file/line number when printing grouped diagnostics #2501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Sources/SwiftDiagnostics/GroupedDiagnostics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ extension GroupedDiagnostics {

// Display file/line/column and diagnostic text for the primary diagnostic.
prefixString =
"\(location.file):\(location.line):\(location.column): \(diagnosticDecorator.decorateDiagnosticMessage(primaryDiag.diagMessage))\n"
"\(location.presumedFile):\(location.presumedLine):\(location.column): \(diagnosticDecorator.decorateDiagnosticMessage(primaryDiag.diagMessage))\n"

// If the primary diagnostic source file is not the same as the root source file, we're pointing into a generated buffer.
// Provide a link back to the original source file where this generated buffer occurred, so it's easy to find if
Expand Down
25 changes: 25 additions & 0 deletions Tests/SwiftDiagnosticsTest/GroupDiagnosticsFormatterTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,31 @@ extension GroupedDiagnostics {
}

final class GroupedDiagnosticsFormatterTests: XCTestCase {
func testSourceLocations() {
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be nice to put the actual file's name somewhere as well, but I have no great suggestions as to where to put it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you elaborate a bit? I'm not quite sure I'm following what you mean. :/

Copy link
Contributor

Choose a reason for hiding this comment

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

With #sourceLocation we have two "files" - the real file on disk (usually generated in this case) and the "presumed" file (the one noted in #sourceLocation). After this change, the file noted in the diagnostic is the presumed file, which matches the Swift compiler. That makes sense since #sourceLocation was added for a reason, but I have found that knowing the underlying file is useful too.

That would have been more clear if I put this comment on the actual #sourceLocation line :)

var group = GroupedDiagnostics()

// Main source file.
_ = group.addTestFile(
"""
#sourceLocation(file: "other.swift", line: 123)
let pi = 3.14159 x
""",
displayName: "main.swift",
diagnosticDescriptors: []
)
let annotated = DiagnosticsFormatter.annotateSources(in: group)
assertStringsEqualWithDiff(
annotated,
"""
other.swift:123:17: error: consecutive statements on a line must be separated by newline or ';'
1 │ #sourceLocation(file: "other.swift", line: 123)
2 │ let pi = 3.14159 x
│ ╰─ error: consecutive statements on a line must be separated by newline or ';'

"""
)
}

func testGroupingForMacroExpansion() {
var group = GroupedDiagnostics()

Expand Down