Skip to content

Return false from TextDocumentFilter.is and TextDocumentRegistrationOptions.is for null/undefined - #1941

Open
Pujitha Paladugu (pujitha24) wants to merge 1 commit into
microsoft:mainfrom
pujitha24:auto/issue-1939
Open

Pujitha Paladugu (pujitha24) wants to merge 1 commit into
microsoft:mainfrom
pujitha24:auto/issue-1939

Conversation

@pujitha24

Copy link
Copy Markdown
Contributor

Motivation:
TextDocumentFilter.is read candidate.language without checking that the
candidate is an object, so TextDocumentFilter.is(null) and
DocumentSelector.is([null]) threw a TypeError instead of returning false.
TextDocumentRegistrationOptions.is(null) returned null rather than a boolean.

Approach:
Guard both functions with Is.objectLiteral, as the neighbouring
NotebookDocumentFilter.is and NotebookCellTextDocumentFilter.is already do.
String filters still short-circuit to true; behaviour for object inputs is
unchanged.

Validation:
Added tests in protocol/src/node/test/documentFilter.test.ts. Before the fix
three of them failed (two TypeErrors, one null !== false); after it
cd protocol && npm run compile && npm run test:node reports 16 passing, and
npm run lint in protocol is clean. Browser tests were not run, since only
platform-independent guard logic changed.

Report: #1939
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5-5 (via Claude Code)

Fixes #1939

…ptions.is for null/undefined

Motivation:
TextDocumentFilter.is read candidate.language without checking that the
candidate is an object, so TextDocumentFilter.is(null) and
DocumentSelector.is([null]) threw a TypeError instead of returning false.
TextDocumentRegistrationOptions.is(null) returned null rather than a boolean.

Approach:
Guard both functions with Is.objectLiteral, as the neighbouring
NotebookDocumentFilter.is and NotebookCellTextDocumentFilter.is already do.
String filters still short-circuit to true; behaviour for object inputs is
unchanged.

Validation:
Added tests in protocol/src/node/test/documentFilter.test.ts. Before the fix
three of them failed (two TypeErrors, one null !== false); after it
`cd protocol && npm run compile && npm run test:node` reports 16 passing, and
`npm run lint` in protocol is clean. Browser tests were not run, since only
platform-independent guard logic changed.

Report: microsoft#1939
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5-5 (via Claude Code)
Copilot AI balanced review requested due to automatic review settings October 31, 2026 16:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain for the changes to handle nullish inputs in document-filter type guards.

No open issues 🎉

Review effort: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TextDocumentFilter.is throws TypeError when called with null or undefined

5 participants