Skip to content

Better checking of assignment declarations #28387

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 1 commit into from
Nov 15, 2018

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Nov 6, 2018

Previously, type checking was turned off for all assignment declarations. This is a problem when the declarations are annotated with jsdoc types:

/** @type {string} */
module.exports = 0

As @DanielRosenwasser points out, this will become more common as people use checkJS more and more.

This PR checks assignment declarations, except for expando initialisers. Expando initialisers are

  1. Empty object types.
  2. Function types.
  3. Class types.
  4. Non-empty object types when the assignment declaration kind is
    prototype assignment or module.exports assignment.

It's probably possible to write a reasonable ad-hoc check for expando initalisers too, but they are (I think) least likely to be annotated since it looks weird:

/** @type { x: number, y: string } */
var ns = {}
ns.x = 1
ns.y = 'oh no'

Fixes #27327

Previously, type checking was turned off for all assignment
declarations. This is a problem when the declarations are annotated with
jsdoc types.

This PR checks assignment declarations, *except* for expando
initialisers. Expando initialisers are

1. Empty object types.
2. Function types.
3. Class types.
4. Non-empty object types when the assignment declaration kind is
prototype assignment or module.exports assignment.
@sandersn sandersn requested a review from weswigham November 6, 2018 23:29
@sandersn sandersn merged commit 53bb4e8 into master Nov 15, 2018
@sandersn sandersn deleted the js/check-assignment-decls-better branch November 15, 2018 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants