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", 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/Biblio.ts b/src/Biblio.ts index 2ae6905b..26cd0748 100644 --- a/src/Biblio.ts +++ b/src/Biblio.ts @@ -109,6 +109,19 @@ export default class Biblio { return this.lookup(ns, env => env._byAoid[aoid]); } + 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) { + out.add(entry.aoid); + } + current = current._parent; + } + return out; + } + getDefinedWords(ns: string): Record { const result = Object.create(null); @@ -316,9 +329,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..bab8dd81 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,19 @@ 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, diff --git a/src/Spec.ts b/src/Spec.ts index 357069db..253e1ff2 100644 --- a/src/Spec.ts +++ b/src/Spec.ts @@ -45,9 +45,11 @@ import { import { lint } from './lint/lint'; import { CancellationToken } from 'prex'; import type { JSDOM } from 'jsdom'; -import type { OrderedListItemNode, 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, PathItem, Seq } from './expr-parser'; const DRAFT_DATE_FORMAT: Intl.DateTimeFormatOptions = { year: 'numeric', @@ -584,10 +586,9 @@ export default class Spec { } } - // right now this just checks that AOs which do/don't return completion records are invoked appropriately - // someday, more! + // checks that AOs which do/don't return completion records are invoked appropriately + // also checks that the appropriate number of arguments are passed private typecheck() { - const isCall = (x: Xref) => x.isInvocation && x.clause != null && x.aoid != null; const isUnused = (t: Type) => t.kind === 'unused' || (t.kind === 'completion' && @@ -598,146 +599,169 @@ 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) { + // TODO strictly speaking this needs to be done in the namespace of the current algorithm + const opNames = this.biblio.getOpNames(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; } - - 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' && expr.type !== 'sdo-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; } - }; - 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) { + if (biblioEntry.kind === 'syntax-directed operation' && expr.type === 'call') { 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} 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`); } - } 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 (biblioEntry.signature == null) { + return; } - } - 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'); + 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 { 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( + `${calleeName} does not return a meaningful value and should only be invoked as \`Perform ${calleeName}(...).\`` + ); + } + + 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(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, opNames); + 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) { @@ -2064,56 +2088,81 @@ 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') { + return textFromPreviousPart(parent, index as number); } - 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; +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 /\s!\s$/.test(previousSibling.textContent!); + return null; } -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 isCalledAsPerform(expr: Expr, path: PathItem[], allowQuestion: boolean) { + const prev = previousText(expr, path); + return prev != null && (allowQuestion ? /\bperform ([?!]\s)?$/i : /\bperform $/i).test(prev); +} + +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') { + // if the previous text ends in `! ` or `? `, this is a completion + const 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 + 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 new file mode 100644 index 00000000..08b88adc --- /dev/null +++ b/src/expr-parser.ts @@ -0,0 +1,640 @@ +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 = + /(?«|«)|(?»|»)|(?\{)|(?\})|(?\()|(?\))|(?(?:, )?and )|(? is )|(?,)|(?\.(?= |$))|(?\b\w+ of )|(? with arguments? )/u; + +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 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[]; +}; +export type Seq = { + type: 'seq'; + items: NonSeq[]; +}; +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) { + super(message); + this.offset = offset; + } +} + +function formatClose(close: CloseTokenType[]) { + 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'; + 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; + } + }); + 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 + const prev = items[items.length - 1]; + if (token.type === 'prose') { + if (prev == null || prev.type !== 'prose') { + items.push(token); + } else { + 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', + 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)); +} + +function isEmpty(s: Seq) { + return s.items.every(i => i.type === 'prose' && isWhitespace(i)); +} + +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')) + ); +} + +class ExprParser { + declare src: FragmentNode[]; + 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[], opNames: Set) { + this.src = src; + this.opNames = opNames; + } + + private peek(): Token { + if (this.next.length === 0) { + 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() { + 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(tokMatcher) : 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!, + source: groups![matchKind], + }); + 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, + source: '', + }); + } + + // guarantees the next token is an element of close + parseSeq(close: CloseTokenType[]): Seq { + const items: NonSeq[] = []; + while (true) { + const next = this.peek(); + switch (next.type) { + case 'and': + case 'is': + case 'period': + case 'with_args': + case 'comma': { + if (!close.includes(next.type)) { + addProse(items, next); + this.next.shift(); + break; + } + if (items.length === 0) { + throw new ParseFailure( + `unexpected ${next.type} (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': { + addProse(items, next); + this.next.shift(); + break; + } + case 'olist': { + this.next.shift(); + const elements: Seq[] = []; + if (this.peek().type !== 'clist') { + while (true) { + elements.push(this.parseSeq(['clist', 'comma'])); + if (this.peek().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() 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().type !== 'cparen') { + while (true) { + args.push(this.parseSeq(['cparen', 'comma'])); + if (this.peek().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() 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(); + 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().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().type)) { + throw new ParseFailure(`expected ${formatClose(['crec', 'comma'])}`, offset); + } + } + if (this.peek().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 }; + } + case 'x_of': { + this.next.shift(); + const callee = next.source.split(' ')[0]; + if (!this.opNames.has(callee)) { + addProse(items, next); + break; + } + const parseNode = this.parseSeq([ + 'eof', + 'period', + 'comma', + 'cparen', + 'clist', + 'crec', + 'with_args', + ]); + const 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, + }); + break; + } + default: { + // @ts-ignore + throw new Error(`unreachable: unknown token type ${next.type}`); + } + } + } + } +} + +export function parse(src: FragmentNode[], opNames: Set): Seq | Failure { + const parser = new ExprParser(src, opNames); + try { + return parser.parseSeq(['eof']); + } catch (e) { + if (e instanceof ParseFailure) { + return { type: 'failure', message: e.message, offset: e.offset }; + } + throw e; + } +} + +export type PathItem = + | { parent: List | Record | Seq | Paren; index: number } + | { parent: Call; index: 'callee' | number } + | { parent: SDOCall; index: 'callee' | number }; +export function walk( + f: (expr: Expr, path: PathItem[]) => void, + current: Expr, + path: PathItem[] = [] +) { + f(current, path); + switch (current.type) { + case 'prose': { + break; + } + case 'list': { + for (let i = 0; i < current.elements.length; ++i) { + 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) { + path.push({ parent: current, index: i }); + walk(f, current.members[i].value, path); + path.pop(); + } + break; + } + 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); + path.pop(); + 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 'paren': + case 'seq': { + for (let i = 0; i < current.items.length; ++i) { + path.push({ parent: current, index: i }); + walk(f, current.items[i], path); + path.pop(); + } + 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..a3673aaf 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(
@@ -25,73 +54,107 @@ describe('typechecking', () => {
         

         

         
-${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',
+        },
+        {
+          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],
         }
       );
     });
 
