Skip to content

Commit

Permalink
fix: constructor-super false positives with loops (#18226)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mdjermanovic
mdjermanovic committed Mar 25, 2024
1 parent de40874 commit dadc5bf
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 94 deletions.
155 changes: 62 additions & 93 deletions lib/rules/constructor-super.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,6 @@
// Helpers
//------------------------------------------------------------------------------

/**
* 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;
}

/**
* Checks whether or not a given node is a constructor.
* @param {ASTNode} node A node to check. This node type is one of
Expand Down Expand Up @@ -165,8 +149,7 @@ module.exports = {
missingAll: "Expected to call 'super()'.",

duplicate: "Unexpected duplicate 'super()'.",
badSuper: "Unexpected 'super()' because 'super' is not a constructor.",
unexpected: "Unexpected 'super()'."
badSuper: "Unexpected 'super()' because 'super' is not a constructor."
}
},

Expand All @@ -186,15 +169,15 @@ module.exports = {
/**
* @type {Record<string, SegmentInfo>}
*/
let segInfoMap = Object.create(null);
const segInfoMap = Object.create(null);

/**
* Gets the flag which shows `super()` is called in some paths.
* @param {CodePathSegment} segment A code path segment to get.
* @returns {boolean} The flag which shows `super()` is called in some paths
*/
function isCalledInSomePath(segment) {
return segment.reachable && segInfoMap[segment.id]?.calledInSomePaths;
return segment.reachable && segInfoMap[segment.id].calledInSomePaths;
}

/**
Expand All @@ -212,17 +195,6 @@ module.exports = {
* @returns {boolean} The flag which shows `super()` is called in all paths.
*/
function isCalledInEveryPath(segment) {

/*
* If specific segment is the looped segment of the current segment,
* skip the segment.
* If not skipped, this never becomes true after a loop.
*/
if (segment.nextSegments.length === 1 &&
segment.nextSegments[0]?.isLoopedPrevSegment(segment)) {
return true;
}

return segment.reachable && segInfoMap[segment.id].calledInEveryPaths;
}

Expand Down Expand Up @@ -279,9 +251,9 @@ module.exports = {
}

// Reports if `super()` lacked.
const seenSegments = codePath.returnedSegments.filter(hasSegmentBeenSeen);
const calledInEveryPaths = seenSegments.every(isCalledInEveryPath);
const calledInSomePaths = seenSegments.some(isCalledInSomePath);
const returnedSegments = codePath.returnedSegments;
const calledInEveryPaths = returnedSegments.every(isCalledInEveryPath);
const calledInSomePaths = returnedSegments.some(isCalledInSomePath);

if (!calledInEveryPaths) {
context.report({
Expand All @@ -296,28 +268,38 @@ module.exports = {
/**
* Initialize information of a given code path segment.
* @param {CodePathSegment} segment A code path segment to initialize.
* @param {CodePathSegment} node Node that starts the segment.
* @returns {void}
*/
onCodePathSegmentStart(segment) {
onCodePathSegmentStart(segment, node) {

funcInfo.currentSegments.add(segment);

if (!(funcInfo && funcInfo.isConstructor && funcInfo.hasExtends)) {
if (!(funcInfo.isConstructor && funcInfo.hasExtends)) {
return;
}

// Initialize info.
const info = segInfoMap[segment.id] = new SegmentInfo();

// When there are previous segments, aggregates these.
const prevSegments = segment.prevSegments;

if (prevSegments.length > 0) {
const seenPrevSegments = prevSegments.filter(hasSegmentBeenSeen);
const seenPrevSegments = segment.prevSegments.filter(hasSegmentBeenSeen);

// When there are previous segments, aggregates these.
if (seenPrevSegments.length > 0) {
info.calledInSomePaths = seenPrevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = seenPrevSegments.every(isCalledInEveryPath);
}

/*
* ForStatement > *.update segments are a special case as they are created in advance,
* without seen previous segments. Since they logically don't affect `calledInEveryPaths`
* calculations, and they can never be a lone previous segment of another one, we'll set
* their `calledInEveryPaths` to `true` to effectively ignore them in those calculations.
* .
*/
if (node.parent && node.parent.type === "ForStatement" && node.parent.update === node) {
info.calledInEveryPaths = true;
}
},

onUnreachableCodePathSegmentStart(segment) {
Expand All @@ -343,25 +325,30 @@ module.exports = {
* @returns {void}
*/
onCodePathSegmentLoop(fromSegment, toSegment) {
if (!(funcInfo && funcInfo.isConstructor && funcInfo.hasExtends)) {
if (!(funcInfo.isConstructor && funcInfo.hasExtends)) {
return;
}

// Update information inside of the loop.
const isRealLoop = toSegment.prevSegments.length >= 2;

funcInfo.codePath.traverseSegments(
{ first: toSegment, last: fromSegment },
segment => {
const info = segInfoMap[segment.id] ?? new SegmentInfo();
(segment, controller) => {
const info = segInfoMap[segment.id];

// skip segments after the loop
if (!info) {
controller.skip();
return;
}

const seenPrevSegments = segment.prevSegments.filter(hasSegmentBeenSeen);
const calledInSomePreviousPaths = seenPrevSegments.some(isCalledInSomePath);
const calledInEveryPreviousPaths = seenPrevSegments.every(isCalledInEveryPath);

// Updates flags.
info.calledInSomePaths = seenPrevSegments.some(isCalledInSomePath);
info.calledInEveryPaths = seenPrevSegments.every(isCalledInEveryPath);
info.calledInSomePaths ||= calledInSomePreviousPaths;
info.calledInEveryPaths ||= calledInEveryPreviousPaths;

// If flags become true anew, reports the valid nodes.
if (info.calledInSomePaths || isRealLoop) {
if (calledInSomePreviousPaths) {
const nodes = info.validNodes;

info.validNodes = [];
Expand All @@ -375,9 +362,6 @@ module.exports = {
});
}
}

// save just in case we created a new SegmentInfo object
segInfoMap[segment.id] = info;
}
);
},
Expand All @@ -388,7 +372,7 @@ module.exports = {
* @returns {void}
*/
"CallExpression:exit"(node) {
if (!(funcInfo && funcInfo.isConstructor)) {
if (!(funcInfo.isConstructor && funcInfo.hasExtends)) {
return;
}

Expand All @@ -398,41 +382,34 @@ module.exports = {
}

// Reports if needed.
if (funcInfo.hasExtends) {
const segments = funcInfo.currentSegments;
let duplicate = false;
let info = null;
const segments = funcInfo.currentSegments;
let duplicate = false;
let info = null;

for (const segment of segments) {
for (const segment of segments) {

if (segment.reachable) {
info = segInfoMap[segment.id];
if (segment.reachable) {
info = segInfoMap[segment.id];

duplicate = duplicate || info.calledInSomePaths;
info.calledInSomePaths = info.calledInEveryPaths = true;
}
duplicate = duplicate || info.calledInSomePaths;
info.calledInSomePaths = info.calledInEveryPaths = true;
}
}

if (info) {
if (duplicate) {
context.report({
messageId: "duplicate",
node
});
} else if (!funcInfo.superIsConstructor) {
context.report({
messageId: "badSuper",
node
});
} else {
info.validNodes.push(node);
}
if (info) {
if (duplicate) {
context.report({
messageId: "duplicate",
node
});
} else if (!funcInfo.superIsConstructor) {
context.report({
messageId: "badSuper",
node
});
} else {
info.validNodes.push(node);
}
} else if (isAnySegmentReachable(funcInfo.currentSegments)) {
context.report({
messageId: "unexpected",
node
});
}
},

Expand All @@ -442,7 +419,7 @@ module.exports = {
* @returns {void}
*/
ReturnStatement(node) {
if (!(funcInfo && funcInfo.isConstructor && funcInfo.hasExtends)) {
if (!(funcInfo.isConstructor && funcInfo.hasExtends)) {
return;
}

Expand All @@ -462,14 +439,6 @@ module.exports = {
info.calledInSomePaths = info.calledInEveryPaths = true;
}
}
},

/**
* Resets state.
* @returns {void}
*/
"Program:exit"() {
segInfoMap = Object.create(null);
}
};
}
Expand Down

0 comments on commit dadc5bf

Please sign in to comment.