Skip to content

Fix type guards throwing on null or undefined in protocol package #1940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Siddharth S (codeXsidd) wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Fix type guards throwing on null or undefined in protocol package #1940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7830fdb
fix: resolve issue #1939 prevent type guards throwing on null or unde…
codeXsidd Oct 31, 2026
d01f458
Merge branch 'main' into fix/issue-1939-text-document-filter-typeguards
codeXsidd Nov 1, 2026
a5adff7
Merge branch 'main' into fix/issue-1939-text-document-filter-typeguards
codeXsidd Nov 3, 2026
58228fc
Clean up imports in documentFilter.test.ts
codeXsidd Nov 7, 2026
2bbebf3
fix(test): close diagnostic test callback and clean up duplicate test…
codeXsidd Nov 7, 2026
93db526
Merge branch 'main' into fix/issue-1939-text-document-filter-typeguards
codeXsidd Nov 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion protocol/src/common/protocol.diagnostic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export type DiagnosticServerCancellationData = {
export namespace DiagnosticServerCancellationData {
export function is(value: any): value is DiagnosticServerCancellationData {
const candidate = value as DiagnosticServerCancellationData;
return candidate && Is.boolean(candidate.retriggerRequest);
return Is.objectLiteral(candidate) && Is.boolean(candidate.retriggerRequest);
}
}

Expand Down
2 changes: 1 addition & 1 deletion protocol/src/common/protocol.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1188,7 +1188,7 @@ export namespace WorkDoneProgressOptions {
}
export function hasWorkDoneProgress(value: any): value is { workDoneProgress: boolean } {
const candidate = value as WorkDoneProgressOptions;
return candidate && Is.boolean(candidate.workDoneProgress);
return Is.objectLiteral(candidate) && Is.boolean(candidate.workDoneProgress);
}
}

Expand Down
91 changes: 71 additions & 20 deletions protocol/src/node/test/documentFilter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,15 @@

import * as assert from 'assert';

import { DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter, RelativePattern, TextDocumentFilter, TextDocumentRegistrationOptions } from '../main';
import {
DiagnosticServerCancellationData, DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter,
RelativePattern, TextDocumentFilter, TextDocumentRegistrationOptions, WorkDoneProgressOptions
} from '../main';

suite('Document filter type guards', () => {

const relativePattern: RelativePattern = { baseUri: 'file:///workspace', pattern: '**/*.ipynb' };

suite('TextDocumentFilter.is', () => {
test('valid filters', () => {
assert.strictEqual(TextDocumentFilter.is('typescript'), true);
assert.strictEqual(TextDocumentFilter.is({ language: 'typescript' }), true);
assert.strictEqual(TextDocumentFilter.is({ scheme: 'file' }), true);
assert.strictEqual(TextDocumentFilter.is({ pattern: '**/*.ts' }), true);
});
test('null and undefined', () => {
assert.strictEqual(TextDocumentFilter.is(null), false);
assert.strictEqual(TextDocumentFilter.is(undefined), false);
});
});

suite('NotebookDocumentFilter.is', () => {
test('string pattern', () => {
assert.strictEqual(NotebookDocumentFilter.is({ pattern: '**/*.ipynb' }), true);
Expand Down Expand Up @@ -54,20 +44,81 @@ suite('Document filter type guards', () => {
});
});

suite('DocumentSelector.is', () => {
test('null and undefined elements', () => {
assert.strictEqual(DocumentSelector.is([null as any]), false);
assert.strictEqual(DocumentSelector.is([undefined as any]), false);
suite('TextDocumentFilter.is', () => {
test('string filter', () => {
assert.strictEqual(TextDocumentFilter.is('typescript'), true);
});
test('language filter', () => {
assert.strictEqual(TextDocumentFilter.is({ language: 'typescript' }), true);
});
test('scheme filter', () => {
assert.strictEqual(TextDocumentFilter.is({ scheme: 'file' }), true);
});
test('pattern filter with string pattern', () => {
assert.strictEqual(TextDocumentFilter.is({ pattern: '**/*.ts' }), true);
});
test('pattern filter with relative pattern', () => {
assert.strictEqual(TextDocumentFilter.is({ pattern: relativePattern }), true);
});
test('null and undefined', () => {
assert.strictEqual(TextDocumentFilter.is(null), false);
assert.strictEqual(TextDocumentFilter.is(undefined), false);
});
test('invalid values', () => {
assert.strictEqual(TextDocumentFilter.is({}), false);
assert.strictEqual(TextDocumentFilter.is(123), false);
assert.strictEqual(TextDocumentFilter.is(true), false);
assert.strictEqual(TextDocumentFilter.is({ pattern: 123 }), false);
});
});

suite('DocumentSelector.is', () => {
test('notebook cell filter with relative pattern', () => {
const documentSelector = [{ notebook: { pattern: relativePattern } }];
assert.strictEqual(DocumentSelector.is(documentSelector), true);
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector }), true);
});
test('TextDocumentRegistrationOptions.is with null and undefined', () => {
test('null and undefined elements', () => {
assert.strictEqual(DocumentSelector.is([null as any]), false);
assert.strictEqual(DocumentSelector.is([undefined as any]), false);
assert.strictEqual(DocumentSelector.is(['typescript', null as any]), false);
});
test('null and undefined selector', () => {
assert.strictEqual(DocumentSelector.is(null), false);
assert.strictEqual(DocumentSelector.is(undefined), false);
});
});

suite('TextDocumentRegistrationOptions.is', () => {
test('valid registration options', () => {
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: ['typescript'] }), true);
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: null }), true);
});
test('invalid registration options', () => {
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: [null] }), false);
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: [undefined] }), false);
});
test('null and undefined', () => {
assert.strictEqual(TextDocumentRegistrationOptions.is(null), false);
assert.strictEqual(TextDocumentRegistrationOptions.is(undefined), false);
assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: [null] }), false);
assert.strictEqual(TextDocumentRegistrationOptions.is(123), false);
});
});

suite('WorkDoneProgressOptions and DiagnosticServerCancellationData null handling', () => {
test('WorkDoneProgressOptions.hasWorkDoneProgress', () => {
assert.strictEqual(WorkDoneProgressOptions.hasWorkDoneProgress({ workDoneProgress: true }), true);
assert.strictEqual(WorkDoneProgressOptions.hasWorkDoneProgress({ workDoneProgress: false }), true);
assert.strictEqual(WorkDoneProgressOptions.hasWorkDoneProgress({}), false);
assert.strictEqual(WorkDoneProgressOptions.hasWorkDoneProgress(null), false);
assert.strictEqual(WorkDoneProgressOptions.hasWorkDoneProgress(undefined), false);
});
test('DiagnosticServerCancellationData.is', () => {
assert.strictEqual(DiagnosticServerCancellationData.is({ retriggerRequest: true }), true);
assert.strictEqual(DiagnosticServerCancellationData.is({ retriggerRequest: false }), true);
assert.strictEqual(DiagnosticServerCancellationData.is({}), false);
assert.strictEqual(DiagnosticServerCancellationData.is(null), false);
assert.strictEqual(DiagnosticServerCancellationData.is(undefined), false);
});
});
});