Skip to content

Class AST: bound/computed properties, executable body #5208

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.

Sign up for GitHub

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 4 commits into from
Apr 29, 2019
Merged

Class AST: bound/computed properties, executable body #5208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c6ad9c2
tests for bound/computed
helixbass Apr 28, 2019
1378413
executable class body aST
helixbass Apr 28, 2019
1648422
remove
helixbass Apr 28, 2019
8839032
computed
helixbass Apr 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/coffeescript/nodes.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions src/nodes.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2668,6 +2668,7 @@ exports.Class = class Class extends Base
else if method.bound
@boundMethods.push method

return unless o.compiling
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the natural representation for executable class body statements is the equivalent AST as if they were just statements in a regular block

So here, we don't have to deal with ExecutableClassBody/HoistTarget at all, we just leave the class body Block as a mix of "class-specific" nodes (ClassMethod/ClassProperty/ClassPrototypeProperty) and other "executable body" nodes, and then the class body AST ends up looking like eg:

# this could correspond to eg:
# class A
#   b: ->
#   x = 3
#   @c: 2
#   if y
#     z()
type: 'ClassBody'
body: [
  {type: 'ClassMethod', ...}
  {type: 'ExpressionStatement', ...}
  {type: 'ClassProperty', ...}
  {type: 'IfStatement', ...}
]

Copy link
Collaborator

Choose a reason for hiding this comment

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

The ones that match class fields should probably output as those Babel node types, I'd think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you give an example of what you mean by "class fields"? See my comment as far as my understanding of how Coffeescript constructs relate to the JS class fields proposal

So basically we are matching Babel for everything that corresponds semantically/structurally to a JS/Babel construct

Eg a static property corresponds to a JS "static class field", so for this:

class A
  @b: 2

we generate an AST node with:

type: 'ClassProperty'
static: true

like you'd see in the Babel AST for

class A {
  static b = 2
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just meant the class fields proposal. Though I guess it doesn’t apply here because these are all on the prototype?

See https://astexplorer.net/#/gist/152f7080b8be9dde369647933ab7747f/c7aea4d9cae9df2a4b6fbf61bbac604f6d8dac2e

class Foo {
  a = 1
}

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup I don't think we have a structure that corresponds to non-static JS class fields

I read over #4552 for context. My understanding is that the answer to his first question there ("does the b property have different semantics?") is "yes", that since b: 3 is set (once) on the prototype, it's not semantically the same as a JS class field eg b = 3

So that's why I introduced ClassPrototypeProperty AST nodes to represent eg b: 3, since they're a different construct that doesn't seem to have an equivalent in JS/Babel classes

if initializer.length isnt expressions.length
@body.expressions = (expression.hoist() for expression in initializer)
new Block expressions
Expand Down Expand Up @@ -2924,7 +2925,7 @@ exports.ClassProperty = class ClassProperty extends Base
key: @name.ast o, LEVEL_LIST
value: @value.ast o, LEVEL_LIST
static: !!@isStatic
computed: no
computed: @name instanceof ComputedPropertyName
operator: @operatorToken?.value ? '='
staticClassName: @staticClassName?.ast(o) ? null

Expand Down Expand Up @@ -3881,14 +3882,15 @@ exports.Code = class Code extends Base
return
static: !!@isStatic
key: @name.ast o
computed: no
computed: @name instanceof ComputedPropertyName or @name.name instanceof ComputedPropertyName
kind:
if @ctor
'constructor'
else
'method'
operator: @operatorToken?.value ? '='
staticClassName: @isStatic.staticClassName?.ast(o) ? null
bound: !!@bound

astProperties: (o) ->
return Object.assign
Expand Down
110 changes: 76 additions & 34 deletions test/abstract_syntax_tree.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,12 +1328,15 @@ test "AST as expected for Class node", ->
async: no
params: []
body: EMPTY_BLOCK
bound: no
]

