From 75e35227f8aa1312d6f7b3d0f3538acb73b9cea0 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sat, 30 Apr 2022 21:43:04 -0700 Subject: [PATCH 1/9] parse expressions --- src/Algorithm.ts | 2 + src/Spec.ts | 119 ++++++- src/expr-parser.ts | 474 +++++++++++++++++++++++++ src/header-parser.ts | 6 +- src/lint/lint.ts | 2 + src/lint/rules/algorithm-line-style.ts | 2 +- test/expr-parser.js | 259 ++++++++++++++ test/lint-algorithms.js | 2 +- test/typecheck.js | 331 ++++++++++++----- test/utils.js | 4 +- 10 files changed, 1102 insertions(+), 99 deletions(-) create mode 100644 src/expr-parser.ts create mode 100644 test/expr-parser.js diff --git a/src/Algorithm.ts b/src/Algorithm.ts index b5f668d1..0850b365 100644 --- a/src/Algorithm.ts +++ b/src/Algorithm.ts @@ -49,6 +49,8 @@ export default class Algorithm extends Builder { // @ts-ignore node.ecmarkdownTree = emdTree; + // @ts-ignore + node.originalHtml = innerHTML; if (spec.opts.lintSpec && spec.locate(node) != null && !node.hasAttribute('example')) { const clause = clauseStack[clauseStack.length - 1]; diff --git a/src/Spec.ts b/src/Spec.ts index 357069db..7fd32c04 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -45,9 +45,16 @@ import { import { lint } from './lint/lint'; import { CancellationToken } from 'prex'; import type { JSDOM } from 'jsdom'; -import type { OrderedListItemNode, parseAlgorithm, UnorderedListItemNode } from 'ecmarkdown'; +import type { + OrderedListItemNode, + OrderedListNode, + parseAlgorithm, + UnorderedListItemNode, +} from 'ecmarkdown'; import { getProductions, rhsMatches, getLocationInGrammarFile } from './lint/utils'; import type { AugmentedGrammarEle } from './Grammar'; +import { offsetToLineAndColumn } from './utils'; +import { parse as parseExpr, walk as walkExpr, Expr } from './expr-parser'; const DRAFT_DATE_FORMAT: Intl.DateTimeFormatOptions = { year: 'numeric', @@ -477,7 +484,8 @@ export default class Spec { if (this.opts.lintSpec) { this.log('Checking types...'); - this.typecheck(); + this.checkCompletionUsage(); + this.checkCalls(); } this.log('Propagating effect annotations...'); this.propagateEffects(); @@ -584,9 +592,8 @@ export default class Spec { } } - // right now this just checks that AOs which do/don't return completion records are invoked appropriately - // someday, more! - private typecheck() { + // checks that AOs which do/don't return completion records are invoked appropriately + private checkCompletionUsage() { const isCall = (x: Xref) => x.isInvocation && x.clause != null && x.aoid != null; const isUnused = (t: Type) => t.kind === 'unused' || @@ -791,6 +798,108 @@ export default class Spec { } } + private checkCalls() { + for (const node of this.doc.querySelectorAll('emu-alg')) { + if (node.hasAttribute('example') || !('ecmarkdownTree' in node)) { + continue; + } + // @ts-ignore + const tree = node.ecmarkdownTree as ReturnType; + if (tree == null) { + continue; + } + // @ts-ignore + const originalHtml: string = node.originalHtml; + + const expressionVisitor = (expr: Expr) => { + if (expr.type !== 'call') { + return; + } + + const { callee, arguments: args } = expr; + if (!(callee.parts.length === 1 && callee.parts[0].name === 'text')) { + return; + } + const calleeName = callee.parts[0].contents; + if ( + [ + 'thisTimeValue', + 'thisStringValue', + 'thisBigIntValue', + 'thisNumberValue', + 'thisSymbolValue', + 'thisBooleanValue', + 'toUppercase', + 'toLowercase', + 'ℝ', + '𝔽', + 'ℤ', + ].includes(calleeName) + ) { + // TODO make the spec not do this + return; + } + const biblioEntry = this.biblio.byAoid(calleeName); + if (biblioEntry == null) { + const { line, column } = offsetToLineAndColumn( + originalHtml, + callee.parts[0].location.start.offset + ); + this.warn({ + type: 'contents', + ruleId: 'typecheck', + message: `could not find definition for ${calleeName}`, + node, + nodeRelativeLine: line, + nodeRelativeColumn: column, + }); + } else if (biblioEntry.signature != null) { + const min = biblioEntry.signature.parameters.length; + const max = min + biblioEntry.signature.optionalParameters.length; + if (args.length < min || args.length > max) { + const { line, column } = offsetToLineAndColumn( + originalHtml, + callee.parts[0].location.start.offset + ); + const count = `${min}${min === max ? '' : `-${max}`}`; + // prettier-ignore + const message = `${calleeName} takes ${count} argument${count === '1' ? '' : 's'}, but this invocation passes ${args.length}`; + this.warn({ + type: 'contents', + ruleId: 'typecheck', + message, + node, + nodeRelativeLine: line, + nodeRelativeColumn: column, + }); + } + } + }; + const walkLines = (list: OrderedListNode) => { + for (const line of list.contents) { + const item = parseExpr(line.contents); + if (item.type === 'failure') { + const { line, column } = offsetToLineAndColumn(originalHtml, item.offset); + this.warn({ + type: 'contents', + ruleId: 'expression-parsing', + message: item.message, + node, + nodeRelativeLine: line, + nodeRelativeColumn: column, + }); + } else { + walkExpr(expressionVisitor, item); + } + if (line.sublist?.name === 'ol') { + walkLines(line.sublist); + } + } + }; + walkLines(tree.contents); + } + } + private toHTML() { const htmlEle = this.doc.documentElement; return '\n' + (htmlEle.hasAttributes() ? htmlEle.outerHTML : htmlEle.innerHTML); diff --git a/src/expr-parser.ts b/src/expr-parser.ts new file mode 100644 index 00000000..dc8d128b --- /dev/null +++ b/src/expr-parser.ts @@ -0,0 +1,474 @@ +import type { parseFragment } from 'ecmarkdown'; +import { formatEnglishList } from './header-parser'; + +// TODO export FragmentNode +type Unarray = T extends Array ? U : T; +type FragmentNode = Unarray>; + +const tokMatcher = + /(?«|«)|(?»|»)|(?\{)|(?\})|(?\()|(?\))/u; +const tokOrCommaMatcher = new RegExp(tokMatcher.source + '|(?,)', 'u'); // gross + +type ProsePart = + | FragmentNode + | { name: 'text'; contents: string; location: { start: { offset: number } } }; +type Prose = { + type: 'prose'; + parts: ProsePart[]; // nonempty +}; +type List = { + type: 'list'; + elements: Seq[]; +}; +type Record = { + type: 'record'; + members: { name: string; value: Seq }[]; +}; +type RecordSpec = { + type: 'record-spec'; + members: { name: string }[]; +}; +type Call = { + type: 'call'; + callee: Prose; + arguments: Seq[]; +}; +type Paren = { + type: 'paren'; + items: NonSeq[]; +}; +type Seq = { + type: 'seq'; + items: NonSeq[]; +}; +type NonSeq = Prose | List | Record | RecordSpec | Call | Paren; +export type Expr = NonSeq | Seq; +type Failure = { type: 'failure'; message: string; offset: number }; + +class ParseFailure extends Error { + declare offset: number; + constructor(message: string, offset: number) { + super(message); + this.offset = offset; + } +} + +function formatClose(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]) { + const mapped = close.map(c => { + switch (c) { + case 'clist': + return 'list close'; + case 'crec': + return 'record close'; + case 'cparen': + return 'close parenthesis'; + case 'eof': + return 'end of line'; + default: + return c; + } + }); + return formatEnglishList(mapped, 'or'); +} + +function isEmpty(s: Seq) { + return s.items.every( + i => i.type === 'prose' && i.parts.every(p => p.name === 'text' && /^\s*$/.test(p.contents)) + ); +} + +function emptyThingHasNewline(s: Seq) { + // only call this function on things which pass isEmpty + return s.items.some(i => + (i as Prose).parts.some(p => (p as { name: 'text'; contents: string }).contents.includes('\n')) + ); +} + +type TokenType = 'eof' | 'olist' | 'clist' | 'orec' | 'crec' | 'oparen' | 'cparen' | 'comma'; +type Token = Prose | { type: TokenType; offset: number }; +class ExprParser { + declare src: FragmentNode[]; + srcIndex = 0; + textTokOffset: number | null = null; // offset into current text node; only meaningful if srcOffset points to a text node + next: Token[] = []; + constructor(src: FragmentNode[]) { + this.src = src; + } + + private peek(matcher: RegExp): Token { + if (this.next.length === 0) { + this.advance(matcher); + } + return this.next[0]; + } + + // this method is complicated because the underlying data is a sequence of ecmarkdown fragments, not a string + private advance(matcher: RegExp) { + const currentProse: ProsePart[] = []; + while (this.srcIndex < this.src.length) { + const tok: ProsePart = + this.textTokOffset == null + ? this.src[this.srcIndex] + : { + name: 'text', + contents: (this.src[this.srcIndex].contents as string).slice(this.textTokOffset), + location: { + start: { + offset: this.src[this.srcIndex].location.start.offset + this.textTokOffset, + }, + }, + }; + const match = tok.name === 'text' ? tok.contents.match(matcher) : null; + if (tok.name !== 'text' || match == null) { + if (!(tok.name === 'text' && tok.contents.length === 0)) { + currentProse.push(tok); + } + ++this.srcIndex; + this.textTokOffset = null; + continue; + } + const { groups } = match; + const before = tok.contents.slice(0, match.index); + if (before.length > 0) { + currentProse.push({ name: 'text', contents: before, location: tok.location }); + } + const matchKind = Object.keys(groups!).find(x => groups![x] != null)!; + if (currentProse.length > 0) { + this.next.push({ type: 'prose', parts: currentProse }); + } + this.textTokOffset = (this.textTokOffset ?? 0) + match.index! + match[0].length; + this.next.push({ + type: matchKind as TokenType, + offset: tok.location.start.offset + match.index!, + }); + return; + } + if (currentProse.length > 0) { + this.next.push({ type: 'prose', parts: currentProse }); + } + this.next.push({ + type: 'eof', + offset: this.src.length === 0 ? 0 : this.src[this.src.length - 1].location.end.offset, + }); + } + + // guarantees the next token is an element of close + parseSeq(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]): Seq { + const items: NonSeq[] = []; + const matcher = close.includes('comma') ? tokOrCommaMatcher : tokMatcher; + while (true) { + const next = this.peek(matcher); + switch (next.type) { + case 'comma': { + if (!close.includes('comma')) { + throw new Error('unreachable: comma while not scanning for commas'); + } + if (items.length === 0) { + throw new ParseFailure( + `unexpected comma (expected some content for element/argument)`, + next.offset + ); + } + return { type: 'seq', items }; + } + case 'eof': { + if (items.length === 0 || !close.includes('eof')) { + throw new ParseFailure(`unexpected eof (expected ${formatClose(close)})`, next.offset); + } + return { type: 'seq', items }; + } + case 'prose': { + this.next.shift(); + items.push(next); + break; + } + case 'olist': { + this.next.shift(); + const elements: Seq[] = []; + if (this.peek(tokOrCommaMatcher).type !== 'clist') { + while (true) { + elements.push(this.parseSeq(['clist', 'comma'])); + if (this.peek(tokOrCommaMatcher).type === 'clist') { + break; + } + this.next.shift(); + } + } + if (elements.length > 0 && isEmpty(elements[elements.length - 1])) { + if (elements.length === 1 || emptyThingHasNewline(elements[elements.length - 1])) { + // allow trailing commas when followed by whitespace + elements.pop(); + } else { + throw new ParseFailure( + `unexpected list close (expected some content for element)`, + (this.peek(tokOrCommaMatcher) as { offset: number }).offset + ); + } + } + items.push({ type: 'list', elements }); + this.next.shift(); // eat the clist + break; + } + case 'clist': { + if (!close.includes('clist')) { + throw new ParseFailure( + 'unexpected list close without corresponding list open', + next.offset + ); + } + return { type: 'seq', items }; + } + case 'oparen': { + const lastPart = items[items.length - 1]; + if (lastPart != null && lastPart.type === 'prose') { + const callee: ProsePart[] = []; + for (let i = lastPart.parts.length - 1; i >= 0; --i) { + const ppart = lastPart.parts[i]; + if (ppart.name === 'text') { + const spaceIndex = ppart.contents.lastIndexOf(' '); + if (spaceIndex !== -1) { + if (spaceIndex < ppart.contents.length - 1) { + const calleePart = ppart.contents.slice(spaceIndex + 1); + if (!/\p{Letter}/u.test(calleePart)) { + // e.g. -(x + 1) + break; + } + lastPart.parts[i] = { + name: 'text', + contents: ppart.contents.slice(0, spaceIndex + 1), + location: ppart.location, + }; + callee.unshift({ + name: 'text', + contents: calleePart, + location: { + start: { offset: ppart.location.start.offset + spaceIndex + 1 }, + }, + }); + } + break; + } + } else if (ppart.name === 'tag') { + break; + } + callee.unshift(ppart); + lastPart.parts.pop(); + } + if (callee.length > 0) { + this.next.shift(); + const args: Seq[] = []; + if (this.peek(tokOrCommaMatcher).type !== 'cparen') { + while (true) { + args.push(this.parseSeq(['cparen', 'comma'])); + if (this.peek(tokOrCommaMatcher).type === 'cparen') { + break; + } + this.next.shift(); + } + } + if (args.length > 0 && isEmpty(args[args.length - 1])) { + if (args.length === 1 || emptyThingHasNewline(args[args.length - 1])) { + // allow trailing commas when followed by a newline + args.pop(); + } else { + throw new ParseFailure( + `unexpected close parenthesis (expected some content for argument)`, + (this.peek(tokOrCommaMatcher) as { offset: number }).offset + ); + } + } + items.push({ + type: 'call', + callee: { type: 'prose', parts: callee }, + arguments: args, + }); + this.next.shift(); // eat the cparen + break; + } + } + this.next.shift(); + items.push({ type: 'paren', items: this.parseSeq(['cparen']).items }); + this.next.shift(); // eat the cparen + break; + } + case 'cparen': { + if (!close.includes('cparen')) { + throw new ParseFailure( + 'unexpected close parenthesis without corresponding open parenthesis', + next.offset + ); + } + return { type: 'seq', items }; + } + case 'orec': { + this.next.shift(); + let type: 'record' | 'record-spec' | null = null; + const members: ({ name: string; value: Seq } | { name: string })[] = []; + while (true) { + const nextTok = this.peek(tokOrCommaMatcher); + if (nextTok.type !== 'prose') { + throw new ParseFailure('expected to find record field name', nextTok.offset); + } + if (nextTok.parts[0].name !== 'text') { + throw new ParseFailure( + 'expected to find record field name', + nextTok.parts[0].location.start.offset + ); + } + const { contents } = nextTok.parts[0]; + const nameMatch = contents.match(/^\s*\[\[(?\w+)\]\]\s*(?:?)/); + 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(tokOrCommaMatcher).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 + ); + } + const { name, colon } = nameMatch.groups!; + if (members.find(x => x.name === name)) { + throw new ParseFailure( + `duplicate record field name ${name}`, + nextTok.parts[0].location.start.offset + contents.match(/^\s*\[\[/)![0].length + ); + } + const shortenedText = nextTok.parts[0].contents.slice(nameMatch[0].length); + const offset = nextTok.parts[0].location.start.offset + nameMatch[0].length; + if (shortenedText.length === 0 && nextTok.parts.length === 1) { + this.next.shift(); + } else if (shortenedText.length === 0) { + this.next[0] = { + type: 'prose', + parts: nextTok.parts.slice(1), + }; + } else { + const shortened: ProsePart = { + name: 'text', + contents: shortenedText, + location: { + start: { offset }, + }, + }; + this.next[0] = { + type: 'prose', + parts: [shortened, ...nextTok.parts.slice(1)], + }; + } + if (colon) { + if (type == null) { + type = 'record'; + } else if (type === 'record-spec') { + throw new ParseFailure( + 'record field has value but preceding field does not', + offset - 1 + ); + } + const value = this.parseSeq(['crec', 'comma']); + if (value.items.length === 0) { + throw new ParseFailure('expected record field to have value', offset); + } + members.push({ name, value }); + } else { + if (type == null) { + type = 'record-spec'; + } else if (type === 'record') { + throw new ParseFailure('expected record field to have value', offset - 1); + } + members.push({ name }); + if (!['crec', 'comma'].includes(this.peek(tokOrCommaMatcher).type)) { + throw new ParseFailure(`expected ${formatClose(['crec', 'comma'])}`, offset); + } + } + if (this.peek(tokOrCommaMatcher).type === 'crec') { + break; + } + this.next.shift(); // eat the comma + } + // @ts-ignore typing this correctly is annoying + items.push({ type, members }); + this.next.shift(); // eat the crec + break; + } + case 'crec': { + if (!close.includes('crec')) { + throw new ParseFailure( + 'unexpected end of record without corresponding start of record', + next.offset + ); + } + return { type: 'seq', items }; + } + default: { + // @ts-ignore + throw new Error(`unreachable: unknown token type ${next.type}`); + } + } + } + } +} + +export function parse(src: FragmentNode[]): Seq | Failure { + const parser = new ExprParser(src); + try { + return parser.parseSeq(['eof']); + } catch (e) { + if (e instanceof ParseFailure) { + return { type: 'failure', message: e.message, offset: e.offset }; + } + throw e; + } +} + +type Index = number | 'callee' | null; +export function walk( + f: (expr: Expr, index: Index, parent: Expr | null) => void, + current: Expr, + index: Index = null, + parent: Expr | null = null +) { + f(current, index, parent); + switch (current.type) { + case 'prose': { + break; + } + case 'list': { + for (let i = 0; i < current.elements.length; ++i) { + walk(f, current.elements[i], i, current); + } + break; + } + case 'record': { + for (let i = 0; i < current.members.length; ++i) { + walk(f, current.members[i].value, i, current); + } + break; + } + case 'record-spec': { + break; + } + case 'call': { + walk(f, current.callee, 'callee', current); + for (let i = 0; i < current.arguments.length; ++i) { + walk(f, current.arguments[i], i, current); + } + break; + } + case 'paren': + case 'seq': { + for (let i = 0; i < current.items.length; ++i) { + walk(f, current.items[i], i, current); + } + break; + } + default: { + // @ts-ignore + throw new Error(`unreachable: unknown expression node type ${current.type}`); + } + } +} diff --git a/src/header-parser.ts b/src/header-parser.ts index 313e13d3..14449de8 100644 --- a/src/header-parser.ts +++ b/src/header-parser.ts @@ -645,7 +645,7 @@ export function formatPreamble( return paras; } -function formatEnglishList(list: Array) { +export function formatEnglishList(list: Array, conjuction = 'and') { if (list.length === 0) { throw new Error('formatEnglishList should not be called with an empty list'); } @@ -653,9 +653,9 @@ function formatEnglishList(list: Array) { return list[0]; } if (list.length === 2) { - return `${list[0]} and ${list[1]}`; + return `${list[0]} ${conjuction} ${list[1]}`; } - return `${list.slice(0, -1).join(', ')}, and ${list[list.length - 1]}`; + return `${list.slice(0, -1).join(', ')}, ${conjuction} ${list[list.length - 1]}`; } function eat(text: string, regex: RegExp) { diff --git a/src/lint/lint.ts b/src/lint/lint.ts index 20fe8c21..3273c1f2 100644 --- a/src/lint/lint.ts +++ b/src/lint/lint.ts @@ -85,6 +85,8 @@ export async function lint( if ('tree' in pair) { // @ts-ignore we are intentionally adding a property here pair.element.ecmarkdownTree = pair.tree; + // @ts-ignore we are intentionally adding a property here + pair.element.originalHtml = pair.element.innerHTML; } } } diff --git a/src/lint/rules/algorithm-line-style.ts b/src/lint/rules/algorithm-line-style.ts index 72832f81..612e0b64 100644 --- a/src/lint/rules/algorithm-line-style.ts +++ b/src/lint/rules/algorithm-line-style.ts @@ -125,7 +125,7 @@ export default function (report: Reporter, node: Element, algorithmSource: strin const hasSubsteps = node.sublist !== null; - // Special case: lines without substeps can end in `pre` tags. + // Special case: only lines without substeps can end in `pre` tags. if (last.name === 'opaqueTag' && /^\s*
/.test(last.contents)) {
         if (hasSubsteps) {
           report({
diff --git a/test/expr-parser.js b/test/expr-parser.js
new file mode 100644
index 00000000..56aa8d57
--- /dev/null
+++ b/test/expr-parser.js
@@ -0,0 +1,259 @@
+'use strict';
+
+let { assertLint, positioned, lintLocationMarker: M, assertLintFree } = require('./utils.js');
+
+describe('expression parsing', () => {
+  describe('valid', () => {
+    it('lists', async () => {
+      await assertLintFree(`
+        
+          1. Let _x_ be «».
+          1. Set _x_ to « ».
+          1. Set _x_ to «0».
+          1. Set _x_ to «0, 1».
+          1. Set _x_ to « 0,1 ».
+        
+      `);
+    });
+
+    it('calls', async () => {
+      await assertLintFree(`
+        
+          1. Let _x_ be _foo_().
+          1. Set _x_ to _foo_( ).
+          1. Set _x_ to _foo_.[[Bar]]().
+          1. Set _x_ to _foo_(0).
+          1. Set _x_ to _foo_( 0 ).
+          1. Set _x_ to _foo_(0, 1).
+        
+      `);
+    });
+
+    it('records', async () => {
+      await assertLintFree(`
+        
+          1. Let _x_ be { [[x]]: 0 }.
+          1. Set _x_ to {[[x]]:0}.
+        
+      `);
+    });
+
+    it('record-spec', async () => {
+      await assertLintFree(`
+        
+          1. For each Record { [[Key]], [[Value]] } _p_ of _entries_, do
+            1. Something.
+        
+      `);
+    });
+
+    it('trailing comma in multi-line list', async () => {
+      await assertLintFree(`
+        
+        1. Let _x_ be «
+            0,
+            1,
+          ».
+        
+      `);
+    });
+
+    it('trailing comma in multi-line call', async () => {
+      await assertLintFree(`
+        
+        1. Let _x_ be _foo_(
+            0,
+            1,
+          ).
+        
+      `);
+    });
+
+    it('trailing comma in multi-line record', async () => {
+      await assertLintFree(`
+        
+        1. Let _x_ be the Record {
+            [[X]]: 0,
+            [[Y]]: 1,
+          }.
+        
+      `);
+    });
+  });
+
+  describe('errors', () => {
+    it('open paren without close', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let (.${M}
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected eof (expected close parenthesis)',
+        }
+      );
+    });
+
+    it('close paren without open', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let ${M}).
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected close parenthesis without corresponding open parenthesis',
+        }
+      );
+    });
+
+    it('mismatched open/close tokens', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be «_foo_(_a_${M}»).
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected list close without corresponding list open',
+        }
+      );
+    });
+
+    it('elision in list', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be «${M},».
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected comma (expected some content for element/argument)',
+        }
+      );
+
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be «_x_, ${M}».
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected list close (expected some content for element)',
+        }
+      );
+    });
+
+    it('elision in call', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be _foo_(${M},)».
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected comma (expected some content for element/argument)',
+        }
+      );
+
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be _foo_(_a_, ${M}).
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'unexpected close parenthesis (expected some content for argument)',
+        }
+      );
+    });
+
+    it('record without names', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be the Record { ${M}}.
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'expected to find record field',
+        }
+      );
+    });
+
+    it('record with malformed names', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be the Record { ${M}[x]: 0 }.
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'expected to find record field',
+        }
+      );
+    });
+
+    it('record with duplicate names', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be the Record { [[A]]: 0, [[${M}A]]: 0 }.
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'duplicate record field name A',
+        }
+      );
+    });
+
+    it('record where only some keys have values', async () => {
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be the Record { [[A]], [[B]]${M}: 0 }.
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'record field has value but preceding field does not',
+        }
+      );
+
+      await assertLint(
+        positioned`
+          
+            1. Let _x_ be the Record { [[A]]: 0, [[B]]${M} }.
+          
+        `,
+        {
+          ruleId: 'expression-parsing',
+          nodeType: 'emu-alg',
+          message: 'expected record field to have value',
+        }
+      );
+    });
+  });
+});
diff --git a/test/lint-algorithms.js b/test/lint-algorithms.js
index abe7193e..8744b271 100644
--- a/test/lint-algorithms.js
+++ b/test/lint-algorithms.js
@@ -250,7 +250,7 @@ describe('linting algorithms', () => {
             1. Substep.
           1. Let _constructorText_ be the source text
           
constructor() {}
- 1. Set _constructor_ to ParseText(_constructorText_, _methodDefinition_). + 1. Set _constructor_ to _parse_(_constructorText_, _methodDefinition_). 1. A highlighted line. 1. Amend the spec with this. 1. Remove this from the spec. diff --git a/test/typecheck.js b/test/typecheck.js index 4fb046dc..76697b8a 100644 --- a/test/typecheck.js +++ b/test/typecheck.js @@ -1,8 +1,37 @@ 'use strict'; -let { assertLint, assertLintFree, positioned, lintLocationMarker: M } = require('./utils.js'); +let { + assertLint, + assertLintFree, + positioned, + lintLocationMarker: M, + getBiblio, +} = require('./utils.js'); + +describe('typechecking completions', () => { + let biblio; + before(async () => { + biblio = await getBiblio(` + +

NormalCompletion ( _x_ )

+
+
+ + +

+ Completion ( + _completionRecord_: a Completion Record, + ): a Completion Record +

+
+ + 1. Assert: _completionRecord_ is a Completion Record. + 1. Return _completionRecord_. + +
+ `); + }); -describe('typechecking', () => { describe('completion-returning AO not consumed as completion', () => { it('positive', async () => { await assertLint( @@ -33,65 +62,57 @@ ${M} 1. Return ExampleAlg(). ruleId: 'invocation-return-type', nodeType: 'emu-alg', message: 'ExampleAlg returns a Completion Record, but is not consumed as if it does', + }, + { + extraBiblios: [biblio], } ); }); - // UUUUGH the check for Completion() assumes it's happening after auto-linking + // the check for Completion() assumes it's happening after auto-linking // so it only works if the Completion AO is defined it('negative', async () => { - await assertLintFree(` - -

- ExampleAlg (): a normal completion containing a Number -

-
-
- - 1. Return NormalCompletion(0). - -
- - -

- Completion ( - _completionRecord_: a Completion Record, - ): a Completion Record -

-
-
description
-
It is used to emphasize that a Completion Record is being returned.
-
- - 1. Assert: _completionRecord_ is a Completion Record. - 1. Return _completionRecord_. - -
+ await assertLintFree( + ` + +

+ ExampleAlg (): a normal completion containing a Number +

+
+
+ + 1. Return NormalCompletion(0). + +
- -

- Example2 () -

-
-
- - 1. Let _a_ be Completion(ExampleAlg()). - 1. Set _a_ to ! ExampleAlg(). - 1. Return ? ExampleAlg(). - -
+ +

+ Example2 () +

+
+
+ + 1. Let _a_ be Completion(ExampleAlg()). + 1. Set _a_ to ! ExampleAlg(). + 1. Return ? ExampleAlg(). + +
- -

- Example3 () -

-
-
- - 1. Return ExampleAlg(). - -
- `); + +

+ Example3 () +

+
+
+ + 1. Return ExampleAlg(). + +
+ `, + { + extraBiblios: [biblio], + } + ); }); }); @@ -167,7 +188,7 @@ ${M} 1. Return ? ExampleAlg().
- 1. Return ${M}? Foo(). + 1. Return ${M}? _foo_. `, @@ -218,25 +239,33 @@ ${M} 1. Return ? ExampleAlg(). nodeType: 'emu-alg', message: 'this would return a Completion Record, but the containing AO is declared not to return a Completion Record', + }, + { + extraBiblios: [biblio], } ); }); it('negative', async () => { - await assertLintFree(` - -

- ExampleAlg (): either a normal completion containing a Number or an abrupt completion -

-
-
- - 1. Return ? Foo(). - 1. Return Completion(_x_). - 1. Throw a new TypeError. - -
- `); + await assertLintFree( + ` + +

+ ExampleAlg (): either a normal completion containing a Number or an abrupt completion +

+
+
+ + 1. Return ? _foo_. + 1. Return Completion(_x_). + 1. Throw a new TypeError. + +
+ `, + { + extraBiblios: [biblio], + } + ); }); }); @@ -251,7 +280,7 @@ ${M} 1. Return ? ExampleAlg().
${M} - 1. Return Foo(). + 1. Return _foo_. `, @@ -273,7 +302,7 @@ ${M} 1. Return ? ExampleAlg().
- 1. Return ? Foo(). + 1. Return ? _foo_. `); @@ -286,7 +315,7 @@ ${M} 1. Return ? ExampleAlg().
- 1. Return Foo(). + 1. Return _foo_. `); @@ -304,7 +333,7 @@ ${M} 1. Return ? ExampleAlg().
- 1. Return Foo(). + 1. Return _foo_. @@ -337,7 +366,7 @@ ${M} 1. Let _x_ be ExampleAlg().
- 1. Return Foo(). + 1. Return _foo_. @@ -452,7 +481,7 @@ ${M} 1. Let _x_ be ExampleAlg().
- 1. Return ? Foo(). + 1. Return ? _foo_. @@ -485,7 +514,7 @@ ${M} 1. Let _x_ be ExampleAlg().
- 1. Return ? Foo(). + 1. Return ? _foo_. @@ -514,7 +543,7 @@ ${M} 1. Let _x_ be ExampleAlg().
- 1. Return Foo(). + 1. Return 0. `, @@ -528,18 +557,146 @@ ${M} 1. Let _x_ be ExampleAlg(). }); it('negative', async () => { - await assertLintFree(` - -

- ExampleAlg (): a normal completion containing either a Number or a boolean -

-
-
+ await assertLintFree( + ` + +

+ ExampleAlg (): a normal completion containing either a Number or a boolean +

+
+
+ + 1. Return NormalCompletion(0). + +
+ `, + { + extraBiblios: [biblio], + } + ); + }); + }); +}); + +describe('signature agreement', async () => { + let biblio; + before(async () => { + biblio = await getBiblio(` + +

TakesNoArgs ()

+
+
+ + +

TakesOneArg ( _x_ )

+
+
+ + +

TakesOneOrTwoArgs ( _x_, [_y_] )

+
+
+ `); + }); + + it('extra args', async () => { + await assertLint( + positioned` - 1. Return NormalCompletion(Foo()). + 1. Return ${M}TakesNoArgs(0). -
- `); - }); + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'TakesNoArgs takes 0 arguments, but this invocation passes 1', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}TakesNoArgs(0). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'TakesNoArgs takes 0 arguments, but this invocation passes 1', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}TakesOneOrTwoArgs(0, 1, 2). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'TakesOneOrTwoArgs takes 1-2 arguments, but this invocation passes 3', + }, + { + extraBiblios: [biblio], + } + ); + }); + + it('too few args args', async () => { + await assertLint( + positioned` + + 1. Return ${M}TakesOneArg(). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'TakesOneArg takes 1 argument, but this invocation passes 0', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}TakesOneOrTwoArgs(). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'TakesOneOrTwoArgs takes 1-2 arguments, but this invocation passes 0', + }, + { + extraBiblios: [biblio], + } + ); + }); + + it('negative', async () => { + await assertLintFree( + ` + + 1. Perform TakesNoArgs(). + 1. Perform TakesOneArg(0). + 1. Perform TakesOneOrTwoArgs(0). + 1. Perform TakesOneOrTwoArgs(0, 1). + 1. Perform TakesNoArgs(). + + `, + { + extraBiblios: [biblio], + } + ); }); }); diff --git a/test/utils.js b/test/utils.js index 94aeb657..acab77af 100644 --- a/test/utils.js +++ b/test/utils.js @@ -119,8 +119,8 @@ async function assertErrorFree(html, opts) { assert.deepStrictEqual(warnings, []); } -async function assertLint(a, b) { - await assertError(a, b, { lintSpec: true }); +async function assertLint(a, b, opts = {}) { + await assertError(a, b, { lintSpec: true, ...opts }); } async function assertLintFree(html, opts = {}) { From a8f96e5e70e250abbbf73df358f68748cab1a79f Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Mon, 4 Jul 2022 16:56:31 -0700 Subject: [PATCH 2/9] make completion-consumption checks based on expr-parser --- src/Spec.ts | 459 +++++++++++++++++++-------------------------- src/expr-parser.ts | 40 ++-- test/typecheck.js | 13 +- 3 files changed, 223 insertions(+), 289 deletions(-) diff --git a/src/Spec.ts b/src/Spec.ts index 7fd32c04..df7f9344 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -45,16 +45,11 @@ import { import { lint } from './lint/lint'; import { CancellationToken } from 'prex'; import type { JSDOM } from 'jsdom'; -import type { - OrderedListItemNode, - OrderedListNode, - parseAlgorithm, - UnorderedListItemNode, -} from 'ecmarkdown'; +import type { OrderedListNode, parseAlgorithm } from 'ecmarkdown'; import { getProductions, rhsMatches, getLocationInGrammarFile } from './lint/utils'; import type { AugmentedGrammarEle } from './Grammar'; import { offsetToLineAndColumn } from './utils'; -import { parse as parseExpr, walk as walkExpr, Expr } from './expr-parser'; +import { parse as parseExpr, walk as walkExpr, Expr, PathItem } from './expr-parser'; const DRAFT_DATE_FORMAT: Intl.DateTimeFormatOptions = { year: 'numeric', @@ -484,8 +479,7 @@ export default class Spec { if (this.opts.lintSpec) { this.log('Checking types...'); - this.checkCompletionUsage(); - this.checkCalls(); + this.typecheck(); } this.log('Propagating effect annotations...'); this.propagateEffects(); @@ -593,8 +587,8 @@ export default class Spec { } // checks that AOs which do/don't return completion records are invoked appropriately - private checkCompletionUsage() { - const isCall = (x: Xref) => x.isInvocation && x.clause != null && x.aoid != null; + // also checks that the appropriate number of arguments are passed + private typecheck() { const isUnused = (t: Type) => t.kind === 'unused' || (t.kind === 'completion' && @@ -605,146 +599,154 @@ export default class Spec { const onlyPerformed: Map = new Map( AOs.filter(e => !isUnused(e.signature!.return!)).map(a => [a.aoid, null]) ); - const alwaysAssertedToBeNormal: Map = new Map( AOs.filter(e => e.signature!.return!.kind === 'completion').map(a => [a.aoid, null]) ); - for (const xref of this._xrefs) { - if (!isCall(xref)) { - continue; - } - const biblioEntry = this.biblio.byAoid(xref.aoid); - if (biblioEntry == null) { - this.warn({ - type: 'node', - node: xref.node, - ruleId: 'xref-biblio', - message: `could not find biblio entry for xref ${JSON.stringify(xref.aoid)}`, - }); - continue; - } - const signature = biblioEntry.signature; - if (signature == null) { + for (const node of this.doc.querySelectorAll('emu-alg')) { + if (node.hasAttribute('example') || !('ecmarkdownTree' in node)) { continue; } - - const { return: returnType } = signature; - if (returnType == null) { + // @ts-ignore + const tree = node.ecmarkdownTree as ReturnType; + if (tree == null) { continue; } + // @ts-ignore + const originalHtml: string = node.originalHtml; - // this is really gross - // i am sorry - // the better approach is to make the xref-linkage logic happen on the level of the ecmarkdown tree - // so that we still have this information at that point - // rather than having to reconstruct it, approximately - // ... someday! - const warn = (message: string) => { - const path = []; - let pointer: HTMLElement | null = xref.node; - let alg: HTMLElement | null = null; - while (pointer != null) { - if (pointer.tagName === 'LI') { - // @ts-ignore - path.unshift([].indexOf.call(pointer.parentElement!.children, pointer)); - } - if (pointer.tagName === 'EMU-ALG') { - alg = pointer; - break; - } - pointer = pointer.parentElement; + const expressionVisitor = (expr: Expr, path: PathItem[]) => { + if (expr.type !== 'call') { + return; } - if (alg?.hasAttribute('example')) { + + const { callee, arguments: args } = expr; + if (!(callee.parts.length === 1 && callee.parts[0].name === 'text')) { return; } - if (alg == null || !{}.hasOwnProperty.call(alg, 'ecmarkdownTree')) { - const ruleId = 'completion-invocation'; - let pointer: Element | null = xref.node; - while (this.locate(pointer) == null) { - pointer = pointer.parentElement; - if (pointer == null) { - break; - } - } - if (pointer == null) { - this.warn({ - type: 'global', - ruleId, - message: message + ` but I wasn't able to find a source location to give you, sorry!`, - }); - } else { - this.warn({ - type: 'node', - node: pointer, - ruleId, - message, - }); - } - } else { - // @ts-ignore - const tree = alg.ecmarkdownTree as ReturnType; - let stepPointer: OrderedListItemNode | UnorderedListItemNode = { - sublist: tree.contents, - } as OrderedListItemNode; - for (const step of path) { - stepPointer = stepPointer.sublist!.contents[step]; - } + const calleeName = callee.parts[0].contents; + + const warn = (message: string) => { + const { line, column } = offsetToLineAndColumn( + originalHtml, + callee.parts[0].location.start.offset + ); this.warn({ type: 'contents', - node: alg, - ruleId: 'invocation-return-type', + ruleId: 'typecheck', message, - nodeRelativeLine: stepPointer.location.start.line, - nodeRelativeColumn: stepPointer.location.start.column, + node, + nodeRelativeLine: line, + nodeRelativeColumn: column, }); + }; + + const biblioEntry = this.biblio.byAoid(calleeName); + if (biblioEntry == null) { + if ( + ![ + 'thisTimeValue', + 'thisStringValue', + 'thisBigIntValue', + 'thisNumberValue', + 'thisSymbolValue', + 'thisBooleanValue', + 'toUppercase', + 'toLowercase', + 'ℝ', + '𝔽', + 'ℤ', + ].includes(calleeName) + ) { + // TODO make the spec not do this + warn(`could not find definition for ${calleeName}`); + } + return; + } else if (biblioEntry.signature == null) { + return; + } + const min = biblioEntry.signature.parameters.length; + const max = min + biblioEntry.signature.optionalParameters.length; + if (args.length < min || args.length > max) { + const count = `${min}${min === max ? '' : `-${max}`}`; + // prettier-ignore + const message = `${calleeName} takes ${count} argument${count === '1' ? '' : 's'}, but this invocation passes ${args.length}`; + warn(message); } - }; - const consumedAsCompletion = isConsumedAsCompletion(xref); - // checks elsewhere ensure that well-formed documents never have a union of completion and non-completion, so checking the first child suffices - // TODO: this is for 'a break completion or a throw completion', which is kind of a silly union; maybe address that in some other way? - const isCompletion = - returnType.kind === 'completion' || - (returnType.kind === 'union' && returnType.types[0].kind === 'completion'); - if (['Completion', 'ThrowCompletion', 'NormalCompletion'].includes(xref.aoid)) { - if (consumedAsCompletion) { + const { return: returnType } = biblioEntry.signature; + if (returnType == null) { + return; + } + + const consumedAsCompletion = isConsumedAsCompletion(expr, path); + + // checks elsewhere ensure that well-formed documents never have a union of completion and non-completion, so checking the first child suffices + // TODO: this is for 'a break completion or a throw completion', which is kind of a silly union; maybe address that in some other way? + const isCompletion = + returnType.kind === 'completion' || + (returnType.kind === 'union' && returnType.types[0].kind === 'completion'); + if (['Completion', 'ThrowCompletion', 'NormalCompletion'].includes(calleeName)) { + if (consumedAsCompletion) { + warn( + `${calleeName} clearly creates a Completion Record; it does not need to be marked as such, and it would not be useful to immediately unwrap its result` + ); + } + } else if (isCompletion && !consumedAsCompletion) { + warn(`${calleeName} returns a Completion Record, but is not consumed as if it does`); + } else if (!isCompletion && consumedAsCompletion) { + warn(`${calleeName} does not return a Completion Record, but is consumed as if it does`); + } + if (returnType.kind === 'unused' && !isCalledAsPerform(expr, path, false)) { warn( - `${xref.aoid} clearly creates a Completion Record; it does not need to be marked as such, and it would not be useful to immediately unwrap its result` + `${calleeName} does not return a meaningful value and should only be invoked as \`Perform ${calleeName}(...).\`` ); } - } else if (isCompletion && !consumedAsCompletion) { - warn(`${xref.aoid} returns a Completion Record, but is not consumed as if it does`); - } else if (!isCompletion && consumedAsCompletion) { - warn(`${xref.aoid} does not return a Completion Record, but is consumed as if it does`); - } - if (returnType.kind === 'unused' && !isCalledAsPerform(xref, false)) { - warn( - `${xref.aoid} does not return a meaningful value and should only be invoked as \`Perform ${xref.aoid}(...).\`` - ); - } - - if (onlyPerformed.has(xref.aoid) && onlyPerformed.get(xref.aoid) !== 'top') { - const old = onlyPerformed.get(xref.aoid); - const performed = isCalledAsPerform(xref, true); - if (!performed) { - onlyPerformed.set(xref.aoid, 'top'); - } else if (old === null) { - onlyPerformed.set(xref.aoid, 'only performed'); + + if (onlyPerformed.has(calleeName) && onlyPerformed.get(calleeName) !== 'top') { + const old = onlyPerformed.get(calleeName); + const performed = isCalledAsPerform(expr, path, true); + if (!performed) { + onlyPerformed.set(calleeName, 'top'); + } else if (old === null) { + onlyPerformed.set(calleeName, 'only performed'); + } } - } - if ( - alwaysAssertedToBeNormal.has(xref.aoid) && - alwaysAssertedToBeNormal.get(xref.aoid) !== 'top' - ) { - const old = alwaysAssertedToBeNormal.get(xref.aoid); - const asserted = isAssertedToBeNormal(xref); - if (!asserted) { - alwaysAssertedToBeNormal.set(xref.aoid, 'top'); - } else if (old === null) { - alwaysAssertedToBeNormal.set(xref.aoid, 'always asserted normal'); + if ( + alwaysAssertedToBeNormal.has(calleeName) && + alwaysAssertedToBeNormal.get(calleeName) !== 'top' + ) { + const old = alwaysAssertedToBeNormal.get(calleeName); + const asserted = isAssertedToBeNormal(expr, path); + if (!asserted) { + alwaysAssertedToBeNormal.set(calleeName, 'top'); + } else if (old === null) { + alwaysAssertedToBeNormal.set(calleeName, 'always asserted normal'); + } } - } + }; + const walkLines = (list: OrderedListNode) => { + for (const line of list.contents) { + const item = parseExpr(line.contents); + if (item.type === 'failure') { + const { line, column } = offsetToLineAndColumn(originalHtml, item.offset); + this.warn({ + type: 'contents', + ruleId: 'expression-parsing', + message: item.message, + node, + nodeRelativeLine: line, + nodeRelativeColumn: column, + }); + } else { + walkExpr(expressionVisitor, item); + } + if (line.sublist?.name === 'ol') { + walkLines(line.sublist); + } + } + }; + walkLines(tree.contents); } for (const [aoid, state] of onlyPerformed) { @@ -798,108 +800,6 @@ export default class Spec { } } - private checkCalls() { - for (const node of this.doc.querySelectorAll('emu-alg')) { - if (node.hasAttribute('example') || !('ecmarkdownTree' in node)) { - continue; - } - // @ts-ignore - const tree = node.ecmarkdownTree as ReturnType; - if (tree == null) { - continue; - } - // @ts-ignore - const originalHtml: string = node.originalHtml; - - const expressionVisitor = (expr: Expr) => { - if (expr.type !== 'call') { - return; - } - - const { callee, arguments: args } = expr; - if (!(callee.parts.length === 1 && callee.parts[0].name === 'text')) { - return; - } - const calleeName = callee.parts[0].contents; - if ( - [ - 'thisTimeValue', - 'thisStringValue', - 'thisBigIntValue', - 'thisNumberValue', - 'thisSymbolValue', - 'thisBooleanValue', - 'toUppercase', - 'toLowercase', - 'ℝ', - '𝔽', - 'ℤ', - ].includes(calleeName) - ) { - // TODO make the spec not do this - return; - } - const biblioEntry = this.biblio.byAoid(calleeName); - if (biblioEntry == null) { - const { line, column } = offsetToLineAndColumn( - originalHtml, - callee.parts[0].location.start.offset - ); - this.warn({ - type: 'contents', - ruleId: 'typecheck', - message: `could not find definition for ${calleeName}`, - node, - nodeRelativeLine: line, - nodeRelativeColumn: column, - }); - } else if (biblioEntry.signature != null) { - const min = biblioEntry.signature.parameters.length; - const max = min + biblioEntry.signature.optionalParameters.length; - if (args.length < min || args.length > max) { - const { line, column } = offsetToLineAndColumn( - originalHtml, - callee.parts[0].location.start.offset - ); - const count = `${min}${min === max ? '' : `-${max}`}`; - // prettier-ignore - const message = `${calleeName} takes ${count} argument${count === '1' ? '' : 's'}, but this invocation passes ${args.length}`; - this.warn({ - type: 'contents', - ruleId: 'typecheck', - message, - node, - nodeRelativeLine: line, - nodeRelativeColumn: column, - }); - } - } - }; - const walkLines = (list: OrderedListNode) => { - for (const line of list.contents) { - const item = parseExpr(line.contents); - if (item.type === 'failure') { - const { line, column } = offsetToLineAndColumn(originalHtml, item.offset); - this.warn({ - type: 'contents', - ruleId: 'expression-parsing', - message: item.message, - node, - nodeRelativeLine: line, - nodeRelativeColumn: column, - }); - } else { - walkExpr(expressionVisitor, item); - } - if (line.sublist?.name === 'ol') { - walkLines(line.sublist); - } - } - }; - walkLines(tree.contents); - } - } - private toHTML() { const htmlEle = this.doc.documentElement; return '\n' + (htmlEle.hasAttributes() ? htmlEle.outerHTML : htmlEle.innerHTML); @@ -2173,56 +2073,75 @@ function pathFromRelativeLink(link: HTMLAnchorElement | HTMLLinkElement) { return link.href.startsWith('about:blank') ? link.href.substring(11) : link.href; } -function isFirstChild(node: Node) { - const p = node.parentElement!; - return ( - p.childNodes[0] === node || (p.childNodes[0].textContent === '' && p.childNodes[1] === node) - ); +function parentSkippingBlankSpace(expr: Expr, path: PathItem[]): PathItem | null { + for (let pointer: Expr = expr, i = path.length - 1; i >= 0; pointer = path[i].parent, --i) { + const { parent } = path[i]; + if ( + parent.type === 'seq' && + parent.items.every( + i => + (i.type === 'prose' && + i.parts.every( + p => p.name === 'tag' || (p.name === 'text' && /^\s*$/.test(p.contents)) + )) || + i === pointer + ) + ) { + // if parent is just whitespace/tags around the call, walk up the tree further + continue; + } + return path[i]; + } + return null; } -// TODO factor some of this stuff out -function isCalledAsPerform(xref: Xref, allowQuestion: boolean) { - let node = xref.node; - if (node.parentElement?.tagName === 'EMU-META' && isFirstChild(node)) { - node = node.parentElement; +function previousText(expr: Expr, path: PathItem[]): string | null { + const part = parentSkippingBlankSpace(expr, path); + if (part == null) { + return null; } - const previousSibling = node.previousSibling; - if (previousSibling?.nodeType !== 3 /* TEXT_NODE */) { - return false; + const { parent, index } = part; + if (parent.type === 'seq' && index > 0) { + const prev = parent.items[(index as number) - 1]; + if (prev.type === 'prose' && prev.parts.length > 0) { + const prevProse = prev.parts[prev.parts.length - 1]; + if (prevProse.name === 'text') { + return prevProse.contents; + } + } } - return (allowQuestion ? /\bperform ([?!]\s)?$/i : /\bperform $/i).test( - previousSibling.textContent! - ); + return null; } -function isAssertedToBeNormal(xref: Xref) { - let node = xref.node; - if (node.parentElement?.tagName === 'EMU-META' && isFirstChild(node)) { - node = node.parentElement; - } - const previousSibling = node.previousSibling; - if (previousSibling?.nodeType !== 3 /* TEXT_NODE */) { - return false; - } - return /\s!\s$/.test(previousSibling.textContent!); +function isCalledAsPerform(expr: Expr, path: PathItem[], allowQuestion: boolean) { + const prev = previousText(expr, path); + return prev != null && (allowQuestion ? /\bperform ([?!]\s)?$/i : /\bperform $/i).test(prev); } -function isConsumedAsCompletion(xref: Xref) { - let node = xref.node; - if (node.parentElement?.tagName === 'EMU-META' && isFirstChild(node)) { - node = node.parentElement; - } - const previousSibling = node.previousSibling; - if (previousSibling?.nodeType !== 3 /* TEXT_NODE */) { +function isAssertedToBeNormal(expr: Expr, path: PathItem[]) { + const prev = previousText(expr, path); + return prev != null && /\s!\s$/.test(prev); +} + +function isConsumedAsCompletion(expr: Expr, path: PathItem[]) { + const part = parentSkippingBlankSpace(expr, path); + if (part == null) { return false; } - if (/[!?]\s$/.test(previousSibling.textContent!)) { - return true; - } - if (previousSibling.textContent! === '(') { - // check for Completion( - const previousPrevious = previousSibling.previousSibling; - if (previousPrevious?.textContent === 'Completion') { + const { parent, index } = part; + if (parent.type === 'seq' && index > 0) { + // if the previous text ends in `! ` or `? `, this is a completion + const prev = parent.items[(index as number) - 1]; // typescript isn't quite smart enough to infer this, alas + if (prev.type === 'prose' && prev.parts.length > 0) { + const prevProse = prev.parts[prev.parts.length - 1]; + if (prevProse.name === 'text' && /[!?]\s$/.test(prevProse.contents)) { + return true; + } + } + } else if (parent.type === 'call' && index === 0 && parent.arguments.length === 1) { + // if this is `Completion(Expr())`, this is a completion + const { parts } = parent.callee; + if (parts.length === 1 && parts[0].name === 'text' && parts[0].contents === 'Completion') { return true; } } diff --git a/src/expr-parser.ts b/src/expr-parser.ts index dc8d128b..84288a85 100644 --- a/src/expr-parser.ts +++ b/src/expr-parser.ts @@ -71,10 +71,12 @@ function formatClose(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]) { return formatEnglishList(mapped, 'or'); } +function isWhitespace(x: Prose) { + return x.parts.every(p => p.name === 'text' && /^\s*$/.test(p.contents)); +} + function isEmpty(s: Seq) { - return s.items.every( - i => i.type === 'prose' && i.parts.every(p => p.name === 'text' && /^\s*$/.test(p.contents)) - ); + return s.items.every(i => i.type === 'prose' && isWhitespace(i)); } function emptyThingHasNewline(s: Seq) { @@ -425,27 +427,33 @@ export function parse(src: FragmentNode[]): Seq | Failure { } } -type Index = number | 'callee' | null; +export type PathItem = + | { parent: List | Record | Seq | Paren; index: number } + | { parent: Call; index: 'callee' | number }; export function walk( - f: (expr: Expr, index: Index, parent: Expr | null) => void, + f: (expr: Expr, path: PathItem[]) => void, current: Expr, - index: Index = null, - parent: Expr | null = null + path: PathItem[] = [] ) { - f(current, index, parent); + // console.log('path', path, current.type); + f(current, path); switch (current.type) { case 'prose': { break; } case 'list': { for (let i = 0; i < current.elements.length; ++i) { - walk(f, current.elements[i], i, current); + path.push({ parent: current, index: i }); + walk(f, current.elements[i], path); + path.pop(); } break; } case 'record': { for (let i = 0; i < current.members.length; ++i) { - walk(f, current.members[i].value, i, current); + path.push({ parent: current, index: i }); + walk(f, current.members[i].value, path); + path.pop(); } break; } @@ -453,16 +461,22 @@ export function walk( break; } case 'call': { - walk(f, current.callee, 'callee', current); + path.push({ parent: current, index: 'callee' }); + walk(f, current.callee, path); + path.pop(); for (let i = 0; i < current.arguments.length; ++i) { - walk(f, current.arguments[i], i, current); + path.push({ parent: current, index: i }); + walk(f, current.arguments[i], path); + path.pop(); } break; } case 'paren': case 'seq': { for (let i = 0; i < current.items.length; ++i) { - walk(f, current.items[i], i, current); + path.push({ parent: current, index: i }); + walk(f, current.items[i], path); + path.pop(); } break; } diff --git a/test/typecheck.js b/test/typecheck.js index 76697b8a..0d29ec69 100644 --- a/test/typecheck.js +++ b/test/typecheck.js @@ -54,12 +54,12 @@ describe('typechecking completions', () => {
-${M} 1. Return ExampleAlg(). + 1. Return ${M}ExampleAlg(). `, { - ruleId: 'invocation-return-type', + ruleId: 'typecheck', nodeType: 'emu-alg', message: 'ExampleAlg returns a Completion Record, but is not consumed as if it does', }, @@ -93,6 +93,7 @@ ${M} 1. Return ExampleAlg(). 1. Let _a_ be Completion(ExampleAlg()). + 1. Let _a_ be Completion(ExampleAlg()). 1. Set _a_ to ! ExampleAlg(). 1. Return ? ExampleAlg(). @@ -138,12 +139,12 @@ ${M} 1. Return ExampleAlg().
-${M} 1. Return ? ExampleAlg(). + 1. Return ? ${M}ExampleAlg(). `, { - ruleId: 'invocation-return-type', + ruleId: 'typecheck', nodeType: 'emu-alg', message: 'ExampleAlg does not return a Completion Record, but is consumed as if it does', } @@ -344,12 +345,12 @@ ${M} 1. Return ? ExampleAlg().
-${M} 1. Let _x_ be ExampleAlg(). + 1. Let _x_ be ${M}ExampleAlg(). `, { - ruleId: 'invocation-return-type', + ruleId: 'typecheck', nodeType: 'emu-alg', message: 'ExampleAlg does not return a meaningful value and should only be invoked as `Perform ExampleAlg(...).`', From d36b8cf5ddf0c807f49940cf6a51cded3852e921 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Mon, 4 Jul 2022 19:59:12 -0700 Subject: [PATCH 3/9] add kind to algorithm biblio entries this is not breaking because it allows 'undefined' --- src/Biblio.ts | 7 +++++++ src/Clause.ts | 31 ++++++++++++++++++------------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/Biblio.ts b/src/Biblio.ts index 2ae6905b..93eb0ad0 100644 --- a/src/Biblio.ts +++ b/src/Biblio.ts @@ -316,9 +316,16 @@ export type Signature = { optionalParameters: Parameter[]; return: null | Type; }; +export type AlgorithmType = + | 'abstract operation' + | 'host-defined abstract operation' + | 'implementation-defined abstract operation' + | 'syntax-directed operation' + | 'numeric method'; export interface AlgorithmBiblioEntry extends BiblioEntryBase { type: 'op'; aoid: string; + kind?: AlgorithmType; signature: null | Signature; effects: string[]; /*@internal*/ _node?: Element; diff --git a/src/Clause.ts b/src/Clause.ts index 0e471aff..0b1391b6 100644 --- a/src/Clause.ts +++ b/src/Clause.ts @@ -1,7 +1,7 @@ import type Note from './Note'; import type Example from './Example'; import type Spec from './Spec'; -import type { PartialBiblioEntry, Signature, Type } from './Biblio'; +import type { AlgorithmType, PartialBiblioEntry, Signature, Type } from './Biblio'; import type { Context } from './Context'; import { ParseError, TypeParser } from './type-parser'; @@ -15,6 +15,15 @@ import { } from './header-parser'; import { offsetToLineAndColumn } from './utils'; +const aoidTypes = [ + 'abstract operation', + 'sdo', + 'syntax-directed operation', + 'host-defined abstract operation', + 'implementation-defined abstract operation', + 'numeric method', +]; + export function extractStructuredHeader(header: Element): Element | null { const dl = header.nextElementSibling; if (dl == null || dl.tagName !== 'DL' || !dl.classList.contains('header')) { @@ -211,18 +220,7 @@ export default class Clause extends Builder { node: this.node, attr: 'aoid', }); - } else if ( - name != null && - type != null && - [ - 'abstract operation', - 'sdo', - 'syntax-directed operation', - 'host-defined abstract operation', - 'implementation-defined abstract operation', - 'numeric method', - ].includes(type) - ) { + } else if (name != null && type != null && aoidTypes.includes(type)) { this.node.setAttribute('aoid', name); this.aoid = name; } @@ -350,10 +348,17 @@ export default class Clause extends Builder { }); } else { const signature = clause.signature; + let kind: AlgorithmType | undefined = + clause.type != null && aoidTypes.includes(clause.type) ? clause.type as AlgorithmType : undefined; + // @ts-ignore + if (kind === 'sdo') { + kind = 'syntax-directed operation'; + } const op: PartialBiblioEntry = { type: 'op', aoid: clause.aoid, refId: clause.id, + kind, signature, effects: clause.effects, _node: clause.node, From 6203aa5fe0c8aa70157bacf26fd44b343808d78a Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Mon, 4 Jul 2022 22:47:58 -0700 Subject: [PATCH 4/9] also parse sdos --- src/Biblio.ts | 15 ++++ src/Spec.ts | 46 ++++++---- src/expr-parser.ts | 218 ++++++++++++++++++++++++++++++++++++++------- test/typecheck.js | 151 ++++++++++++++++++++++++++++++- 4 files changed, 378 insertions(+), 52 deletions(-) diff --git a/src/Biblio.ts b/src/Biblio.ts index 93eb0ad0..398c18df 100644 --- a/src/Biblio.ts +++ b/src/Biblio.ts @@ -109,6 +109,21 @@ export default class Biblio { return this.lookup(ns, env => env._byAoid[aoid]); } + getSDONames(ns: string): Set { + const out = new Set(); + let current = this._nsToEnvRec[ns]; + while (current) { + const entries = current._byType['op'] || []; + for (const entry of entries) { + if (entry.kind === 'syntax-directed operation') { + out.add(entry.aoid); + } + } + current = current._parent; + } + return out; + } + getDefinedWords(ns: string): Record { const result = Object.create(null); diff --git a/src/Spec.ts b/src/Spec.ts index df7f9344..56cf08a7 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -49,7 +49,7 @@ import type { OrderedListNode, parseAlgorithm } from 'ecmarkdown'; import { getProductions, rhsMatches, getLocationInGrammarFile } from './lint/utils'; import type { AugmentedGrammarEle } from './Grammar'; import { offsetToLineAndColumn } from './utils'; -import { parse as parseExpr, walk as walkExpr, Expr, PathItem } from './expr-parser'; +import { parse as parseExpr, walk as walkExpr, Expr, PathItem, Seq } from './expr-parser'; const DRAFT_DATE_FORMAT: Intl.DateTimeFormatOptions = { year: 'numeric', @@ -603,6 +603,10 @@ export default class Spec { AOs.filter(e => e.signature!.return!.kind === 'completion').map(a => [a.aoid, null]) ); + // TODO strictly speaking this needs to be done in the namespace of the current algorithm + const sdoNames = this.biblio.getSDONames(this.namespace); + + // TODO move declarations out of loop for (const node of this.doc.querySelectorAll('emu-alg')) { if (node.hasAttribute('example') || !('ecmarkdownTree' in node)) { continue; @@ -616,7 +620,8 @@ export default class Spec { const originalHtml: string = node.originalHtml; const expressionVisitor = (expr: Expr, path: PathItem[]) => { - if (expr.type !== 'call') { + // console.log(require('util').inspect(expr, { depth: Infinity })); + if (expr.type !== 'call' && expr.type !== 'sdo-call') { return; } @@ -727,7 +732,7 @@ export default class Spec { }; const walkLines = (list: OrderedListNode) => { for (const line of list.contents) { - const item = parseExpr(line.contents); + const item = parseExpr(line.contents, sdoNames); if (item.type === 'failure') { const { line, column } = offsetToLineAndColumn(originalHtml, item.offset); this.warn({ @@ -739,6 +744,7 @@ export default class Spec { nodeRelativeColumn: column, }); } else { + // console.log(require('util').inspect(item, { depth: Infinity})); walkExpr(expressionVisitor, item); } if (line.sublist?.name === 'ol') { @@ -2101,13 +2107,22 @@ function previousText(expr: Expr, path: PathItem[]): string | null { return null; } const { parent, index } = part; - if (parent.type === 'seq' && index > 0) { - const prev = parent.items[(index as number) - 1]; - if (prev.type === 'prose' && prev.parts.length > 0) { - const prevProse = prev.parts[prev.parts.length - 1]; - if (prevProse.name === 'text') { - return prevProse.contents; - } + if (parent.type === 'seq') { + return textFromPreviousPart(parent, index as number); + } + return null; +} + +function textFromPreviousPart(seq: Seq, index: number): string | null { + const prev = seq.items[index - 1]; + if (prev?.type === 'prose' && prev.parts.length > 0) { + let prevIndex = prev.parts.length - 1; + while (prevIndex > 0 && prev.parts[prevIndex].name === 'tag') { + --prevIndex; + } + const prevProse = prev.parts[prevIndex]; + if (prevProse.name === 'text') { + return prevProse.contents; } } return null; @@ -2129,14 +2144,11 @@ function isConsumedAsCompletion(expr: Expr, path: PathItem[]) { return false; } const { parent, index } = part; - if (parent.type === 'seq' && index > 0) { + if (parent.type === 'seq') { // if the previous text ends in `! ` or `? `, this is a completion - const prev = parent.items[(index as number) - 1]; // typescript isn't quite smart enough to infer this, alas - if (prev.type === 'prose' && prev.parts.length > 0) { - const prevProse = prev.parts[prev.parts.length - 1]; - if (prevProse.name === 'text' && /[!?]\s$/.test(prevProse.contents)) { - return true; - } + let text = textFromPreviousPart(parent, index as number); + if (text != null && /[!?]\s$/.test(text)) { + return true; } } else if (parent.type === 'call' && index === 0 && parent.arguments.length === 1) { // if this is `Completion(Expr())`, this is a completion diff --git a/src/expr-parser.ts b/src/expr-parser.ts index 84288a85..775c53f2 100644 --- a/src/expr-parser.ts +++ b/src/expr-parser.ts @@ -6,8 +6,7 @@ type Unarray = T extends Array ? U : T; type FragmentNode = Unarray>; const tokMatcher = - /(?«|«)|(?»|»)|(?\{)|(?\})|(?\()|(?\))/u; -const tokOrCommaMatcher = new RegExp(tokMatcher.source + '|(?,)', 'u'); // gross + /(?«|«)|(?»|»)|(?\{)|(?\})|(?\()|(?\))|(?(?:, )?and )|(? is )|(?,)|(?\.(?= |$))|(?\b\w+ of )|(? with arguments? )/u; type ProsePart = | FragmentNode @@ -33,18 +32,50 @@ type Call = { callee: Prose; arguments: Seq[]; }; +type SDOCall = { + type: 'sdo-call'; + callee: Prose; // could just be string, but this way we get location and symmetry with Call + parseNode: Seq; + arguments: Seq[]; +}; type Paren = { type: 'paren'; items: NonSeq[]; }; -type Seq = { +export type Seq = { type: 'seq'; items: NonSeq[]; }; -type NonSeq = Prose | List | Record | RecordSpec | Call | Paren; +type NonSeq = Prose | List | Record | RecordSpec | Call | SDOCall | Paren; export type Expr = NonSeq | Seq; type Failure = { type: 'failure'; message: string; offset: number }; +type TokenType = + | 'eof' + | 'olist' + | 'clist' + | 'orec' + | 'crec' + | 'oparen' + | 'cparen' + | 'and' + | 'is' + | 'comma' + | 'period' + | 'x_of' + | 'with_args'; +type CloseTokenType = + | 'clist' + | 'crec' + | 'cparen' + | 'and' + | 'is' + | 'comma' + | 'period' + | 'eof' + | 'with_args'; +type Token = Prose | { type: TokenType; offset: number; source: string }; + class ParseFailure extends Error { declare offset: number; constructor(message: string, offset: number) { @@ -53,7 +84,7 @@ class ParseFailure extends Error { } } -function formatClose(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]) { +function formatClose(close: CloseTokenType[]) { const mapped = close.map(c => { switch (c) { case 'clist': @@ -64,6 +95,16 @@ function formatClose(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]) { return 'close parenthesis'; case 'eof': return 'end of line'; + case 'with_args': + return '"with argument(s)"'; + case 'comma': + return 'comma'; + case 'period': + return 'period'; + case 'and': + return '"and"'; + case 'is': + return '"is"'; default: return c; } @@ -71,6 +112,50 @@ function formatClose(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]) { return formatEnglishList(mapped, 'or'); } +function addProse(items: NonSeq[], token: Token) { + // sometimes we determine after seeing a token that it should not have been treated as a token + // in that case we want to join it with the preceding prose, if any + let prev = items[items.length - 1]; + if (token.type === 'prose') { + if (prev == null || prev.type !== 'prose') { + items.push(token); + } else { + let lastPartOfPrev = prev.parts[prev.parts.length - 1]; + let firstPartOfThis = token.parts[0]; + if (lastPartOfPrev?.name === 'text' && firstPartOfThis?.name === 'text') { + items[items.length - 1] = { + type: 'prose', + parts: [ + ...prev.parts.slice(0, -1), + { + name: 'text', + contents: lastPartOfPrev.contents + firstPartOfThis.contents, + location: { start: { offset: lastPartOfPrev.location.start.offset } }, + }, + ...token.parts.slice(1), + ], + }; + } else { + items[items.length - 1] = { + type: 'prose', + parts: [...prev.parts, ...token.parts], + }; + } + } + } else { + addProse(items, { + type: 'prose', + parts: [ + { + name: 'text', + contents: token.source, + location: { start: { offset: token.offset } }, + }, + ], + }); + } +} + function isWhitespace(x: Prose) { return x.parts.every(p => p.name === 'text' && /^\s*$/.test(p.contents)); } @@ -86,26 +171,26 @@ function emptyThingHasNewline(s: Seq) { ); } -type TokenType = 'eof' | 'olist' | 'clist' | 'orec' | 'crec' | 'oparen' | 'cparen' | 'comma'; -type Token = Prose | { type: TokenType; offset: number }; class ExprParser { declare src: FragmentNode[]; + declare sdoNames: Set; srcIndex = 0; textTokOffset: number | null = null; // offset into current text node; only meaningful if srcOffset points to a text node next: Token[] = []; - constructor(src: FragmentNode[]) { + constructor(src: FragmentNode[], sdoNames: Set) { this.src = src; + this.sdoNames = sdoNames; } - private peek(matcher: RegExp): Token { + private peek(): Token { if (this.next.length === 0) { - this.advance(matcher); + this.advance(); } return this.next[0]; } // this method is complicated because the underlying data is a sequence of ecmarkdown fragments, not a string - private advance(matcher: RegExp) { + private advance() { const currentProse: ProsePart[] = []; while (this.srcIndex < this.src.length) { const tok: ProsePart = @@ -120,7 +205,7 @@ class ExprParser { }, }, }; - const match = tok.name === 'text' ? tok.contents.match(matcher) : null; + const match = tok.name === 'text' ? tok.contents.match(tokMatcher) : null; if (tok.name !== 'text' || match == null) { if (!(tok.name === 'text' && tok.contents.length === 0)) { currentProse.push(tok); @@ -142,6 +227,7 @@ class ExprParser { this.next.push({ type: matchKind as TokenType, offset: tok.location.start.offset + match.index!, + source: groups![matchKind], }); return; } @@ -151,23 +237,29 @@ class ExprParser { this.next.push({ type: 'eof', offset: this.src.length === 0 ? 0 : this.src[this.src.length - 1].location.end.offset, + source: '', }); } // guarantees the next token is an element of close - parseSeq(close: ('clist' | 'crec' | 'cparen' | 'comma' | 'eof')[]): Seq { + parseSeq(close: CloseTokenType[]): Seq { const items: NonSeq[] = []; - const matcher = close.includes('comma') ? tokOrCommaMatcher : tokMatcher; while (true) { - const next = this.peek(matcher); + const next = this.peek(); switch (next.type) { + case 'and': + case 'is': + case 'period': + case 'with_args': case 'comma': { - if (!close.includes('comma')) { - throw new Error('unreachable: comma while not scanning for commas'); + if (!close.includes(next.type)) { + addProse(items, next); + this.next.shift(); + break; } if (items.length === 0) { throw new ParseFailure( - `unexpected comma (expected some content for element/argument)`, + `unexpected ${next.type} (expected some content for element/argument)`, next.offset ); } @@ -180,17 +272,17 @@ class ExprParser { return { type: 'seq', items }; } case 'prose': { + addProse(items, next); this.next.shift(); - items.push(next); break; } case 'olist': { this.next.shift(); const elements: Seq[] = []; - if (this.peek(tokOrCommaMatcher).type !== 'clist') { + if (this.peek().type !== 'clist') { while (true) { elements.push(this.parseSeq(['clist', 'comma'])); - if (this.peek(tokOrCommaMatcher).type === 'clist') { + if (this.peek().type === 'clist') { break; } this.next.shift(); @@ -203,7 +295,7 @@ class ExprParser { } else { throw new ParseFailure( `unexpected list close (expected some content for element)`, - (this.peek(tokOrCommaMatcher) as { offset: number }).offset + (this.peek() as { offset: number }).offset ); } } @@ -259,10 +351,10 @@ class ExprParser { if (callee.length > 0) { this.next.shift(); const args: Seq[] = []; - if (this.peek(tokOrCommaMatcher).type !== 'cparen') { + if (this.peek().type !== 'cparen') { while (true) { args.push(this.parseSeq(['cparen', 'comma'])); - if (this.peek(tokOrCommaMatcher).type === 'cparen') { + if (this.peek().type === 'cparen') { break; } this.next.shift(); @@ -275,7 +367,7 @@ class ExprParser { } else { throw new ParseFailure( `unexpected close parenthesis (expected some content for argument)`, - (this.peek(tokOrCommaMatcher) as { offset: number }).offset + (this.peek() as { offset: number }).offset ); } } @@ -307,7 +399,7 @@ class ExprParser { let type: 'record' | 'record-spec' | null = null; const members: ({ name: string; value: Seq } | { name: string })[] = []; while (true) { - const nextTok = this.peek(tokOrCommaMatcher); + const nextTok = this.peek(); if (nextTok.type !== 'prose') { throw new ParseFailure('expected to find record field name', nextTok.offset); } @@ -323,7 +415,7 @@ class ExprParser { 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(tokOrCommaMatcher).type === 'crec') { + if (this.peek().type === 'crec') { this.next.shift(); break; } @@ -383,11 +475,11 @@ class ExprParser { throw new ParseFailure('expected record field to have value', offset - 1); } members.push({ name }); - if (!['crec', 'comma'].includes(this.peek(tokOrCommaMatcher).type)) { + if (!['crec', 'comma'].includes(this.peek().type)) { throw new ParseFailure(`expected ${formatClose(['crec', 'comma'])}`, offset); } } - if (this.peek(tokOrCommaMatcher).type === 'crec') { + if (this.peek().type === 'crec') { break; } this.next.shift(); // eat the comma @@ -406,6 +498,59 @@ class ExprParser { } return { type: 'seq', items }; } + case 'x_of': { + this.next.shift(); + let callee = next.source.split(' ')[0]; + if (!this.sdoNames.has(callee)) { + addProse(items, next); + break; + } + let parseNode = this.parseSeq([ + 'eof', + 'period', + 'comma', + 'cparen', + 'clist', + 'crec', + 'with_args', + ]); + let args: Seq[] = []; + if (this.peek().type === 'with_args') { + this.next.shift(); + while (true) { + args.push( + this.parseSeq([ + 'eof', + 'period', + 'and', + 'is', + 'comma', + 'cparen', + 'clist', + 'crec', + 'with_args', + ]) + ); + if (!['and', 'comma'].includes(this.peek().type)) { + break; + } + this.next.shift(); + } + } + items.push({ + type: 'sdo-call', + callee: { + type: 'prose', + parts: [ + { name: 'text', contents: callee, location: { start: { offset: next.offset } } }, + ], + }, + parseNode, + arguments: args, + }); + // console.log(require('util').inspect(items[items.length - 1], { depth: Infinity })); + break; + } default: { // @ts-ignore throw new Error(`unreachable: unknown token type ${next.type}`); @@ -415,8 +560,8 @@ class ExprParser { } } -export function parse(src: FragmentNode[]): Seq | Failure { - const parser = new ExprParser(src); +export function parse(src: FragmentNode[], sdoNames: Set): Seq | Failure { + const parser = new ExprParser(src, sdoNames); try { return parser.parseSeq(['eof']); } catch (e) { @@ -429,7 +574,8 @@ export function parse(src: FragmentNode[]): Seq | Failure { export type PathItem = | { parent: List | Record | Seq | Paren; index: number } - | { parent: Call; index: 'callee' | number }; + | { parent: Call; index: 'callee' | number } + | { parent: SDOCall; index: 'callee' | number }; export function walk( f: (expr: Expr, path: PathItem[]) => void, current: Expr, @@ -460,6 +606,14 @@ export function walk( case 'record-spec': { break; } + case 'sdo-call': { + for (let i = 0; i < current.arguments.length; ++i) { + path.push({ parent: current, index: i }); + walk(f, current.arguments[i], path); + path.pop(); + } + break; + } case 'call': { path.push({ parent: current, index: 'callee' }); walk(f, current.callee, path); diff --git a/test/typecheck.js b/test/typecheck.js index 0d29ec69..79afab77 100644 --- a/test/typecheck.js +++ b/test/typecheck.js @@ -67,10 +67,38 @@ describe('typechecking completions', () => { extraBiblios: [biblio], } ); + + await assertLint( + positioned` + +

+ ExampleAlg (): a normal completion containing a Number +

+
+
+ + +

+ Example2 () +

+
+
+ + 1. Return ${M}ExampleAlg of _foo_. + +
+ `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'ExampleAlg returns a Completion Record, but is not consumed as if it does', + }, + { + extraBiblios: [biblio], + } + ); }); - // the check for Completion() assumes it's happening after auto-linking - // so it only works if the Completion AO is defined it('negative', async () => { await assertLintFree( ` @@ -85,6 +113,15 @@ describe('typechecking completions', () => { + +

+ ExampleSDO ( + optional _x_: a number, + ): a normal completion containing a Number +

+
+
+

Example2 () @@ -96,6 +133,10 @@ describe('typechecking completions', () => { 1. Let _a_ be Completion(ExampleAlg()). 1. Set _a_ to ! ExampleAlg(). 1. Return ? ExampleAlg(). + 1. Let _a_ be Completion(ExampleSDO of _foo_). + 1. Let _a_ be Completion(ExampleSDO of _foo_ with argument 0). + 1. If ? ExampleSDO of _foo_ is *true*, then + 1. Something. @@ -597,6 +638,21 @@ describe('signature agreement', async () => {

TakesOneOrTwoArgs ( _x_, [_y_] )

+ + +

SDOTakesNoArgs ()

+
+
+ + +

SDOTakesOneArg ( _x_ )

+
+
+ + +

SDOTakesOneOrTwoArgs ( _x_, [_y_] )

+
+
`); }); @@ -650,7 +706,57 @@ describe('signature agreement', async () => { ); }); - it('too few args args', async () => { + it('extra args for sdo', async () => { + await assertLint( + positioned` + + 1. Return ${M}SDOTakesNoArgs of _foo_ with argument 1. + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDOTakesNoArgs takes 0 arguments, but this invocation passes 1', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}SDOTakesNoArgs of _foo_ with argument 0. + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDOTakesNoArgs takes 0 arguments, but this invocation passes 1', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}SDOTakesOneOrTwoArgs of _foo_ with arguments 0, 1, and 2. + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDOTakesOneOrTwoArgs takes 1-2 arguments, but this invocation passes 3', + }, + { + extraBiblios: [biblio], + } + ); + }); + + it('too few args', async () => { await assertLint( positioned` @@ -684,6 +790,40 @@ describe('signature agreement', async () => { ); }); + it('too few args for sdo', async () => { + await assertLint( + positioned` + + 1. Return ${M}SDOTakesOneArg of _foo_. + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDOTakesOneArg takes 1 argument, but this invocation passes 0', + }, + { + extraBiblios: [biblio], + } + ); + + await assertLint( + positioned` + + 1. Return ${M}SDOTakesOneOrTwoArgs(). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDOTakesOneOrTwoArgs takes 1-2 arguments, but this invocation passes 0', + }, + { + extraBiblios: [biblio], + } + ); + }); + it('negative', async () => { await assertLintFree( ` @@ -693,6 +833,11 @@ describe('signature agreement', async () => { 1. Perform TakesOneOrTwoArgs(0). 1. Perform TakesOneOrTwoArgs(0, 1). 1. Perform TakesNoArgs(). + 1. Perform SDOTakesNoArgs of _foo_. + 1. Perform SDOTakesOneArg of _foo_ with argument 0. + 1. Perform SDOTakesOneOrTwoArgs of _foo_ with argument 0. + 1. Perform SDOTakesOneOrTwoArgs of _foo_ with arguments 0 and 1. + 1. Perform SDOTakesNoArgs of _foo_. `, { From 77be522b7a2961d2ed0b99d1c490ecea53648099 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Mon, 4 Jul 2022 23:01:58 -0700 Subject: [PATCH 5/9] lint for AO/SDO invoked as SDO/AO --- src/Biblio.ts | 6 ++-- src/Spec.ts | 14 +++++++-- src/expr-parser.ts | 12 ++++---- test/typecheck.js | 76 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 94 insertions(+), 14 deletions(-) diff --git a/src/Biblio.ts b/src/Biblio.ts index 398c18df..26cd0748 100644 --- a/src/Biblio.ts +++ b/src/Biblio.ts @@ -109,15 +109,13 @@ export default class Biblio { return this.lookup(ns, env => env._byAoid[aoid]); } - getSDONames(ns: string): Set { + getOpNames(ns: string): Set { const out = new Set(); let current = this._nsToEnvRec[ns]; while (current) { const entries = current._byType['op'] || []; for (const entry of entries) { - if (entry.kind === 'syntax-directed operation') { - out.add(entry.aoid); - } + out.add(entry.aoid); } current = current._parent; } diff --git a/src/Spec.ts b/src/Spec.ts index 56cf08a7..90453020 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -604,7 +604,7 @@ export default class Spec { ); // TODO strictly speaking this needs to be done in the namespace of the current algorithm - const sdoNames = this.biblio.getSDONames(this.namespace); + const opNames = this.biblio.getOpNames(this.namespace); // TODO move declarations out of loop for (const node of this.doc.querySelectorAll('emu-alg')) { @@ -667,7 +667,15 @@ export default class Spec { warn(`could not find definition for ${calleeName}`); } return; - } else if (biblioEntry.signature == null) { + } + + if (biblioEntry.kind === 'syntax-directed operation' && expr.type === 'call') { + warn(`${calleeName} is a syntax-directed operation and should not be invoked like a regular call`); + } else if (biblioEntry.kind != null && biblioEntry.kind !== 'syntax-directed operation' && expr.type === 'sdo-call') { + warn(`${calleeName} is not a syntax-directed operation but here is being invoked as one`); + } + + if (biblioEntry.signature == null) { return; } const min = biblioEntry.signature.parameters.length; @@ -732,7 +740,7 @@ export default class Spec { }; const walkLines = (list: OrderedListNode) => { for (const line of list.contents) { - const item = parseExpr(line.contents, sdoNames); + const item = parseExpr(line.contents, opNames); if (item.type === 'failure') { const { line, column } = offsetToLineAndColumn(originalHtml, item.offset); this.warn({ diff --git a/src/expr-parser.ts b/src/expr-parser.ts index 775c53f2..c2b79646 100644 --- a/src/expr-parser.ts +++ b/src/expr-parser.ts @@ -173,13 +173,13 @@ function emptyThingHasNewline(s: Seq) { class ExprParser { declare src: FragmentNode[]; - declare sdoNames: Set; + declare opNames: Set; srcIndex = 0; textTokOffset: number | null = null; // offset into current text node; only meaningful if srcOffset points to a text node next: Token[] = []; - constructor(src: FragmentNode[], sdoNames: Set) { + constructor(src: FragmentNode[], opNames: Set) { this.src = src; - this.sdoNames = sdoNames; + this.opNames = opNames; } private peek(): Token { @@ -501,7 +501,7 @@ class ExprParser { case 'x_of': { this.next.shift(); let callee = next.source.split(' ')[0]; - if (!this.sdoNames.has(callee)) { + if (!this.opNames.has(callee)) { addProse(items, next); break; } @@ -560,8 +560,8 @@ class ExprParser { } } -export function parse(src: FragmentNode[], sdoNames: Set): Seq | Failure { - const parser = new ExprParser(src, sdoNames); +export function parse(src: FragmentNode[], opNames: Set): Seq | Failure { + const parser = new ExprParser(src, opNames); try { return parser.parseSeq(['eof']); } catch (e) { diff --git a/test/typecheck.js b/test/typecheck.js index 79afab77..a3673aaf 100644 --- a/test/typecheck.js +++ b/test/typecheck.js @@ -810,7 +810,7 @@ describe('signature agreement', async () => { await assertLint( positioned` - 1. Return ${M}SDOTakesOneOrTwoArgs(). + 1. Return ${M}SDOTakesOneOrTwoArgs of _foo_. `, { @@ -846,3 +846,77 @@ describe('signature agreement', async () => { ); }); }); + +describe('invocation kind', async () => { + let biblio; + before(async () => { + biblio = await getBiblio(` + +

AO ()

+
+
+ + +

SDO ()

+
+
+ `); + }); + + it('SDO invoked as AO', async () => { + await assertLint( + positioned` + + 1. Return ${M}SDO(). + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'SDO is a syntax-directed operation and should not be invoked like a regular call', + }, + { + extraBiblios: [biblio], + } + ); + }); + + it('AO invoked as SDO', async () => { + await assertLint( + positioned` + + 1. Return ${M}AO of _foo_. + + `, + { + ruleId: 'typecheck', + nodeType: 'emu-alg', + message: 'AO is not a syntax-directed operation but here is being invoked as one', + }, + { + extraBiblios: [biblio], + } + ); + }); + + it('negative', async () => { + await assertLintFree( + ` + +

+ Example () +

+
+
+ + 1. Perform AO(). + 1. Perform SDO of _foo_. + +
+ `, + { + extraBiblios: [biblio], + } + ); + }); +}); From 0b378243b0008f61dbaeaeb510591cbcee305949 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Mon, 4 Jul 2022 23:02:22 -0700 Subject: [PATCH 6/9] lint --- src/Clause.ts | 4 +++- src/Spec.ts | 12 +++++++++--- src/expr-parser.ts | 12 ++++++------ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/Clause.ts b/src/Clause.ts index 0b1391b6..bab8dd81 100644 --- a/src/Clause.ts +++ b/src/Clause.ts @@ -349,7 +349,9 @@ export default class Clause extends Builder { } else { const signature = clause.signature; let kind: AlgorithmType | undefined = - clause.type != null && aoidTypes.includes(clause.type) ? clause.type as AlgorithmType : undefined; + clause.type != null && aoidTypes.includes(clause.type) + ? (clause.type as AlgorithmType) + : undefined; // @ts-ignore if (kind === 'sdo') { kind = 'syntax-directed operation'; diff --git a/src/Spec.ts b/src/Spec.ts index 90453020..daa4d0cc 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -670,8 +670,14 @@ export default class Spec { } if (biblioEntry.kind === 'syntax-directed operation' && expr.type === 'call') { - warn(`${calleeName} is a syntax-directed operation and should not be invoked like a regular call`); - } else if (biblioEntry.kind != null && biblioEntry.kind !== 'syntax-directed operation' && expr.type === 'sdo-call') { + warn( + `${calleeName} is a syntax-directed operation and should not be invoked like a regular call` + ); + } else if ( + biblioEntry.kind != null && + biblioEntry.kind !== 'syntax-directed operation' && + expr.type === 'sdo-call' + ) { warn(`${calleeName} is not a syntax-directed operation but here is being invoked as one`); } @@ -2154,7 +2160,7 @@ function isConsumedAsCompletion(expr: Expr, path: PathItem[]) { const { parent, index } = part; if (parent.type === 'seq') { // if the previous text ends in `! ` or `? `, this is a completion - let text = textFromPreviousPart(parent, index as number); + const text = textFromPreviousPart(parent, index as number); if (text != null && /[!?]\s$/.test(text)) { return true; } diff --git a/src/expr-parser.ts b/src/expr-parser.ts index c2b79646..2aab9333 100644 --- a/src/expr-parser.ts +++ b/src/expr-parser.ts @@ -115,13 +115,13 @@ function formatClose(close: CloseTokenType[]) { function addProse(items: NonSeq[], token: Token) { // sometimes we determine after seeing a token that it should not have been treated as a token // in that case we want to join it with the preceding prose, if any - let prev = items[items.length - 1]; + const prev = items[items.length - 1]; if (token.type === 'prose') { if (prev == null || prev.type !== 'prose') { items.push(token); } else { - let lastPartOfPrev = prev.parts[prev.parts.length - 1]; - let firstPartOfThis = token.parts[0]; + const lastPartOfPrev = prev.parts[prev.parts.length - 1]; + const firstPartOfThis = token.parts[0]; if (lastPartOfPrev?.name === 'text' && firstPartOfThis?.name === 'text') { items[items.length - 1] = { type: 'prose', @@ -500,12 +500,12 @@ class ExprParser { } case 'x_of': { this.next.shift(); - let callee = next.source.split(' ')[0]; + const callee = next.source.split(' ')[0]; if (!this.opNames.has(callee)) { addProse(items, next); break; } - let parseNode = this.parseSeq([ + const parseNode = this.parseSeq([ 'eof', 'period', 'comma', @@ -514,7 +514,7 @@ class ExprParser { 'crec', 'with_args', ]); - let args: Seq[] = []; + const args: Seq[] = []; if (this.peek().type === 'with_args') { this.next.shift(); while (true) { From 26322f029cdc12a542a17428aec8199ce945b376 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 6 Jul 2022 18:54:52 -0700 Subject: [PATCH 7/9] bump ecmarkdown --- package-lock.json | 14 +++++++------- package.json | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index e39caf25..01a9558b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,7 @@ "command-line-args": "^5.2.0", "command-line-usage": "^6.1.1", "dedent-js": "^1.0.1", - "ecmarkdown": "^7.0.0", + "ecmarkdown": "^7.1.0", "eslint-formatter-codeframe": "^7.32.1", "fast-glob": "^3.2.7", "grammarkdown": "^3.2.0", @@ -1115,9 +1115,9 @@ } }, "node_modules/ecmarkdown": { - "version": "7.0.0", - "resolved": "https://registry.npmjs.org/ecmarkdown/-/ecmarkdown-7.0.0.tgz", - "integrity": "sha512-hJxPALjSOpSMMcFjSzwzJBk8EWOu20mYlTfV7BnVTh9er0FEaT2eSx16y36YxqQfdFxPUsa0CSH4fLf0qUclKw==", + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/ecmarkdown/-/ecmarkdown-7.1.0.tgz", + "integrity": "sha512-xTrf1Qj6nCsHGSHaOrAPfALoEH2nPSs+wclpzXEsozVJbEYcTkims59rJiJQB6TxAW2J4oFfoaB2up1wbb9k4w==", "dependencies": { "escape-html": "^1.0.1" } @@ -4370,9 +4370,9 @@ } }, "ecmarkdown": { - "version": "7.0.0", - "resolved": "https://registry.npmjs.org/ecmarkdown/-/ecmarkdown-7.0.0.tgz", - "integrity": "sha512-hJxPALjSOpSMMcFjSzwzJBk8EWOu20mYlTfV7BnVTh9er0FEaT2eSx16y36YxqQfdFxPUsa0CSH4fLf0qUclKw==", + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/ecmarkdown/-/ecmarkdown-7.1.0.tgz", + "integrity": "sha512-xTrf1Qj6nCsHGSHaOrAPfALoEH2nPSs+wclpzXEsozVJbEYcTkims59rJiJQB6TxAW2J4oFfoaB2up1wbb9k4w==", "requires": { "escape-html": "^1.0.1" } diff --git a/package.json b/package.json index f728888d..c438abe3 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,7 @@ "command-line-args": "^5.2.0", "command-line-usage": "^6.1.1", "dedent-js": "^1.0.1", - "ecmarkdown": "^7.0.0", + "ecmarkdown": "^7.1.0", "eslint-formatter-codeframe": "^7.32.1", "fast-glob": "^3.2.7", "grammarkdown": "^3.2.0", From 4bca649372985e3abd93c2fe7464630404ce4578 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 6 Jul 2022 19:26:06 -0700 Subject: [PATCH 8/9] remove some stray commented out console.log --- src/Spec.ts | 2 -- src/expr-parser.ts | 2 -- 2 files changed, 4 deletions(-) diff --git a/src/Spec.ts b/src/Spec.ts index daa4d0cc..2864db9a 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -620,7 +620,6 @@ export default class Spec { const originalHtml: string = node.originalHtml; const expressionVisitor = (expr: Expr, path: PathItem[]) => { - // console.log(require('util').inspect(expr, { depth: Infinity })); if (expr.type !== 'call' && expr.type !== 'sdo-call') { return; } @@ -758,7 +757,6 @@ export default class Spec { nodeRelativeColumn: column, }); } else { - // console.log(require('util').inspect(item, { depth: Infinity})); walkExpr(expressionVisitor, item); } if (line.sublist?.name === 'ol') { diff --git a/src/expr-parser.ts b/src/expr-parser.ts index 2aab9333..08b88adc 100644 --- a/src/expr-parser.ts +++ b/src/expr-parser.ts @@ -548,7 +548,6 @@ class ExprParser { parseNode, arguments: args, }); - // console.log(require('util').inspect(items[items.length - 1], { depth: Infinity })); break; } default: { @@ -581,7 +580,6 @@ export function walk( current: Expr, path: PathItem[] = [] ) { - // console.log('path', path, current.type); f(current, path); switch (current.type) { case 'prose': { From d5c2fa5694aba84f9ed4f9b84493198a2fe63b43 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 6 Jul 2022 19:26:56 -0700 Subject: [PATCH 9/9] remove hardcoded exception for bbb names --- src/Spec.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Spec.ts b/src/Spec.ts index 2864db9a..253e1ff2 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -657,9 +657,6 @@ export default class Spec { 'thisBooleanValue', 'toUppercase', 'toLowercase', - 'ℝ', - '𝔽', - 'ℤ', ].includes(calleeName) ) { // TODO make the spec not do this