Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting: misc improvements #239

Merged
merged 5 commits into from
Aug 18, 2020
Merged

Linting: misc improvements #239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f65a563
lint: allow dunder method names
bakkot Aug 18, 2020
75c2306
lint: lowcase messages from spellchecker
bakkot Aug 18, 2020
45a2cfb
lint: forbid hardcoded clause numbers
bakkot Aug 18, 2020
0bc4039
lint: forbid multiple spaces
bakkot Aug 18, 2020
55e5f70
lint: forbid leading/trailing whitespace within tags
bakkot Aug 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/lint/collect-header-diagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ export function collectHeaderDiagnostics(
// Object.fromEntries
// ForIn/OfHeadEvaluation
// Array.prototype [ @@iterator ]
/^[A-Za-z][A-Za-z0-9/]*(\.[A-Za-z][A-Za-z0-9/]*)*( \[ @@[a-z][a-zA-Z]+ \])?\s*$/,
// Object.prototype.__defineGetter__
/^[A-Za-z][A-Za-z0-9/]*(\.[A-Za-z][A-Za-z0-9]*)*(\.__[a-z][A-Za-z0-9]*__| \[ @@[a-z][a-zA-Z]+ \])?\s*$/,

// %ForInIteratorPrototype%.next
// %TypedArray%.prototype [ @@iterator ]
Expand Down
39 changes: 28 additions & 11 deletions src/lint/collect-spelling-diagnostics.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,52 +9,69 @@ let ruleId = 'spelling';
let matchers = [
{
pattern: /\*this\* object/giu,
message: 'Prefer "*this* value"',
message: 'prefer "*this* value"',
},
{
pattern: /1's complement/giu,
message: 'Prefer "one\'s complement"',
message: 'prefer "one\'s complement"',
},
{
pattern: /2's complement/giu,
message: 'Prefer "two\'s complement"',
message: 'prefer "two\'s complement"',
},
{
pattern: /\*0\*(?!<sub>ℤ<\/sub>)/gu,
message: 'The Number value 0 should be written "*+0*", to unambiguously exclude "*-0*"',
message: 'the Number value 0 should be written "*+0*", to unambiguously exclude "*-0*"',
},
{
pattern: /behavior/giu,
message: 'ECMA-262 uses Oxford spelling ("behaviour")',
},
{
pattern: /[Tt]he empty string/gu,
message: 'Prefer "the empty String"',
message: 'prefer "the empty String"',
},
{
pattern: /[ \t]+\n/gu,
message: 'Trailing spaces are not allowed',
message: 'trailing spaces are not allowed',
},
{
pattern: /(?<=(^|[^\n])\n\n)\n+/gu,
message: 'No more than one blank line is allowed',
message: 'no more than one blank line is allowed',
},
{
pattern: /(?<=<emu-clause.*>\n)\n\s*<h1>/giu,
message: "There should not be a blank line between a clause's opening tag and its header",
message: "there should not be a blank line between a clause's opening tag and its header",
},
{
pattern: /(?<=(^|[^\n])\n)\n+[ \t]*<\/emu-clause>/giu,
message:
'There should not be a blank line between the last line of a clause and its closing tag',
'there should not be a blank line between the last line of a clause and its closing tag',
},
{
pattern: /\r/gu,
message: 'Only Unix-style (LF) linebreaks are allowed',
message: 'only Unix-style (LF) linebreaks are allowed',
},
{
pattern: /(?<=\b[Ss]teps? )\d/gu,
message: 'Prefer using labeled steps and <emu-xref> tags over hardcoding step numbers',
message: 'prefer using labeled steps and <emu-xref> tags over hardcoding step numbers',
},
{
pattern: /(?<=\b[Cc]lauses? )\d/gu,
message:
'clauses should be referenced using <emu-xref> tags rather than hardcoding clause numbers',
},
{
pattern: /(?<=\S) +(?! |<\/(td|th|dd|dt)>)/gu,
message: 'multiple consecutive spaces are not allowed',
},
{
pattern: /(?<=<[a-z]+( [a-z]+(="[^"\n]+")?)*>)(?<!(td|th|dd|dt)>) /gu,
message: 'tags should not contain leading whitespace',
},
{
pattern: /(?<=[^ \n]) +<\/(?!td|th|dd|dt)/gu,
message: 'tags should not contain trailing whitespace',
},
];

Expand Down
91 changes: 77 additions & 14 deletions test/lint-spelling.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('spelling', function () {
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Prefer "*this* value"',
message: 'prefer "*this* value"',
}
);
});
Expand All @@ -24,7 +24,7 @@ describe('spelling', function () {
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Prefer "one\'s complement"',
message: 'prefer "one\'s complement"',
}
);
});
Expand All @@ -37,7 +37,7 @@ describe('spelling', function () {
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Prefer "two\'s complement"',
message: 'prefer "two\'s complement"',
}
);
});
Expand All @@ -50,7 +50,7 @@ describe('spelling', function () {
{
ruleId: 'spelling',
nodeType: 'html',
message: 'The Number value 0 should be written "*+0*", to unambiguously exclude "*-0*"',
message: 'the Number value 0 should be written "*+0*", to unambiguously exclude "*-0*"',
}
);
});
Expand All @@ -76,20 +76,20 @@ describe('spelling', function () {
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Prefer "the empty String"',
message: 'prefer "the empty String"',
}
);
});

