Handle empty object capabilities as truthy where applicable

[bstaletic]

Another one of those "be vscode plox" things.
Servers seem to expect things like

{
	"capabilities": {
		"whateverProvider": {}
	}
}

to mean that the server is really a provider of whatever. Earliest offender is the swift server being a code action provider.

Please take extra care wen reviewing this thing.

Unresolved things:

1. Are we correctly handling textDocumentSync?
2. Type hierarchy needs to get the same treatment once Implement simple type hierarchy support #1704 is accepted.

---

This change is 

[puremourning]

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

---

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

def _IsCapabilityProvided( capabilities, query ):
  capability = capabilities.get( query )
  return bool( capability or capability == {} )

this looks a little odd, the RHS of or is already a bool, so I think bool( capability ) or capability == {} maybe.

But perhaps what we're really saying is that we should return True if:

• the query is in the dict
• the value of query in the dict is not False or None (json null)

Interestingly, not all providers are allowed to be False, for example:

	 * The server provides code lens.
	 */
	codeLensProvider?: CodeLensOptions;

This may be:

• not supplied (return False)
• null/None (return False)
• a CodeLensOptions object (return True)

Wheres:

	codeActionProvider?: boolean | CodeActionOptions;

This may be:

• not supplied (return False)
• null/None (return False)
• True (return True)
• False (return False)
• a CodeActionOptions object (return True)

sadly, we may need a third argument to this function allow_boolean which says if True/False is valid for the entry in the spec.

something like

def _IsCapabilityProvided( c, q, allow_boolean=True ):
   if not q in c: return False
   v = c[q]
   if type( v ) == bool:
     if not allow_boolean: return False
     return v
   if type( v ) == dict:
     return True
   return False

maybe maybe?

[bstaletic]

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

---

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

bool( capability ) or capability == {}

That is what I intended to write!

I definitely was not considering getting a bool where it is not expected. Or rather I was and thought it would be fine to accept True even against the protocol.
Yes, that's questionable.

That last suggestion of yours seems expensive. For semantic tokens and signature help we query capabilities on every request. Maybe we need to start being careful about those?

The following should cover all cases "by the book" without unnecessary type comparisons and dict lookups:

def _IsCapabilityProvided( c, q, allow_boolean=True ):
    v = c.get( q )
    if v is None: return False
    t = type( v )
    if t == bool:
        return v if allow_boolean else False
    return t == dict

[puremourning]

Reviewable status: 0 of 2 LGTMs obtained (waiting on @bstaletic)

---

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

Previously, bstaletic (Boris Staletic) wrote…

bool( capability ) or capability == {}

That is what I intended to write!

I definitely was not considering getting a bool where it is not expected. Or rather I was and thought it would be fine to accept True even against the protocol.
Yes, that's questionable.

That last suggestion of yours seems expensive. For semantic tokens and signature help we query capabilities on every request. Maybe we need to start being careful about those?

The following should cover all cases "by the book" without unnecessary type comparisons and dict lookups:

def _IsCapabilityProvided( c, q, allow_boolean=True ):
    v = c.get( q )
    if v is None: return False
    t = type( v )
    if t == bool:
        return v if allow_boolean else False
    return t == dict

I think you're right. Just fix the bool() expression in your first version and keep it simple. The pedantic-but-correct version is harder to follow, harder to explain and just a bit ugly :D

[bstaletic]

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

---

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

Previously, puremourning (Ben Jackson) wrote…

I think you're right. Just fix the bool() expression in your first version and keep it simple. The pedantic-but-correct version is harder to follow, harder to explain and just a bit ugly :D

Done.

[puremourning]

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

[bstaletic]

CI is green. I am happy with this PR. Codecov got rate limited again.
@puremourning I'm only not merging this in case you wish to inspect the latest changes.

[puremourning]

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

[mergify]

Thanks for sending a PR!