Skip to content

Commit

Permalink
docs: check config comments in rule examples (#17815)
Browse files Browse the repository at this point in the history
* docs: check config comments in rule examples

* move `parseDirective` out of `SourceCode`

* apply suggestions
  • Loading branch information
fasttime committed Dec 8, 2023
1 parent fd28363 commit 4391b71
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 72 deletions.
1 change: 1 addition & 0 deletions docs/src/rules/capitalized-comments.md
Expand Up @@ -33,6 +33,7 @@ Examples of **correct** code for this rule:
:::correct

```js
/* eslint capitalized-comments: error */

// Capitalized comment

Expand Down
4 changes: 4 additions & 0 deletions docs/src/rules/no-buffer-constructor.md
Expand Up @@ -21,6 +21,8 @@ Examples of **incorrect** code for this rule:
::: incorrect

```js
/* eslint no-buffer-constructor: error */

new Buffer(5);
new Buffer([1, 2, 3]);

Expand All @@ -38,6 +40,8 @@ Examples of **correct** code for this rule:
::: correct

```js
/* eslint no-buffer-constructor: error */

Buffer.alloc(5);
Buffer.allocUnsafe(5);
Buffer.from([1, 2, 3]);
Expand Down
1 change: 1 addition & 0 deletions docs/src/rules/no-implicit-globals.md
Expand Up @@ -264,6 +264,7 @@ Examples of **correct** code for `/* exported variableName */` operation:
::: correct { "sourceType": "script" }

```js
/* eslint no-implicit-globals: error */
/* exported global_var */

var global_var = 42;
Expand Down
20 changes: 0 additions & 20 deletions docs/src/rules/strict.md
Expand Up @@ -271,19 +271,13 @@ This option ensures that all functions are executed in strict mode. A strict mod

Examples of **incorrect** code for this rule with the earlier default option which has been removed:

::: incorrect { "sourceType": "script" }

```js
// "strict": "error"

function foo() {
}
```

:::

::: incorrect { "sourceType": "script" }

```js
// "strict": "error"

Expand All @@ -294,12 +288,8 @@ function foo() {
}());
```

:::

Examples of **correct** code for this rule with the earlier default option which has been removed:

::: correct { "sourceType": "script" }

```js
// "strict": "error"

Expand All @@ -309,10 +299,6 @@ function foo() {
}
```

:::

::: correct { "sourceType": "script" }

```js
// "strict": "error"

Expand All @@ -321,10 +307,6 @@ function foo() {
}
```

:::

::: correct { "sourceType": "script" }

```js
// "strict": "error"

Expand All @@ -336,8 +318,6 @@ function foo() {
}());
```

:::

## When Not To Use It

In a codebase that has both strict and non-strict code, either turn this rule off, or [selectively disable it](../use/configure/rules#disabling-rules) where necessary. For example, functions referencing `arguments.callee` are invalid in strict mode. A [full list of strict mode differences](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode/Transitioning_to_strict_mode#Differences_from_non-strict_to_strict) is available on MDN.
36 changes: 35 additions & 1 deletion lib/linter/config-comment-parser.js
Expand Up @@ -15,7 +15,10 @@ const levn = require("levn"),
Legacy: {
ConfigOps
}
} = require("@eslint/eslintrc/universal");
} = require("@eslint/eslintrc/universal"),
{
directivesPattern
} = require("../shared/directives");

const debug = require("debug")("eslint:config-comment-parser");

Expand Down Expand Up @@ -148,4 +151,35 @@ module.exports = class ConfigCommentParser {
return items;
}

/**
* Extract the directive and the justification from a given directive comment and trim them.
* @param {string} value The comment text to extract.
* @returns {{directivePart: string, justificationPart: string}} The extracted directive and justification.
*/
extractDirectiveComment(value) {
const match = /\s-{2,}\s/u.exec(value);

if (!match) {
return { directivePart: value.trim(), justificationPart: "" };
}

const directive = value.slice(0, match.index).trim();
const justification = value.slice(match.index + match[0].length).trim();

return { directivePart: directive, justificationPart: justification };
}

/**
* Parses a directive comment into directive text and value.
* @param {Comment} comment The comment node with the directive to be parsed.
* @returns {{directiveText: string, directiveValue: string}} The directive text and value.
*/
parseDirective(comment) {
const { directivePart } = this.extractDirectiveComment(comment.value);
const match = directivesPattern.exec(directivePart);
const directiveText = match[1];
const directiveValue = directivePart.slice(match.index + directiveText.length);

return { directiveText, directiveValue };
}
};
24 changes: 3 additions & 21 deletions lib/linter/linter.js
Expand Up @@ -316,24 +316,6 @@ function createDisableDirectives(options) {
return result;
}

