From 847a36a208ab71ac820c24036b1a385a924a5d7c Mon Sep 17 00:00:00 2001 From: Leonid Svyatov Date: Tue, 21 Jul 2026 19:45:16 +0300 Subject: [PATCH] fix(parser): return instead of throwing on an unterminated scope A body line that begins with a word followed by "(" reaches scope() through footer() and token(). A second "(" before the closing ")" is a syntax error there, and scope() was the one sub-parser that threw rather than returning the Error, so the throw escaped message() and the whole commit failed to parse. Both callers already handle an Error return. summary() treats it as "no scope", token() skips the node. Return the error so they get the chance, and rewind past the consumed "(" as well, since abort() rewinds only to the scope node, which starts one character later. summary() re-raises this specific error, because the grammar excludes parens from a scope, so an unclosed "(" in a summary really is a syntax error. Existing error messages and positions are unchanged. Closes #54 --- lib/parser.js | 13 ++++++++++++- test/parser.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/parser.js b/lib/parser.js index 33cf7b2..72bda68 100644 --- a/lib/parser.js +++ b/lib/parser.js @@ -87,6 +87,9 @@ function summary (scanner) { // ... "(" ")" ... let s = scope(scanner) if (s instanceof Error) { + // An opened scope that never closes is a syntax error in a , where + // the grammar demands the ")". Everywhere else it just means "no scope here". + if (s.unterminatedScope) return s s = null } else { node.children.push(s) @@ -157,6 +160,7 @@ function text (scanner) { * "(" ")" ::= 1* */ function scope (scanner) { + const start = scanner.position() if (scanner.peek() !== '(') { return scanner.abort(scanner.enter('scope', '')) } else { @@ -174,7 +178,14 @@ function scope (scanner) { } if (scanner.peek() !== ')') { - throw scanner.abort(node, [')']) + // Return rather than throw, so a line that merely looks like a scope + // ("foo(bar(1))" at the start of a line) can fall back to . abort() + // rewinds only as far as the scope node, which starts after the "(", so + // rewind past the "(" too and leave the scanner where the caller found it. + const error = scanner.abort(node, [')']) + scanner.rewind(start) + error.unterminatedScope = true + return error } else { scanner.exit(node) scanner.next() diff --git a/test/parser.js b/test/parser.js index 328a993..a693f3d 100644 --- a/test/parser.js +++ b/test/parser.js @@ -109,6 +109,30 @@ describe('', () => { const parsed = parser('feat: breaking change\n\nBREAKING CHANGE: introduces breaking change\nsecond line') parsed.should.matchSnapshot() }) + it('treats a line starting with nested parens as , not a ', () => { + const parsed = parser('fix: address major bug\n\nfoo(bar(1))') + expect(bodyChildren(parsed)).to.deep.equal([['text', 'foo(bar(1))']]) + }) + it('treats a line starting with an unterminated paren as ', () => { + // https://github.com/conventional-commits/parser/pull/48 + const parsed = parser('fix(foo): some renovate commit\n\nfoo(\n)') + expect(bodyChildren(parsed)).to.deep.equal([['text', 'foo('], ['newline', '\n'], ['text', ')']]) + }) + it('still parses footers after such a line', () => { + const parsed = parser('fix: address major bug\n\nz.array(z.boolean())\n\nRefs #392') + expect(bodyChildren(parsed)).to.deep.equal([['text', 'z.array(z.boolean())']]) + expect(parsed.children.filter(c => c.type === 'footer')).to.have.lengthOf(1) + }) + it('contains valid positions for a line starting with nested parens', () => { + assertNodePositions('fix: address major bug\n\nfoo(bar(1))') + }) + it('still throws for nested parens in a scope', () => { + // ::= 1*, so this is + // a genuine syntax error, unlike the same characters in a . + expect(() => { + parser('fix(a(b)): x') + }).to.throw("unexpected token '(' at 1:6, valid tokens [)]") + }) }) describe(', *,