Skip to content
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

Distinct var declaration proposal: x @= 3 #5477

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

cosmicexplorer
Copy link
Contributor

@cosmicexplorer cosmicexplorer commented Nov 24, 2024

This PR is a +55/-0 (very small!) diff on top of #5475: see the comparison at https://github.com/cosmicexplorer/coffeescript/compare/scope-refactoring...cosmicexplorer:coffeescript:distinct-var-decl?expand=1.

Problem

We would like to be able to attach JSDoc comments to variable declarations for two reasons:

  1. generating type annotations (e.g. .d.ts) for our exported interfaces.
  2. type-checking our code itself.

As discussed in the thread at #5307 (comment), the current mechanism of generating var declarations implicitly and declaring them all at once in a single statement at the top of the function means users are unable to associate a block comment with a specific variable declaration (we currently coalesce all comments in assignments together on top of the single var statement):

> coffee -c -b -s --no-header <<EOF
y = 3
###* @type {number} ###
x = 3
EOF
/** @type {number} */
var x, y;

y = 3;

x = 3;

We want to be able to associate that particular comments with x and x alone.

Solution

With #5475, this becomes much easier:

  • Create a new syntax (bikeshedding: @=) and a new node (Declaration) to separate it from Assign behavior entirely.
  • Ensure @= is only called at the top level of a var scope (either a function scope or the top-level scope).
  • Ensure @= is the first assignment in the scope, and that there is exactly one @= in the scope for a given variable name.

Result

For valid distinct declarations, block comments are correctly assigned to the output so that JSDoc syntax can be observed:

> coffee -c -b -s --no-header <<EOF
y = 3
###* @type {number} ###
x @= 3
EOF
var y;

y = 3;

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

tsc integration prototype

I have added a script build-support/typescript-compile.coffee which performs typechecking against tsc and rewrites error locations using source maps. I'm not sure how reliable this technique is since I don't really know how source maps are supposed to work, but it seems to work in extremely small examples:

# e.g. run these shell commands from the repo root (you'll need to have npm installed typescript):
; cat > test-map.coffee <<EOF
y = 3

###* @type {string} ###
x @= 3
EOF
; coffee build-support/typescript-compile.coffee test-map.coffee
test-map.coffee:4:1 - error TS2322: Type 'number' is not assignable to type 'string'.

4 x @= 3
  ~
# it also generates adjacent .js and .js.map files:
; cat test-map.js
var y;

y = 3;

/** @type {string} */
var x = 3;

//# sourceMappingURL=test-map.js.map
; cat test-map.js.map
{
  "version": 3,
  "file": "test-map.js",
  "sourceRoot": "/home/cosmicexplorer/tools/coffeescript",
  "sources": [
    "test-map.coffee"
  ],
  "names": [],
  "mappings": "AAAA,IAAA;;AAAA,CAAA,GAAI,EAAJ;;;AAGA,IAAA,CAAA,GAAK",
  "sourcesContent": [
    "y = 3\n\n###* @type {string} ###\nx @= 3\n"
  ]
}
# finally, it also generates a .d.ts type definition file:
; cat test-map.d.ts
declare var y: any;
/** @type {string} */
declare var x: string;

- annotate delimiter pairing logic
- generate a var statement for the given node
- [FIX!] update scope to use positions and call .type() recursively!

this fixes the following code:
```coffee
import x from "./lib/coffeescript/index.js"
do ->
  x = 3
  x
```
- rename "scope" to FunctionScope
- break out a separate TopLevelScope so we can look at modules more clearly
- add lots of TODOs and fix namedMethod?()
- remove extraneous 9e9 limit from .splice call
- add quite a lot of notes to compileWithDeclarations
- add block scopes!!!!
- imports and exports are so much better supported now!!!
- assigning to an imported symbol is an error
@edemaine
Copy link
Contributor

FYI: #5395 offers an alternative approach, which is to attach the comment to the first assignment to the variable. I'm not sure it's worth adding additional syntax when a simple rule like this might suffice. In any case I think this is worth a discussion.

(However, I never revised #5395 so it's in hiatus now, though it's still used by CoffeeSense I believe. Feel free to pick it up if it's of interest. Or I could try if there's interest. My efforts have mostly shifted to Civet.)

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