it('trailing whitespace', async function () {
await assertLint(
positioned`
<p>something</p>${M}
<p>something</p>${M}
`,
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Trailing spaces are not allowed',
message: 'trailing spaces are not allowed',
}
);
});
Expand All @@ -105,7 +105,7 @@ ${M}
{
ruleId: 'spelling',
nodeType: 'html',
message: 'No more than one blank line is allowed',
message: 'no more than one blank line is allowed',
}
);

Expand All @@ -120,7 +120,7 @@ ${M}
{
ruleId: 'spelling',
nodeType: 'html',
message: 'No more than one blank line is allowed',
message: 'no more than one blank line is allowed',
}
);
});
Expand All @@ -136,7 +136,7 @@ ${M}
{
ruleId: 'spelling',
nodeType: 'html',
message: "There should not be a blank line between a clause's opening tag and its header",
message: "there should not be a blank line between a clause's opening tag and its header",
}
);
});
Expand All @@ -153,7 +153,7 @@ ${M}
ruleId: 'spelling',
nodeType: 'html',
message:
'There should not be a blank line between the last line of a clause and its closing tag',
'there should not be a blank line between the last line of a clause and its closing tag',
}
);
});
Expand All @@ -166,7 +166,7 @@ windows:${M}\r
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Only Unix-style (LF) linebreaks are allowed',
message: 'only Unix-style (LF) linebreaks are allowed',
}
);
});
Expand All @@ -179,7 +179,60 @@ windows:${M}\r
{
ruleId: 'spelling',
nodeType: 'html',
message: 'Prefer using labeled steps and <emu-xref> tags over hardcoding step numbers',
message: 'prefer using labeled steps and <emu-xref> tags over hardcoding step numbers',
}
);
});

it('clause numbers', async function () {
await assertLint(
positioned`
See clause ${M}1.
`,
{
ruleId: 'spelling',
nodeType: 'html',
message:
'clauses should be referenced using <emu-xref> tags rather than hardcoding clause numbers',
}
);
});

it('multiple internal spaces', async function () {
await assertLint(
positioned`
I am writing on a typewriter.${M} In the 21st century, for some reason.
`,
{
ruleId: 'spelling',
nodeType: 'html',
message: 'multiple consecutive spaces are not allowed',
}
);
});

it('leading whitespace within tags', async function () {
await assertLint(
positioned`
<p>${M} This is an example.</p>
`,
{
ruleId: 'spelling',
nodeType: 'html',
message: 'tags should not contain leading whitespace',
}
);
});

it('trailing whitespace within tags', async function () {
await assertLint(
positioned`
<p>This is an example:${M} </p>
`,
{
ruleId: 'spelling',
nodeType: 'html',
message: 'tags should not contain trailing whitespace',
}
);
});
Expand All @@ -206,7 +259,17 @@ windows:${M}\r
<emu-alg>
1. [id="step-label"] Foo.
</emu-alg>
Something about step <emu-xref href="#step-label"></emu-xref>.
<p>Something about step <emu-xref href="#step-label"></emu-xref>.</p>
<p>See clause <emu-xref href="#example"></emu-xref>.</p>

<table><tr><td>Spaces for aligning table cells are acceptable.</td>
<td> For example, it is nice for these to line up. </td>
<td> Also to pad them a bit, in some cases. </td>
</tr></table>

<p>
Paragraphs with the open/close tags on their own line are OK.
</p>
`);
});
});
19 changes: 5 additions & 14 deletions test/lint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ describe('linting whole program', function () {
it('error in inline grammar', async function () {
await assertLint(
positioned`
<emu-grammar type="definition"> Statement[${M}a]: \`;\` </emu-grammar>
<emu-grammar type="definition">Statement[${M}a]: \`;\`</emu-grammar>
`,
{
ruleId: 'grammarkdown:2008',
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('linting whole program', function () {
it('error in inline grammar', async function () {
await assertLint(
positioned`
<emu-grammar> ${M}Statement: EmptyStatement </emu-grammar>
<emu-grammar>${M}Statement: EmptyStatement</emu-grammar>
<emu-alg>
1. Return Foo.
</emu-alg>
Expand Down Expand Up @@ -156,18 +156,6 @@ describe('linting whole program', function () {
message: 'expected header to have a single space before the argument list',
}
);

await assertLint(
positioned`
<emu-clause id="foo">
<h1>Example ${M} ( )</h1>
`,
{
ruleId: 'header-format',
nodeType: 'h1',
message: 'expected header to have a single space before the argument list',
}
);
});

it('arg format', async function () {
Expand Down Expand Up @@ -211,6 +199,9 @@ describe('linting whole program', function () {
<emu-clause id="i8">
<h1>ForIn/OfHeadEvaluation ( )</h1>
</emu-clause>
<emu-clause id="i9">
<h1>Object.prototype.__defineGetter__ ( )</h1>
</emu-clause>
`);
});

Expand Down