Skip to content

Replace usages of fatalError that are recoverable by non-fatal error propagation constructs #1162

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 6 commits into from
Apr 5, 2024
Merged

Replace usages of fatalError that are recoverable by non-fatal error propagation constructs #1162

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8e9c19a
Don’t `fatalError` when constructing `DocumentURI` from an invalid URL
ahoppen Apr 4, 2024
34acdb0
Don’t `fatalError` for Fix-Its that make no changes
ahoppen Apr 2, 2024
7a65050
Don’t `fatalError` if encoding a message to the client could not be e…
ahoppen Apr 4, 2024
cb14bd0
Don’t `fatalError` if `TextDocumentContentChangeEvent` could not be c…
ahoppen Apr 2, 2024
f41feb7
Don’t `fatalError` if applying an edit in sourcekitd fails
ahoppen Apr 2, 2024
e3bc561
Don’t `fatalError` when calling `splitToSingleLineRanges` on a range …
ahoppen Apr 2, 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
5 changes: 3 additions & 2 deletions Sources/SourceKitLSP/Swift/SyntaxHighlightingTokenParser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ struct SyntaxHighlightingTokenParser {
}
}

extension Range where Bound == Position {
extension Range<Position> {
/// Splits a potentially multi-line range to multiple single-line ranges.
func splitToSingleLineRanges(in snapshot: DocumentSnapshot) -> [Self] {
if isEmpty {
Expand All @@ -220,7 +220,8 @@ extension Range where Bound == Position {
guard let startIndex = snapshot.index(of: lowerBound),
let endIndex = snapshot.index(of: upperBound)
else {
fatalError("Range \(self) reaches outside of the document")
logger.fault("Range \(self) reaches outside of the document")
return [self]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just bound the ends to the document range?

Copy link
Member Author

Choose a reason for hiding this comment

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

This made me re-think recovery inside position conversions altogether and I think positions conversions should never fail and always return the closest valid position, which also simplifies all client code. I’ll create a separate PR for that.

Copy link
Member Author

Choose a reason for hiding this comment

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

And here we go: #1168

}

let text = snapshot.text[startIndex..<endIndex]
Expand Down