Skip to content

Commit

Permalink
Fix default param values with implied this param
Browse files Browse the repository at this point in the history
Resolves #2698
  • Loading branch information
Gerrit0 committed Sep 6, 2024
1 parent 43f1dc2 commit 6a48e4c
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- `@link` tags to symbols which are not included in the documentation will produce invalid link warnings again, #2681.
- Fixed handling of `@param` tags on comments attached to function callback parameters, #2683.
- The `alphabetical` and `alphabetical-ignoring-documents` sort options now use `localeCompare` to sort, #2684.
- Fixed incorrect placement of parameter default values in some signatures with a `this` parameter, #2698.

## v0.26.6 (2024-08-18)

Expand Down
15 changes: 13 additions & 2 deletions src/lib/converter/factories/signature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ function convertParameters(
| readonly ts.JSDocParameterTag[]
| undefined,
) {
// #2698 if `satisfies` is used to imply a this parameter, we might have
// more parameters than parameter nodes and need to shift the parameterNode
// access index. Very ugly, but it does the job.
const parameterNodeOffset =
parameterNodes?.length !== parameters.length ? -1 : 0;

return parameters.map((param, i) => {
const declaration = param.valueDeclaration;
assert(
Expand Down Expand Up @@ -254,7 +260,9 @@ function convertParameters(
paramRefl.type = removeUndefined(paramRefl.type);
}

paramRefl.defaultValue = convertDefaultValue(parameterNodes?.[i]);
paramRefl.defaultValue = convertDefaultValue(
parameterNodes?.[i + parameterNodeOffset],
);
paramRefl.setFlag(ReflectionFlag.Optional, isOptional);

// If we have no declaration, then this is an implicitly defined parameter in JS land
Expand All @@ -268,7 +276,10 @@ function convertParameters(
}

paramRefl.setFlag(ReflectionFlag.Rest, isRest);
checkForDestructuredParameterDefaults(paramRefl, parameterNodes?.[i]);
checkForDestructuredParameterDefaults(
paramRefl,
parameterNodes?.[i + parameterNodeOffset],
);
return paramRefl;
});
}
Expand Down
21 changes: 21 additions & 0 deletions src/test/converter2/issues/gh2698.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
type Square = {
color: "red" | "blue" | "green";
};

type TAnimator = {
(
this: Square,
numSpins: number,
direction: "clockwise" | "counterclockwise",
): string;
};

export const animator = function (
numSpins: number = 2,
direction: "clockwise" | "counterclockwise" = "counterclockwise",
) {
console.log(this.color, numSpins, direction);
return "Hello World";
} satisfies TAnimator;

animator.call({ color: "blue" }, 77);
14 changes: 14 additions & 0 deletions src/test/issues.c2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1707,4 +1707,18 @@ describe("Issue Tests", () => {
// had a chance to copy the data's @param to the parameter.
equal(data2.comment, undefined);
});

it("#2698 handles this parameters present in type but not node", () => {
const project = convert();
const animator = querySig(project, "animator");
equal(
animator.parameters?.map((p) => p.name),
["this", "numSpins", "direction"],
);

equal(
animator.parameters.map((p) => p.defaultValue),
[undefined, "2", '"counterclockwise"'],
);
});
});

0 comments on commit 6a48e4c

Please sign in to comment.