/**
* Extract the directive and the justification from a given directive comment and trim them.
* @param {string} value The comment text to extract.
* @returns {{directivePart: string, justificationPart: string}} The extracted directive and justification.
*/
function extractDirectiveComment(value) {
const match = /\s-{2,}\s/u.exec(value);

if (!match) {
return { directivePart: value.trim(), justificationPart: "" };
}

const directive = value.slice(0, match.index).trim();
const justification = value.slice(match.index + match[0].length).trim();

return { directivePart: directive, justificationPart: justification };
}

/**
* Parses comments in file to extract file-specific config of rules, globals
* and environments and merges them with global config; also code blocks
Expand All @@ -355,7 +337,7 @@ function getDirectiveComments(sourceCode, ruleMapper, warnInlineConfig) {
});

sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);
const { directivePart, justificationPart } = commentParser.extractDirectiveComment(comment.value);

const match = directivesPattern.exec(directivePart);

Expand Down Expand Up @@ -500,7 +482,7 @@ function getDirectiveCommentsForFlatConfig(sourceCode, ruleMapper) {
const disableDirectives = [];

sourceCode.getInlineConfigNodes().filter(token => token.type !== "Shebang").forEach(comment => {
const { directivePart, justificationPart } = extractDirectiveComment(comment.value);
const { directivePart, justificationPart } = commentParser.extractDirectiveComment(comment.value);

const match = directivesPattern.exec(directivePart);

Expand Down Expand Up @@ -620,7 +602,7 @@ function findEslintEnv(text) {
if (match[0].endsWith("*/")) {
retv = Object.assign(
retv || {},
commentParser.parseListConfig(extractDirectiveComment(match[1]).directivePart)
commentParser.parseListConfig(commentParser.extractDirectiveComment(match[1]).directivePart)
);
}
}
Expand Down
25 changes: 2 additions & 23 deletions lib/source-code/source-code.js
Expand Up @@ -212,24 +212,6 @@ function isSpaceBetween(sourceCode, first, second, checkInsideOfJSXText) {
// Directive Comments
//-----------------------------------------------------------------------------

/**
* Extract the directive and the justification from a given directive comment and trim them.
* @param {string} value The comment text to extract.
* @returns {{directivePart: string, justificationPart: string}} The extracted directive and justification.
*/
function extractDirectiveComment(value) {
const match = /\s-{2,}\s/u.exec(value);

if (!match) {
return { directivePart: value.trim(), justificationPart: "" };
}

const directive = value.slice(0, match.index).trim();
const justification = value.slice(match.index + match[0].length).trim();

return { directivePart: directive, justificationPart: justification };
}

/**
* Ensures that variables representing built-in properties of the Global Object,
* and any globals declared by special block comments, are present in the global
Expand Down Expand Up @@ -921,7 +903,7 @@ class SourceCode extends TokenStore {
return false;
}

const { directivePart } = extractDirectiveComment(comment.value);
const { directivePart } = commentParser.extractDirectiveComment(comment.value);

const directiveMatch = directivesPattern.exec(directivePart);

Expand Down Expand Up @@ -977,10 +959,7 @@ class SourceCode extends TokenStore {

this.getInlineConfigNodes().forEach(comment => {

const { directivePart } = extractDirectiveComment(comment.value);
const match = directivesPattern.exec(directivePart);
const directiveText = match[1];
const directiveValue = directivePart.slice(match.index + directiveText.length);
const { directiveText, directiveValue } = commentParser.parseDirective(comment);

switch (directiveText) {
case "exported":
Expand Down
10 changes: 9 additions & 1 deletion tests/fixtures/bad-examples.md
@@ -1,5 +1,5 @@
---
title: Lorem Ipsum
title: no-restricted-syntax
---

This file contains rule example code with syntax errors.
Expand All @@ -24,3 +24,11 @@ const foo = "baz";
````

:::

:::correct

```js
/* eslint another-rule: error */
```

:::
24 changes: 24 additions & 0 deletions tests/lib/linter/config-comment-parser.js
Expand Up @@ -248,4 +248,28 @@ describe("ConfigCommentParser", () => {
});
});

describe("parseDirective", () => {

it("should parse a directive comment with a justification", () => {
const comment = { value: " eslint no-unused-vars: error -- test " };
const result = commentParser.parseDirective(comment);

assert.deepStrictEqual(result, {
directiveText: "eslint",
directiveValue: " no-unused-vars: error"
});
});

it("should parse a directive comment without a justification", () => {
const comment = { value: "global foo" };
const result = commentParser.parseDirective(comment);

assert.deepStrictEqual(result, {
directiveText: "global",
directiveValue: " foo"
});
});

});

});
3 changes: 2 additions & 1 deletion tests/tools/check-rule-examples.js
Expand Up @@ -61,8 +61,9 @@ describe("check-rule-examples", () => {
"\x1B[0m \x1B[2m12:1\x1B[22m \x1B[31merror\x1B[39m Syntax error: 'import' and 'export' may appear only with 'sourceType: module'\x1B[0m\n" +
"\x1B[0m \x1B[2m20:5\x1B[22m \x1B[31merror\x1B[39m Nonstandard language tag 'ts': use one of 'javascript', 'js' or 'jsx'\x1B[0m\n" +
"\x1B[0m \x1B[2m23:7\x1B[22m \x1B[31merror\x1B[39m Syntax error: Identifier 'foo' has already been declared\x1B[0m\n" +
"\x1B[0m \x1B[2m31:1\x1B[22m \x1B[31merror\x1B[39m Example code should contain a configuration comment like /* eslint no-restricted-syntax: \"error\" */\x1B[0m\n" +
"\x1B[0m\x1B[0m\n" +
"\x1B[0m\x1B[31m\x1B[1m✖ 4 problems (4 errors, 0 warnings)\x1B[22m\x1B[39m\x1B[0m\n" +
"\x1B[0m\x1B[31m\x1B[1m✖ 5 problems (5 errors, 0 warnings)\x1B[22m\x1B[39m\x1B[0m\n" +
"\x1B[0m\x1B[31m\x1B[1m\x1B[22m\x1B[39m\x1B[0m\n"
}
);
Expand Down
44 changes: 39 additions & 5 deletions tools/check-rule-examples.js
Expand Up @@ -7,10 +7,13 @@
const { parse } = require("espree");
const { readFile } = require("fs").promises;
const glob = require("glob");
const matter = require("gray-matter");
const markdownIt = require("markdown-it");
const markdownItContainer = require("markdown-it-container");
const { promisify } = require("util");
const markdownItRuleExample = require("../docs/tools/markdown-it-rule-example");
const ConfigCommentParser = require("../lib/linter/config-comment-parser");
const rules = require("../lib/rules");

//------------------------------------------------------------------------------
// Typedefs
Expand All @@ -26,19 +29,22 @@ const markdownItRuleExample = require("../docs/tools/markdown-it-rule-example");

const STANDARD_LANGUAGE_TAGS = new Set(["javascript", "js", "jsx"]);

const commentParser = new ConfigCommentParser();

/**
* Tries to parse a specified JavaScript code with Playground presets.
* @param {string} code The JavaScript code to parse.
* @param {ParserOptions} parserOptions Explicitly specified parser options.
* @returns {SyntaxError | null} A `SyntaxError` object if the code cannot be parsed, or `null`.
* @returns {{ ast: ASTNode } | { error: SyntaxError }} An AST with comments, or a `SyntaxError` object if the code cannot be parsed.
*/
function tryParseForPlayground(code, parserOptions) {
try {
parse(code, { ecmaVersion: "latest", ...parserOptions });
const ast = parse(code, { ecmaVersion: "latest", ...parserOptions, comment: true });

return { ast };
} catch (error) {
return error;
return { error };
}
return null;
}

/**
Expand All @@ -48,6 +54,8 @@ function tryParseForPlayground(code, parserOptions) {
*/
async function findProblems(filename) {
const text = await readFile(filename, "UTF-8");
const { title } = matter(text).data;
const isRuleRemoved = !rules.has(title);
const problems = [];
const ruleExampleOptions = markdownItRuleExample({
open({ code, parserOptions, codeBlockToken }) {
Expand All @@ -72,7 +80,33 @@ async function findProblems(filename) {
});
}

const error = tryParseForPlayground(code, parserOptions);
const { ast, error } = tryParseForPlayground(code, parserOptions);

if (ast && !isRuleRemoved) {
const hasRuleConfigComment = ast.comments.some(
comment => {
if (comment.type !== "Block" || !/^\s*eslint(?!\S)/u.test(comment.value)) {
return false;
}
const { directiveValue } = commentParser.parseDirective(comment);
const parseResult = commentParser.parseJsonConfig(directiveValue, comment.loc);

return parseResult.success && Object.prototype.hasOwnProperty.call(parseResult.config, title);
}
);

if (!hasRuleConfigComment) {
const message = `Example code should contain a configuration comment like /* eslint ${title}: "error" */`;

problems.push({
fatal: false,
severity: 2,
message,
line: codeBlockToken.map[0] + 2,
column: 1
});
}
}

if (error) {
const message = `Syntax error: ${error.message}`;
Expand Down

0 comments on commit 4391b71

Please sign in to comment.