Skip to content

Commit

Permalink
fix(require-param): do not cache by comment node; fixes #901
Browse files Browse the repository at this point in the history
May target same comment node but with different JS node (e.g., if `any` context is used, we don't want an export declaration's comment to be cached (but not used) and then this prevent the export's `FunctionDeclaration` from being evaluated
  • Loading branch information
brettz9 committed Oct 23, 2022
1 parent 4fabdd6 commit 867edc3
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 27 deletions.
9 changes: 1 addition & 8 deletions src/iterateJsdoc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
getReducedASTNode,
getJSDocComment,
commentHandler,
parseComment,
Expand Down Expand Up @@ -1147,14 +1146,8 @@ const iterateAllJsdocs = (iterator, ruleConfig, contexts, additiveCommentContext

return {
'*:not(Program)' (node) {
const reducedNode = getReducedASTNode(node, sourceCode);

if (node !== reducedNode) {
return;
}

const commentNode = getJSDocComment(sourceCode, node, settings);
if (trackedJsdocs.has(commentNode)) {
if (!ruleConfig.noTracking && trackedJsdocs.has(commentNode)) {
return;
}

Expand Down
47 changes: 28 additions & 19 deletions src/rules/requireParam.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,6 @@ export default iterateJsdoc(({
utils,
context,
}) => {
const preferredTagName = utils.getPreferredTagName({
tagName: 'param',
});
if (!preferredTagName) {
return;
}

const jsdocParameterNames = utils.getJsdocTagsDeep(preferredTagName);

const shallowJsdocParameterNames = jsdocParameterNames.filter((tag) => {
return !tag.name.includes('.');
}).map((tag, idx) => {
return {
...tag,
idx,
};
});

if (utils.avoidDocs()) {
return;
}
Expand All @@ -73,10 +55,32 @@ export default iterateJsdoc(({
useDefaultObjectProperties = false,
} = context.options[0] || {};

const preferredTagName = utils.getPreferredTagName({
tagName: 'param',
});
if (!preferredTagName) {
return;
}

const functionParameterNames = utils.getFunctionParameterNames(useDefaultObjectProperties);
if (!functionParameterNames.length) {
return;
}

const jsdocParameterNames = utils.getJsdocTagsDeep(preferredTagName);

const shallowJsdocParameterNames = jsdocParameterNames.filter((tag) => {
return !tag.name.includes('.');
}).map((tag, idx) => {
return {
...tag,
idx,
};
});

const checkTypesRegex = utils.getRegexFromString(checkTypesPattern);

const missingTags = [];
const functionParameterNames = utils.getFunctionParameterNames(useDefaultObjectProperties);
const flattenedRoots = utils.flattenRoots(functionParameterNames).names;

const paramIndex = {};
Expand Down Expand Up @@ -486,4 +490,9 @@ export default iterateJsdoc(({
],
type: 'suggestion',
},

// We cannot cache comment nodes as the contexts may recur with the
// same comment node but a different JS node, and we may need the different
// JS node to ensure we iterate its context
noTracking: true,
});
43 changes: 43 additions & 0 deletions test/rules/assertions/requireParam.js
Original file line number Diff line number Diff line change
Expand Up @@ -2408,6 +2408,49 @@ export default {
`,
parser: require.resolve('@typescript-eslint/parser'),
},
{
code: `
/**
* Helper function to warp to a custom stage/level.
*
* @param name The name
* @param firstFloor Optional.
*/
export function setCustomStage(
name: string,
firstFloor = true,
verbose = false,
): void {}
`,
errors: [
{
message: 'Missing JSDoc @param "verbose" declaration.',
},
],
ignoreReadme: true,
options: [
{
contexts: [
'any',
],
},
],
output: `
/**
* Helper function to warp to a custom stage/level.
*
* @param name The name
* @param firstFloor Optional.
* @param verbose
*/
export function setCustomStage(
name: string,
firstFloor = true,
verbose = false,
): void {}
`,
parser: require.resolve('@typescript-eslint/parser'),
},
],
valid: [
{
Expand Down

0 comments on commit 867edc3

Please sign in to comment.