typescript-go icon indicating copy to clipboard operation
typescript-go copied to clipboard

Type nodes for JSDoc

Open sandersn opened this issue 6 months ago • 0 comments

In Javascript, you can give a type and modifiers (for classes) to expandos, exports and object literal properties:

/** @type {number} */
module.exports = 12
const o = {
  /** @type {number} */
  p: 12
}
/** @type {number} */
f.p = 12
class C {
  constructor() {
    /** 
     * @private
     * @type {number}
     */
    this.p = 12
  }
}

Alternate PR name: Types where they shouldn't be.

image

This PR adds Type nodes to nodes that can only get types from jsdoc. Then it checks those types, replacing the previous hack of reparsing a type assertion. In order to make this work on commonjs exports it starts calling withJSDoc on synthetic commonjs nodes. It also (sorry), refactors reparser.go into separate functions, and stops cloning type nodes since we're moving to a no-clone reparsing. I'll remove the other 3 clones later, as part of the PR that corrects synthetic-parent walks for @template.

This is the first infrastructure/fix PR of the ones I'll spend most of June on. I hope they'll get more coherent but this one sprawls a bit.

Nodes with new Type nodes

  • PropertyAssignment
  • ShorthandPropertyAssignment
  • [JS]ExportAssignment
  • CommonJSExport
  • BinaryExpression, which also adds Modifiers to support private on this.p assignments. These additions are going to cost memory, which we agreed on at the design meeting.

Returns do not add type annotations because I think this is a low-value feature, and TS type checking has no equivalent whatsoever.

Notes

To fix contextual typing on commonjs exports, which of course walks up the parent chain, I should also have to introduce a Host property on BinaryExpression, because CommonJSExport is reparsed from BinaryExpression. This was a bridge too far for me, so I cheated: both CommonJSExport and BinaryExpression have the same type annotation, so I let the @type double-attach and have special-case code only in getContextualTypeForBinaryOperand to recognise the BinaryExpression syntax for module.exports.

ShorthandPropertyAssignment type checking was inconsistent in the two places it existed. When I refactored it to a single function, I left the (unannotated) behaviour the same, but it means that the inDestructuringPattern argument seems backwards. We should revisit this because I think the consistent behaviour may be right.

I started removing Clones in reparsing, starting with type nodes. This required me to add a synthetic-parent lookup in 2 more places (the existing one is in nameresolver.go). I expect there are a few more, so I created a utility. In order to nudge us toward making Parent a method, I gave it the name getEffectiveTypeParent. But I'm not sure how hard/how important a Parent method is.

The reparser refactor obscures a few changes (sorry!), but not many. The cases in reparseHosted for PropertyAssignment, etc just change from makeNewTypeAssertion to makeNewType.

sandersn avatar Jun 02 '25 16:06 sandersn