[READY] Add support for DocumentSymbol in document outline requests

[bstaletic]

I'm not too happy about this.
Omnisharp LSP assumes clients support this.
We could, but it comes at the cost of manually flattening the hierarchy. That's what this pull request does.

Alternatives include:

• Implementing this, but not advetising the capability. That way, servers that assume client support for it work, but those that don't won't have a performance impact.
• Leave this mess out of the LanguageServerCompleter class and only add this hack into the LSP Omnisharp completer. This was done in [RFC] Switch C# completer to LSP #1752

EDIT: Switched to the first alternative, as discussed in gitter.

---

This change is 

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.92%. Comparing base (1026c83) to head (3027d22).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1757      +/-   ##
==========================================
+ Coverage   95.90%   95.92%   +0.01%     
==========================================
  Files          84       84              
  Lines        8450     8462      +12     
  Branches      163      163              
==========================================
+ Hits         8104     8117      +13     
+ Misses        296      295       -1     
  Partials       50       50              

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained

---

ycmd/completers/language_server/language_server_completer.py line 2659 at r1 (raw file):

    if any( 'range' in s for s in result ):
      LOGGER.debug( 'Hierarchical DocumentSymbol not supported.' )

I'm still logging this, to maybe catch another non-conforming server.

---

ycmd/tests/language_server/language_server_completer_test.py line 107 at r1 (raw file):

class LanguageServerCompleterTest( TestCase ):
  @IsolatedYcmd()
  def test_LanguageServerCompleter_DocumentSymbol_Hierarchical( self, app ):

This test was added just because we don't yet have a naughty server. Omnisharp would exercise this codepath properly.

[puremourning]

some bike shed comments included.

Reviewed 1 of 2 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: 1 of 2 LGTMs obtained (waiting on @bstaletic)

---

ycmd/completers/language_server/language_server_completer.py line 3433 at r2 (raw file):

  result = []
  for s in symbols:
    partial_results = [ s ]

if I' reading this right, partial_results doesn't do antything that results doesn't.

as in, is this not equivalent?

result = []
for s in symbols:
  result.append(s)
  result.extend(_FlattenDocumentSymbolHierarchy( s.get('children', [] ) ) )

maybe I just don't brain today.

---

ycmd/completers/language_server/language_server_completer.py line 3444 at r2 (raw file):

  """Convert a list of LSP DocumentSymbol into a YCM GoTo response"""

  def BuildGoToLocationFromSymbol( symbol ):

This is so similar to the same def in _SymbolListToGoTo that the diff algorithm confused the hell out of me.

Perhaps it's time to pull this out to a top-level function and save some duplication?

---

ycmd/tests/language_server/language_server_completer_test.py line 107 at r1 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

This test was added just because we don't yet have a naughty server. Omnisharp would exercise this codepath properly.

nice. integration tests are great, but unit tests have their place too!

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

---

ycmd/completers/language_server/language_server_completer.py line 3433 at r2 (raw file):

Previously, puremourning (Ben Jackson) wrote…

if I' reading this right, partial_results doesn't do antything that results doesn't.

as in, is this not equivalent?

result = []
for s in symbols:
  result.append(s)
  result.extend(_FlattenDocumentSymbolHierarchy( s.get('children', [] ) ) )

maybe I just don't brain today.

You're right. My brain just prefers to work with partial results when writing a recursive algorithm.

Fixed.

---

ycmd/completers/language_server/language_server_completer.py line 3444 at r2 (raw file):

Previously, puremourning (Ben Jackson) wrote…

This is so similar to the same def in _SymbolListToGoTo that the diff algorithm confused the hell out of me.

Perhaps it's time to pull this out to a top-level function and save some duplication?

Yes, the diff was confused.

Hmm... if I pull that out, then _DocumentSymbolListToGoTo and _SymbolInfoListToGoTo are exactly the same.

Please double check my implementation and the specs, because workspace symbol is also quite similar.

I did deduplicate, like you suggested.

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

---

ycmd/completers/language_server/language_server_completer.py line 3444 at r2 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

Yes, the diff was confused.

Hmm... if I pull that out, then _DocumentSymbolListToGoTo and _SymbolInfoListToGoTo are exactly the same.

Please double check my implementation and the specs, because workspace symbol is also quite similar.

I did deduplicate, like you suggested.

Well, all the tests are passing, which gives me more confidence in the changes.

[bstaletic]

Reviewable status: 0 of 2 LGTMs obtained (and 1 stale) (waiting on @puremourning)

---

ycmd/completers/language_server/language_server_completer.py line 3444 at r2 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

Well, all the tests are passing, which gives me more confidence in the changes.

I have rechecked the LSP specs. This implementation is fine.

[bstaletic]

I'll merge this, to progress the C# stuff.