Skip to content

Add test coverage for defensive branches in JSON parser #1536

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

Merged
sergey-shandar merged 5 commits into from
Aug 13, 2026
Merged

Add test coverage for defensive branches in JSON parser #1536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d92da09
Cover defensive branches in JSON parser module
claude Aug 13, 2026
b9ae16f
Merge branch 'main' into claude/module-coverage-improvement-4ykmg5
sergey-shandar Aug 13, 2026
12c87f7
Merge branch 'main' into claude/module-coverage-improvement-4ykmg5
sergey-shandar Aug 13, 2026
bb79b67
Restructure away unreachable branches instead of testing them
claude Aug 13, 2026
a2a2375
Add CHANGELOG entry for #1536
claude Aug 13, 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
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ history.

## Unreleased

- `media/json/parser`: `endArray`/`endObject` no longer branch on `state.top`
and `tokenToValue` drops its defensive default arm — the parser's state
machine already guarantees these invariants, so the dead branches are gone
instead of tested
[#1536](https://github.com/functionalscript/functionalscript/pull/1536)
- `text/code_point`: new `eofFlush` factory builds the end-of-input step
`decoder` takes. The UTF-8 and UTF-16 decoders derive their eof ops from it
instead of each writing the flush out, so "leftover state becomes exactly one
Expand Down
33 changes: 25 additions & 8 deletions fjs/media/json/parser/module.f.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
* @import { List } from '../../../types/list/types.ts'
* @import { Fold } from '../../../types/function/operator/types.ts'
* @import { JsonToken } from '../tokenizer/types.ts'
* @import { _JsonObject, _JsonArray, _StateParse, _JsonState, _JsonStack } from './types.ts'
* @import { _JsonObject, _JsonArray, _StateParse, _JsonState, _JsonStack, _ValueToken } from './types.ts'
*/

import { error, ok } from '../../../types/result/module.f.mjs'
Expand Down Expand Up @@ -63,9 +63,14 @@ const popStack = stack => {
: { status: '', top: ne.first, stack: ne.tail }
}

/** @type {(state: _StateParse) => _JsonState} */
/**
* `endArray` only ever runs while parsing the array `startArray` opened
* (status `'['`/`'[v'`), so `state.top` is always that array here.
*
* @type {(state: _StateParse) => _JsonState}
*/
const endArray = state => {
const array = state.top !== null ? toArray(state.top.values) : null
const array = toArray(/** @type {_JsonArray} */ (state.top).values)

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the required CHANGELOG entry

This PR changes authored parser code, but the ## Unreleased section of CHANGELOG.md has no entry for PR #1536. Add the required short entry with the real PR number before merging; the repository workflow explicitly requires a CHANGELOG entry for every code-changing PR.

AGENTS.md reference: AGENTS.md:L133-L135

Useful? React with 👍 / 👎.

@sergey-shandar sergey-shandar Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added in a2a2375.

Generated by Claude Code

const newState = popStack(state.stack)
return pushValue(newState)(array)
}
Expand All @@ -76,26 +81,38 @@ const startObject = state => {
return { status: '{', top: { kind: 'object', values: null, key: '' }, stack: newStack }
}

/** @type {(state: _StateParse) => _JsonState} */
/**
* `endObject` only ever runs while parsing the object `startObject` opened
* (status `'{'`/`'{v'`), so `state.top` is always that object here.
*
* @type {(state: _StateParse) => _JsonState}
*/
const endObject = state => {
const obj = state.top?.kind === 'object' ? fromMap(state.top.values) : null
const obj = fromMap(/** @type {_JsonObject} */ (state.top).values)
const newState = popStack(state.stack)
return pushValue(newState)(obj)
}

/** @type {(token: JsonToken) => Unknown} */
/**
* Only ever called on a token `isValueToken` has already confirmed carries a
* value, so the switch covers every `_ValueToken` case with no fallback arm.
*
* @type {(token: _ValueToken) => Unknown}
*/
const tokenToValue = token => {
switch (token.kind) {
case 'null': return null
case 'false': return false
case 'true': return true
case 'number': return parseFloat(token.value)
case 'string': return token.value
default: return null
}
}

/** @type {(token: JsonToken) => boolean} */
/**
* @param {JsonToken} token
* @returns {token is _ValueToken}
*/
const isValueToken = token => {
switch (token.kind) {
case 'null':
Expand Down
4 changes: 4 additions & 0 deletions fjs/media/json/parser/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
import type { Unknown } from '../types.ts'
import type { OrderedMap } from '../../../types/ordered_map/types.ts'
import type { List } from '../../../types/list/types.ts'
import type { JsonToken } from '../tokenizer/types.ts'

/** JSON tokens that carry a directly-usable value. */
export type _ValueToken = Extract<JsonToken, { readonly kind: 'null' | 'false' | 'true' | 'string' | 'number' }>

export type _JsonObject = {
readonly kind: 'object'
Expand Down
Loading