Skip to content

Commit

Permalink
feat: Implement onUnreachableCodePathStart/End (#17511)
Browse files Browse the repository at this point in the history
* feat: Implement onUnreachableCodePathStart/End

refs # 17457

* Finish up onUnreachable* work

* Refactor to account for out-of-order events

* Update lib/rules/no-unreachable.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/rules/no-unreachable.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update tests/lib/linter/code-path-analysis/code-path-analyzer.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Incorporate feedback

* Clean up rules and docs

* Update docs

* Fix code example

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/rules/consistent-return.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update lib/rules/no-this-before-super.js

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Fix examples

* Add deprecation notices to RuleTester/FlatRuleTester

* Update config

* Add deprecation notices to RuleTester/FlatRuleTester

* Fix lint warning

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Update docs/src/extend/code-path-analysis.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Fix test

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
nzakas and mdjermanovic authored Sep 7, 2023
1 parent de86b3b commit da09f4e
Show file tree
Hide file tree
Showing 20 changed files with 990 additions and 287 deletions.
482 changes: 320 additions & 162 deletions docs/src/extend/code-path-analysis.md

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion eslint.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,6 @@ module.exports = [
files: [INTERNAL_FILES.RULE_TESTER_PATTERN],
rules: {
"n/no-restricted-require": ["error", [
...createInternalFilesPatterns(INTERNAL_FILES.RULE_TESTER_PATTERN),
resolveAbsolutePath("lib/cli-engine/index.js")
]]
}
Expand Down
56 changes: 32 additions & 24 deletions lib/linter/code-path-analysis/code-path-analyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -192,15 +192,18 @@ function forwardCurrentToHead(analyzer, node) {
headSegment = headSegments[i];

if (currentSegment !== headSegment && currentSegment) {
debug.dump(`onCodePathSegmentEnd ${currentSegment.id}`);

if (currentSegment.reachable) {
analyzer.emitter.emit(
"onCodePathSegmentEnd",
currentSegment,
node
);
}
const eventName = currentSegment.reachable
? "onCodePathSegmentEnd"
: "onUnreachableCodePathSegmentEnd";

debug.dump(`${eventName} ${currentSegment.id}`);

analyzer.emitter.emit(
eventName,
currentSegment,
node
);
}
}

Expand All @@ -213,16 +216,19 @@ function forwardCurrentToHead(analyzer, node) {
headSegment = headSegments[i];

if (currentSegment !== headSegment && headSegment) {
debug.dump(`onCodePathSegmentStart ${headSegment.id}`);

const eventName = headSegment.reachable
? "onCodePathSegmentStart"
: "onUnreachableCodePathSegmentStart";

debug.dump(`${eventName} ${headSegment.id}`);

CodePathSegment.markUsed(headSegment);
if (headSegment.reachable) {
analyzer.emitter.emit(
"onCodePathSegmentStart",
headSegment,
node
);
}
analyzer.emitter.emit(
eventName,
headSegment,
node
);
}
}

Expand All @@ -241,15 +247,17 @@ function leaveFromCurrentSegment(analyzer, node) {

for (let i = 0; i < currentSegments.length; ++i) {
const currentSegment = currentSegments[i];
const eventName = currentSegment.reachable
? "onCodePathSegmentEnd"
: "onUnreachableCodePathSegmentEnd";

debug.dump(`onCodePathSegmentEnd ${currentSegment.id}`);
if (currentSegment.reachable) {
analyzer.emitter.emit(
"onCodePathSegmentEnd",
currentSegment,
node
);
}
debug.dump(`${eventName} ${currentSegment.id}`);

analyzer.emitter.emit(
eventName,
currentSegment,
node
);
}

state.currentSegments = [];
Expand Down
1 change: 1 addition & 0 deletions lib/linter/code-path-analysis/code-path.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ class CodePath {
/**
* Current code path segments.
* @type {CodePathSegment[]}
* @deprecated
*/
get currentSegments() {
return this.internal.currentSegments;
Expand Down
1 change: 1 addition & 0 deletions lib/linter/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,7 @@ const DEPRECATED_SOURCECODE_PASSTHROUGHS = {
getTokensBetween: "getTokensBetween"
};


const BASE_TRAVERSAL_CONTEXT = Object.freeze(
Object.keys(DEPRECATED_SOURCECODE_PASSTHROUGHS).reduce(
(contextInfo, methodName) =>
Expand Down
29 changes: 28 additions & 1 deletion lib/rule-tester/flat-rule-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ const
equal = require("fast-deep-equal"),
Traverser = require("../shared/traverser"),
{ getRuleOptionsSchema } = require("../config/flat-config-helpers"),
{ Linter, SourceCodeFixer, interpolate } = require("../linter");
{ Linter, SourceCodeFixer, interpolate } = require("../linter"),
CodePath = require("../linter/code-path-analysis/code-path");

const { FlatConfigArray } = require("../config/flat-config-array");
const { defaultConfig } = require("../config/default-config");

Expand Down Expand Up @@ -274,6 +276,21 @@ function getCommentsDeprecation() {
);
}

/**
* Emit a deprecation warning if rule uses CodePath#currentSegments.
* @param {string} ruleName Name of the rule.
* @returns {void}
*/
function emitCodePathCurrentSegmentsWarning(ruleName) {
if (!emitCodePathCurrentSegmentsWarning[`warned-${ruleName}`]) {
emitCodePathCurrentSegmentsWarning[`warned-${ruleName}`] = true;
process.emitWarning(
`"${ruleName}" rule uses CodePath#currentSegments and will stop working in ESLint v9. Please read the documentation for how to update your code: https://eslint.org/docs/latest/extend/code-path-analysis#usage-examples`,
"DeprecationWarning"
);
}
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -664,6 +681,7 @@ class FlatRuleTester {

// Verify the code.
const { getComments } = SourceCode.prototype;
const originalCurrentSegments = Object.getOwnPropertyDescriptor(CodePath.prototype, "currentSegments");
let messages;

// check for validation errors
Expand All @@ -677,11 +695,20 @@ class FlatRuleTester {

try {
SourceCode.prototype.getComments = getCommentsDeprecation;
Object.defineProperty(CodePath.prototype, "currentSegments", {
get() {
emitCodePathCurrentSegmentsWarning(ruleName);
return originalCurrentSegments.get.call(this);
}
});

messages = linter.verify(code, configs, filename);
} finally {
SourceCode.prototype.getComments = getComments;
Object.defineProperty(CodePath.prototype, "currentSegments", originalCurrentSegments);
}


const fatalErrorMessage = messages.find(m => m.fatal);

assert(!fatalErrorMessage, `A fatal parsing error occurred: ${fatalErrorMessage && fatalErrorMessage.message}`);
Expand Down
27 changes: 26 additions & 1 deletion lib/rule-tester/rule-tester.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ const
equal = require("fast-deep-equal"),
Traverser = require("../../lib/shared/traverser"),
{ getRuleOptionsSchema, validate } = require("../shared/config-validator"),
{ Linter, SourceCodeFixer, interpolate } = require("../linter");
{ Linter, SourceCodeFixer, interpolate } = require("../linter"),
CodePath = require("../linter/code-path-analysis/code-path");

const ajv = require("../shared/ajv")({ strictDefaults: true });

Expand Down Expand Up @@ -375,6 +376,21 @@ function emitDeprecatedContextMethodWarning(ruleName, methodName) {
}
}

/**
* Emit a deprecation warning if rule uses CodePath#currentSegments.
* @param {string} ruleName Name of the rule.
* @returns {void}
*/
function emitCodePathCurrentSegmentsWarning(ruleName) {
if (!emitCodePathCurrentSegmentsWarning[`warned-${ruleName}`]) {
emitCodePathCurrentSegmentsWarning[`warned-${ruleName}`] = true;
process.emitWarning(
`"${ruleName}" rule uses CodePath#currentSegments and will stop working in ESLint v9. Please read the documentation for how to update your code: https://eslint.org/docs/latest/extend/code-path-analysis#usage-examples`,
"DeprecationWarning"
);
}
}

//------------------------------------------------------------------------------
// Public Interface
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -746,13 +762,22 @@ class RuleTester {

// Verify the code.
const { getComments } = SourceCode.prototype;
const originalCurrentSegments = Object.getOwnPropertyDescriptor(CodePath.prototype, "currentSegments");
let messages;

try {
SourceCode.prototype.getComments = getCommentsDeprecation;
Object.defineProperty(CodePath.prototype, "currentSegments", {
get() {
emitCodePathCurrentSegmentsWarning(ruleName);
return originalCurrentSegments.get.call(this);
}
});

messages = linter.verify(code, config, filename);
} finally {
SourceCode.prototype.getComments = getComments;
Object.defineProperty(CodePath.prototype, "currentSegments", originalCurrentSegments);
}

const fatalErrorMessage = messages.find(m => m.fatal);
Expand Down
47 changes: 36 additions & 11 deletions lib/rules/array-callback-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,6 @@ const astUtils = require("./utils/ast-utils");
const TARGET_NODE_TYPE = /^(?:Arrow)?FunctionExpression$/u;
const TARGET_METHODS = /^(?:every|filter|find(?:Last)?(?:Index)?|flatMap|forEach|map|reduce(?:Right)?|some|sort|toSorted)$/u;

/**
* Checks a given code path segment is reachable.
* @param {CodePathSegment} segment A segment to check.
* @returns {boolean} `true` if the segment is reachable.
*/
function isReachable(segment) {
return segment.reachable;
}

/**
* Checks a given node is a member access which has the specified name's
* property.
Expand All @@ -38,6 +29,22 @@ function isTargetMethod(node) {
return astUtils.isSpecificMemberAccess(node, null, TARGET_METHODS);
}

/**
* Checks all segments in a set and returns true if any are reachable.
* @param {Set<CodePathSegment>} segments The segments to check.
* @returns {boolean} True if any segment is reachable; false otherwise.
*/
function isAnySegmentReachable(segments) {

for (const segment of segments) {
if (segment.reachable) {
return true;
}
}

return false;
}

/**
* Returns a human-legible description of an array method
* @param {string} arrayMethodName A method name to fully qualify
Expand Down Expand Up @@ -205,7 +212,7 @@ module.exports = {
messageId = "expectedNoReturnValue";
}
} else {
if (node.body.type === "BlockStatement" && funcInfo.codePath.currentSegments.some(isReachable)) {
if (node.body.type === "BlockStatement" && isAnySegmentReachable(funcInfo.currentSegments)) {
messageId = funcInfo.hasReturn ? "expectedAtEnd" : "expectedInside";
}
}
Expand Down Expand Up @@ -242,7 +249,8 @@ module.exports = {
methodName &&
!node.async &&
!node.generator,
node
node,
currentSegments: new Set()
};
},

Expand All @@ -251,6 +259,23 @@ module.exports = {
funcInfo = funcInfo.upper;
},

onUnreachableCodePathSegmentStart(segment) {
funcInfo.currentSegments.add(segment);
},

onUnreachableCodePathSegmentEnd(segment) {
funcInfo.currentSegments.delete(segment);
},

onCodePathSegmentStart(segment) {
funcInfo.currentSegments.add(segment);
},

onCodePathSegmentEnd(segment) {
funcInfo.currentSegments.delete(segment);
},


// Checks the return statement is valid.
ReturnStatement(node) {

Expand Down
39 changes: 32 additions & 7 deletions lib/rules/consistent-return.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,19 @@ const { upperCaseFirst } = require("../shared/string-utils");
//------------------------------------------------------------------------------

/**
* Checks whether or not a given code path segment is unreachable.
* @param {CodePathSegment} segment A CodePathSegment to check.
* @returns {boolean} `true` if the segment is unreachable.
* Checks all segments in a set and returns true if all are unreachable.
* @param {Set<CodePathSegment>} segments The segments to check.
* @returns {boolean} True if all segments are unreachable; false otherwise.
*/
function isUnreachable(segment) {
return !segment.reachable;
function areAllSegmentsUnreachable(segments) {

for (const segment of segments) {
if (segment.reachable) {
return false;
}
}

return true;
}

/**
Expand Down Expand Up @@ -88,7 +95,7 @@ module.exports = {
* When unreachable, all paths are returned or thrown.
*/
if (!funcInfo.hasReturnValue ||
funcInfo.codePath.currentSegments.every(isUnreachable) ||
areAllSegmentsUnreachable(funcInfo.currentSegments) ||
astUtils.isES5Constructor(node) ||
isClassConstructor(node)
) {
Expand Down Expand Up @@ -141,13 +148,31 @@ module.exports = {
hasReturn: false,
hasReturnValue: false,
messageId: "",
node
node,
currentSegments: new Set()
};
},
onCodePathEnd() {
funcInfo = funcInfo.upper;
},

onUnreachableCodePathSegmentStart(segment) {
funcInfo.currentSegments.add(segment);
},

onUnreachableCodePathSegmentEnd(segment) {
funcInfo.currentSegments.delete(segment);
},

onCodePathSegmentStart(segment) {
funcInfo.currentSegments.add(segment);
},

onCodePathSegmentEnd(segment) {
funcInfo.currentSegments.delete(segment);
},


// Reports a given return statement if it's inconsistent.
ReturnStatement(node) {
const argument = node.argument;
Expand Down
Loading

0 comments on commit da09f4e

Please sign in to comment.