From 6c7e1a97d3177411d7a4145d634da999c50748bd Mon Sep 17 00:00:00 2001
From: dpiercey <dpiercey@ebay.com>
Date: Mon, 11 Dec 2023 09:25:15 -0700
Subject: [PATCH] fix: add for tag by attribute types

---
 .changeset/brave-points-eat.md                |   8 +
 .../__snapshots__/for-tag.expected/index.md   | 259 +++++++++++-------
 .../fixtures/script/for-tag/index.marko       |  12 +
 packages/language-tools/marko.internal.d.ts   |  18 +-
 4 files changed, 190 insertions(+), 107 deletions(-)
 create mode 100644 .changeset/brave-points-eat.md

diff --git a/.changeset/brave-points-eat.md b/.changeset/brave-points-eat.md
new file mode 100644
index 00000000..d791d34c
--- /dev/null
+++ b/.changeset/brave-points-eat.md
@@ -0,0 +1,8 @@
+---
+"@marko/language-server": patch
+"@marko/language-tools": patch
+"@marko/type-check": patch
+"marko-vscode": patch
+---
+
+Add `by` attribute types for `<for>` tag.
diff --git a/packages/language-server/src/__tests__/fixtures/script/for-tag/__snapshots__/for-tag.expected/index.md b/packages/language-server/src/__tests__/fixtures/script/for-tag/__snapshots__/for-tag.expected/index.md
index 56eb9d97..373a2687 100644
--- a/packages/language-server/src/__tests__/fixtures/script/for-tag/__snapshots__/for-tag.expected/index.md
+++ b/packages/language-server/src/__tests__/fixtures/script/for-tag/__snapshots__/for-tag.expected/index.md
@@ -44,162 +44,223 @@
   17 |
 ```
 
-### Ln 25, Col 3
+### Ln 18, Col 18
 ```marko
-  23 |
-  24 | <effect() {
-> 25 |   hoistedFromForOf;
+  16 | </for>
+  17 |
+> 18 | <for of=list by=(item, index) => `${item}-${index}`>
+     |                  ^ (parameter) item: {
+    readonly value: 1;
+} | {
+    readonly value: 2;
+} | {
+    readonly value: 3;
+}
+  19 | //               ^?    ^?
+  20 | </for>
+  21 |
+```
+
+### Ln 18, Col 24
+```marko
+  16 | </for>
+  17 |
+> 18 | <for of=list by=(item, index) => `${item}-${index}`>
+     |                        ^ (parameter) index: number
+  19 | //               ^?    ^?
+  20 | </for>
+  21 |
+```
+
+### Ln 29, Col 3
+```marko
+  27 |
+  28 | <effect() {
+> 29 |   hoistedFromForOf;
      |   ^ const hoistedFromForOf: 1 | 2 | 3
-  26 | //^?
-  27 | }/>
-  28 |
+  30 | //^?
+  31 | }/>
+  32 |
 ```
 
-### Ln 32, Col 5
+### Ln 36, Col 5
 ```marko
-  30 |
-  31 | <for|key, value| in=record>
-> 32 |   ${key} ${value}
+  34 |
+  35 | <for|key, value| in=record>
+> 36 |   ${key} ${value}
      |     ^ (parameter) key: "a" | "b"
-  33 | //  ^?     ^?
-  34 | </for>
-  35 |
+  37 | //  ^?     ^?
+  38 | </for>
+  39 |
 ```
 
-### Ln 32, Col 12
+### Ln 36, Col 12
 ```marko
-  30 |
-  31 | <for|key, value| in=record>
-> 32 |   ${key} ${value}
+  34 |
+  35 | <for|key, value| in=record>
+> 36 |   ${key} ${value}
      |            ^ (parameter) value: 1 | 2
-  33 | //  ^?     ^?
-  34 | </for>
-  35 |
+  37 | //  ^?     ^?
+  38 | </for>
+  39 |
+```
+
+### Ln 40, Col 20
+```marko
+  38 | </for>
+  39 |
+> 40 | <for in=record by=(value, key) => `${value}-${key}`>
+     |                    ^ (parameter) value: 1 | 2
+  41 | //                 ^?     ^?
+  42 | </for>
+  43 |
 ```
 
-### Ln 41, Col 3
+### Ln 40, Col 27
 ```marko
+  38 | </for>
   39 |
-  40 | <effect() {
-> 41 |   hoistedFromForIn;
+> 40 | <for in=record by=(value, key) => `${value}-${key}`>
+     |                           ^ (parameter) key: "a" | "b"
+  41 | //                 ^?     ^?
+  42 | </for>
+  43 |
+```
+
+### Ln 49, Col 3
+```marko
+  47 |
+  48 | <effect() {
+> 49 |   hoistedFromForIn;
      |   ^ const hoistedFromForIn: "a" | "b"
-  42 | //^?
-  43 | }/>
-  44 |
+  50 | //^?
+  51 | }/>
+  52 |
 ```
 
-### Ln 46, Col 5
+### Ln 54, Col 5
 ```marko
-  44 |
-  45 | <for|index| to=10>
-> 46 |   ${index}
+  52 |
+  53 | <for|index| to=10>
+> 54 |   ${index}
      |     ^ (parameter) index: number
-  47 | //  ^?
-  48 | </for>
-  49 |
+  55 | //  ^?
+  56 | </for>
+  57 |
+```
+
+### Ln 58, Col 16
+```marko
+  56 | </for>
+  57 |
+> 58 | <for to=10 by=(index) => `${index}`>
+     |                ^ (parameter) index: number
+  59 | //             ^?
+  60 | </for>
+  61 |
 ```
 
-### Ln 51, Col 5
+### Ln 63, Col 5
 ```marko
-  49 |
-  50 | <for|index| from=1 to=10>
-> 51 |   ${index}
+  61 |
+  62 | <for|index| from=1 to=10>
+> 63 |   ${index}
      |     ^ (parameter) index: number
-  52 | //  ^?
-  53 | </for>
-  54 |
+  64 | //  ^?
+  65 | </for>
+  66 |
 ```
 
-### Ln 56, Col 5
+### Ln 68, Col 5
 ```marko
-  54 |
-  55 | <for|index| to=10 step=2>
-> 56 |   ${index}
+  66 |
+  67 | <for|index| to=10 step=2>
+> 68 |   ${index}
      |     ^ (parameter) index: number
-  57 | //  ^?
-  58 | </for>
-  59 |
+  69 | //  ^?
+  70 | </for>
+  71 |
 ```
 
-### Ln 65, Col 3
+### Ln 77, Col 3
 ```marko
-  63 |
-  64 | <effect() {
-> 65 |   hoistedFromForTo;
+  75 |
+  76 | <effect() {
+> 77 |   hoistedFromForTo;
      |   ^ const hoistedFromForTo: number
-  66 | //^?
-  67 | }/>
-  68 |
+  78 | //^?
+  79 | }/>
+  80 |
 ```
 
 ## Diagnostics
-### Ln 22, Col 6
+### Ln 26, Col 6
 ```marko
-  20 | </for>
-  21 |
-> 22 | <for|item| of=list></for>
+  24 | </for>
+  25 |
+> 26 | <for|item| of=list></for>
      |      ^^^^ 'item' is declared but its value is never read.
-  23 |
-  24 | <effect() {
-  25 |   hoistedFromForOf;
+  27 |
+  28 | <effect() {
+  29 |   hoistedFromForOf;
 ```
 
-### Ln 24, Col 2
+### Ln 28, Col 2
 ```marko
-  22 | <for|item| of=list></for>
-  23 |
-> 24 | <effect() {
+  26 | <for|item| of=list></for>
+  27 |
+> 28 | <effect() {
      |  ^^^^^^ Cannot find name 'effect'.
-  25 |   hoistedFromForOf;
-  26 | //^?
-  27 | }/>
+  29 |   hoistedFromForOf;
+  30 | //^?
+  31 | }/>
 ```
 
-### Ln 40, Col 2
+### Ln 48, Col 2
 ```marko
-  38 | </for>
-  39 |
-> 40 | <effect() {
+  46 | </for>
+  47 |
+> 48 | <effect() {
      |  ^^^^^^ Cannot find name 'effect'.
-  41 |   hoistedFromForIn;
-  42 | //^?
-  43 | }/>
+  49 |   hoistedFromForIn;
+  50 | //^?
+  51 | }/>
 ```
 
-### Ln 64, Col 2
+### Ln 76, Col 2
 ```marko
-  62 | </for>
-  63 |
-> 64 | <effect() {
+  74 | </for>
+  75 |
+> 76 | <effect() {
      |  ^^^^^^ Cannot find name 'effect'.
-  65 |   hoistedFromForTo;
-  66 | //^?
-  67 | }/>
+  77 |   hoistedFromForTo;
+  78 | //^?
+  79 | }/>
 ```
 
-### Ln 69, Col 2
+### Ln 81, Col 2
 ```marko
-  67 | }/>
-  68 |
-> 69 | <for|index|>
+  79 | }/>
+  80 |
+> 81 | <for|index|>
      |  ^^^^^^^^^ No overload matches this call.
   The last overload gave the following error.
-    Argument of type '{ renderBody: (index: any) => MarkoReturn<void>; }' is not assignable to parameter of type '({ from?: number | undefined; to: number; step?: number | undefined; } | { in: unknown; } | { of: readonly unknown[] | Iterable<unknown>; }) & { renderBody?: AnyMarkoBody | undefined; }'.
-      Type '{ renderBody: (index: any) => MarkoReturn<void>; }' is not assignable to type '{ of: readonly unknown[] | Iterable<unknown>; } & { renderBody?: AnyMarkoBody | undefined; }'.
+    Argument of type '{ renderBody: (index: any) => MarkoReturn<void>; }' is not assignable to parameter of type '({ from?: number | undefined; to: number; step?: number | undefined; } | { in: unknown; } | { of: readonly unknown[] | Iterable<unknown>; }) & { renderBody?: AnyMarkoBody | undefined; by?: ((...args: unknown[]) => string) | undefined; }'.
+      Type '{ renderBody: (index: any) => MarkoReturn<void>; }' is not assignable to type '{ of: readonly unknown[] | Iterable<unknown>; } & { renderBody?: AnyMarkoBody | undefined; by?: ((...args: unknown[]) => string) | undefined; }'.
         Property 'of' is missing in type '{ renderBody: (index: any) => MarkoReturn<void>; }' but required in type '{ of: readonly unknown[] | Iterable<unknown>; }'.
-  70 |   Should error
-  71 | </for>
-  72 |
+  82 |   Should error
+  83 | </for>
+  84 |
 ```
 
-### Ln 69, Col 6
+### Ln 81, Col 6
 ```marko
-  67 | }/>
-  68 |
-> 69 | <for|index|>
+  79 | }/>
+  80 |
+> 81 | <for|index|>
      |      ^^^^^ 'index' is declared but its value is never read.
-  70 |   Should error
-  71 | </for>
-  72 |
+  82 |   Should error
+  83 | </for>
+  84 |
 ```
 
diff --git a/packages/language-server/src/__tests__/fixtures/script/for-tag/index.marko b/packages/language-server/src/__tests__/fixtures/script/for-tag/index.marko
index ac2b154c..85d18c3a 100644
--- a/packages/language-server/src/__tests__/fixtures/script/for-tag/index.marko
+++ b/packages/language-server/src/__tests__/fixtures/script/for-tag/index.marko
@@ -15,6 +15,10 @@
 //  ^?      ^?       ^?
 </for>
 
+<for of=list by=(item, index) => `${item}-${index}`>
+//               ^?    ^?
+</for>
+
 <for|item| of=list>
   <const/{ value: hoistedFromForOf } = item/>
 </for>
@@ -33,6 +37,10 @@
 //  ^?     ^?
 </for>
 
+<for in=record by=(value, key) => `${value}-${key}`>
+//                 ^?     ^?
+</for>
+
 <for|key| in=record>
   <const/hoistedFromForIn = key/>
 </for>
@@ -47,6 +55,10 @@
 //  ^?
 </for>
 
+<for to=10 by=(index) => `${index}`>
+//             ^?
+</for>
+
 <for|index| from=1 to=10>
   ${index}
 //  ^?
diff --git a/packages/language-tools/marko.internal.d.ts b/packages/language-tools/marko.internal.d.ts
index bd95a90e..b8f2e3af 100644
--- a/packages/language-tools/marko.internal.d.ts
+++ b/packages/language-tools/marko.internal.d.ts
@@ -153,19 +153,19 @@ declare global {
 
       export function forTag<
         Value,
+        Item extends Value extends
+          | readonly (infer Item)[]
+          | Iterable<infer Item>
+          ? Item
+          : unknown,
         RenderBody extends Marko.Body<
-          [
-            value: Value extends readonly (infer Item)[] | Iterable<infer Item>
-              ? Item
-              : unknown,
-            index: number,
-            all: Value,
-          ],
+          [item: Item, index: number, all: Value],
           void
         >,
       >(input: {
         of: Value;
         renderBody: RenderBody;
+        by?: (item: Item, index: number) => string;
       }): ReturnAndScope<RenderBodyScope<RenderBody>, void>;
 
       export function forTag<
@@ -177,6 +177,7 @@ declare global {
       >(input: {
         in: Value;
         renderBody: RenderBody;
+        by?: (value: Value[keyof Value], key: keyof Value) => string;
       }): ReturnAndScope<RenderBodyScope<RenderBody>, void>;
 
       export function forTag<
@@ -189,6 +190,7 @@ declare global {
         to: To;
         step?: Step;
         renderBody: RenderBody;
+        by?: (index: number) => string;
       }): ReturnAndScope<RenderBodyScope<RenderBody>, void>;
 
       export function forTag<RenderBody extends AnyMarkoBody>(
@@ -204,7 +206,7 @@ declare global {
           | {
               of: readonly unknown[] | Iterable<unknown>;
             }
-        ) & { renderBody?: RenderBody },
+        ) & { renderBody?: RenderBody; by?: (...args: unknown[]) => string },
       ): ReturnAndScope<RenderBodyScope<RenderBody>, void>;
 
       export function forAttrTag<