sepBy mutates position even on no match
gilbert opened this issue · 4 comments
Describe the bug
When sepBy fails to match at all, it still updates the cursor position. This causes subsequent parsers to fail due to skipped input.
In contrast, many – which also always succeeds – does not update cursor position when it fails to match at all.
Reproduction
I can't figure out how to get past the pre-commit hooks, so here's a diff of the test and fix instead of a PR:
The test & fix
diff --git a/src/__tests__/combinators/sepBy.spec.ts b/src/__tests__/combinators/sepBy.spec.ts
index ca348b9..0cda6a1 100644
--- a/src/__tests__/combinators/sepBy.spec.ts
+++ b/src/__tests__/combinators/sepBy.spec.ts
@@ -1,4 +1,4 @@
-import { sepBy, sepBy1 } from '@combinators'
+import { sepBy, sepBy1, sequence } from '@combinators'
import { string } from '@parsers'
import { run, result, should, describe, it } from '@testing'
@@ -26,6 +26,17 @@ describe('sepBy', () => {
should.matchState(actual, expected)
})
+
+ it('should successfully continue if nothing matched', () => {
+ const parser = sequence(
+ sepBy(string('hello'), string('?')),
+ sepBy(string('bye'), string('?')),
+ )
+ const actual = run(parser, 'bye?bye?')
+ const expected = result(true, [[], ['bye', 'bye']])
+
+ should.matchState(actual, expected)
+ })
})
describe('sepBy1', () => {
diff --git a/src/combinators/sepBy.ts b/src/combinators/sepBy.ts
index 51b81b1..f42bc32 100644
--- a/src/combinators/sepBy.ts
+++ b/src/combinators/sepBy.ts
@@ -14,6 +14,7 @@ import type { Parser } from '@types'
export function sepBy<T, S>(parser: Parser<T>, sep: Parser<S>): Parser<Array<T>> {
return {
parse(input, pos) {
// Run the parser once to get the first value.
const resultP = parser.parse(input, pos)
@@ -37,8 +38,8 @@ export function sepBy<T, S>(parser: Parser<T>, sep: Parser<S>): Parser<Array<T>>
return {
isOk: true,
- span: [pos, resultP.pos],
- pos: resultP.pos,
+ span: [pos, pos],
+ pos,
value: []
}
}And here's the test again for visibility, which fails on the current version (3.6.2):
it('should successfully continue if nothing matched', () => {
const parser = sequence(
sepBy(string('hello'), string('?')),
sepBy(string('bye'), string('?')),
)
const actual = run(parser, 'bye?bye?')
const expected = result(true, [[], ['bye', 'bye']])
should.matchState(actual, expected)
})Hi! Thanks for the issue and the diff, much appreciated. Kinda ashamed I've overlooked this. I'll roll out a fix on the weekend.
I can't figure out how to get past the pre-commit hooks
That's a bummer, can you share the specifics? Maybe I could help.
No problem, and thank you for creating this beauty of a library
Here's the output when I try to commit:
git commit output
% git commit
✔ Preparing lint-staged...
❯ Running tasks for staged files...
❯ package.json — 2 files
✔ *.{js,ts,json} — 2 files
❯ *.{js,ts} — 2 files
✖ eslint --fix [FAILED]
↓ Skipped because of errors from tasks. [SKIPPED]
✔ Reverting to original state because of errors...
✔ Cleaning up temporary files...
✖ eslint --fix:
/Users/me/p/src/sigma/src/__tests__/combinators/sepBy.spec.ts
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
at q (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:6391)
at Object.ce [as getTsconfig] (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:7466)
at /Users/me/p/src/sigma/node_modules/eslint-import-resolver-typescript/lib/index.cjs:289:36
at Array.map (<anonymous>)
at initMappers (/Users/me/p/src/sigma/node_modules/eslint-import-resolver-typescript/lib/index.cjs:285:26)
at Object.resolve (/Users/me/p/src/sigma/node_modules/eslint-import-resolver-typescript/lib/index.cjs:178:3)
at v2 (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:116:23)
at withResolver (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:121:14)
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/namespace
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity, same trace as above --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/order
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity, same trace as above --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/no-unresolved
1:1 warning Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity, same trace as above --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/no-duplicates
1:41 error Unable to resolve path to module '@combinators' import/no-unresolved
2:24 error Unable to resolve path to module '@parsers' import/no-unresolved
3:51 error Unable to resolve path to module '@testing' import/no-unresolved
/Users/me/p/src/sigma/src/combinators/sepBy.ts
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/namespace
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/order
1:1 warning Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/no-duplicates
1:1 error Resolve error: File '@vue/tsconfig/tsconfig.web.json' not found.
at te (/Users/me/p/src/sigma/node_modules/get-tsconfig/dist/index.js:3:5610)
--omitted for brevity --
at fullResolve (/Users/me/p/src/sigma/node_modules/eslint-module-utils/resolve.js:138:22) import/no-unresolved
3:22 error Unable to resolve path to module './many' import/no-unresolved
4:26 error Unable to resolve path to module './sequence' import/no-unresolved
✖ 13 problems (11 errors, 2 warnings)
husky - pre-commit hook exited with code 1 (error)
Here's the output when I try to commit:
git commit output
Okay, I figured it out. tl;dr: should have done cd docs && npm i before committing.
We have an eslint plugin for imports, and right now it's configured like this:
"import/resolver": {
"typescript": {
"alwaysTryTypes": true,
"project": ["./tsconfig.json", "./*/tsconfig.json"]
}
}The culprit here is "./*/tsconfig.json", which instructs the plugin to pick up not only the root tsconfig, but also the one in docs which in turn extends @vue/tsconfig/tsconfig.web.json, so, since docs didn't have its dependencies installed, everything failed.
The proper way to address this and some other issues with the current setup would be to migrate to monorepo setup, but I have no capacity to do that right now. So for the time being we will install deps for all packages in postinstall when doing npm install (done in #78).