Replace usages of fatalError that are recoverable by non-fatal error propagation constructs
#1162
Conversation
…onverted to a `SourceEdit`
…that exceeds past the document
ahoppen
force-pushed
the
fewer-fatal-errors
branch
from
54ef06a to
e3bc561
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
1 similar comment
|
@swift-ci Please test Windows |
| @@ -110,7 +110,8 @@ extension CodeAction { | |||
| case (true, false): | |||
| return "Insert '\(newText)'" | |||
| case (true, true): | |||
| preconditionFailure("FixIt makes no changes") | |||
There was a problem hiding this comment.
FWIW I'm not a huge fan of constructing a title out of an edit regardless (but I prefer this to a fatal error).
| @@ -518,7 +518,12 @@ extension SwiftLanguageService { | |||
| do { | |||
| _ = try await self.sourcekitd.send(req, fileContents: nil) | |||
| } catch { | |||
| fatalError("failed to apply edit") | |||
There was a problem hiding this comment.
This and the other edit case are pretty bad. We could theoretically send an close/open in this case. Not sure about the other though, if we fail to decode an edit... 🤷
There was a problem hiding this comment.
Yes, I think it’s very unlikely this is going to happen and I don’t think it’s worth optimizing for this case.
| @@ -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] | |||
There was a problem hiding this comment.
Could just bound the ends to the document range?
There was a problem hiding this comment.
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.
Best reviews commit-by-commit.