Skip to content
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.

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

expr-parser: skip tags in records #480

Merged
merged 2 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "ecmarkup",
"version": "14.1.2",
"version": "14.1.3",
"description": "Custom element definitions and core utilities for markup that specifies ECMAScript and related technologies.",
"main": "lib/ecmarkup.js",
"typings": "lib/ecmarkup.d.ts",
Expand Down
61 changes: 52 additions & 9 deletions src/expr-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,14 @@ class ExprParser {
},
};
const match = tok.name === 'text' ? tok.contents.match(tokMatcher) : null;
// the `tok.name !== 'text'` part in the test below is redundant but makes TS happier
if (tok.name !== 'text' || match == null) {
if (!(tok.name === 'text' && tok.contents.length === 0)) {
const empty =
(tok.name === 'text' && tok.contents.length === 0) ||
tok.name === 'tag' ||
tok.name === 'opaqueTag' ||
tok.name === 'comment';
if (!empty) {
currentProse.push(tok);
}
++this.srcIndex;
Expand Down Expand Up @@ -241,6 +247,36 @@ class ExprParser {
});
}

// returns true if this ate a newline
private eatWhitespace(): boolean {
let next;
let hadNewline = false;
while ((next = this.peek())?.type === 'prose') {
const firstNonWhitespaceIdx = next.parts.findIndex(
part => part.name !== 'text' || /\S/.test(part.contents)
);
if (firstNonWhitespaceIdx === -1) {
hadNewline ||= next.parts.some(
part => part.name === 'text' && part.contents.includes('\n')
);
this.next.shift();
} else if (firstNonWhitespaceIdx === 0) {
return hadNewline;
} else {
hadNewline ||= next.parts.some(
(part, i) =>
i < firstNonWhitespaceIdx && part.name === 'text' && part.contents.includes('\n')
);
this.next[0] = {
type: 'prose',
parts: next.parts.slice(firstNonWhitespaceIdx),
};
return hadNewline;
}
}
return hadNewline;
}

// guarantees the next token is an element of close
parseSeq(close: CloseTokenType[]): Seq {
const items: NonSeq[] = [];
Expand Down Expand Up @@ -421,7 +457,20 @@ class ExprParser {
let type: 'record' | 'record-spec' | null = null;
const members: ({ name: string; value: Seq } | { name: string })[] = [];
while (true) {
const hadNewline = this.eatWhitespace();
const nextTok = this.peek();
if (nextTok.type === 'crec') {
if (!hadNewline) {
// ideally this would be a lint failure, or better yet a formatting thing, but whatever
throw new ParseFailure(
members.length > 0
? 'trailing commas are only allowed when followed by a newline'
: 'records cannot be empty',
nextTok.offset
);
}
break;
}
if (nextTok.type !== 'prose') {
throw new ParseFailure('expected to find record field name', nextTok.offset);
}
Expand All @@ -434,14 +483,6 @@ class ExprParser {
const { contents } = nextTok.parts[0];
const nameMatch = contents.match(/^\s*\[\[(?<name>\w+)\]\]\s*(?<colon>:?)/);
if (nameMatch == null) {
if (members.length > 0 && /^\s*$/.test(contents) && contents.includes('\n')) {
// allow trailing commas when followed by a newline
this.next.shift(); // eat the whitespace
if (this.peek().type === 'crec') {
this.next.shift();
break;
}
}
throw new ParseFailure(
'expected to find record field',
nextTok.parts[0].location.start.offset + contents.match(/^\s*/)![0].length
Expand Down Expand Up @@ -581,6 +622,8 @@ class ExprParser {
}
}

// Note: this does not necessarily represent the entire input
// in particular it may omit some whitespace, tags, and comments
export function parse(src: FragmentNode[], opNames: Set<String>): Seq | Failure {
const parser = new ExprParser(src, opNames);
try {
Expand Down
13 changes: 12 additions & 1 deletion test/expr-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,17 @@ describe('expression parsing', () => {
</emu-alg>
`);
});

it('tags in record', async () => {
await assertLintFree(`
<emu-alg>
1. Let _x_ be a new Record { [[Foo]]: 0, <!-- comment --> [[Bar]]: 1 }.
1. Let _x_ be a new Record { [[Foo]]: 0, <ins>[[Bar]]: 1</ins> }.
1. Let _x_ be a new Record { [[Foo]]: 0, <ins>[[Bar]]: 1, [[Baz]]: 2</ins> }.
1. Let _x_ be a new Record { [[Foo]]: 0, <ins>[[Bar]]: 1,</ins> [[Baz]]: 2 }.
</emu-alg>
`);
});
});

describe('errors', () => {
Expand Down Expand Up @@ -193,7 +204,7 @@ describe('expression parsing', () => {
{
ruleId: 'expression-parsing',
nodeType: 'emu-alg',
message: 'expected to find record field',
message: 'records cannot be empty',
}
);
});
Expand Down