Skip to content

Commit

Permalink
✨ Add support for eslint-disable-line block comments and directive …
Browse files Browse the repository at this point in the history
…comments with descriptions. (#43)
  • Loading branch information
ota-meshi authored May 17, 2020
1 parent e93ac75 commit 027e20c
Show file tree
Hide file tree
Showing 14 changed files with 704 additions and 36 deletions.
36 changes: 20 additions & 16 deletions lib/internal/disabled-area.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"use strict"

const utils = require("./utils")
const COMMENT_DIRECTIVE = /^\s*(eslint-(?:en|dis)able(?:(?:-next)?-line)?)\s*(?:(\S|\S[\s\S]*\S)\s*)?$/u
const DELIMITER = /[\s,]+/gu
const pool = new WeakMap()

Expand Down Expand Up @@ -162,39 +161,44 @@ module.exports = class DisabledArea {
}

/**
* Scan the souce code and setup disabled area list.
* Scan the source code and setup disabled area list.
*
* @param {eslint.SourceCode} sourceCode - The source code to scan.
* @returns {void}
* @private
*/
_scan(sourceCode) {
for (const comment of sourceCode.getAllComments()) {
const m = COMMENT_DIRECTIVE.exec(comment.value)
if (m == null) {
const directiveComment = utils.parseDirectiveComment(comment)
if (directiveComment == null) {
continue
}
const kind = m[1]
const ruleIds = m[2] ? m[2].split(DELIMITER) : null

if (comment.type === "Block" && kind === "eslint-disable") {
const kind = directiveComment.kind
if (
kind !== "eslint-disable" &&
kind !== "eslint-enable" &&
kind !== "eslint-disable-line" &&
kind !== "eslint-disable-next-line"
) {
continue
}
const ruleIds = directiveComment.value
? directiveComment.value.split(DELIMITER)
: null

if (kind === "eslint-disable") {
this._disable(comment, comment.loc.start, ruleIds, "block")
} else if (comment.type === "Block" && kind === "eslint-enable") {
} else if (kind === "eslint-enable") {
this._enable(comment, comment.loc.start, ruleIds, "block")
} else if (
comment.type === "Line" &&
kind === "eslint-disable-line"
) {
} else if (kind === "eslint-disable-line") {
const line = comment.loc.start.line
const start = { line, column: 0 }
const end = { line: line + 1, column: -1 }

this._disable(comment, start, ruleIds, "line")
this._enable(comment, end, ruleIds, "line")
} else if (
comment.type === "Line" &&
kind === "eslint-disable-next-line"
) {
} else if (kind === "eslint-disable-next-line") {
const line = comment.loc.start.line
const start = { line: line + 1, column: 0 }
const end = { line: line + 2, column: -1 }
Expand Down
54 changes: 54 additions & 0 deletions lib/internal/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
const escapeStringRegexp = require("escape-string-regexp")
const LINE_PATTERN = /[^\r\n\u2028\u2029]*(?:\r\n|[\r\n\u2028\u2029]|$)/gu

const DIRECTIVE_PATTERN = /^(eslint(?:-env|-enable|-disable(?:(?:-next)?-line)?)?|exported|globals?)(?:\s|$)/u
const LINE_COMMENT_PATTERN = /^eslint-disable-(next-)?line$/u

module.exports = {
/**
* Make the location ignoring `eslint-disable` comments.
Expand Down Expand Up @@ -95,4 +98,55 @@ module.exports = {
lte(a, b) {
return a.line < b.line || (a.line === b.line && a.column <= b.column)
},

/**
* Parse the given comment token as a directive comment.
*
* @param {Token} comment - The comment token to parse.
* @returns {{kind: string, value: string, description: string | null}|null} The parsed data of the given comment. If `null`, it is not a directive comment.
*/
parseDirectiveComment(comment) {
const { text, description } = divideDirectiveComment(comment.value)
const match = DIRECTIVE_PATTERN.exec(text)

if (!match) {
return null
}
const directiveText = match[1]
const lineCommentSupported = LINE_COMMENT_PATTERN.test(directiveText)

if (comment.type === "Line" && !lineCommentSupported) {
return null
}

if (
lineCommentSupported &&
comment.loc.start.line !== comment.loc.end.line
) {
// disable-line comment should not span multiple lines.
return null
}

const directiveValue = text.slice(match.index + directiveText.length)

return {
kind: directiveText,
value: directiveValue.trim(),
description,
}
},
}

/**
* Divides and trims description text and directive comments.
* @param {string} value The comment text to strip.
* @returns {{text: string, description: string | null}} The stripped text.
*/
function divideDirectiveComment(value) {
const divided = value.split(/\s-{2,}\s/u)
const text = divided[0].trim()
return {
text,
description: divided.length > 1 ? divided[1].trim() : null,
}
}
24 changes: 14 additions & 10 deletions lib/rules/no-unlimited-disable.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,6 @@

const utils = require("../internal/utils")

const PATTERNS = {
Block: /^\s*(eslint-disable)\s*(\S)?/u,
Line: /^\s*(eslint-disable(?:-next)?-line)\s*(\S)?/u,
}

module.exports = {
meta: {
docs: {
Expand All @@ -32,18 +27,27 @@ module.exports = {
return {
Program() {
for (const comment of sourceCode.getAllComments()) {
const pattern = PATTERNS[comment.type]
if (pattern == null) {
const directiveComment = utils.parseDirectiveComment(
comment
)
if (directiveComment == null) {
continue
}

const m = pattern.exec(comment.value)
if (m && !m[2]) {
const kind = directiveComment.kind
if (
kind !== "eslint-disable" &&
kind !== "eslint-disable-line" &&
kind !== "eslint-disable-next-line"
) {
continue
}
if (!directiveComment.value) {
context.report({
loc: utils.toForceLocation(comment.loc),
message:
"Unexpected unlimited '{{kind}}' comment. Specify some rule names to disable.",
data: { kind: m[1] },
data: { kind: directiveComment.kind },
})
}
}
Expand Down
13 changes: 5 additions & 8 deletions lib/rules/no-use.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@
"use strict"

const utils = require("../internal/utils")
const PATTERNS = {
Block: /^\s*(eslint(?:-disable|-enable|-env)?|exported|globals?)(?:\s|$)/u,
Line: /^\s*(eslint-disable(?:-next)?-line)(?:\s|$)/u,
}

module.exports = {
meta: {
Expand Down Expand Up @@ -58,13 +54,14 @@ module.exports = {
return {
Program() {
for (const comment of sourceCode.getAllComments()) {
const pattern = PATTERNS[comment.type]
if (pattern == null) {
const directiveComment = utils.parseDirectiveComment(
comment
)
if (directiveComment == null) {
continue
}

const m = pattern.exec(comment.value)
if (m != null && !allowed.has(m[1])) {
if (!allowed.has(directiveComment.kind)) {
context.report({
loc: utils.toForceLocation(comment.loc),
message: "Unexpected ESLint directive comment.",
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"nyc": "^14.1.1",
"opener": "^1.4.3",
"rimraf": "^2.6.2",
"semver": "^7.3.2",
"string-replace-loader": "^2.1.1",
"vue-eslint-editor": "^1.1.0",
"vuepress": "^1.0.1"
Expand Down
98 changes: 98 additions & 0 deletions tests/lib/illegal-eslint-disable-line.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/**
* Test that multi-line eslint-disable-line comments are not false positives.
*/
"use strict"

const assert = require("assert")
const fs = require("fs")
const path = require("path")
const spawn = require("cross-spawn")
const rimraf = require("rimraf")
const semver = require("semver")
const eslintVersion = require("eslint/package").version

/**
* Run eslint CLI command with a given source code.
* @param {string} code The source code to lint.
* @returns {Promise<Message[]>} The result message.
*/
function runESLint(code) {
return new Promise((resolve, reject) => {
const cp = spawn(
"eslint",
[
"--stdin",
"--stdin-filename",
"test.js",
"--no-eslintrc",
"--plugin",
"eslint-comments",
"--format",
"json",
],
{ stdio: ["pipe", "pipe", "inherit"] }
)
const chunks = []
let totalLength = 0

cp.stdout.on("data", chunk => {
chunks.push(chunk)
totalLength += chunk.length
})
cp.stdout.on("end", () => {
try {
const resultsStr = String(Buffer.concat(chunks, totalLength))
const results = JSON.parse(resultsStr)
resolve(results[0].messages)
} catch (error) {
reject(error)
}
})
cp.on("error", reject)

cp.stdin.end(code)
})
}

describe("multi-line eslint-disable-line comments", () => {
before(() => {
// Register this plugin.
const selfPath = path.resolve(__dirname, "../../")
const pluginPath = path.resolve(
__dirname,
"../../node_modules/eslint-plugin-eslint-comments"
)

if (fs.existsSync(pluginPath)) {
rimraf.sync(pluginPath)
}
fs.symlinkSync(selfPath, pluginPath, "junction")
})

describe("`eslint-comments/*` rules are valid", () => {
for (const code of [
`/* eslint eslint-comments/no-use:[error, {allow: ['eslint']}] */
/* eslint-disable-line
*/
/* eslint-disable-next-line
*/`,
`/* eslint eslint-comments/no-duplicate-disable:error */
/*eslint-disable no-undef*/
/*eslint-disable-line
no-undef*/
`,
]) {
it(code, () =>
runESLint(code).then(messages => {
if (semver.satisfies(eslintVersion, ">=5.0.0")) {
assert.strictEqual(messages.length > 0, true)
}
const normalMessages = messages.filter(
message => message.ruleId != null
)
assert.strictEqual(normalMessages.length, 0)
})
)
}
})
})
40 changes: 40 additions & 0 deletions tests/lib/rules/disable-enable-pair.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
"use strict"

const semver = require("semver")
const eslintVersion = require("eslint/package").version
const RuleTester = require("eslint").RuleTester
const rule = require("../../../lib/rules/disable-enable-pair")
const tester = new RuleTester()
Expand All @@ -24,6 +26,8 @@ tester.run("disable-enable-pair", rule, {
`,
"//eslint-disable-line",
"//eslint-disable-next-line",
"/*eslint-disable-line*/",
"/*eslint-disable-next-line*/",
"/*eslint no-undef: off */",
`
function foo() {
Expand Down Expand Up @@ -76,6 +80,19 @@ var foo = 1
`,
options: [{ allowWholeFile: true }],
},
// -- description
...(semver.satisfies(eslintVersion, ">=7.0.0 || <6.0.0")
? [
`
/*eslint-disable no-undef -- description*/
/*eslint-enable no-undef*/
`,
`
/*eslint-disable no-undef,no-unused-vars -- description*/
/*eslint-enable no-undef,no-unused-vars*/
`,
]
: []),
],
invalid: [
{
Expand Down Expand Up @@ -193,5 +210,28 @@ console.log();
},
],
},
// -- description
...(semver.satisfies(eslintVersion, ">=7.0.0 || <6.0.0")
? [
{
code: `
{
/*eslint-disable no-unused-vars -- description */
}
`,
options: [{ allowWholeFile: true }],
errors: [
{
message:
"Requires 'eslint-enable' directive for 'no-unused-vars'.",
line: 3,
column: 18,
endLine: 3,
endColumn: 32,
},
],
},
]
: []),
],
})
Loading

0 comments on commit 027e20c

Please sign in to comment.