Fix 14 bugs found during code audit#409
Open
JordanBoltonMN wants to merge 3 commits into
Open
Conversation
Bug fixes: - Pattern regex escaping (patterns.ts) - StringUtils grapheme boundary (stringUtils.ts) - Token kind mismatch (token.ts) - Lexer off-by-one (lexer.ts) - LexerSnapshot cache invalidation (lexerSnapshot.ts) - Context node cleanup (contextUtils.ts) - copyState currentContextNode isolation (parseStateUtils.ts) - NodeIdMapIterator bounds check (nodeIdMapIterator.ts) - Validator type predicates (combineOperatorsAndOperands.ts) - Disambiguation indent normalization (disambiguationUtils.ts) - ParserUtils error propagation (parserUtils.ts) - Tokenizer incremental sync (tokenizerIncremental.test.ts) - Redundant ParseContextUtils.newState() call (parseStateUtils.ts) - Package version bump (package.json) All tests pass (779 passing, 1 pending). Lint clean (0 errors, 0 warnings). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e467f33 to
a24496d
Compare
Maintain explanatory comments describing why each test exists. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a set of correctness bugs found during a code audit across the lexer, parser, and shared utilities, and adds/updates regression tests to lock in the fixes. It also bumps the package version to reflect the set of bugfixes.
Changes:
- Fixes multiple lexer/parser correctness issues (token kinds, incremental retokenization, snapshot position accounting, disambiguation error handling, node iteration bounds, parse-state copying isolation).
- Corrects several shared utility regex/string edge cases (numeric exponent sign, whitespace regex, identifier character classes, escaped-quote scanning).
- Adds focused regression tests covering the reported bugs, plus bumps the package version.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/libraryTest/tokenizer/tokenizerIncremental.test.ts | Updates expected scope name for text literal token content. |
| src/test/libraryTest/parser/parseNodeIdMapUtils.test.ts | Adds regression tests ensuring copyState isolates currentContextNode references. |
| src/test/libraryTest/parser/parseBehavior.test.ts | Ensures invalid parseBehavior hits an invariant path; adds type-directive regression coverage (one test marked pending). |
| src/test/libraryTest/lexer/lexSimple.test.ts | Adds regression tests for numeric exponent escaping and end-of-input .. handling; adds whitespace-pattern regression tests. |
| src/test/libraryTest/lexer/lexMultilineTokens.test.ts | Adds regression test validating LineTokenKindAdditions.TextLiteralContent enum value matches its member name. |
| src/test/libraryTest/lexer/lexIncremental.test.ts | Adds regression test ensuring incremental retokenization does not drop lines. |
| src/test/libraryTest/lexer/lexerSnapshotCache.test.ts | Adds regression tests for codeUnit correctness in grapheme position computations. |
| src/test/libraryTest/language/astUtils.test.ts | Adds tests covering validator operand type predicates for nested as expressions and equality operands. |
| src/test/libraryTest/common/stringUtils.test.ts | Adds tests for consecutive/multiple escaped quotes in findQuotes. |
| src/test/libraryTest/common/cancellationToken.test.ts | Adds tests verifying CounterCancellationToken consumes exactly one count per call. |
| src/powerquery-parser/parser/parseState/parseStateUtils.ts | Ensures copyState uses the copied contextState and rebinds currentContextNode to the copied node instance. |
| src/powerquery-parser/parser/parsers/combinatorialParserV2/combineOperatorsAndOperands.ts | Fixes validator type predicates for as and equality/relational operand typing. |
| src/powerquery-parser/parser/parser/parserUtils.ts | Makes the parseBehavior default branch explicitly throw via Assert.isNever. |
| src/powerquery-parser/parser/nodeIdMap/nodeIdMapIterator.ts | Fixes sibling bounds check to correctly return undefined only when out of range. |
| src/powerquery-parser/parser/disambiguation/disambiguationUtils.ts | Improves catch handling to validate Error and rethrow cancellation errors before restoring checkpoints. |
| src/powerquery-parser/parser/context/contextUtils.ts | Fixes isNodeKind runtime behavior for union input (array vs single kind). |
| src/powerquery-parser/lexer/lexerSnapshot.ts | Fixes grapheme position codeUnit to reflect token start rather than token end. |
| src/powerquery-parser/lexer/lexer.ts | Fixes incremental retokenization early-exit to not drop the current line. |
| src/powerquery-parser/language/token.ts | Fixes TextLiteralContent enum value mismatch ("TextContent" → "TextLiteralContent"). |
| src/powerquery-parser/common/stringUtils.ts | Fixes findQuotes loop control for escaped quote handling. |
| src/powerquery-parser/common/patterns.ts | Fixes regexes for identifier chars, whitespace, and numeric exponent sign matching. |
| package.json | Bumps package version to 0.20.0. |
| package-lock.json | Updates lockfile version fields to 0.20.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolve conflicts: - package.json: keep version 0.20.0 - package-lock.json: keep HEAD version - parseNodeIdMapUtils.test.ts: combine imports (ArrayUtils + TaskUtils) - parseBehavior.test.ts: add non-null assertions for array access Fix lint issues introduced by merge: - Fix sort-imports ordering across 8 files - Fix switch-exhaustiveness-check in stringUtils.ts - Auto-fix prettier formatting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
c8cf2c9 to
fb3e418
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug fixes:
All tests pass (779 passing, 1 pending).
Lint clean (0 errors, 0 warnings).