Skip to content

Commit

Permalink
core: allow "id" to be name of argument to functions (#6912)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
sipsma committed Mar 21, 2024
1 parent 88d89e8 commit 5974f19
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 22 deletions.
23 changes: 12 additions & 11 deletions core/integration/module_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
})
}
})
Expand Down
2 changes: 1 addition & 1 deletion core/integration/testdata/modules/go/id/arg/main.go
Expand Up @@ -3,5 +3,5 @@ package main
type Test struct{}

func (m *Test) Fn(id string) string {
return "NOOOO!!!!"
return id
}
2 changes: 1 addition & 1 deletion core/integration/testdata/modules/python/id/arg/main.py
Expand Up @@ -2,4 +2,4 @@

@function
def fn(id: str) -> str:
return "NOOOO!!!!"
return id
6 changes: 3 additions & 3 deletions 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;
}
}
6 changes: 0 additions & 6 deletions core/module.go
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 5974f19

Please sign in to comment.