-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
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
feat: implement rfc 2021-suppression-support #15459
Changes from 8 commits
e80163e
ccf6efa
8f148ee
6eb1260
451b116
f094bee
b05aeca
d904a5c
09d33c2
c5d6219
039b6e3
3d3fdc8
bfe22b1
52f0dea
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,62 +197,99 @@ function processUnusedDisableDirectives(allDirectives) { | |
* for the exported function, except that `reportUnusedDisableDirectives` is not supported | ||
* (this function always reports unused disable directives). | ||
* @returns {{problems: Problem[], unusedDisableDirectives: Problem[]}} An object with a list | ||
* of filtered problems and unused eslint-disable directives | ||
* of problems (including suppressed ones) and unused eslint-disable directives | ||
*/ | ||
Yiwei-Ding marked this conversation as resolved.
Show resolved
Hide resolved
|
||
function applyDirectives(options) { | ||
const problems = []; | ||
let nextDirectiveIndex = 0; | ||
let currentGlobalDisableDirective = null; | ||
const disabledRuleMap = new Map(); | ||
|
||
// enabledRules is only used when there is a current global disable directive. | ||
const enabledRules = new Set(); | ||
const usedDisableDirectives = new Set(); | ||
|
||
for (const problem of options.problems) { | ||
mdjermanovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
let nextDirectiveIndex = 0; | ||
mdjermanovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// isProblemSuppressedBefore indicates whether the problem is suppressed by block directives. | ||
const isProblemSuppressedBefore = problem.suppressions ? true : false; // eslint-disable-line no-unneeded-ternary -- problem.suppressions might be changed afterwards. | ||
|
||
const usedDisableDirectivesForProblem = new Set(); | ||
mdjermanovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
while ( | ||
nextDirectiveIndex < options.directives.length && | ||
compareLocations(options.directives[nextDirectiveIndex], problem) <= 0 | ||
) { | ||
const directive = options.directives[nextDirectiveIndex++]; | ||
const suppression = { kind: "directive", justification: directive.unprocessedDirective.justification }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to add location information to the suppression to show where the directive is? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a good idea. The location info can be leveraged for auditing purposes as well. I also find a location property in But I'd like to add location info to the suppressions in another PR because:
|
||
|
||
switch (directive.type) { | ||
case "disable": | ||
if (directive.ruleId === null) { | ||
currentGlobalDisableDirective = directive; | ||
disabledRuleMap.clear(); | ||
enabledRules.clear(); | ||
} else if (currentGlobalDisableDirective) { | ||
enabledRules.delete(directive.ruleId); | ||
disabledRuleMap.set(directive.ruleId, directive); | ||
} else { | ||
disabledRuleMap.set(directive.ruleId, directive); | ||
if (directive.ruleId === null || directive.ruleId === problem.ruleId) { | ||
Yiwei-Ding marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (problem.suppressions) { | ||
problem.suppressions.push(suppression); | ||
} else { | ||
problem.suppressions = [suppression]; | ||
} | ||
} | ||
|
||
if (!isProblemSuppressedBefore) { | ||
if (directive.ruleId === null) { | ||
currentGlobalDisableDirective = directive; | ||
} else if (directive.ruleId === problem.ruleId && !currentGlobalDisableDirective) { | ||
usedDisableDirectivesForProblem.add(directive); | ||
} | ||
} | ||
break; | ||
|
||
case "enable": | ||
if (directive.ruleId === null) { | ||
currentGlobalDisableDirective = null; | ||
disabledRuleMap.clear(); | ||
} else if (currentGlobalDisableDirective) { | ||
enabledRules.add(directive.ruleId); | ||
disabledRuleMap.delete(directive.ruleId); | ||
} else { | ||
disabledRuleMap.delete(directive.ruleId); | ||
if (directive.ruleId === null || directive.ruleId === problem.ruleId) { | ||
if ( | ||
problem.suppressions && ( | ||
directive.unprocessedDirective.type === "disable-line" || | ||
directive.unprocessedDirective.type === "disable-next-line" | ||
) | ||
) { | ||
problem.suppressions.pop(); | ||
} else { | ||
problem.suppressions = []; | ||
} | ||
} | ||
|
||
if (!isProblemSuppressedBefore) { | ||
if (directive.ruleId === null || currentGlobalDisableDirective) { | ||
currentGlobalDisableDirective = null; | ||
} | ||
if (directive.ruleId === null || directive.ruleId === problem.ruleId) { | ||
usedDisableDirectivesForProblem.forEach(usedDirective => { | ||
if ( | ||
usedDirective.ruleId === problem.ruleId && | ||
(!problem.suppressions || problem.suppressions.length === 0) | ||
) { | ||
usedDisableDirectivesForProblem.delete(usedDirective); | ||
} | ||
}); | ||
} | ||
} | ||
break; | ||
|
||
// no default | ||
} | ||
} | ||
|
||
if (disabledRuleMap.has(problem.ruleId)) { | ||
usedDisableDirectives.add(disabledRuleMap.get(problem.ruleId)); | ||
} else if (currentGlobalDisableDirective && !enabledRules.has(problem.ruleId)) { | ||
usedDisableDirectives.add(currentGlobalDisableDirective); | ||
} else { | ||
problems.push(problem); | ||
if (currentGlobalDisableDirective) { | ||
const usedDisableDirectiveWithRuleId = [...usedDisableDirectivesForProblem].filter(directive => directive.ruleId === problem.ruleId); | ||
|
||
if (usedDisableDirectiveWithRuleId) { | ||
usedDisableDirectivesForProblem.delete(usedDisableDirectiveWithRuleId[0]); | ||
} | ||
usedDisableDirectivesForProblem.add(currentGlobalDisableDirective); | ||
} | ||
usedDisableDirectivesForProblem.forEach(usedDirective => { | ||
usedDisableDirectives.add(usedDirective); | ||
}); | ||
mdjermanovic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (typeof problem.suppressions === "undefined" || problem.suppressions.length === 0) { | ||
delete problem.suppressions; | ||
} | ||
problems.push(problem); | ||
} | ||
|
||
const unusedDisableDirectivesToReport = options.directives | ||
|
@@ -282,22 +319,23 @@ function applyDirectives(options) { | |
|
||
/** | ||
* Given a list of directive comments (i.e. metadata about eslint-disable and eslint-enable comments) and a list | ||
* of reported problems, determines which problems should be reported. | ||
* of reported problems, adds the suppression information to the problems. | ||
* @param {Object} options Information about directives and problems | ||
* @param {{ | ||
* type: ("disable"|"enable"|"disable-line"|"disable-next-line"), | ||
* ruleId: (string|null), | ||
* line: number, | ||
* column: number | ||
* column: number, | ||
* justification: string | ||
* }} options.directives Directive comments found in the file, with one-based columns. | ||
* Two directive comments can only have the same location if they also have the same type (e.g. a single eslint-disable | ||
* comment for two different rules is represented as two directives). | ||
* @param {{ruleId: (string|null), line: number, column: number}[]} options.problems | ||
* A list of problems reported by rules, sorted by increasing location in the file, with one-based columns. | ||
* @param {"off" | "warn" | "error"} options.reportUnusedDisableDirectives If `"warn"` or `"error"`, adds additional problems for unused directives | ||
* @param {boolean} options.disableFixes If true, it doesn't make `fix` properties. | ||
* @returns {{ruleId: (string|null), line: number, column: number}[]} | ||
* A list of reported problems that were not disabled by the directive comments. | ||
* @returns {{ruleId: (string|null), line: number, column: number, suppressions?: {kind: string, justification: string}}[]} | ||
* An object with a list of reported problems, the suppressed of which contain the suppression information. | ||
*/ | ||
module.exports = ({ directives, disableFixes, problems, reportUnusedDisableDirectives = "off" }) => { | ||
const blockDirectives = directives | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This link is broken as there's no
getsuppressedmessages
section in the document.