`prepareRename` response `{ defaultBehavior: true }` is treated the same as `null`

[ndonfris]

The LSP 3.17 spec defines distinct semantics for null vs { defaultBehavior: true } from textDocument/prepareRename:

If { defaultBehavior: boolean } is returned (since 3.16) the rename position is valid and the client should use its default behavior to compute the rename range. If null is returned then it is deemed that a textDocument/rename request is not valid at the given position.

From what I can tell, the current implementation of textDocument/prepareRename packaged in the method client.renameProvider() which treats both null and { defaultBehavior: true } as the same case (i.e., the rename position is invalid which skips textDocument/rename requests from being sent).

Tip

For the time being, I got around the unexpected behavior of { defaultBehavior: true } being treated as null by returning the range and placeholder values passed into textDocument/prepareRename

To make the use case of { defaultBehavior: true } consistent across language-clients, textDocument/prepareRename should be able to determine this case is a valid rename position for clients who use the vscode-languageclient package

Prior discussion of this issue occurred in coc.nvim issue #5583, which importantly points out that coc.nvim's design mirrors the behavior of the client source code in this repo. This makes the fix merged in coc.nvim PR #5591 very easy to translate to this repo, in file client/src/common/rename.ts#L100-L102:

} else if (this.isDefaultBehavior(result)) {
-  return result.defaultBehavior === true ? null : Promise.reject(new Error(`The element can't be renamed.`))
+  return result.defaultBehavior === true ? result : Promise.reject(new Error(`The element can't be renamed.`))
}

I submitted #1748 to reflect this change + I added some new tests for verifing the behavior of textDocument/prepareRename the /client & /client-node-tests for the new rename related features.

Although, I should probably mention that I don't normally use a language client that provides its own defaultBehavior rename provider (neovim user), so maybe the confusion I've encountered and my purposed fix isn't needed here. In which case, I still think it might be beneficial to allow more distinction between defaultBehavior: true vs null prepareRename request responses

Happy to make any adjustments to the PR if needed, just let me know!

Thanks for all the work y'all maintainers have put into this project over the years! It's amazing