Skip to content

Commit f91d77f

Browse files
authored
debt - make vscode.executeDocumentSymbolProvider very correct wrt what it returns (microsoft#251190)
1 parent d1d4260 commit f91d77f

File tree

3 files changed

+61
-33
lines changed

3 files changed

+61
-33
lines changed

src/vs/workbench/api/common/extHostApiCommands.ts

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -41,28 +41,21 @@ const newCommands: ApiCommand[] = [
4141
if (isFalsyOrEmpty(value)) {
4242
return undefined;
4343
}
44-
class MergedInfo extends types.SymbolInformation implements vscode.DocumentSymbol {
45-
static to(symbol: languages.DocumentSymbol): MergedInfo {
46-
const res = new MergedInfo(
47-
symbol.name,
48-
typeConverters.SymbolKind.to(symbol.kind),
49-
symbol.containerName || '',
50-
new types.Location(apiArgs[0], typeConverters.Range.to(symbol.range))
51-
);
52-
res.detail = symbol.detail;
53-
res.range = res.location.range;
54-
res.selectionRange = typeConverters.Range.to(symbol.selectionRange);
55-
res.children = symbol.children ? symbol.children.map(MergedInfo.to) : [];
56-
return res;
57-
}
5844

59-
detail!: string;
60-
range!: vscode.Range;
61-
selectionRange!: vscode.Range;
62-
children!: vscode.DocumentSymbol[];
63-
override containerName: string = '';
45+
function wrap(symbol: languages.DocumentSymbol): types.SymbolInformationAndDocumentSymbol {
46+
return new types.SymbolInformationAndDocumentSymbol(
47+
symbol.name,
48+
typeConverters.SymbolKind.to(symbol.kind),
49+
symbol.detail,
50+
symbol.containerName || '',
51+
apiArgs[0],
52+
typeConverters.Range.to(symbol.range),
53+
typeConverters.Range.to(symbol.selectionRange),
54+
symbol.children ? symbol.children.map(wrap) : []
55+
);
6456
}
65-
return value.map(MergedInfo.to);
57+
58+
return value.map(wrap);
6659

6760
})
6861
),

src/vs/workbench/api/common/extHostTypes.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,19 +1383,8 @@ export class SymbolInformation {
13831383
}
13841384
}
13851385

1386-
@es5ClassCompat
1387-
export class DocumentSymbol {
1388-
1389-
static validate(candidate: DocumentSymbol): void {
1390-
if (!candidate.name) {
1391-
throw new Error('name must not be falsy');
1392-
}
1393-
if (!candidate.range.contains(candidate.selectionRange)) {
1394-
throw new Error('selectionRange must be contained in fullRange');
1395-
}
1396-
candidate.children?.forEach(DocumentSymbol.validate);
1397-
}
13981386

1387+
abstract class AbstractDocumentSymbol {
13991388
name: string;
14001389
detail: string;
14011390
kind: SymbolKind;
@@ -1411,11 +1400,56 @@ export class DocumentSymbol {
14111400
this.range = range;
14121401
this.selectionRange = selectionRange;
14131402
this.children = [];
1403+
}
1404+
}
1405+
1406+
@es5ClassCompat
1407+
export class DocumentSymbol extends AbstractDocumentSymbol {
1408+
1409+
static validate(candidate: DocumentSymbol): void {
1410+
if (!candidate.name) {
1411+
throw new Error('name must not be falsy');
1412+
}
1413+
if (!candidate.range.contains(candidate.selectionRange)) {
1414+
throw new Error('selectionRange must be contained in fullRange');
1415+
}
1416+
candidate.children?.forEach(DocumentSymbol.validate);
1417+
}
14141418

1419+
constructor(name: string, detail: string, kind: SymbolKind, range: Range, selectionRange: Range) {
1420+
super(name, detail, kind, range, selectionRange);
14151421
DocumentSymbol.validate(this);
14161422
}
1423+
1424+
static override[Symbol.hasInstance](candidate: unknown): boolean {
1425+
if (!isObject(candidate)) {
1426+
throw new TypeError();
1427+
}
1428+
return candidate instanceof AbstractDocumentSymbol
1429+
|| candidate instanceof SymbolInformationAndDocumentSymbol;
1430+
}
14171431
}
14181432

1433+
// This is a special type that's used from the `vscode.executeDocumentSymbolProvider` API
1434+
// command which implements both shapes, vscode.SymbolInformation _and_ vscode.DocumentSymbol
1435+
export class SymbolInformationAndDocumentSymbol extends SymbolInformation implements vscode.DocumentSymbol {
1436+
1437+
detail: string;
1438+
range: vscode.Range;
1439+
selectionRange: vscode.Range;
1440+
children: vscode.DocumentSymbol[];
1441+
override containerName: string;
1442+
1443+
constructor(name: string, kind: vscode.SymbolKind, detail: string, containerName: string, uri: URI, range: Range, selectionRange: Range, children?: SymbolInformationAndDocumentSymbol[]) {
1444+
super(name, kind, containerName, new Location(uri, range));
1445+
1446+
this.containerName = containerName;
1447+
this.detail = detail;
1448+
this.range = range;
1449+
this.selectionRange = selectionRange;
1450+
this.children = children ?? [];
1451+
}
1452+
}
14191453

14201454
export enum CodeActionTriggerKind {
14211455
Invoke = 1,

src/vs/workbench/api/test/browser/extHostApiCommands.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,8 +774,9 @@ suite('ExtHostLanguageFeatureCommands', function () {
774774
assert.strictEqual(values.length, 2);
775775
const [first, second] = values;
776776
assert.strictEqual(first instanceof types.SymbolInformation, true);
777-
assert.strictEqual(first instanceof types.DocumentSymbol, false);
777+
assert.strictEqual(first instanceof types.DocumentSymbol, true);
778778
assert.strictEqual(second instanceof types.SymbolInformation, true);
779+
assert.strictEqual(second instanceof types.DocumentSymbol, true);
779780
assert.strictEqual(first.name, 'DocumentSymbol');
780781
assert.strictEqual(first.children.length, 1);
781782
assert.strictEqual(second.name, 'SymbolInformation');

0 commit comments

Comments
 (0)