From e8f02c3ca69f9c1f29ed94139136986eaaee40a2 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Tue, 19 May 2020 23:31:38 -0700 Subject: [PATCH 1/7] add lint for header format --- src/lint/collect-header-diagnostics.ts | 104 +++++++++++++++++++++++++ src/lint/collect-nodes.ts | 4 +- src/lint/lint.ts | 9 ++- test/lint.js | 100 +++++++++++++++++++++++- 4 files changed, 214 insertions(+), 3 deletions(-) create mode 100644 src/lint/collect-header-diagnostics.ts diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts new file mode 100644 index 00000000..2cf01580 --- /dev/null +++ b/src/lint/collect-header-diagnostics.ts @@ -0,0 +1,104 @@ +import type { LintingError } from './algorithm-error-reporter-type'; + +import { getLocation } from './utils'; + +const ruleId = 'header-format'; + +export function collectHeaderDiagnostics( + dom: any, + headers: { element: Element; contents: string }[] +) { + let lintingErrors: LintingError[] = []; + + for (let { element, contents } of headers) { + if (!/\(.*\)$/.test(contents) || / Operator \( `[^`]+` \)$/.test(contents)) { + continue; + } + + function indexToLineAndColumn(index: number) { + let headerLines = contents.split('\n'); + let headerLine = 0; + let seen = 0; + while (true) { + if (seen + headerLines[headerLine].length >= index) { + break; + } + seen += headerLines[headerLine].length + 1; // +1 for the '\n' + ++headerLine; + } + let headerColumn = index - seen; + + let elementLoc = getLocation(dom, element); + let line = elementLoc.startTag.line + headerLine; + let column = + headerLine === 0 + ? elementLoc.startTag.col + + (elementLoc.startTag.endOffset - elementLoc.startTag.startOffset) + + headerColumn + : headerColumn + 1; + + return { line, column }; + } + + let name = contents.substring(0, contents.indexOf('(')); + let params = contents.substring(contents.indexOf('(') + 1, contents.length - 1); + + if (/ $/.test(name)) { + name = name.substring(0, name.length - 1); + } else { + let { line, column } = indexToLineAndColumn(name.length); + lintingErrors.push({ + ruleId, + nodeType: 'H1', + line, + column, + message: 'expected header to have a space before the argument list', + }); + } + + let nameMatches = [ + /^(Runtime|Static) Semantics: [A-Z][A-Za-z0-9/]*$/, + /^(Number|BigInt)::[a-z][A-Za-z0-9]*$/, + /^\[\[[A-Z][A-Za-z0-9]*\]\]$/, + /^_[A-Z][A-Za-z0-9]*_$/, + /^[A-Za-z][A-Za-z0-9]*(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?$/, + /^%[A-Z][A-Za-z0-9]*%(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?$/, + ].some(r => r.test(name)); + + if (!nameMatches) { + let { line, column } = indexToLineAndColumn(0); + lintingErrors.push({ + ruleId, + nodeType: 'H1', + line, + column, + message: `expected operation to have a name like 'Example', 'Runtime Semantics: Foo', 'Example.prop', etc, but found ${JSON.stringify( + name + )}`, + }); + } + + let paramsMatches = + params.match(/\[/g)?.length === params.match(/\]/g)?.length && + [ + /^ $/, + /^ \. \. \. $/, + /^ (_[A-Za-z0-9]+_, )*\.\.\._[A-Za-z0-9]+_ $/, + /^ (_[A-Za-z0-9]+_, )*… (, _[A-Za-z0-9]+_)+ $/, + /^ (\[ )?_[A-Za-z0-9]+_(, _[A-Za-z0-9]+_)*( \[ , _[A-Za-z0-9]+_(, _[A-Za-z0-9]+_)*)*( \])* $/, + ].some(r => r.test(params)); + + if (!paramsMatches) { + let { line, column } = indexToLineAndColumn(name.length + 1); + lintingErrors.push({ + ruleId, + nodeType: 'H1', + line, + column, + message: `expected parameter list to look like '( _a_, [ , _b_ ] )', '( _foo_, _bar_, ..._baz_ )', '( _foo_, … , _bar_ )', or '( . . . )'`, + }); + } + } + + return lintingErrors; +} diff --git a/src/lint/collect-nodes.ts b/src/lint/collect-nodes.ts index 58cd3cb7..8fd315ff 100644 --- a/src/lint/collect-nodes.ts +++ b/src/lint/collect-nodes.ts @@ -3,6 +3,7 @@ import type { Node as EcmarkdownNode } from 'ecmarkdown'; import { getLocation } from './utils'; export function collectNodes(sourceText: string, dom: any, document: Document) { + let headers: { element: Element; contents: string }[] = []; let mainGrammar: { element: Element; source: string }[] = []; let sdos: { grammar: Element; alg: Element }[] = []; let earlyErrors: { grammar: Element; lists: HTMLUListElement[] }[] = []; @@ -28,6 +29,7 @@ export function collectNodes(sourceText: string, dom: any, document: Document) { let first = node.firstElementChild; if (first !== null && first.nodeName === 'H1') { let title = first.textContent ?? ''; + headers.push({ element: first, contents: title }); if (title.trim() === 'Static Semantics: Early Errors') { let grammar = null; let lists: HTMLUListElement[] = []; @@ -101,5 +103,5 @@ export function collectNodes(sourceText: string, dom: any, document: Document) { } visitCurrentNode(); - return { mainGrammar, sdos, earlyErrors, algorithms }; + return { mainGrammar, headers, sdos, earlyErrors, algorithms }; } diff --git a/src/lint/lint.ts b/src/lint/lint.ts index 014245e0..84149afe 100644 --- a/src/lint/lint.ts +++ b/src/lint/lint.ts @@ -3,6 +3,7 @@ import { emit } from 'ecmarkdown'; import { collectNodes } from './collect-nodes'; import { collectGrammarDiagnostics } from './collect-grammar-diagnostics'; import { collectAlgorithmDiagnostics } from './collect-algorithm-diagnostics'; +import { collectHeaderDiagnostics } from './collect-header-diagnostics'; import type { Reporter } from './algorithm-error-reporter-type'; /* @@ -16,7 +17,11 @@ There's more to do: https://github.com/tc39/ecmarkup/issues/173 */ export function lint(report: Reporter, sourceText: string, dom: any, document: Document) { - let { mainGrammar, sdos, earlyErrors, algorithms } = collectNodes(sourceText, dom, document); + let { mainGrammar, headers, sdos, earlyErrors, algorithms } = collectNodes( + sourceText, + dom, + document + ); let { grammar, oneOffGrammars, lintingErrors } = collectGrammarDiagnostics( dom, @@ -28,6 +33,8 @@ export function lint(report: Reporter, sourceText: string, dom: any, document: D lintingErrors.push(...collectAlgorithmDiagnostics(dom, sourceText, algorithms)); + lintingErrors.push(...collectHeaderDiagnostics(dom, headers)); + if (lintingErrors.length > 0) { report(lintingErrors, sourceText); return; diff --git a/test/lint.js b/test/lint.js index 0bbbb688..c87d3e61 100644 --- a/test/lint.js +++ b/test/lint.js @@ -1,6 +1,6 @@ 'use strict'; -let { assertLint, positioned, lintLocationMarker: M } = require('./lint-helpers'); +let { assertLint, assertLintFree, positioned, lintLocationMarker: M } = require('./lint-helpers'); describe('linting whole program', function () { describe('grammar validity', function () { @@ -110,4 +110,102 @@ describe('linting whole program', function () { ); }); }); + + describe('header format', function () { + it('name format', async function () { + await assertLint( + positioned` + +

${M}something: ( )

+ `, + { + ruleId: 'header-format', + nodeType: 'H1', + message: + "expected operation to have a name like 'Example', 'Runtime Semantics: Foo', 'Example.prop', etc, but found \"something:\"", + } + ); + }); + + it('spacing', async function () { + await assertLint( + positioned` + +

Example${M}( )

+ `, + { + ruleId: 'header-format', + nodeType: 'H1', + message: 'expected header to have a space before the argument list', + } + ); + }); + + it('arg format', async function () { + await assertLint( + positioned` + +

Example ${M}(_a_)

+ `, + { + ruleId: 'header-format', + nodeType: 'H1', + message: + "expected parameter list to look like '( _a_, [ , _b_ ] )', '( _foo_, _bar_, ..._baz_ )', '( _foo_, … , _bar_ )', or '( . . . )'", + } + ); + }); + + it('legal names', async function () { + await assertLintFree(` + +

Example ( )

+ + +

Runtime Semantics: Example ( )

+ + +

The * Operator ( \`*\` )

+ + +

Number::example ( )

+ + +

[[Example]] ( )

+ + +

_Example_ ( )

+ + +

%Foo%.bar [ @@iterator ] ( )

+ + `); + }); + + it('legal argument lists', async function () { + await assertLintFree(` + +

Example ( )

+ + +

Example ( _foo_ )

+ + +

Example ( [ _foo_ ] )

+ + +

Date ( _year_, _month_ [ , _date_ [ , _hours_ [ , _minutes_ [ , _seconds_ [ , _ms_ ] ] ] ] ] )

+ + +

Object ( . . . )

+ + +

String.raw ( _template_, ..._substitutions_ )

+ + +

Function ( _p1_, _p2_, … , _pn_, _body_ )

+ + `); + }); + }); }); From 70f4337d345a8176dcb1a3cd5f24e9df0b69823a Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 14:20:47 -0700 Subject: [PATCH 2/7] factor location logic out --- src/lint/collect-header-diagnostics.ts | 45 +++++++++----------------- src/lint/utils.ts | 28 ++++++++++++++++ 2 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index 2cf01580..7112583a 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -1,6 +1,6 @@ import type { LintingError } from './algorithm-error-reporter-type'; -import { getLocation } from './utils'; +import { getLocation, indexWithinElementToTrueLocation } from './utils'; const ruleId = 'header-format'; @@ -15,38 +15,17 @@ export function collectHeaderDiagnostics( continue; } - function indexToLineAndColumn(index: number) { - let headerLines = contents.split('\n'); - let headerLine = 0; - let seen = 0; - while (true) { - if (seen + headerLines[headerLine].length >= index) { - break; - } - seen += headerLines[headerLine].length + 1; // +1 for the '\n' - ++headerLine; - } - let headerColumn = index - seen; - - let elementLoc = getLocation(dom, element); - let line = elementLoc.startTag.line + headerLine; - let column = - headerLine === 0 - ? elementLoc.startTag.col + - (elementLoc.startTag.endOffset - elementLoc.startTag.startOffset) + - headerColumn - : headerColumn + 1; - - return { line, column }; - } - let name = contents.substring(0, contents.indexOf('(')); let params = contents.substring(contents.indexOf('(') + 1, contents.length - 1); if (/ $/.test(name)) { name = name.substring(0, name.length - 1); } else { - let { line, column } = indexToLineAndColumn(name.length); + let { line, column } = indexWithinElementToTrueLocation( + getLocation(dom, element), + contents, + name.length + ); lintingErrors.push({ ruleId, nodeType: 'H1', @@ -66,7 +45,11 @@ export function collectHeaderDiagnostics( ].some(r => r.test(name)); if (!nameMatches) { - let { line, column } = indexToLineAndColumn(0); + let { line, column } = indexWithinElementToTrueLocation( + getLocation(dom, element), + contents, + 0 + ); lintingErrors.push({ ruleId, nodeType: 'H1', @@ -89,7 +72,11 @@ export function collectHeaderDiagnostics( ].some(r => r.test(params)); if (!paramsMatches) { - let { line, column } = indexToLineAndColumn(name.length + 1); + let { line, column } = indexWithinElementToTrueLocation( + getLocation(dom, element), + contents, + name.length + 1 + ); lintingErrors.push({ ruleId, nodeType: 'H1', diff --git a/src/lint/utils.ts b/src/lint/utils.ts index edbcb7f8..2f51f09b 100644 --- a/src/lint/utils.ts +++ b/src/lint/utils.ts @@ -14,6 +14,34 @@ import type { import { Grammar as GrammarFile, SyntaxKind } from 'grammarkdown'; +export function indexWithinElementToTrueLocation( + elementLoc: ReturnType, + string: string, + index: number +) { + let headerLines = string.split('\n'); + let headerLine = 0; + let seen = 0; + while (true) { + if (seen + headerLines[headerLine].length >= index) { + break; + } + seen += headerLines[headerLine].length + 1; // +1 for the '\n' + ++headerLine; + } + let headerColumn = index - seen; + + let line = elementLoc.startTag.line + headerLine; + let column = + headerLine === 0 + ? elementLoc.startTag.col + + (elementLoc.startTag.endOffset - elementLoc.startTag.startOffset) + + headerColumn + : headerColumn + 1; + + return { line, column }; +} + export function grammarkdownLocationToTrueLocation( elementLoc: ReturnType, gmdLine: number, From a6077eb9bf8234e36c369529dca32fe2c47fc455 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 14:25:33 -0700 Subject: [PATCH 3/7] match multiple spaces after name --- src/lint/collect-header-diagnostics.ts | 20 +++++++++----------- test/lint.js | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index 7112583a..b995fe65 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -18,9 +18,7 @@ export function collectHeaderDiagnostics( let name = contents.substring(0, contents.indexOf('(')); let params = contents.substring(contents.indexOf('(') + 1, contents.length - 1); - if (/ $/.test(name)) { - name = name.substring(0, name.length - 1); - } else { + if (!/[\S] $/.test(name)) { let { line, column } = indexWithinElementToTrueLocation( getLocation(dom, element), contents, @@ -31,17 +29,17 @@ export function collectHeaderDiagnostics( nodeType: 'H1', line, column, - message: 'expected header to have a space before the argument list', + message: 'expected header to have a single space before the argument list', }); } let nameMatches = [ - /^(Runtime|Static) Semantics: [A-Z][A-Za-z0-9/]*$/, - /^(Number|BigInt)::[a-z][A-Za-z0-9]*$/, - /^\[\[[A-Z][A-Za-z0-9]*\]\]$/, - /^_[A-Z][A-Za-z0-9]*_$/, - /^[A-Za-z][A-Za-z0-9]*(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?$/, - /^%[A-Z][A-Za-z0-9]*%(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?$/, + /^(Runtime|Static) Semantics: [A-Z][A-Za-z0-9/]*\s*$/, + /^(Number|BigInt)::[a-z][A-Za-z0-9]*\s*$/, + /^\[\[[A-Z][A-Za-z0-9]*\]\]\s*$/, + /^_[A-Z][A-Za-z0-9]*_\s*$/, + /^[A-Za-z][A-Za-z0-9]*(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/, + /^%[A-Z][A-Za-z0-9]*%(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/, ].some(r => r.test(name)); if (!nameMatches) { @@ -75,7 +73,7 @@ export function collectHeaderDiagnostics( let { line, column } = indexWithinElementToTrueLocation( getLocation(dom, element), contents, - name.length + 1 + name.length ); lintingErrors.push({ ruleId, diff --git a/test/lint.js b/test/lint.js index c87d3e61..5d704fae 100644 --- a/test/lint.js +++ b/test/lint.js @@ -122,7 +122,7 @@ describe('linting whole program', function () { ruleId: 'header-format', nodeType: 'H1', message: - "expected operation to have a name like 'Example', 'Runtime Semantics: Foo', 'Example.prop', etc, but found \"something:\"", + "expected operation to have a name like 'Example', 'Runtime Semantics: Foo', 'Example.prop', etc, but found \"something: \"", } ); }); @@ -136,7 +136,19 @@ describe('linting whole program', function () { { ruleId: 'header-format', nodeType: 'H1', - message: 'expected header to have a space before the argument list', + message: 'expected header to have a single space before the argument list', + } + ); + + await assertLint( + positioned` + +

Example ${M}( )

+ `, + { + ruleId: 'header-format', + nodeType: 'H1', + message: 'expected header to have a single space before the argument list', } ); }); From 39628424963844d5054d560028ae8769ce910720 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 14:26:05 -0700 Subject: [PATCH 4/7] relax restriction on methods --- src/lint/collect-header-diagnostics.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index b995fe65..35920bed 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -35,7 +35,7 @@ export function collectHeaderDiagnostics( let nameMatches = [ /^(Runtime|Static) Semantics: [A-Z][A-Za-z0-9/]*\s*$/, - /^(Number|BigInt)::[a-z][A-Za-z0-9]*\s*$/, + /^[A-Z][A-Za-z0-9]*::[a-z][A-Za-z0-9]*\s*$/, /^\[\[[A-Z][A-Za-z0-9]*\]\]\s*$/, /^_[A-Z][A-Za-z0-9]*_\s*$/, /^[A-Za-z][A-Za-z0-9]*(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/, From 77b73ce717f9a217a0a2966aba0641140d06c992 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 14:34:53 -0700 Subject: [PATCH 5/7] add sample comments --- src/lint/collect-header-diagnostics.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index 35920bed..5f76e69d 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -34,11 +34,25 @@ export function collectHeaderDiagnostics( } let nameMatches = [ + // Runtime Semantics: Foo /^(Runtime|Static) Semantics: [A-Z][A-Za-z0-9/]*\s*$/, + + // Number::foo /^[A-Z][A-Za-z0-9]*::[a-z][A-Za-z0-9]*\s*$/, + + // [[GetOwnProperty]] /^\[\[[A-Z][A-Za-z0-9]*\]\]\s*$/, + + // _NativeError_ /^_[A-Z][A-Za-z0-9]*_\s*$/, + + // CreateForInIterator + // Object.fromEntries + // Array.prototype [ @@iterator ] /^[A-Za-z][A-Za-z0-9]*(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/, + + // %ForInIteratorPrototype%.next + // %TypedArray%.prototype [ @@iterator ] /^%[A-Z][A-Za-z0-9]*%(\.[A-Za-z][A-Za-z0-9]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/, ].some(r => r.test(name)); @@ -62,10 +76,20 @@ export function collectHeaderDiagnostics( let paramsMatches = params.match(/\[/g)?.length === params.match(/\]/g)?.length && [ + // Foo ( ) /^ $/, + + // Object ( . . . ) /^ \. \. \. $/, + + // String.raw ( _template_, ..._substitutions_ ) /^ (_[A-Za-z0-9]+_, )*\.\.\._[A-Za-z0-9]+_ $/, + + // Function ( _p1_, _p2_, … , _pn_, _body_ ) /^ (_[A-Za-z0-9]+_, )*… (, _[A-Za-z0-9]+_)+ $/, + + // Example ( _foo_ , [ _bar_ ] ) + // Example ( [ _foo_ ] ) /^ (\[ )?_[A-Za-z0-9]+_(, _[A-Za-z0-9]+_)*( \[ , _[A-Za-z0-9]+_(, _[A-Za-z0-9]+_)*)*( \])* $/, ].some(r => r.test(params)); From a4630e05f87452429f213702d8aceeeab3de9892 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 14:35:41 -0700 Subject: [PATCH 6/7] element.tagName --- src/lint/collect-header-diagnostics.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index 5f76e69d..986bbc8f 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -26,7 +26,7 @@ export function collectHeaderDiagnostics( ); lintingErrors.push({ ruleId, - nodeType: 'H1', + nodeType: element.tagName, line, column, message: 'expected header to have a single space before the argument list', @@ -64,7 +64,7 @@ export function collectHeaderDiagnostics( ); lintingErrors.push({ ruleId, - nodeType: 'H1', + nodeType: element.tagName, line, column, message: `expected operation to have a name like 'Example', 'Runtime Semantics: Foo', 'Example.prop', etc, but found ${JSON.stringify( @@ -101,7 +101,7 @@ export function collectHeaderDiagnostics( ); lintingErrors.push({ ruleId, - nodeType: 'H1', + nodeType: element.tagName, line, column, message: `expected parameter list to look like '( _a_, [ , _b_ ] )', '( _foo_, _bar_, ..._baz_ )', '( _foo_, … , _bar_ )', or '( . . . )'`, From bfaeba84dc065e7f8fb8177d402fe7f96ec91aac Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Wed, 20 May 2020 17:10:54 -0700 Subject: [PATCH 7/7] move location --- src/lint/collect-header-diagnostics.ts | 2 +- test/lint.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lint/collect-header-diagnostics.ts b/src/lint/collect-header-diagnostics.ts index 986bbc8f..d44542e1 100644 --- a/src/lint/collect-header-diagnostics.ts +++ b/src/lint/collect-header-diagnostics.ts @@ -22,7 +22,7 @@ export function collectHeaderDiagnostics( let { line, column } = indexWithinElementToTrueLocation( getLocation(dom, element), contents, - name.length + name.length - 1 ); lintingErrors.push({ ruleId, diff --git a/test/lint.js b/test/lint.js index 5d704fae..76f303c6 100644 --- a/test/lint.js +++ b/test/lint.js @@ -131,7 +131,7 @@ describe('linting whole program', function () { await assertLint( positioned` -

Example${M}( )

+

Exampl${M}e( )

`, { ruleId: 'header-format', @@ -143,7 +143,7 @@ describe('linting whole program', function () { await assertLint( positioned` -

Example ${M}( )

+

Example ${M} ( )

`, { ruleId: 'header-format',