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.

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

docs: check config comments in rule examples #17815

Merged
merged 3 commits into from Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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