Description
When we get a textDocument/codeAction request, the context contains some Diagnostics. These are (allegedly) diagnostics that are a) visible to the user and b) overlap with the requested range.
However, it's fairly unclear what the client is allowed to do in populating this field. In particular, it's not clear that they have to give it back to us as we sent it to them. The spec also indicates that this field is unreliable, the doc for it says
They are provided so that the server knows which errors are currently presented to the user for the given range. There is no guarantee that these accurately reflect the error state of the resource. The primary parameter to compute code actions is the provided range.
(emphasis mine)
We rely on the context Diagnostics quite a bit:
- Various code actions that trigger off diagnostics inspect them
- This seems questionable since in most cases we want to trigger based on the objective fact about the code that triggers the diagnostic (e.g. "redundant import"), and not on whether this fact is presented to the user.
- In some places we compare them against existing diagnostics we know about
- This can go wrong if the client messes with the diagnostic, see e.g. 'Remove redundant imports' code action not showing in neovim 0.9 #3857
- If we actually care about this we should put identifying information in the
datafield.
Moreover, we don't really need to do this at all: we can just consult our diagnostic store and filter it based on the given range. This is more direct and simpler, and we should probably just do this in almost all cases. The exception would be cases where we actually do care about presentation: for example, the "suppress this warning" code action does care about whether the diagnostic it is proposing to suppress is in fact visible to the user. But this is an exceptional case.