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

Fix Ruby LSP formatting range to comply with LSP specification #2438

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

vitallium
Copy link
Contributor

Motivation

Hi! While working on the Ruby LSP extension for the IntelliJ platform, I noticed an issue (or not?) with the formatting response from the Ruby Language Server. Thanks!

Implementation

The current implementation of formatting
does not use the proper range for a response.
Consider the following Ruby code:

class A
  def my_method
   [1]
  end
end

Let's pretend that our RuboCop configuration specifies omitting whitespaces inside array declarations, which means the code above is not properly formatted.
The code should be formatted like this:

class A
  def my_method
   [ 1 ]
  end
end

When a client asks Ruby LSP to format the first code, the server sends a response (taken from the Zed editor) where the reported end property of the Range interface has line and character keys that do not match the source document.
Both of them equal the character size of the document.

{
   "id":6,
   "result":[
      {
         "range":{
            "start":{
               "line":0,
               "character":0
            },
            "end":{
               "line":43,
               "character":43
            }
         },
         "newText":"class A\n  def my_method\n    [ 1 ]\n  end\nend\n"
      }
   ],
   "jsonrpc":"2.0"
}

For some reason, Visual Studio Code considers this response valid, and the code is formatted properly.
However, some editors perform a check to ensure that the reported response from Ruby LSP is not out of bounds of the current text document.
According to the LSP specification from the link below: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range A range in a text document expressed as (zero-based) start and end positions. The correct response should be:

{
   "id":35,
   "result":[
      {
         "range":{
            "start":{
               "line":0,
               "character":0
            },
            "end":{
               "line":5,
               "character":0
            }
         },
         "newText":"class A\n  def my_method\n    [ 1 ]\n  end\nend\n"
      }
   ],
   "jsonrpc":"2.0"
}

Automated Tests

I am happy to add unit tests for that, but I am not sure where I should start. I didn't find any tests regarding testing requests/responses in accordance with LSP specs.

Manual Tests

Unfortunately, there is no way to test this in Visual Studio Code because formatting, for some reason, works just fine there.

The current implementation of formatting
does not use the proper range for a response.
Consider the following Ruby code:

```ruby
class A
  def my_method
   [1]
  end
end
```

Let's pretend that our RuboCop configuration specifies
omitting whitespaces inside array declarations, which
means the code above is not properly formatted.
The code should be formatted like this:

```ruby
class A
  def my_method
   [ 1 ]
  end
end
```

When a client asks Ruby LSP to format the first code, the server
sends a response (taken from the Zed editor) where the reported
`end` property of the `Range` interface has `line` and `character` keys
that do not match the source document.
Both of them equal the character size of the document.

```json
{
   "id":6,
   "result":[
      {
         "range":{
            "start":{
               "line":0,
               "character":0
            },
            "end":{
               "line":43,
               "character":43
            }
         },
         "newText":"class A\n  def my_method\n    [ 1 ]\n  end\nend\n"
      }
   ],
   "jsonrpc":"2.0"
}
```

For some reason, Visual Studio Code considers this response valid,
and the code is formatted properly.
However, some editors perform a check to ensure that
the reported response from Ruby LSP is not out of bounds
of the current text document.
According to the LSP specification from the link below:
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#range
A range in a text document expressed as (zero-based) start and end positions.
The correct response should be:

```json
{
   "id":35,
   "result":[
      {
         "range":{
            "start":{
               "line":0,
               "character":0
            },
            "end":{
               "line":5,
               "character":0
            }
         },
         "newText":"class A\n  def my_method\n    [ 1 ]\n  end\nend\n"
      }
   ],
   "jsonrpc":"2.0"
}
```
@vitallium vitallium requested a review from a team as a code owner August 13, 2024 16:44
@vitallium vitallium requested review from andyw8 and vinistock August 13, 2024 16:44
@vinistock vinistock added bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes labels Aug 28, 2024
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

@vinistock vinistock merged commit 16d418f into Shopify:main Aug 28, 2024
20 of 22 checks passed
@vitallium
Copy link
Contributor Author

@vinistock thanks a lot for finishing it! I’ve been a bit slow recently in updating my contributions.

@vitallium vitallium deleted the vs/fix-lsp-range-formatting branch August 29, 2024 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR will fix an existing bug server This pull request should be included in the server gem's release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants