From 7830fdb7a793ba1f5d1aa01b90861d4476644b8c Mon Sep 17 00:00:00 2001 From: codeXsidd Date: Sat, 31 Oct 2026 22:12:12 +0530 Subject: [PATCH 1/3] fix: resolve issue #1939 prevent type guards throwing on null or undefined --- protocol/src/common/protocol.diagnostic.ts | 2 +- protocol/src/common/protocol.ts | 6 +- protocol/src/node/test/documentFilter.test.ts | 75 ++++++++++++++++++- 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/protocol/src/common/protocol.diagnostic.ts b/protocol/src/common/protocol.diagnostic.ts index 8971b12c6..abb82d23d 100644 --- a/protocol/src/common/protocol.diagnostic.ts +++ b/protocol/src/common/protocol.diagnostic.ts @@ -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); } } diff --git a/protocol/src/common/protocol.ts b/protocol/src/common/protocol.ts index af6a9f23e..b5d0025f3 100644 --- a/protocol/src/common/protocol.ts +++ b/protocol/src/common/protocol.ts @@ -241,7 +241,7 @@ export type TextDocumentFilter = TextDocumentFilterLanguage | TextDocumentFilter export namespace TextDocumentFilter { export function is(value: any): value is TextDocumentFilter { const candidate: TextDocumentFilter = value; - return Is.string(candidate) || (Is.string(candidate.language) || Is.string(candidate.scheme) || GlobPattern.is(candidate.pattern)); + return Is.string(candidate) || (Is.objectLiteral(candidate) && (Is.string(candidate.language) || Is.string(candidate.scheme) || GlobPattern.is(candidate.pattern))); } } @@ -1159,7 +1159,7 @@ export interface TextDocumentRegistrationOptions { export namespace TextDocumentRegistrationOptions { export function is(value: any): value is TextDocumentRegistrationOptions { const candidate = value as TextDocumentRegistrationOptions; - return candidate && (candidate.documentSelector === null || DocumentSelector.is(candidate.documentSelector)); + return Is.objectLiteral(candidate) && (candidate.documentSelector === null || DocumentSelector.is(candidate.documentSelector)); } } @@ -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); } } diff --git a/protocol/src/node/test/documentFilter.test.ts b/protocol/src/node/test/documentFilter.test.ts index 69ac21282..449a96899 100644 --- a/protocol/src/node/test/documentFilter.test.ts +++ b/protocol/src/node/test/documentFilter.test.ts @@ -6,7 +6,10 @@ import * as assert from 'assert'; -import { DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter, RelativePattern, TextDocumentRegistrationOptions } from '../main'; +import { + DiagnosticServerCancellationData, DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter, + RelativePattern, TextDocumentFilter, TextDocumentRegistrationOptions, WorkDoneProgressOptions +} from '../main'; suite('Document filter type guards', () => { @@ -41,11 +44,81 @@ suite('Document filter type guards', () => { }); }); + 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('null and undefined elements', () => { + assert.strictEqual(DocumentSelector.is([null]), false); + assert.strictEqual(DocumentSelector.is([undefined]), false); + assert.strictEqual(DocumentSelector.is(['typescript', null]), 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(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); + }); }); }); From 58228fc9548a404dfb4540525a0bb686decff283 Mon Sep 17 00:00:00 2001 From: Siddharth S Date: Sat, 7 Nov 2026 10:03:34 +0530 Subject: [PATCH 2/3] Clean up imports in documentFilter.test.ts Removed redundant imports from documentFilter.test.ts Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- protocol/src/node/test/documentFilter.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/protocol/src/node/test/documentFilter.test.ts b/protocol/src/node/test/documentFilter.test.ts index 466242942..8de1da429 100644 --- a/protocol/src/node/test/documentFilter.test.ts +++ b/protocol/src/node/test/documentFilter.test.ts @@ -10,7 +10,6 @@ import { DiagnosticServerCancellationData, DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter, RelativePattern, TextDocumentFilter, TextDocumentRegistrationOptions, WorkDoneProgressOptions } from '../main'; -import { DocumentSelector, NotebookCellTextDocumentFilter, NotebookDocumentFilter, RelativePattern, TextDocumentFilter, TextDocumentRegistrationOptions } from '../main'; suite('Document filter type guards', () => { From 2bbebf3c172dcf98457abba1f49d2c4fbdda3472 Mon Sep 17 00:00:00 2001 From: codeXsidd Date: Sat, 7 Nov 2026 10:10:09 +0530 Subject: [PATCH 3/3] fix(test): close diagnostic test callback and clean up duplicate test suites in documentFilter.test.ts --- protocol/src/node/test/documentFilter.test.ts | 27 +++---------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/protocol/src/node/test/documentFilter.test.ts b/protocol/src/node/test/documentFilter.test.ts index 8de1da429..cd0375270 100644 --- a/protocol/src/node/test/documentFilter.test.ts +++ b/protocol/src/node/test/documentFilter.test.ts @@ -15,19 +15,6 @@ 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); @@ -86,19 +73,15 @@ 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); - }); 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('null and undefined elements', () => { - assert.strictEqual(DocumentSelector.is([null]), false); - assert.strictEqual(DocumentSelector.is([undefined]), false); - assert.strictEqual(DocumentSelector.is(['typescript', null]), false); + 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); @@ -136,10 +119,6 @@ suite('Document filter type guards', () => { assert.strictEqual(DiagnosticServerCancellationData.is({}), false); assert.strictEqual(DiagnosticServerCancellationData.is(null), false); assert.strictEqual(DiagnosticServerCancellationData.is(undefined), false); - test('TextDocumentRegistrationOptions.is with null and undefined', () => { - assert.strictEqual(TextDocumentRegistrationOptions.is(null), false); - assert.strictEqual(TextDocumentRegistrationOptions.is(undefined), false); - assert.strictEqual(TextDocumentRegistrationOptions.is({ documentSelector: [null] }), false); }); }); });