Avoid crashing on invalid range in editDocument

[hamishknight]

Rename LineTable.replace(utf8Offset:length:with) to tryReplace and bail if the provided range is out of bounds of the buffer. This ensures we match the behavior of SourceKit when handling an editor.replacetext request.

rdar://90385969

[ahoppen]

Nice find

[ahoppen]

I think it’s odd that the UTF-8-based replace methods now continue on error while the UTF-16-based methods trap (even though it is indicated by tryReplace vs replace). What do you think about changing replace to throws and then let the client handle an out-of-bounds error. For SourceKit-LSP I think crashing is still a reasonable resolution because we would definitely have an out-of-date document state and if we crash, the editor should re-launch us and thus send the new document state. For the plugin, I think wrapping it in orLog is reasonable to match the current behavior of SourceKitService. Although I find it quite concerning if we get out-of-bound replacements.

[hamishknight]

For SourceKit-LSP I think crashing is still a reasonable resolution

Maybe I'm misunderstanding, but doesn't LSP currently recover by clipping the provided location to the start and end locations?

sourcekit-lsp/Sources/SKUtilities/LineTable.swift

Lines 241 to 245 in 69150c2

	switch self.lineSlice(at: line, callerFile: callerFile, callerLine: callerLine) {
	case .beforeFirstLine:
	return self.content.startIndex
	case .afterLastLine:
	return self.content.endIndex

But yeah, I changed the name to tryReplace to indicate it has a different behavior to replace. Happy to make it throw + orLog instead.

[hamishknight]

Updated to throw an error

[ahoppen]

Huh, I could have sworn that we crashed for out-of-bound edits but I changed that in #1798. Sorry for the back and forth but I think my main motivation is that the UTF-8 and UTF-16-based methods should behave consistently. With that new information, I think I’d prefer if we’d just match the UTF-16 behavior for UTF-8 and use

let start = self.stringIndexOf(line: fromLine, utf8Column: fromOff)
let end = self.stringIndexOf(line: toLine, utf8Column: toOff)

That should fix the crash with the standard log behavior, right?

[hamishknight]

@swift-ci please test