Skip to content

Brackets and postfix= in @param add undefined#22514

Merged
sandersn merged 2 commits into
masterfrom
param-brackets-add-undefined
Mar 13, 2018
Merged

Brackets and postfix= in @param add undefined#22514
sandersn merged 2 commits into
masterfrom
param-brackets-add-undefined

Conversation

@sandersn
Copy link
Copy Markdown
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 a user and mhegazy March 13, 2018 20:21
@sandersn
Copy link
Copy Markdown
Member Author

This should be ported to 2.8.1 as well.

Comment thread src/compiler/checker.ts Outdated
let isOptional = false;
if (includeOptionality) {
if (isInJavaScriptFile(declaration) && isParameterDeclaration(declaration)) {
const parameterTags = getJSDocParameterTags(declaration as ParameterDeclaration);
Copy link
Copy Markdown

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
Copy Markdown
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.

Comment thread src/compiler/checker.ts Outdated
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
Copy Markdown

@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
Copy Markdown
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
Copy Markdown
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