Bug: Block comments should be placed above variable assignment, not declaration (Fix JSDoc etc)

[phil294]

Input Code

For example, if you want to output usable JSDoc to integrate with TypeScript:

###*
# @param a {string}
###
method1 = (a) ->
 
###*
# @param b {string}
###
method2 = (b) ->

Expected Behavior

var method1, method2;

/**
 * @param a {string}
 */
method1 = function(a) {};

/**
 * @param b {string}
 */
method2 = function(b) {};

Current Behavior

/**
 * @param a {string}
 */
/**
 * @param b {string}
 */
var method1, method2;

method1 = function(a) {};

method2 = function(b) {};

Possible Solution

Possible current workaround (pretty ugly):

method1 = method2 = null

#
###*
# @param a {string}
###
method1 = (a) ->
 
#
###*
# @param b {string}
###
method2 = (b) ->

Alternatively, you can of course do inline typed params

method1 = (###* @type string ### b) ->

but this is not always a feasible solution of course

Context

Besides, should we maybe update the docs to state the possiblity of type-checking via JSDoc+TS? I know there is a TypeScript discussion issue going on in #5307 but this jsdoc thing is already possible.

• How has this issue affected you? What are you trying to accomplish? *
  I am currently exploring building a basic CS LSP implementation based on piping CS compiler output to TSC, bundled in a VSCode extension. It works pretty well so far, I'll post in the other thread soon (edit: POC / WIP here

[GeoffreyBooth]

Yes, the block comments shouldn’t be hoisted with the var declarations. That’s a useless place for the comments to be output, and as you point out, it breaks JSDoc.

The place to look to understand comments is #4572, in particular #4572 (comment), #4572 (comment), #4572 (comment), #4572 (comment). It’s a long thread, but those comments should give you a general idea of how comments work in the compiler. It’s a bit of a hack because comments aren’t part of JavaScript syntax itself (they’re not nodes in the abstract syntax tree); but this is the same approach that Babel takes, so it’s pretty solid.

It’s probably either in scope.litcoffee or in nodes.coffee Assign that we create the special var line and move it to the top of the function scope. To fix this, you need to find where an Assign node gets split into two nodes, the declaration var line and the assignment line, and make sure any comments attached to the original node get moved onto the assignment node (and not the declaration node, which is where they must be going now hence this issue).

[phil294]

@robert-boulanger found a similar issue with comment blocks above fat arrow class methods: phil294/coffeesense#1 (comment)

[edemaine]

I think #5395 could help with the original issue here. Currently, that PR doesn't push the var declaration down to first assignment if there's an attached comment (so that the comment doesn't get lost). But it could instead move the comment to the pushed declaration.

I assume it's better to leave #5395 as is, so it's easier to review, and I can work on this after it gets merged. If you'd rather me work on it first, as a proof of concept that pushing var down is useful, let me know.

[phil294]

Possible current workaround (pretty ugly):

#
###*
# @param a {string}
###
method1 = (a) ->

Side note: I found another, slightly shorter workaround that does not require an extra line:

``###*
# @param a {string}
###
method1 = (a) ->

Edit: I stand corrected, it does not always work. Example:

a =
  b: 1
  ``### abc ###
  c: 2

-> error: unexpected newline

[phil294]

Yes, the block comments shouldn’t be hoisted with the var declarations. That’s a useless place for the comments to be output, and as you point out, it breaks JSDoc.

I'm not convinced anymore this is the case, because sometimes, you need to declare and type a var before assigning it inside some callback:

###* @type {number} ###
var x
cb = =>
  x = 2
  null
cb()
console.log(x)

not sure how this would be solved. Sure, instead of var x you could write x = 1 but that is different logic, and not feasible when your type is not number but perhaps a complicated third party interface.

So there's two problems here:

• Most of the time, comment blocks need to be positioned before assignments
• To deliberately declare a variable at a higher function level than it is used, you sometimes the comment block at the declaration level again, in which case it would also be necessary to somehow define the declaration manually with var.

It would be questionable to re-introduce the var keyword just for this, but it's a noteworthy limitation on the usage of JSDoc inside CoffeeScript. A workaround with

`var x`

wouldn't work because cb ships its own var x which overrules the parent one.

The proper solution for the programmer is to make cb return the value of x and assigning it outside of cb.

TL;DR: This is not a showstopper and the logic should still be changed as planned

[GeoffreyBooth]

sometimes, you need to declare and type a var before assigning it inside some callback

CoffeeScript doesn't allow variable declarations, only assignments. In your code where you have var x you would instead write x = undefined, which is syntactically identical and is an assignment. The complier generates the declaration farther up, at the top of the function scope. We want the comment output at this assignment, not at the computer's generated declaration.

[phil294]

x = undefined

Sure! But then you'll have to mark x as @type {number | undefined} and the effective type changes.

In contrast, consider this JS code, perhaps a better example:

/** @type {number} */
var x
var cb = () => {
    x.toFixed()
};
x = 3
cb();

no type error, and runs successfully. Not possible with var x = undefined.

Not great code though, I understand that. You'd risk exposing yourself to some used x before being defined at some point if you mess up assignment order.

[GeoffreyBooth]

As I understand it, you don’t need the | undefined part for JSDoc:

/** @type {number} */
var x;
x = undefined;
x = 3;

This doesn’t error in the TypeScript playground: https://www.typescriptlang.org/play?#code/PQKhAIAEBcE8AcCm4DeA7ArgWwEaIE4C+4IwAUAG4CG+4AHgNxl3gC84GaAJogGYCWaRFyYt2AZiZA

This does show that we need the JSDoc output above the var declaration, though, not above the assignment, in order for TypeScript to detect it. So I guess that’s something to consider.

[phil294]

ah cool. Not with strictNullChecks: true, though. playground link

(and also, you need to set language to JavaScript as JSDoc has no effect inside .ts files)