-    // UUUUGH 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).
-        
-        
+      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_.
-        
-        
+          
+          

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

+          

+          
 
-        
-        

-          Example2 ()
-        

-        

-        

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

+            Example2 ()
+          

+          

+          

+          
+            1. Let _a_ be Completion(ExampleAlg()).
+            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.
+          
+          
 
-        
-        

-          Example3 ()
-        

-        

-        

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

+            Example3 ()
+          

+          

+          

+          
+            1. Return ExampleAlg().
+          
+          
+        `,
+        {
+          extraBiblios: [biblio],
+        }
+      );
     });
   });
 
@@ -117,12 +180,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',
         }
@@ -167,7 +230,7 @@ ${M}      1. Return ? ExampleAlg().
         

         

         
-          1. Return ${M}? Foo().
+          1. Return ${M}? _foo_.
         
         
         `,
@@ -218,25 +281,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 +322,7 @@ ${M}      1. Return ? ExampleAlg().
         

         

         ${M}
-          1. Return Foo().
+          1. Return _foo_.
         
         
         `,
@@ -273,7 +344,7 @@ ${M}      1. Return ? ExampleAlg().
         

         

         
-          1. Return ? Foo().
+          1. Return ? _foo_.
         
         
       `);
@@ -286,7 +357,7 @@ ${M}      1. Return ? ExampleAlg().
         

         

         
-          1. Return Foo().
+          1. Return _foo_.
         
         
       `);
@@ -304,7 +375,7 @@ ${M}      1. Return ? ExampleAlg().
           

           

           
-            1. Return Foo().
+            1. Return _foo_.
           
           
 
@@ -315,12 +386,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(...).`',
@@ -337,7 +408,7 @@ ${M}          1. Let _x_ be ExampleAlg().
         

         

         
-          1. Return Foo().
+          1. Return _foo_.
         
         
 
@@ -452,7 +523,7 @@ ${M}          1. Let _x_ be ExampleAlg().
           

           

           
-            1. Return ? Foo().
+            1. Return ? _foo_.
           
           
 
@@ -485,7 +556,7 @@ ${M}          1. Let _x_ be ExampleAlg().
           

           

           
-            1. Return ? Foo().
+            1. Return ? _foo_.
           
           
 
@@ -514,7 +585,7 @@ ${M}          1. Let _x_ be ExampleAlg().
           

           

           
-            1. Return Foo().
+            1. Return 0.
           
           
         `,
@@ -528,18 +599,324 @@ ${M}          1. Let _x_ be ExampleAlg().
     });
 
     it('negative', async () => {
-      await assertLintFree(`
+      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_] )

+        

+      
+
+      
+        
SDOTakesNoArgs ()

+        

+      
+
+      
+        
SDOTakesOneArg ( _x_ )

+        

+      
+
+      
+        
SDOTakesOneOrTwoArgs ( _x_, [_y_] )

+        

+      
+    `);
+  });
+
+  it('extra args', async () => {
+    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}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('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`
+        
+          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('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 of _foo_.
+        
+      `,
+      {
+        ruleId: 'typecheck',
+        nodeType: 'emu-alg',
+        message: 'SDOTakesOneOrTwoArgs 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().
+          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_.
+        
+      `,
+      {
+        extraBiblios: [biblio],
+      }
+    );
+  });
+});
+
+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(
+      `
         
         

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

         

         

-        
-          1. Return NormalCompletion(Foo()).
+        
+          1. Perform AO().
+          1. Perform SDO of _foo_.
         
         
-      `);
-    });
+      `,
+      {
+        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 = {}) {