diff --git a/package-lock.json b/package-lock.json index 3f19bbf9..8e65871a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "ecmarkup", - "version": "16.2.0", + "version": "17.0.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "ecmarkup", - "version": "16.2.0", + "version": "17.0.0", "license": "MIT", "dependencies": { "chalk": "^4.1.2", diff --git a/package.json b/package.json index 69e274b5..1b7ce221 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ecmarkup", - "version": "16.2.0", + "version": "17.0.0", "description": "Custom element definitions and core utilities for markup that specifies ECMAScript and related technologies.", "main": "lib/ecmarkup.js", "typings": "lib/ecmarkup.d.ts", diff --git a/src/lint/collect-algorithm-diagnostics.ts b/src/lint/collect-algorithm-diagnostics.ts index a34f97ff..8bf77855 100644 --- a/src/lint/collect-algorithm-diagnostics.ts +++ b/src/lint/collect-algorithm-diagnostics.ts @@ -1,4 +1,4 @@ -import type { Node as EcmarkdownNode, OrderedListItemNode } from 'ecmarkdown'; +import type { Node as EcmarkdownNode, OrderedListItemNode, OrderedListNode } from 'ecmarkdown'; import type { LintingError, Reporter } from './algorithm-error-reporter-type'; import type { default as Spec, Warning } from '../Spec'; @@ -11,14 +11,16 @@ import lintAlgorithmStepNumbering from './rules/algorithm-step-numbering'; import lintAlgorithmStepLabels from './rules/algorithm-step-labels'; import lintForEachElement from './rules/for-each-element'; import lintStepAttributes from './rules/step-attributes'; +import lintIfElseConsistency from './rules/if-else-consistency'; import { checkVariableUsage } from './rules/variable-use-def'; import { parse, Seq } from '../expr-parser'; type LineRule = ( report: Reporter, - stepSeq: Seq | null, step: OrderedListItemNode, - algorithmSource: string + algorithmSource: string, + parsedSteps: Map, + parent: OrderedListNode ) => void; const stepRules: LineRule[] = [ lintAlgorithmLineStyle, @@ -26,6 +28,7 @@ const stepRules: LineRule[] = [ lintAlgorithmStepLabels, lintForEachElement, lintStepAttributes, + lintIfElseConsistency, ]; export function collectAlgorithmDiagnostics( @@ -70,11 +73,8 @@ export function collectAlgorithmDiagnostics( } const parsedSteps: Map = new Map(); let allNodesParsedSuccessfully = true; - function walk(visit: LineRule, step: OrderedListItemNode) { - // we don't know the names of ops at this point - // TODO maybe run later in the process? but not worth worrying about for now + function parseStep(step: OrderedListItemNode) { const parsed = parse(step.contents, new Set()); - visit(reporter, parsed.name === 'seq' ? parsed : null, step, algorithmSource); // TODO reconsider algorithmSource if (parsed.name === 'failure') { allNodesParsedSuccessfully = false; } else { @@ -82,14 +82,29 @@ export function collectAlgorithmDiagnostics( } if (step.sublist?.name === 'ol') { for (const substep of step.sublist.contents) { - walk(visit, substep); + parseStep(substep); + } + } + } + + function applyRule(visit: LineRule, step: OrderedListItemNode, parent: OrderedListNode) { + // we don't know the names of ops at this point + // TODO maybe run later in the process? but not worth worrying about for now + visit(reporter, step, algorithmSource, parsedSteps, parent); + if (step.sublist?.name === 'ol') { + for (const substep of step.sublist.contents) { + applyRule(visit, substep, step.sublist); } } } if (tree != null && !element.hasAttribute('example')) { + for (const step of tree.contents.contents) { + parseStep(step); + } + for (const rule of stepRules) { for (const step of tree.contents.contents) { - walk(rule, step); + applyRule(rule, step, tree.contents); } } if (allNodesParsedSuccessfully) { diff --git a/src/lint/rules/algorithm-line-style.ts b/src/lint/rules/algorithm-line-style.ts index 29d939a9..bae78fe8 100644 --- a/src/lint/rules/algorithm-line-style.ts +++ b/src/lint/rules/algorithm-line-style.ts @@ -28,10 +28,11 @@ Checks that every algorithm step has one of these forms: */ export default function ( report: Reporter, - stepSeq: Seq | null, node: OrderedListItemNode, - algorithmSource: string + algorithmSource: string, + parsedSteps: Map ) { + const stepSeq = parsedSteps.get(node); if (stepSeq == null || stepSeq.items.length === 0) { return; } diff --git a/src/lint/rules/algorithm-step-labels.ts b/src/lint/rules/algorithm-step-labels.ts index 58bbc37b..612a7a81 100644 --- a/src/lint/rules/algorithm-step-labels.ts +++ b/src/lint/rules/algorithm-step-labels.ts @@ -1,18 +1,12 @@ import type { OrderedListItemNode } from 'ecmarkdown'; import type { Reporter } from '../algorithm-error-reporter-type'; -import type { Seq } from '../../expr-parser'; const ruleId = 'algorithm-step-labels'; /* Checks that step labels all start with `step-`. */ -export default function ( - report: Reporter, - stepSeq: Seq | null, - node: OrderedListItemNode, - algorithmSource: string -) { +export default function (report: Reporter, node: OrderedListItemNode, algorithmSource: string) { const idAttr = node.attrs.find(({ key }) => key === 'id'); if (idAttr != null && !/^step-/.test(idAttr.value)) { const itemSource = algorithmSource.slice( diff --git a/src/lint/rules/algorithm-step-numbering.ts b/src/lint/rules/algorithm-step-numbering.ts index 587526c4..7d5893b2 100644 --- a/src/lint/rules/algorithm-step-numbering.ts +++ b/src/lint/rules/algorithm-step-numbering.ts @@ -1,18 +1,12 @@ import type { OrderedListItemNode } from 'ecmarkdown'; import type { Reporter } from '../algorithm-error-reporter-type'; -import type { Seq } from '../../expr-parser'; const ruleId = 'algorithm-step-numbering'; /* Checks that step numbers are all `1`. */ -export default function ( - report: Reporter, - stepSeq: Seq | null, - node: OrderedListItemNode, - algorithmSource: string -) { +export default function (report: Reporter, node: OrderedListItemNode, algorithmSource: string) { const itemSource = algorithmSource.slice(node.location.start.offset, node.location.end.offset); const match = itemSource.match(/^(\s*)(\d+\.) /)!; if (match[2] !== '1.') { diff --git a/src/lint/rules/for-each-element.ts b/src/lint/rules/for-each-element.ts index b62eef9a..f9a0656b 100644 --- a/src/lint/rules/for-each-element.ts +++ b/src/lint/rules/for-each-element.ts @@ -1,12 +1,19 @@ import type { Reporter } from '../algorithm-error-reporter-type'; import type { Seq } from '../../expr-parser'; +import type { OrderedListItemNode } from 'ecmarkdown'; const ruleId = 'for-each-element'; /* Checks that "For each" loops name a type or say "element" before the variable. */ -export default function (report: Reporter, stepSeq: Seq | null) { +export default function ( + report: Reporter, + step: OrderedListItemNode, + algorithmSource: string, + parsedSteps: Map +) { + const stepSeq = parsedSteps.get(step); if (stepSeq == null || stepSeq.items.length < 2) { return; } diff --git a/src/lint/rules/if-else-consistency.ts b/src/lint/rules/if-else-consistency.ts new file mode 100644 index 00000000..44aaddd3 --- /dev/null +++ b/src/lint/rules/if-else-consistency.ts @@ -0,0 +1,57 @@ +import type { Reporter } from '../algorithm-error-reporter-type'; +import type { Seq } from '../../expr-parser'; +import type { OrderedListItemNode, OrderedListNode } from 'ecmarkdown'; +import { offsetToLineAndColumn } from '../../utils'; + +const ruleId = 'if-else-consistency'; + +/* +Checks that `if`/`else` statements are both single-line or both multi-line. +*/ +export default function ( + report: Reporter, + step: OrderedListItemNode, + algorithmSource: string, + parsedSteps: Map, + parent: OrderedListNode +) { + const stepSeq = parsedSteps.get(step); + if (stepSeq == null) { + return; + } + const firstSeqItem = stepSeq.items[0]; + if (firstSeqItem?.name !== 'text' || !/^(?:If|Else if)\b/.test(firstSeqItem.contents)) { + return; + } + const idx = parent.contents.indexOf(step); + if (idx >= parent.contents.length - 1) { + return; + } + const nextStep = parent.contents[idx + 1]; + const nextSeq = parsedSteps.get(nextStep); + if (nextSeq == null) { + return; + } + const nextFirstSeqitem = nextSeq.items[0]; + if ( + nextFirstSeqitem?.name !== 'text' || + !/^(?:Else|Otherwise)\b/.test(nextFirstSeqitem.contents) + ) { + return; + } + if (step.sublist != null && nextStep.sublist == null) { + const location = offsetToLineAndColumn(algorithmSource, nextFirstSeqitem.location.start.offset); + report({ + ruleId, + ...location, + message: '"Else" steps should be multiline whenever their corresponding "If" is', + }); + } else if (step.sublist == null && nextStep.sublist != null) { + const location = offsetToLineAndColumn(algorithmSource, firstSeqItem.location.start.offset); + report({ + ruleId, + ...location, + message: '"If" steps should be multiline whenever their corresponding "Else" is', + }); + } +} diff --git a/src/lint/rules/step-attributes.ts b/src/lint/rules/step-attributes.ts index 99d36a18..c5842170 100644 --- a/src/lint/rules/step-attributes.ts +++ b/src/lint/rules/step-attributes.ts @@ -1,6 +1,5 @@ import type { OrderedListItemNode } from 'ecmarkdown'; import type { Reporter } from '../algorithm-error-reporter-type'; -import type { Seq } from '../../expr-parser'; import { SPECIAL_KINDS } from '../../Clause'; @@ -11,7 +10,7 @@ const KNOWN_ATTRIBUTES = ['id', 'fence-effects', 'declared', ...SPECIAL_KINDS]; /* Checks for unknown attributes on steps. */ -export default function (report: Reporter, stepSeq: Seq | null, node: OrderedListItemNode) { +export default function (report: Reporter, node: OrderedListItemNode) { for (const attr of node.attrs) { if (!KNOWN_ATTRIBUTES.includes(attr.key)) { report({ diff --git a/test/lint-algorithms.js b/test/lint-algorithms.js index e61150dc..87b77b7c 100644 --- a/test/lint-algorithms.js +++ b/test/lint-algorithms.js @@ -467,4 +467,132 @@ describe('linting algorithms', () => { `); }); }); + + describe('if/else consistency', () => { + const ruleId = 'if-else-consistency'; + it('rejects single-line if with multiline else', async () => { + await assertLint( + positioned` + + 1. ${M}If some condition holds, do something. + 1. Else, + 1. Do something else. + `, + { + ruleId, + nodeType, + message: '"If" steps should be multiline whenever their corresponding "Else" is', + } + ); + }); + + it('rejects single-line if with multiline else-if', async () => { + await assertLint( + positioned` + + 1. ${M}If some condition holds, do something. + 1. Else if another condition holds, then + 1. Do something else. + 1. Else, + 1. Do something yet otherwise. + `, + { + ruleId, + nodeType, + message: '"If" steps should be multiline whenever their corresponding "Else" is', + } + ); + }); + + it('rejects single-line else-if with multiline else', async () => { + await assertLint( + positioned` + + 1. If some condition holds, do something. + 1. ${M}Else if another condition holds, do something else. + 1. Else, + 1. Do something yet otherwise. + `, + { + ruleId, + nodeType, + message: '"If" steps should be multiline whenever their corresponding "Else" is', + } + ); + }); + + it('rejects multi-line if with single-line else', async () => { + await assertLint( + positioned` + + 1. If some condition holds, then + 1. Do something. + 1. ${M}Else do something else. + `, + { + ruleId, + nodeType, + message: '"Else" steps should be multiline whenever their corresponding "If" is', + } + ); + }); + + it('rejects multi-line if with single-line else-f', async () => { + await assertLint( + positioned` + + 1. If some condition holds, then + 1. Do something. + 1. ${M}Else if another condition holds do something else. + 1. Else, do something yet otherwise. + `, + { + ruleId, + nodeType, + message: '"Else" steps should be multiline whenever their corresponding "If" is', + } + ); + }); + + it('rejects multi-line else-if with single-line else', async () => { + await assertLint( + positioned` + + 1. If some condition holds, then + 1. Do something. + 1. Else if another condition holds, then + 1. Do something else. + 1. ${M}Else, do something yet otherwise. + `, + { + ruleId, + nodeType, + message: '"Else" steps should be multiline whenever their corresponding "If" is', + } + ); + }); + + it('negative', async () => { + await assertLintFree(` + + 1. If some condition holds, do something simple. + 1. Else, do something yet otherwise simple. + 1. If some condition holds, do something simple. + 1. Else if another condition holds, do something else simple. + 1. Else, do something yet otherwise simple. + 1. NOTE: Also works for multiline. + 1. If some condition holds, then + 1. Do something simple. + 1. Else, + 1. Do something yet otherwise simple. + 1. If some condition holds, then + 1. Do something simple. + 1. Else if another condition holds, then + 1. Do something else simple. + 1. Else, + 1. Do something yet otherwise simple. + + `); + }); + }); });