From 823c9fcb0309f298c6865dd0059cd9507408d846 Mon Sep 17 00:00:00 2001
From: Erik Sipsma <erik@dagger.io>
Date: Thu, 21 Mar 2024 09:15:52 -0700
Subject: [PATCH] core: allow "id" to be name of argument to functions

I think this restriction got added along with restrictions around using
it for field/function names, but it was overzealous; can't see any
reason why it can't be an arg name.

Signed-off-by: Erik Sipsma <erik@dagger.io>
---
 core/integration/module_test.go               | 23 ++++++++++---------
 .../testdata/modules/go/id/arg/main.go        |  2 +-
 .../testdata/modules/python/id/arg/main.py    |  2 +-
 .../modules/typescript/id/arg/index.ts        |  6 ++---
 core/module.go                                |  6 -----
 5 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/core/integration/module_test.go b/core/integration/module_test.go
index a2dc1ea11e4..507cd03e732 100644
--- a/core/integration/module_test.go
+++ b/core/integration/module_test.go
@@ -4528,13 +4528,13 @@ func TestModuleLoops(t *testing.T) {
 }
 
 //go:embed testdata/modules/go/id/arg/main.go
-var badIDArgGoSrc string
+var goodIDArgGoSrc string
 
 //go:embed testdata/modules/python/id/arg/main.py
-var badIDArgPySrc string
+var goodIDArgPySrc string
 
 //go:embed testdata/modules/typescript/id/arg/index.ts
-var badIDArgTSSrc string
+var goodIDArgTSSrc string
 
 //go:embed testdata/modules/go/id/field/main.go
 var badIDFieldGoSrc string
@@ -4565,20 +4565,21 @@ func TestModuleReservedWords(t *testing.T) {
 		t.Parallel()
 
 		t.Run("arg", func(t *testing.T) {
+			// id used to be disallowed as an arg name, but is allowed now, test it works
 			t.Parallel()
 
 			for _, tc := range []testCase{
 				{
 					sdk:    "go",
-					source: badIDArgGoSrc,
+					source: goodIDArgGoSrc,
 				},
 				{
 					sdk:    "python",
-					source: badIDArgPySrc,
+					source: goodIDArgPySrc,
 				},
 				{
 					sdk:    "typescript",
-					source: badIDArgTSSrc,
+					source: goodIDArgTSSrc,
 				},
 			} {
 				tc := tc
@@ -4587,11 +4588,11 @@ func TestModuleReservedWords(t *testing.T) {
 					t.Parallel()
 					c, ctx := connect(t)
 
-					_, err := modInit(ctx, t, c, tc.sdk, tc.source).
-						With(daggerQuery(`{test{fn(id:"no")}}`)).
-						Sync(ctx)
-
-					require.ErrorContains(t, err, "cannot define argument with reserved name \"id\"")
+					out, err := modInit(ctx, t, c, tc.sdk, tc.source).
+						With(daggerQuery(`{test{fn(id:"YES!!!!")}}`)).
+						Stdout(ctx)
+					require.NoError(t, err)
+					require.JSONEq(t, `{"test":{"fn":"YES!!!!"}}`, out)
 				})
 			}
 		})
diff --git a/core/integration/testdata/modules/go/id/arg/main.go b/core/integration/testdata/modules/go/id/arg/main.go
index f55d552a7d0..014bf9bf37f 100644
--- a/core/integration/testdata/modules/go/id/arg/main.go
+++ b/core/integration/testdata/modules/go/id/arg/main.go
@@ -3,5 +3,5 @@ package main
 type Test struct{}
 
 func (m *Test) Fn(id string) string {
-	return "NOOOO!!!!"
+	return id
 }
diff --git a/core/integration/testdata/modules/python/id/arg/main.py b/core/integration/testdata/modules/python/id/arg/main.py
index ee882b86595..492ec2b3243 100644
--- a/core/integration/testdata/modules/python/id/arg/main.py
+++ b/core/integration/testdata/modules/python/id/arg/main.py
@@ -2,4 +2,4 @@
 
 @function
 def fn(id: str) -> str:
-    return "NOOOO!!!!"
+    return id
diff --git a/core/integration/testdata/modules/typescript/id/arg/index.ts b/core/integration/testdata/modules/typescript/id/arg/index.ts
index 6750b95ac85..33978772b83 100644
--- a/core/integration/testdata/modules/typescript/id/arg/index.ts
+++ b/core/integration/testdata/modules/typescript/id/arg/index.ts
@@ -1,9 +1,9 @@
-import { object, func } from "@dagger.io/dagger"
+import { object, func } from "@dagger.io/dagger";
 
 @object()
 class Test {
   @func()
   fn(id: string): string {
-    return "NOOOO!!!!"
-  } 
+    return id;
+  }
 }
diff --git a/core/module.go b/core/module.go
index da2842519c9..37340118939 100644
--- a/core/module.go
+++ b/core/module.go
@@ -408,9 +408,6 @@ func (mod *Module) validateObjectTypeDef(ctx context.Context, typeDef *TypeDef)
 		}
 
 		for _, arg := range fn.Args {
-			if gqlArgName(arg.Name) == "id" {
-				return fmt.Errorf("cannot define argument with reserved name %q on function %q", arg.Name, fn.Name)
-			}
 			argType, ok, err := mod.Deps.ModTypeFor(ctx, arg.TypeDef)
 			if err != nil {
 				return fmt.Errorf("failed to get mod type for type def: %w", err)
@@ -457,9 +454,6 @@ func (mod *Module) validateInterfaceTypeDef(ctx context.Context, typeDef *TypeDe
 		}
 
 		for _, arg := range fn.Args {
-			if gqlArgName(arg.Name) == "id" {
-				return fmt.Errorf("cannot define argument with reserved name %q on function %q", arg.Name, fn.Name)
-			}
 			if err := mod.validateTypeDef(ctx, arg.TypeDef); err != nil {
 				return err
 			}