Skip to content
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.

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

Support CodeAction resolve for Java code actions #299

Merged

Conversation

datho7561
Copy link
Contributor

@datho7561 datho7561 commented Oct 6, 2022

Resolve workspace edits only when the code action is applied for all code actions for Java files that modify source files. This should increase the performance of editing Java files with lsp4mp active, since the edits of all the code actions don't need to be computed.

I also adjusted the title of the "Add @operation annotation" code action to include the class that the code action will apply to.

Closes #171

@@ -0,0 +1,78 @@
/*******************************************************************************
* Copyright (c) 2019 Red Hat Inc. and others.
Copy link
Contributor

Choose a reason for hiding this comment

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

2022

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't new, I just copied it over from microprofile.ls

@@ -0,0 +1,78 @@
/*******************************************************************************
* Copyright (c) 2019 Red Hat Inc. and others.
Copy link
Contributor

Choose a reason for hiding this comment

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

2022

@angelozerr
Copy link
Contributor

The code looks promising, I need to play with your PR.

@datho7561
Copy link
Contributor Author

The tests should pass now, but I would like to clean up the code (Eclipse has been formatting strange ever since I installed and uninstalled the editorconfig extension)

@datho7561 datho7561 force-pushed the 171-codeaction-resolve-just-jdt branch from 1cf916f to bf9c0da Compare October 14, 2022 15:31
@datho7561 datho7561 changed the title Java file CodeAction resolve Support CodeAction resolve for Java code actions Oct 14, 2022
@datho7561 datho7561 marked this pull request as ready for review October 14, 2022 15:33
@datho7561 datho7561 force-pushed the 171-codeaction-resolve-just-jdt branch from bf9c0da to 1605fbe Compare October 14, 2022 15:53
Resolve workspace edits only when the code action is applied
for all code actions for Java files that modify source files.
This should increase the performance of editing Java files with lsp4mp
active, since the edits of all the code actions don't need to be
computed.

I also adjusted the title of the "Add @operation annotation" code action
to include the class that the code action will apply to.

Closes eclipse#171

Signed-off-by: David Thompson <davthomp@redhat.com>
@angelozerr
Copy link
Contributor

@datho7561 when I see the codeAction trace:

[Trace - 15:37:54] Received response 'textDocument/codeAction - (21)' in 18ms.
Result: [
    {
        "title": "Let 'DontImplementHealthCheck' implement '@HealthCheck'",
        "kind": "quickfix",
        "diagnostics": [
            {
                "range": {
                    "start": {
                        "line": 9,
                        "character": 13
                    },
                    "end": {
                        "line": 9,
                        "character": 37
                    }
                },
                "code": "ImplementHealthCheck",
                "source": "microprofile-health",
                "message": "The class org.acme.health.DontImplementHealthCheck using the @Liveness, @Readiness, or @Health annotation should implement the HealthCheck interface."
            }
        ],
        "data": {
            "participantId": "org.eclipse.lsp4mp.jdt.internal.health.java.ImplementHealthCheckQuickFix",
            "documentUri": "file:///c%3A/Users/azerr/git/lsp4mp/microprofile.jdt/org.eclipse.lsp4mp.jdt.test/projects/maven/microprofile-health-quickstart/src/main/java/org/acme/health/DontImplementHealthCheck.java",
            "range": {
                "start": {
                    "line": 9,
                    "character": 13
                },
                "end": {
                    "line": 9,
                    "character": 37
                }
            },
            "resourceOperationSupported": true,
            "commandConfigurationUpdateSupported": true
        }
    }
]

the diagnostic range and the code action range are the same. I wonder if spec allows to returns a coe action without range and in this usecase you could use the diagnostic range, what do you think?

Map<String, Object> extendedData = new HashMap<>();
extendedData.put(ANNOTATION_KEY, annotations);
codeAction.setData(new CodeActionResolveData(context.getUri(), getParticipantId(),
context.getParams().getRange(), extendedData,
Copy link
Contributor

@angelozerr angelozerr Oct 17, 2022

Choose a reason for hiding this comment

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

I think you dont need this range since you can use the diagnostic range, no? I think this range can be usefull when your code action have several diagnostic, so perhaps you can keep this range in the data, but when there is just one diagnostic you could use the diagnostic range and not fill the code action range.

@datho7561
Copy link
Contributor Author

I wonder if spec allows to returns a code action without range and in this usecase you could use the diagnostic range, what do you think?

I put the CodeAction range into the CodeAction data, because one of our codeactions doesn't have an associated diagnostic (OpenAPI @Operator). I am using (a child type of) CodeActionContext as the parameters for resolving, since it means less changes to implement code action resolve, since I have all the same data as the original codeaction

@angelozerr angelozerr merged commit f5571c3 into eclipse:master Oct 19, 2022
@angelozerr
Copy link
Contributor

It works like a charm @datho7561, great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve CodeAction performance with CodeAction#data & resolveCodeAction
2 participants