Microsoft/vscode
1
0
Fork 0
mirror of https://github.com/microsoft/vscode.git synced 2025-12-24 12:19:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use readonly arrays for the vscode.DiagnosticCollection api

Browse Source 
## Problem
The diagnostic collection object is set up so that it does not mutate the arrays of diagnostics you pass to it. It also does not expect or allow mutation of diagnostics that it returns.

However it it currently typed using normal arrays. This means that if an extension (such as JS/TS) wishes to use readonly diagnostics intnernally, it cannot do so without casting.

## Proposed Fix
Use `ReadonlyArray` in diagnostic collection. This should be a safe change for the `set` type methods. The changes to `get` and `forEach` have the risk of breaking the typing of some extensions, but `get` already returned a frozen array of diagnostic so trying to mutate the array itself would have resulted in runtime error.
This commit is contained in:
Matt Bierner
2019-06-07 11:41:33 -07:00
parent dc2245f164
commit 8448512143
4 changed files with 20 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
extensions/typescript-language-features/src/features/diagnostics.ts
View File

@@ -208,7 +208,7 @@ export class DiagnosticsManager extends Disposable {
public configFileDiagnosticsReceived(
file: vscode.Uri,
diagnostics: vscode.Diagnostic[]
diagnostics: ReadonlyArray<vscode.Diagnostic>
): void {
this._currentDiagnostics.set(file, diagnostics);
}
@@ -218,7 +218,7 @@ export class DiagnosticsManager extends Disposable {
this._diagnostics.delete(resource);
}
public getDiagnostics(file: vscode.Uri): vscode.Diagnostic[] {
public getDiagnostics(file: vscode.Uri): ReadonlyArray<vscode.Diagnostic> {
return this._currentDiagnostics.get(file) || [];
}

8
src/vs/vscode.d.ts vendored
View File

@@ -4354,7 +4354,7 @@ declare module 'vscode' {
* @param uri A resource identifier.
* @param diagnostics Array of diagnostics or `undefined`
*/
set(uri: Uri, diagnostics: Diagnostic[] | undefined): void;
set(uri: Uri, diagnostics: ReadonlyArray<Diagnostic> | undefined): void;
/**
* Replace all entries in this collection.
@@ -4366,7 +4366,7 @@ declare module 'vscode' {
*
* @param entries An array of tuples, like `[[file1, [d1, d2]], [file2, [d3, d4, d5]]]`, or `undefined`.
*/
set(entries: [Uri, Diagnostic[] | undefined][]): void;
set(entries: ReadonlyArray<[Uri, ReadonlyArray<Diagnostic> | undefined]>): void;
/**
* Remove all diagnostics from this collection that belong
@@ -4388,7 +4388,7 @@ declare module 'vscode' {
* @param callback Function to execute for each entry.
* @param thisArg The `this` context used when invoking the handler function.
*/
forEach(callback: (uri: Uri, diagnostics: Diagnostic[], collection: DiagnosticCollection) => any, thisArg?: any): void;
forEach(callback: (uri: Uri, diagnostics: ReadonlyArray<Diagnostic>, collection: DiagnosticCollection) => any, thisArg?: any): void;
/**
* Get the diagnostics for a given resource. *Note* that you cannot
@@ -4397,7 +4397,7 @@ declare module 'vscode' {
* @param uri A resource identifier.
* @returns An immutable array of [diagnostics](#Diagnostic) or `undefined`.
*/
get(uri: Uri): Diagnostic[] | undefined;
get(uri: Uri): ReadonlyArray<Diagnostic> | undefined;
/**
* Check if this collection contains diagnostics for a

22
src/vs/workbench/api/common/extHostDiagnostics.ts
View File

@@ -47,9 +47,9 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
return this._name;
}
set(uri: vscode.Uri, diagnostics: vscode.Diagnostic[]): void;
set(entries: [vscode.Uri, vscode.Diagnostic[]][]): void;
set(first: vscode.Uri | [vscode.Uri, vscode.Diagnostic[]][], diagnostics?: vscode.Diagnostic[]) {
set(uri: vscode.Uri, diagnostics: ReadonlyArray<vscode.Diagnostic>): void;
set(entries: ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic>]>): void;
set(first: vscode.Uri | ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic>]>, diagnostics?: ReadonlyArray<vscode.Diagnostic>) {
if (!first) {
// this set-call is a clear-call
@@ -167,7 +167,7 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
this._proxy.$clear(this._owner);
}
forEach(callback: (uri: URI, diagnostics: vscode.Diagnostic[], collection: DiagnosticCollection) => any, thisArg?: any): void {
forEach(callback: (uri: URI, diagnostics: ReadonlyArray<vscode.Diagnostic>, collection: DiagnosticCollection) => any, thisArg?: any): void {
this._checkDisposed();
this._data.forEach((value, key) => {
const uri = URI.parse(key);
@@ -175,11 +175,11 @@ export class DiagnosticCollection implements vscode.DiagnosticCollection {
});
}
get(uri: URI): vscode.Diagnostic[] {
get(uri: URI): ReadonlyArray<vscode.Diagnostic> {
this._checkDisposed();
const result = this._data.get(uri.toString());
if (Array.isArray(result)) {
return <vscode.Diagnostic[]>Object.freeze(result.slice(0));
return <ReadonlyArray<vscode.Diagnostic>>Object.freeze(result.slice(0));
}
return [];
}
@@ -278,10 +278,10 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape {
return result;
}
getDiagnostics(resource: vscode.Uri): vscode.Diagnostic[];
getDiagnostics(): [vscode.Uri, vscode.Diagnostic[]][];
getDiagnostics(resource?: vscode.Uri): vscode.Diagnostic[] | [vscode.Uri, vscode.Diagnostic[]][];
getDiagnostics(resource?: vscode.Uri): vscode.Diagnostic[] | [vscode.Uri, vscode.Diagnostic[]][] {
getDiagnostics(resource: vscode.Uri): ReadonlyArray<vscode.Diagnostic>;
getDiagnostics(): ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic>]>;
getDiagnostics(resource?: vscode.Uri): ReadonlyArray<vscode.Diagnostic> | ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic>]>;
getDiagnostics(resource?: vscode.Uri): ReadonlyArray<vscode.Diagnostic> | ReadonlyArray<[vscode.Uri, ReadonlyArray<vscode.Diagnostic>]> {
if (resource) {
return this._getDiagnostics(resource);
} else {
@@ -302,7 +302,7 @@ export class ExtHostDiagnostics implements ExtHostDiagnosticsShape {
}
}
private _getDiagnostics(resource: vscode.Uri): vscode.Diagnostic[] {
private _getDiagnostics(resource: vscode.Uri): ReadonlyArray<vscode.Diagnostic> {
let res: vscode.Diagnostic[] = [];
this._collections.forEach(collection => {
if (collection.has(resource)) {

6
src/vs/workbench/test/electron-browser/api/extHostDiagnostics.test.ts
View File

@@ -91,18 +91,18 @@ suite('ExtHostDiagnostics', () => {
new Diagnostic(new Range(0, 0, 1, 1), 'message-2')
]);
let array = collection.get(URI.parse('foo:bar'));
let array = collection.get(URI.parse('foo:bar')) as Diagnostic[];
assert.throws(() => array.length = 0);
assert.throws(() => array.pop());
assert.throws(() => array[0] = new Diagnostic(new Range(0, 0, 0, 0), 'evil'));
collection.forEach((uri, array) => {
collection.forEach((uri, array: Diagnostic[]) => {
assert.throws(() => array.length = 0);
assert.throws(() => array.pop());
assert.throws(() => array[0] = new Diagnostic(new Range(0, 0, 0, 0), 'evil'));
});
array = collection.get(URI.parse('foo:bar'));
array = collection.get(URI.parse('foo:bar')) as Diagnostic[];
assert.equal(array.length, 2);
collection.dispose();