Shopify / ruby-lsp

An opinionated language server for Ruby
https://shopify.github.io/ruby-lsp/
MIT License
1.58k stars 155 forks source link

Syntax highlighting broken when a variable is inserted in a heredoc #2492

Open fletchto99 opened 2 months ago

fletchto99 commented 2 months ago

Description

Syntax highlighting is broken after the heredoc when a variable is inserted if it's not surrounded by double quotes. See the examples provided below. The only change was on line 6, I removed the " around the pr_number variable.

Ruby LSP Information

Ruby LSP Information

VS Code Version

1.92.2

Ruby LSP Extension Version

0.7.17

Ruby LSP Server Version

0.17.16

Ruby LSP Addons

Ruby Version

3.2.2

Ruby Version Manager

none

Installed Extensions

Click to expand - github-markdown-preview (0.3.0) - markdown-checkbox (0.4.0) - markdown-emoji (0.3.0) - markdown-footnotes (0.1.1) - markdown-mermaid (1.23.1) - markdown-preview-github-styles (2.0.4) - markdown-yaml-preamble (0.1.0) - markdown-table-prettify (3.6.0) - vscode-markdownlint (0.55.0) - languagetool-linter (0.21.4) - vscode-eslint (3.0.10) - prettier-vscode (11.0.0) - markdown-table-formatter (3.0.0) - copilot (1.223.0) - copilot-chat (0.18.2) - vscode-github-actions (0.26.3) - vscode-pull-request-github (0.94.0) - go (0.42.0) - vscode-docker (1.29.2) - debugpy (2024.10.0) - isort (2023.10.1) - python (2024.12.3) - vscode-pylance (2024.8.2) - azurecli (0.6.0) - vscode-typescript-next (5.7.20240825) - wordcount (0.1.0) - vsliveshare (1.0.5936) - vscode-xml (0.27.1) - vscode-yaml (1.15.0) - remove-tabs-on-save (1.2.4) - ruby-extensions-pack (0.1.11) - ruby-lsp (0.7.17) - vscode-fileutils (3.10.3) - sorbet-vscode-extension (0.3.36) - code-spell-checker (3.0.1) - vscode-stylelint (1.4.0) - shellcheck (0.37.1) - alex-linter (0.6.6) - write-good-linter (0.1.5) - markdown-all-in-one (3.6.2)

Ruby LSP Settings

Click to expand ##### Workspace ```json {} ``` ##### User ```json { "enableExperimentalFeatures": false, "enabledFeatures": { "codeActions": true, "diagnostics": true, "documentHighlights": true, "documentLink": true, "documentSymbols": true, "foldingRanges": true, "formatting": true, "hover": true, "inlayHint": true, "onTypeFormatting": true, "selectionRanges": true, "semanticHighlighting": true, "completion": true, "codeLens": true, "definition": true, "workspaceSymbol": true, "signatureHelp": true, "typeHierarchy": true }, "featuresConfiguration": {}, "rubyVersionManager": { "identifier": "none" }, "customRubyCommand": "", "formatter": "auto", "linters": null, "bundleGemfile": "", "testTimeout": 30, "branch": "", "pullDiagnosticsOn": "both", "useBundlerCompose": false, "bypassTypechecker": false, "rubyExecutablePath": "", "indexing": {}, "erbSupport": true } ```

Reproduction steps

Write a heredoc with a variable that isn't surrounded by dobule quotes.

Normal highlighting:

Image

Broken highlighting (but still valid ruby):

Image

snutij commented 2 days ago

Hi @fletchto99, do we still have this issue, even with the latest version of VS Code (v1.95) and the ruby-lsp extension (v0.8.10)? I'm asking because I can't reproduce the same behavior. If you still experience this, could you try the extension bisect debug to point out the faulty one?

code snippets ```ruby class DemoOK def check_automerge_status(pr_number) query = <<~GRAPHQL query { repository(owner: "foo", name: "bar") { pullRequest(number: "#{pr_number}") { isInMergeQueue } } } GRAPHQL response = client.post("/graphql", { query: query }.to_json) response[:data][:repository][:pullRequest][:isInMergeQueue] end end class DemoKO def check_automerge_status(pr_number) query = <<~GRAPHQL query { repository(owner: "foo", name: "bar") { pullRequest(number: #{pr_number}) { isInMergeQueue } } } GRAPHQL response = client.post("/graphql", { query: query }.to_json) response[:data][:repository][:pullRequest][:isInMergeQueue] end end ```

Image

vinistock commented 2 days ago

The problem here is the intersection of grammars. Inside a heredoc using the GRAPHQL delimiter, we mark the language as graphql so that if you have an extension providing a grammar for GraphQL, it will highlight it properly.

However, if you interpolate something in the middle, then it's likely that it will break the GraphQL grammar as it will try to understand the interpolation, but that's not valid GraphQL.

I'm not sure if we could use semantic highlighting to inform the editor that only the variable interpolation is Ruby source and the rest is still GraphQL source.

fletchto99 commented 2 days ago

Ahh I think I found the issue. I also had the GraphQL: Syntax Highlighting plugin enabled, which supports highlighting in ruby files. That's what seems to break the highlighting. I guess this would be a bug with the GraphQL Syntax highlighting plugin then?

With it enabled:

Image

With it disabled:

Image

vinistock commented 2 days ago

Does it support highlighting in Ruby files? Or does it register for the graphql language ID? If it's the former, then it's a bug in that extension. If it's the latter, then it's the issue I mentioned.

Inside the heredoc, the language ID is graphql, which means the grammar is applied, but a string interpolation is not valid GraphQL, which leads to the incorrect highlighting.

This will happen when using any heredoc with a language identifier and interpolation.

# Inside this block, the language ID is SQL. If you have an extension registered for it, it will
# attempt to run its grammar on this code. But the interpolation syntax #{} is not valid SQL code
# so that will lead to highlighting errors
<<~SQL
  SELECT #{some_var} FROM #{some_table}
SQL
SampsonCrowley commented 2 days ago

I don't even have a GraphQL extension installed whatsoever, but heredoc highlighting is severely broken with interpolation...

Thankfully it only seems to last for the duration of the line, but the areas within the doc aren't even recognized as being a string...

Image Image

vinistock commented 2 days ago

I don't even have a GraphQL extension installed whatsoever

A GraphQL extension wouldn't affect a SQL block since the language ID there is sql and not graphql. It's likely a SQL extension that provides the grammar for that.

I don't believe there's an easy way to improve this. We would need to make the text mate grammar, which is based on regexes, be able to parse Ruby code embedded in the middle of another language, identified by the heredoc delimiter.

Which would also mean, being able to parse with the grammar whatever language is identified by the delimiter (SQL, GraphQL or whatever). This would essentially mean trying to support every possible language someone might want to put in a heredoc in our grammar file, which does not scale. But beyond that, it doesn't actually fix the issue since there's runtime dependencies.

The problem here is not that the Ruby grammar we provide is failing to parse the heredoc. It's that the grammars responsible for the other language IDs don't expect the interpolation and therefore fail to highlight the content. And they have no way of understanding the interpolation since it may depend on runtime aspects.

Consider this

<<~SQL
  SELECT #{columns_i_want.reject { |c| IGNORED_COLUMNS.any?(c) }.join(",")} FROM users;
SQL

This SQL statement is only valid SQL code after the interpolation happens, which depends on runtime values. There's no way for a text mate grammar to process this and know with confidence if this is valid SQL or not.