testExpression '''
a = class A
b: ->
c
d: =>
e
''',
type: 'AssignmentExpression'
right:
Expand All @@ -1359,14 +1362,34 @@ test "AST as expected for Class node", ->
expression: ID 'c'
]
operator: ':'
bound: no
,
type: 'ClassMethod'
static: no
key: ID 'd'
computed: no
kind: 'method'
id: null
generator: no
async: no
params: []
body:
type: 'BlockStatement'
body: [
type: 'ExpressionStatement'
expression: ID 'e'
]
operator: ':'
bound: yes
]

testStatement '''
class A
@b: ->
@c = ->
@c = =>
@d: 1
@e = 2
j = 5
A.f = 3
A.g = ->
this.h = ->
Expand All @@ -1392,6 +1415,7 @@ test "AST as expected for Class node", ->
staticClassName:
type: 'ThisExpression'
shorthand: yes
bound: no
,
type: 'ClassMethod'
static: yes
Expand All @@ -1407,6 +1431,7 @@ test "AST as expected for Class node", ->
staticClassName:
type: 'ThisExpression'
shorthand: yes
bound: yes
,
type: 'ClassProperty'
static: yes
Expand All @@ -1427,6 +1452,12 @@ test "AST as expected for Class node", ->
staticClassName:
type: 'ThisExpression'
shorthand: yes
,
type: 'ExpressionStatement'
expression:
type: 'AssignmentExpression'
left: ID 'j'
right: NUMBER 5
,
type: 'ClassProperty'
static: yes
Expand All @@ -1448,6 +1479,7 @@ test "AST as expected for Class node", ->
body: EMPTY_BLOCK
operator: '='
staticClassName: ID 'A'
bound: no
,
type: 'ClassMethod'
static: yes
Expand All @@ -1463,6 +1495,7 @@ test "AST as expected for Class node", ->
staticClassName:
type: 'ThisExpression'
shorthand: no
bound: no
,
type: 'ClassProperty'
static: yes
Expand All @@ -1479,6 +1512,9 @@ test "AST as expected for Class node", ->
class A
b: 1
[c]: 2
[d]: ->
@[e]: ->
@[f]: 3
''',
type: 'ClassDeclaration'
id: ID 'A'
Expand All @@ -1495,41 +1531,47 @@ test "AST as expected for Class node", ->
key: ID 'c'
value: NUMBER 2
computed: yes
,
type: 'ClassMethod'
static: no
key: ID 'd'
computed: yes
kind: 'method'
id: null
generator: no
async: no
params: []
body: EMPTY_BLOCK
operator: ':'
bound: no
,
type: 'ClassMethod'
static: yes
key: ID 'e'
computed: yes
kind: 'method'
id: null
generator: no
async: no
params: []
body: EMPTY_BLOCK
operator: ':'
bound: no
staticClassName:
type: 'ThisExpression'
shorthand: yes
,
type: 'ClassProperty'
static: yes
key: ID 'f'
computed: yes
value: NUMBER 3
operator: ':'
staticClassName:
type: 'ThisExpression'
shorthand: yes
]

# test "AST as expected for ExecutableClassBody node", ->
# code = """
# class Klass
# privateStatic = if 42 then yes else no
# getPrivateStatic: -> privateStatic
# """
# testExpression code,
# type: 'Class'
# variable:
# value: 'Klass'
# body:
# type: 'Block'
# expressions: [
# type: 'Assign'
# variable:
# value: 'privateStatic'
# value:
# type: 'If'
# ,
# type: 'Obj'
# generated: yes
# properties: [
# type: 'Assign'
# variable:
# value: 'getPrivateStatic'
# value:
# type: 'Code'
# body:
# type: 'Value'
# properties: []
# ]
# ]

test "AST as expected for ModuleDeclaration node", ->
testStatement 'export {X}',
type: 'ExportNamedDeclaration'
Expand Down
Loading