Skip to content

Commit

Permalink
basic linting behind --lint-spec (#199)
Browse files Browse the repository at this point in the history
  • Loading branch information
bakkot authored May 16, 2020
1 parent 6aa114e commit bf29f13
Show file tree
Hide file tree
Showing 20 changed files with 1,196 additions and 15 deletions.
1 change: 1 addition & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
],
"rules": {
"prettier/prettier": "error",
"no-inner-declarations": "off",
"consistent-return": "off",
"no-eq-null": "error",
"no-floating-decimal": "error",
Expand Down
12 changes: 6 additions & 6 deletions package-lock.json

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

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"build-release": "tsc -p src",
"prepublish": "safe-publish-latest && npm run build-release",
"pretest": "npm run build",
"test": "mocha && npm run lint",
"test": "mocha",
"lint": "eslint --ext .js,.ts .",
"format": "prettier --write .",
"update-pages": "node bin/ecmarkup.js spec/index.html _index.html && git checkout gh-pages && rm index.html && mv _index.html index.html && git add index.html && git commit -m \"update pages\" && git checkout master",
Expand All @@ -34,8 +34,8 @@
"dependencies": {
"bluebird": "^3.7.2",
"chalk": "^1.1.3",
"ecmarkdown": "4.0.0",
"grammarkdown": "^2.1.2",
"ecmarkdown": "5.0.1",
"grammarkdown": "^2.2.0",
"highlight.js": "^9.17.1",
"html-escape": "^1.0.2",
"js-yaml": "^3.13.1",
Expand Down
9 changes: 8 additions & 1 deletion src/Algorithm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@ import * as emd from 'ecmarkdown';
/*@internal*/
export default class Algorithm extends Builder {
static enter(context: Context) {
context.inAlg = true;
const { node } = context;

if ('ecmarkdownOut' in node) {
// i.e., we already parsed this during the lint phase
// @ts-ignore
node.innerHTML = node.ecmarkdownOut.replace(/((?:\s+|>)[!?])\s+(\w+\s*\()/g, '$1 $2');
return;
}

// replace spaces after !/? with   to prevent bad line breaking
const html = emd
.algorithm(node.innerHTML)
.replace(/((?:\s+|>)[!?])\s+(\w+\s*\()/g, '$1 $2');
node.innerHTML = html;
context.inAlg = true;
}

static exit(context: Context) {
Expand Down
11 changes: 10 additions & 1 deletion src/Grammar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,13 @@ const globalEndTagRe = /<\/?(emu-\w+|h?\d|p|ul|table|pre|code)\b[^>]*>/gi;
/*@internal*/
export default class Grammar extends Builder {
static enter({ spec, node }: Context) {
if ('grammarkdownOut' in node) {
// i.e., we already parsed this during the lint phase
// @ts-ignore
node.innerHTML = node.grammarkdownOut;
return;
}

let content: string;
let possiblyMalformed = true;
if (spec.sourceText) {
Expand All @@ -21,8 +28,9 @@ export default class Grammar extends Builder {
// the parser was able to find a matching end tag.
const start = location.startTag.endOffset as number;
const end = location.endTag.startOffset as number;
content = spec.sourceText.slice(start, end);
content = spec.sourceText!.slice(start, end);
} else {
// TODO this is not reached
// the parser was *not* able to find a matching end tag. Try to recover by finding a
// possible end tag, otherwise read the rest of the source text.
const start = (globalEndTagRe.lastIndex = location.endOffset as number);
Expand All @@ -35,6 +43,7 @@ export default class Grammar extends Builder {
globalEndTagRe.lastIndex = 0;
}
} else {
// TODO this is not reached
// can't read location for whatever reason, so fallback to innerHTML
content = node.innerHTML;
}
Expand Down
13 changes: 12 additions & 1 deletion src/Spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import Xref from './Xref';
import Eqn from './Eqn';
import Biblio from './Biblio';
import { autolink, replacerForNamespace, NO_CLAUSE_AUTOLINK } from './autolinker';
import { lint } from './lint/lint';
import { CancellationToken } from 'prex';

const DRAFT_DATE_FORMAT = { year: 'numeric', month: 'long', day: 'numeric', timeZone: 'UTC' };
Expand Down Expand Up @@ -105,7 +106,7 @@ export default class Spec {
fetch: (file: string, token: CancellationToken) => PromiseLike<string>,
dom: any,
opts?: Options,
sourceText?: string,
sourceText?: string, // TODO we need not support this being undefined
token = CancellationToken.none
) {
opts = opts || {};
Expand Down Expand Up @@ -217,6 +218,16 @@ export default class Spec {
};

const document = this.doc;

if (this.opts.reportLintErrors) {
this._log('Linting...');
const source = this.sourceText;
if (source === undefined) {
throw new Error('Cannot lint when source text is not available');
}
lint(this.opts.reportLintErrors, source, this.dom, document);
}

const walker = document.createTreeWalker(document.body, 1 | 4 /* elements and text nodes */);

walk(walker, context);
Expand Down
6 changes: 6 additions & 0 deletions src/args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ export const argParser = nomnom
jsOut: { full: 'js-out', metavar: 'FILE', help: 'Write Emu JS dependencies to FILE' },
toc: { flag: true, help: "Don't include the table of contents" },
oldToc: { full: 'old-toc', flag: true, help: 'Use the old table of contents styling' },
lintSpec: {
full: 'lint-spec',
flag: true,
default: false,
help: 'Enforce some style and correctness checks',
},
verbose: { flag: true, default: false, help: 'Display document build progress' },
version: {
abbr: 'v',
Expand Down
31 changes: 29 additions & 2 deletions src/cli.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import type { Options } from './ecmarkup';
import type { LintingError } from './lint/lint';

import { argParser } from './args';
const args = argParser.parse();

import * as ecmarkup from './ecmarkup';
import * as fs from 'fs';
import * as chalk from 'chalk';
import * as ecmarkup from './ecmarkup';
import * as utils from './utils';

const debounce: (_: () => Promise<void>) => () => Promise<void> = require('promise-debounce');
Expand All @@ -16,10 +20,33 @@ if (args.js) {
args.jsOut = args.js;
}

if (args.lintSpec && args.watch) {
console.error('Cannot use --lint-spec with --watch');
process.exit(1);
}

const watching = new Map<string, fs.FSWatcher>();
const build = debounce(async function build() {
try {
const spec = await ecmarkup.build(args.infile, utils.readFile, args);
const opts: Options = { ...args };
if (args.lintSpec) {
opts.reportLintErrors = (errors: LintingError[]) => {
console.error(
chalk.underline('Linting errors:') +
'\n' +
errors.map(e => `${args.infile}:${e.line}:${e.column}: ${e.message}`).join('\n') +
'\n'
);
throw new Error(
`There ${
errors.length === 1
? 'was a linting error'
: 'were ' + errors.length + ' linting errors'
}.`
);
};
}
const spec = await ecmarkup.build(args.infile, utils.readFile, opts);

const pending: Promise<any>[] = [];
if (args.biblio) {
Expand Down
3 changes: 3 additions & 0 deletions src/ecmarkup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { BiblioEntry } from './Biblio';
import type { LintingError } from './lint/lint';

import Spec from './Spec';
import * as utils from './utils';
Expand All @@ -24,13 +25,15 @@ export interface Options {
contributors?: string;
toc?: boolean;
oldToc?: boolean;
lintSpec?: boolean;
verbose?: boolean;
cssOut?: string;
jsOut?: string;
assets?: 'none' | 'inline' | 'external';
outfile?: string;
boilerplate?: Boilerplate;
ecma262Biblio?: boolean;
reportLintErrors?: (errors: LintingError[]) => void;
}

export async function build(
Expand Down
9 changes: 9 additions & 0 deletions src/lint/algorithm-error-reporter-type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export type Reporter = ({
line,
column,
message,
}: {
line: number;
column: number;
message: string;
}) => void;
72 changes: 72 additions & 0 deletions src/lint/collect-algorithm-diagnostics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { Node as EcmarkdownNode, Observer } from 'ecmarkdown';

import type { Reporter } from './algorithm-error-reporter-type';
import type { LintingError } from './lint';

import { parseAlgorithm, visit } from 'ecmarkdown';

import { getLocation } from './utils';
import lintAlgorithmLineEndings from './rules/algorithm-line-endings';

let algorithmRules = [lintAlgorithmLineEndings];

function composeObservers(...observers: Observer[]): Observer {
return {
enter(node: EcmarkdownNode) {
for (let observer of observers) {
observer.enter?.(node);
}
},
exit(node: EcmarkdownNode) {
for (let observer of observers) {
observer.exit?.(node);
}
},
};
}

export function collectAlgorithmDiagnostics(
dom: any,
sourceText: string,
algorithms: { element: Element; tree?: EcmarkdownNode }[]
) {
let lintingErrors: LintingError[] = [];

for (let algorithm of algorithms) {
let element = algorithm.element;
let location = getLocation(dom, element);

let reporter: Reporter = ({
line,
column,
message,
}: {
line: number;
column: number;
message: string;
}) => {
// jsdom's lines and columns are both 1-based
// ecmarkdown has 1-based line numbers and 0-based column numbers
// we want 1-based for both
let trueLine = location.startTag.line + line - 1;
let trueCol = column;
if (line === 1) {
trueCol +=
location.startTag.col + (location.startTag.endOffset - location.startTag.startOffset);
} else {
trueCol += 1;
}
lintingErrors.push({ line: trueLine, column: trueCol, message });
};

let observer = composeObservers(...algorithmRules.map(f => f(reporter, element)));
let tree = parseAlgorithm(
sourceText.slice(location.startTag.endOffset, location.endTag.startOffset),
{ trackPositions: true }
);
visit(tree, observer);
algorithm.tree = tree;
}

return lintingErrors;
}
Loading

0 comments on commit bf29f13

Please sign in to comment.