WIP Fix HandleLSPDiagnostics buffer match logic.

[hsanson]

To find the buffer corresponding to URIs reported by LSP the
HandleLSPDiagnostics() method uses the built-in bufnr() function. From
the documentation we learn that the first parameter of bufnr() is
an expression, not a path.

EclipseLSP will report project wide errors (e.g. gradle errors) that are
not related to any actual source file with an URI that corresponds to the
project root folder, e.g:

file:///home/username/Projects/gradle-simple

This URI will match any open buffer of files within the project root
hiearchy, thus project-wide errors appear as part of every file within
the project, e.g:

file:///home/username/Projects/gradle-simple/src/main/java/Hello.java

To fix this, this MR adds '^' to the beginning and '$' at the end of the
URI path to force an exact match. This is how is recommended in vim
help (see :h bufname).

Where are the tests? Have you added tests? Have you updated the tests? Read the
comment above and the documentation referenced in it first. Write tests!

Seriously, read :help ale-dev and write tests.

[w0rp]

Looks good. Add a test for this and apply this to the tsserver function too.

[hsanson]

@w0rp I have modified the TS diagnostics function too to force exact match of files. Regarding tests for this I require some guidance since the s:HandleLSPDiagnostics() method is private to the lsp_linter.vim script, thus cannot be directly tested without renaming it.

Another way would be to modify the ale#lsp#response#ReadDiagnostics() function to do the buffer resolution and return it as part of the l:loclist. This would allow us to test the buffer resolution by simple modifications to the test_read_lsp_diagnostics.vader test. Not sure if this would break a lot of other things.

[w0rp]

Have a look at test/test_engine_lsp_response_handling.vader. There are a few tests there, mostly for tsserver, which call the callback function which calls the two script functions. You can add a test to ensure that results for directories don't populate the loclist for a filename in a directory.

[hsanson]

@w0rp I am having some issues getting one of my LSP tests to pass:

Execute(LSP diagnostics responses on project root should not populate loclist):
  let b:ale_linters = ['eclipselsp']
  runtime ale_linters/java/eclipselsp.vim
  call ale#test#SetFilename('filename.java')
  call ale#engine#InitBufferInfo(bufnr(''))
  call ale#lsp_linter#SetLSPLinterMap({'1': 'eclipselsp'})

  call ale#lsp_linter#HandleLSPResponse(1, {
  \ 'jsonrpc':'2.0',
  \ 'method':'textDocument/publishDiagnostics',
  \ 'params': {
  \     'uri':'file://' . g:dir,
  \     'diagnostics': [
  \        {
  \          'range': {
  \             'start': {
  \               'line': 0,
  \               'character':0
  \             },
  \             'end': {
  \               'line': 0,
  \               'character':0
  \             }
  \          },
  \          'severity': 2,
  \          'code': "",
  \          'source': 'Java',
  \          'message': 'Missing JRE 1-8'
  \       }
  \     ]
  \  }
  \})

  AssertEqual
  \ [
  \ ],
  \ ale#test#GetLoclistWithoutModule()

I have verified the LSP linter works as expected in vim/nvim (e.g. the project wide errors do not appear on the loclist). But in the test the loclist seems to be always populated with the error.

I have verified that the "l:bufffer" variable is indeed set to "-1", therefore the "empty(l:info)" condition is true and the s:HandleLSPDiagnostics() method returns at that point:

function! s:HandleLSPDiagnostics(conn_id, response) abort
    let l:linter_name = s:lsp_linter_map[a:conn_id]
    let l:filename = ale#path#FromURI(a:response.params.uri)
    let l:buffer = bufnr('^' . l:filename . '$')
    let l:info = get(g:ale_buffer_info, l:buffer, {})

    if empty(l:info)
        return                        <--- returns here
    endif

   ....

Not sure what the problem could be here but it seems to affect only the test.

[w0rp]

I'll see if I can fix the test today.

[w0rp]

I've fixed the tests now. Data in tests wasn't being set up and torn down properly in a few places. The tests should now pass when you run them in your editor with :Vader as well.

[w0rp]

Cheers! 🍻