Skip to content

Brackets and postfix= in @param add undefined #22514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 13, 2018

Conversation

sandersn
Copy link
Member

Previously they only added optionality.
Note that, unlike Typescript, when a parameter initializer is specified in jsdoc, it does not remove undefined in the body of the function. That's because TS will generate initialisation code, but JS won't, so the author will have to manually write code to remove undefined from the type.

/** @param {number} [a=101] */
function f(a) {
  // a: number | undefined here
  if (!a) {
    a = 101
  }
  // a: number here
}

Note that we don't check that

  1. the initializer value is actually assigned to the parameter.
  2. the initializer's type matches the declared type of the parameter.

Pretty much we just parse it and leave it alone.

Fixes #22412

Previously they only added optionality.
Note that, unlike Typescript, when a parameter initializer is specified
in jsdoc, it does not remove undefined in the *body* of the function.
That's because TS will generate initialisation code, but JS won't, so
the author will have to manually write code to remove undefined from the
type.

```js
/** @param {number} [a=101] */
function f(a) {
  // a: number | undefined here
  if (!a) {
    a = 101
  }
  // a: number here
}
```

Note that we don't check that
1. the initializer value is actually assigned to the parameter.
2. the initializer's type matches the declared type of the parameter.

Pretty much we just parse it and leave it alone.
@sandersn sandersn requested review from mhegazy and a user March 13, 2018 20:21
@sandersn
Copy link
Member Author

This should be ported to 2.8.1 as well.

let isOptional = false;
if (includeOptionality) {
if (isInJavaScriptFile(declaration) && isParameterDeclaration(declaration)) {
const parameterTags = getJSDocParameterTags(declaration as ParameterDeclaration);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cast not valid, isParameterDeclaration returns true for non-parameters (maybe the name could use an improvement).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, isParameter isn't obvious since it's one of the few types that don't match the name of their syntax kind.

if (includeOptionality) {
if (isInJavaScriptFile(declaration) && isParameterDeclaration(declaration)) {
const parameterTags = getJSDocParameterTags(declaration as ParameterDeclaration);
if (parameterTags && parameterTags.length > 0 && find(parameterTags, tag => tag.isBracketed)) {
Copy link

@ghost ghost Mar 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simpler to write isOptional = ... than if (...) isOptional = true;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so. Doesn't match the next branch, so it makes the code a little less obvious to read.

@sandersn sandersn merged commit 0fa838a into master Mar 13, 2018
sandersn added a commit that referenced this pull request Mar 13, 2018
* Brackets and postfix= in `@param` add undefined

Previously they only added optionality.
Note that, unlike Typescript, when a parameter initializer is specified
in jsdoc, it does not remove undefined in the *body* of the function.
That's because TS will generate initialisation code, but JS won't, so
the author will have to manually write code to remove undefined from the
type.

```js
/** @param {number} [a=101] */
function f(a) {
  // a: number | undefined here
  if (!a) {
    a = 101
  }
  // a: number here
}
```

Note that we don't check that
1. the initializer value is actually assigned to the parameter.
2. the initializer's type matches the declared type of the parameter.

Pretty much we just parse it and leave it alone.

* Address PR comments
@sandersn
Copy link
Member Author

I cherry-picked it into 2.8 too.

@sandersn sandersn deleted the param-brackets-add-undefined branch March 13, 2018 23:01
sandersn added a commit that referenced this pull request Mar 14, 2018
From #22510 and #22514, which remove lots of bogus `@param` errors and
add lots of `Object is possibly undefined` errors, respectively.

There are quite a few effects of the correct addition of undefined to
types based on postfix= in jsdoc, actually.
sandersn added a commit that referenced this pull request Mar 14, 2018
From #22510 and #22514, which remove lots of bogus `@param` errors and
add lots of `Object is possibly undefined` errors, respectively.

There are quite a few effects of the correct addition of undefined to
types based on postfix= in jsdoc, actually.
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant