Merge pull request #1631 from DanielXMoore/readonly-at

Fix mixing access modifiers (e.g. `readonly`) with `@` arguments in constructors
DanielXMoore · Dec 3, 2024 · abbf5dc · abbf5dc
2 parents 9b5b016 + 84dcc4e
commit abbf5dc
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 14 deletions.
diff --git a/source/parser.hera b/source/parser.hera
@@ -2001,6 +2001,7 @@ NWBindingIdentifier
       type: "AtBinding",
       children: [ref],
       ref,
+      names: [],
     }
   # Likewise for private identifiers
   Hash AtIdentifierRef:ref ->
@@ -2009,6 +2010,7 @@ NWBindingIdentifier
       type: "AtBinding",
       children: [ref],
       ref,
+      names: [],
     }
   Identifier:id
   # NOTE: Support for return := 1 and let return: number

diff --git a/source/parser/binding.civet b/source/parser/binding.civet
@@ -2,7 +2,6 @@ import type {
   ArrayBindingPattern
   ASTNode
   ASTNodeObject
-  AtBinding
   BindingPattern
   BindingRestElement
   Children
@@ -113,30 +112,27 @@ function gatherBindingCode(statements: ASTNode, opts?: { injectParamProps?: bool
 
   function insertRestSplices(s, p: unknown[], thisAssignments: ThisAssignments): void
     gatherRecursiveAll(s, (n) => n.blockPrefix or (opts?.injectParamProps and n.accessModifier) or n.type is "AtBinding")
-      .forEach((n) => {
+      .forEach (n) =>
         // Insert `this` assignments
         if n.type is "AtBinding"
           { ref } := n as! AtBinding
           { id } := ref
           thisAssignments.push([`this.${id} = `, ref])
           return
 
-        if (opts?.injectParamProps and n.type is "Parameter" and n.accessModifier)
-          n.names.forEach((id) => {
-            thisAssignments.push({
-              type: "AssignmentExpression",
-              children: [`this.${id} = `, id],
+        if opts?.injectParamProps and n.type is "Parameter" and n.accessModifier
+          for each id of n.names
+            thisAssignments.push
+              type: "AssignmentExpression"
+              children: [`this.${id} = `, id]
               js: true
-            })
-          })
           return
 
         { blockPrefix } := n
         p.push(blockPrefix)
 
         // Search for any further nested splices, and at bindings
         insertRestSplices(blockPrefix, p, thisAssignments)
-      })
 
   insertRestSplices(statements, splices, thisAssignments)
 

diff --git a/source/parser/function.civet b/source/parser/function.civet
@@ -843,21 +843,26 @@ function processParams(f): void
       index-- while classExpressions[index-1]?[1] is like {type: "MethodDefinition", name: "constructor"}
       fStatement := classExpressions[index]
       for each parameter of gatherRecursive parameters, .type is "Parameter"
-        continue unless parameter.typeSuffix
+        {accessModifier} := parameter
+        continue unless accessModifier or parameter.typeSuffix
         for each binding of gatherRecursive parameter, .type is "AtBinding"
           // TODO: Handle typeSuffix of entire complex parameter pattern
           // (Currently just handle `@prop ::` individual typing,
           // where parent is AtBindingProperty or BindingElement,
           // or when the parent is the whole Parameter.)
           typeSuffix := binding.parent?.typeSuffix
-          continue unless typeSuffix
+          continue unless accessModifier or typeSuffix
+          // Remove accessModifier from parameter if we lift it to a field
+          if parameter.accessModifier
+            replaceNode parameter.accessModifier, undefined
+            parameter.accessModifier = undefined
           id := binding.ref.id
           continue if fields.has id
           classExpressions.splice index++, 0, [fStatement[0], {
             type: "FieldDefinition"
             id
             typeSuffix
-            children: [id, typeSuffix]
+            children: [accessModifier, id, typeSuffix]
           }, ";"]
           // Only the first definition gets an indent, stolen from fStatement
           fStatement[0] = ""

diff --git a/source/parser/types.civet b/source/parser/types.civet
@@ -567,6 +567,7 @@ export type AtBinding =
   ref: ASTRef
   children: Children & [ASTRef]
   parent?: Parent
+  names: []
 
 export type BlockStatement =
   type: "BlockStatement"
@@ -1199,7 +1200,7 @@ export type VoidType = ASTLeafWithType "VoidType"
 
 export type TypeLiteralNode = ASTLeaf | VoidType
 
-export type ThisAssignments = [string, ASTRef][]
+export type ThisAssignments = ([string, ASTRef] | AssignmentExpression)[]
 
 export type WithClause =
   type: "WithClause"

diff --git a/test/types/class.civet b/test/types/class.civet
@@ -188,6 +188,17 @@ describe "[TS] class", ->
     }
   """
 
+  testCase """
+    constructor with readonly param
+    ---
+    class Foo
+      @(readonly @var)
+    ---
+    class Foo {
+      readonly var;constructor(_var){this.var = _var;}
+    }
+  """
+
   testCase """
     const assignment becomes readonly field
     ---
@@ -467,6 +478,17 @@ describe "[TS] class", ->
       }
     """
 
+    testCase """
+      with access modifiers
+      ---
+      class UserAccount
+        @(public @name: string, readonly @var: number)
+      ---
+      class UserAccount {
+        public name: string;readonly var: number;constructor(name: string, _var: number){this.name = name;this.var = _var;}
+      }
+    """
+
     testCase """
       nested object fields
       ---