Skip to content

Commit

Permalink
fix(extension/#3787): Fix completion item parsing resiliency (#3800)
Browse files Browse the repository at this point in the history
Fix #3787 

__Issue:__ Completion items were coming with `null` value in the JSON, for example: 
```
{
...
'c': null,
...
}
```
(Note that the completion item JSON format uses a condensed field name to minimize data over the wire, see: https://github.com/onivim/vscode-exthost/blob/fc0cb0ba738b856da6ab36d0efe0aa3446959a98/src/vs/workbench/api/common/extHost.protocol.ts#L1383)

This was causing parsing to fail.

__Defect:__ Our parser was expecting the `detail` field (along with others) to either be present as a string or not present at all. The parser wasn't resilient in the case where the field was there, but `null`.

__Fix:__ Instead of using `field.optional`, use `field.withDefault` along with a `None` default value.

__Future work:__
Potentially `field.optional` should just work like this out-of-the-box...

__Todo:__
- [x] Changes entry
  • Loading branch information
bryphe authored Aug 23, 2021
1 parent 07b08c9 commit fda6bf7
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGES_CURRENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
### Bug Fixes

- #3791 - SCM: Fix 'Git: Checkout to' command (fixes #2525)
- #3800 - Completion: Fix completion item parsing resiliency (fixes #3787)

### Performance

Expand Down
25 changes: 19 additions & 6 deletions src/Exthost/SuggestItem.re
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,25 @@ let sortText = ({sortText, label, _}) => {
};
};

module SuggestItemLabel = {
type t = string;

let decode =
Json.Decode.(
one_of([
("string", string),
("record", obj(({field, _}) => {field.required("label", string)})),
])
);
};

module Dto = {
let decode = (~defaultRange) => {
Json.Decode.(
obj(({field, _}) => {
// These fields come from the `ISuggestDataDtoField` definition:
// https://github.com/onivim/vscode-exthost/blob/50bef147f7bbd250015361a4e3cad3305f65bc27/src/vs/workbench/api/common/extHost.protocol.ts#L1089
let label = field.required("a", string);
let label = field.required("a", SuggestItemLabel.decode);

let kind =
field.withDefault(
Expand All @@ -147,11 +159,12 @@ module Dto = {
),
);

let detail = field.optional("c", string);
let documentation = field.optional("d", MarkdownString.decode);
let sortText = field.optional("e", string);
let filterText = field.optional("f", string);
let insertText = field.optional("h", string);
let detail = field.withDefault("c", None, nullable(string));
let documentation =
field.withDefault("d", None, nullable(MarkdownString.decode));
let sortText = field.withDefault("e", None, nullable(string));
let filterText = field.withDefault("f", None, nullable(string));
let insertText = field.withDefault("h", None, nullable(string));
let insertTextRules =
field.withDefault(
"i",
Expand Down

0 comments on commit fda6bf7

Please sign in to comment.