LSP4E allows invalid code action request range to a language server

[rgrunber]

CC @mickaelistria

I'm using https://github.com/rgrunber/eclipse.jdt.ls.client/blob/master/com.redhat.eclipseide.jdtlsclient/src/com/redhat/eclipseide/jdtlsclient/JDTLSProductConnectionProvider.java (via. LSP4E). I noticed that the client was sending a range that JDT-LS can't handle.

https://github.com/eclipse-platform/eclipse.platform.text/blob/81a446557dd3f8f354a272de447fe787881dbe1f/org.eclipse.jface.text/src/org/eclipse/jface/text/quickassist/QuickAssistAssistant.java#L71

[t=1670291196426
] LSP4E to com.redhat.eclipseide.jdtlsclient.extenalServer: {
    "jsonrpc": "2.0",
    "id": "22",
    "method": "textDocument/codeAction",
    "params": {
        "textDocument": {
            "uri": "file:///home/rgrunber/runtime-New_configuration/Test/src/org/example/Test.java"
        },
        "range": {
            "start": {
                "line": 9,
                "character": 20
            },
            "end": {
                "line": 9,
                "character": 19
            }
        },
        "context": {
            "diagnostics": []
        }
    }
}

This results in code action requests being sent to JDT-LS which fail because for refactoring we assert the length should be >= 0 :
https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/b8bdee93b96fabed18ac1f3f50c1e51c3802c010/org.eclipse.jdt.core.manipulation/core%20extension/org/eclipse/jdt/internal/corext/dom/Selection.java#L52

However, looking at https://github.com/eclipse-platform/eclipse.platform.text/blob/81a446557dd3f8f354a272de447fe787881dbe1f/org.eclipse.jface.text/src/org/eclipse/jface/text/quickassist/IQuickAssistInvocationContext.java#L38 , this is valid.

So I think this needs to be adjusted at

lsp4e/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/codeactions/LSPCodeActionQuickAssistProcessor.java

Line 224 in 89dc090

private static CodeActionParams prepareCodeActionParams(List<LSPDocumentInfo> infos, int offset, int length) {

, where LSP4E can replace the context with a valid length.

[mickaelistria]

@rgrunber Are you interested in submitting a PR for that?

[rgrunber]

I'm definitely open to it. It should be pretty simple. The last thing I'd like to do is find the spot between eclipse.platform.text <===> eclipse.jdt.ui where that negative length gets corrected. It has to get corrected somewhere because platform has -1 as valid according to API, and JDT has code that will fail if that happens.

Also, I couldn't find anything in the LSP spec that explicitly states this kind of "inverted" range is invalid, though the fact that I can't easily find a bug in JDT-LS that complains about this is a good sign that clients should avoid doing this.

[rgrunber]

Ok, here's where JDT decides to ignore the -1 : https://github.com/eclipse-jdt/eclipse.jdt.ui/blob/b8bdee93b96fabed18ac1f3f50c1e51c3802c010/org.eclipse.jdt.ui/ui/org/eclipse/jdt/internal/ui/text/correction/JavaCorrectionProcessor.java#L235-L238 . It completely ignored the length and re-computes from the text viewer. Maybe we can even copy the exact logic.