Fix IndexOutOfBounds from the highlight pass #2252
Conversation
mkondratek
changed the title
| startOffset > this.textLength || | ||
| endOffset > this.textLength || | ||
| startOffset > endOffset) { | ||
| throw IllegalArgumentException( |
There was a problem hiding this comment.
Initially I wanted to bound the values here. But maybe we should not do that? We should be aware of what arguments we pass here.
| @@ -97,6 +101,12 @@ class CodyFixHighlightPass(val file: PsiFile, val editor: Editor) : | |||
| null | |||
| } | |||
| } | |||
| .toList() | |||
|
|
|||
| if (protocolDiagnostics.isEmpty()) { | |||
There was a problem hiding this comment.
it was possible that we have been sending an empty list 😅 that was completely redundant
| val endOffset = max(cursor, range.startOffset + completionText.length) | ||
| codeStyleManager.reformatText(psiFile, cursor, endOffset) |
There was a problem hiding this comment.
Unrelated to highlight pass but I managed to get it once
| ProtocolLocation( | ||
| uri = protocolTextDocument.uri, | ||
| range = document.codyRange(highlight.startOffset, highlight.endOffset)) | ||
| val location = ProtocolLocation(uri = protocolTextDocument.uri, range = range) |
There was a problem hiding this comment.
Perhaps we can get rid of that whole protocolTextDocument?
It seems we need it only for uri, and we can get is using ProtocolTextDocument::uriFor.
It is not marked as requiring EDT so there is a good chance we will also not need that future, etc..
There was a problem hiding this comment.
btw, doCollectInformation is called from the background thread by IntelliJ
(we are extending HighlightPass and collectInformation calls doCollectInformation)
There was a problem hiding this comment.
good point about the URI - fixing!
| @@ -106,15 +116,18 @@ class CodyFixHighlightPass(val file: PsiFile, val editor: Editor) : | |||
| if (progress.isCanceled) { | |||
| break | |||
| } | |||
| if (highlight.startOffset > document.textLength || | |||
There was a problem hiding this comment.
We are now reading document in outside of the EDT.
Your check probably highly decreases chance of the issue, but I doubt it eliminates it as state can change even between this and the next instruction.
I think document.codyRange should be called on EDT.
| highlight.startOffset > highlight.endOffset) { | ||
| break | ||
| } | ||
|
|
||
| val range = document.codyRange(highlight.startOffset, highlight.endOffset) |
There was a problem hiding this comment.
Is that running in the EDT?
There was a problem hiding this comment.
It runs in EDT. I added @RequiresEdt on it.
Test plan
Try these examples: