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

Request textDocument/documentHighlight failed #678

Open
yoshinori-ma opened this issue Aug 23, 2023 · 6 comments
Open

Request textDocument/documentHighlight failed #678

yoshinori-ma opened this issue Aug 23, 2023 · 6 comments

Comments

@yoshinori-ma
Copy link

I am using the vscode-solargraph extension. When I hover over code in the editor, I receive the following error:

[Error - 15:18:20] Request textDocument/documentHighlight failed.
  Message: [ArgumentError] dependency name must be a String, was nil
  Code: -32603

I also encountered an error when running solargraph scan
Upon investigating the error message, I traced it back to this line in the Solargraph codebase: yard_map.rb#L285. It appears that the name becomes nil and causes the program to crash.
I've noticed that the issue occurs when requiring a path with a leading /, like so: require '/app/spec/hoge'.

environments

$ code -v 
1.81.1
6c3e3dba23e8fadc360aed75ce363ba185c49794
arm64
$ solargraph -v
0.49.0
@r-glebov
Copy link

r-glebov commented Nov 3, 2023

Yeah exactly my case, fixed it locally with this updated line in a yard_map.rb:

name = path.split('/').first.empty? ? path.split('/')[1] : path.split('/').first

@r-glebov
Copy link

r-glebov commented Nov 3, 2023

Hopefully we'll have a fix in upcoming versions, so we don't need to have a patched version of the gem.

@castwide
Copy link
Owner

castwide commented Nov 4, 2023

Can anyone provide a reproducible example? I'm totally okay with making this change, but I'd like to have a better understanding of the root cause.

@r-glebov
Copy link

r-glebov commented Nov 6, 2023

I can share an example code, which didn't work with current Solargraph version until I fixed that myself.

require 'pathname'

module SomeApp
  ROOT = Pathname.pwd.freeze

  class << self
    def path(*args)
      root.join(*args)
    end

    def root
      ROOT
    end
end

And then I require any file in any part of the project this way:

require SomeApp.path('path', 'to', 'file')

That doesn't work with Solargraph because generated path looks like /app/path/to/file, and path.split('/').first in this case returns empty string as a first result of a split.

@castwide
Copy link
Owner

Thanks, @r-glebov.

I took a slightly different approach here. YardMap should only get documentation for external gem dependencies, so absolute paths should get ignored.

The fix is on master and will be included in the next release.

(Slightly related: #675)

@castwide
Copy link
Owner

castwide commented Dec 5, 2023

Released in v0.50.0.

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

No branches or pull requests

3 participants