feat: allow empty arguments for field resolvers

[Noroth]

This PR introduces support to call field resolvers without any arguments. Currently we only check if field resolvers provide arguments with parentheses.

Summary by CodeRabbit

• Bug Fixes

  • Enhanced error handling and validation for field resolution
  • Improved support for optional arguments in field resolvers
  • Better error messages when field definitions cannot be resolved

• Tests

  • Expanded test coverage for field resolver edge cases

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Checklist

• I have discussed my proposed changes in an issue and have received approval to proceed.
• I have followed the coding standards of the project.
• Tests or benchmarks have been added or updated.

[coderabbitai]

Walkthrough

Refactors field resolution in gRPC datasource processing from fieldRef-based to fieldDefRef-based (field definition references) throughout execution plan construction and visitor logic. Adds strict validation for field definitions with error handling, conditional argument allocation in resolver RPC calls, and corresponding test coverage for optional resolver arguments.

Changes

Cohort / File(s)	Summary
Execution Plan Field Definition Resolution v2/pkg/engine/datasource/grpc_datasource/execution_plan.go	Replaces fieldRef with fieldDefRef for field resolution across buildRequiredField, buildFieldMessage, buildCompositeFields, and related routines. Adds field definition lookup with strict validation. Introduces fieldDefinitionRefForType helper. Implements conditional argument allocation in resolver RPC calls. Updates error messages to reference field names via fieldDefRef context.
Visitor Field Resolver Detection v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go, execution_plan_visitor_federation.go	Updates EnterField to use fieldDefRef for resolver detection instead of raw field ref. Adds field definition resolution in LeaveField with defensive error reporting if definition not found. Changes resolver-check logic to depend on validated fieldDefRef state.
Field Resolver Tests v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go	Adds test case for resolver with optional argument (fooResolverOptionalArgument). Extends GraphQL schema and test mappings to include new optional-argument resolver wiring and RPC routing configuration.
Integration Tests v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go	Adds test case "Query with field resolver without parentheses (nullable parameter)" validating that popularityScore defaults to 50 when threshold parameter is omitted.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Areas requiring extra attention:

• Field definition reference propagation across execution_plan.go functions; verify fieldDefRef consistency through buildRequiredField → buildFieldMessage → nested resolution chains
• Error handling logic in LeaveField of both visitor files; confirm error reporting path when field definitions cannot be resolved
• Conditional argument allocation in resolver RPC calls; ensure error guards within loops properly prevent partial state corruption
• fieldDefinitionRefForType helper usage in composite field resolution; validate correctness of field-name-to-definition mapping
• Test coverage for new optional-argument resolver pathway; verify RPC call sequencing and dependency handling match expected patterns

Possibly related PRs

• feat: validate presence of optional @requires dependencies #1297: Introduces and propagates field-definition-reference helpers with direct logic overlap in field definition resolution patterns.
• feat: add support for field resolvers #1323: Modifies grpc_datasource execution plan and visitor code for field-resolver handling, affecting the same execution_plan.go and visitor files.
• feat: add support for multiple key directives #1262: Modifies execution_plan.go and visitor files within grpc_datasource planning logic, touching overlapping code paths.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat: allow empty arguments for field resolvers' directly aligns with the PR's main objective to add support for calling field resolvers without any arguments, as confirmed across all modified files.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ludwig/eng-8592-allow-providing-no-arguments-to-field-resolvers

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 701ea8d and 5d80fce.

📒 Files selected for processing (5)

• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (6 hunks)
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go (3 hunks)
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go (2 hunks)
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go (2 hunks)
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go (1 hunks)

🧰 Additional context used

🧠 Learnings (8)

📓 Common learnings

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go:406-429
Timestamp: 2025-11-19T09:42:17.644Z
Learning: In the wundergraph/graphql-go-tools gRPC datasource implementation (v2/pkg/engine/datasource/grpc_datasource), field resolvers must have arguments. The system does not currently support defining field resolvers without arguments. This invariant ensures that the `parentCallID` increment in `enterFieldResolver` is always matched by a decrement in `LeaveField` (which checks `r.operation.FieldHasArguments(ref)`).

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1039-1097
Timestamp: 2025-11-19T10:53:06.342Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling, the `resolveRequiredFields` function intentionally uses two distinct approaches: for simple GraphQL object types it populates `message.Fields`, while for composite types (interface/union) it exclusively uses `message.FieldSelectionSet` with fragment-based selections. This differs from `buildFieldMessage` (regular queries) because field resolver responses returning composite types must align with protobuf oneOf structure, where all selections—including common interface fields—are handled through fragment selections built by `buildCompositeField`. The two approaches cannot be mixed in field resolver responses.

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1053-1118
Timestamp: 2025-12-02T08:25:26.682Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling for composite types (interface/union), buildFieldResolverTypeMessage correctly combines both message.Fields and message.FieldSelectionSet: message.Fields contains interface-level fields that can be selected directly on the interface (such as __typename or fields defined in the interface itself), while message.FieldSelectionSet contains type-specific fields from inline fragments. This mixing is intentional and correct for GraphQL interfaces, as interface-level fields exist outside inline fragment selections and must be handled separately from type-specific fragment selections.

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go:1418-1526
Timestamp: 2025-11-19T09:38:25.112Z
Learning: In v2/pkg/engine/datasource/grpc_datasource test files, ResolvePath fields use snake_case (e.g., "test_containers.id") because they reference proto message field names, while JSONPath fields use camelCase (e.g., "testContainers") because they reference GraphQL/JSON response field names. Both casing styles are intentional and correct.

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1323
File: v2/pkg/engine/datasource/grpc_datasource/compiler.go:683-702
Timestamp: 2025-10-16T13:05:19.838Z
Learning: In GraphQL field resolver context resolution (v2/pkg/engine/datasource/grpc_datasource/compiler.go), when traversing paths in resolveContextDataForPath, the code can safely assume that intermediate path segments will only be messages or lists, never scalars. This is because field resolvers are only defined on GraphQL object types, not scalar types, so the parent function must return either a message or a list. This invariant is enforced by the GraphQL type system design.

📚 Learning: 2025-11-19T09:42:17.644Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go:406-429
Timestamp: 2025-11-19T09:42:17.644Z
Learning: In the wundergraph/graphql-go-tools gRPC datasource implementation (v2/pkg/engine/datasource/grpc_datasource), field resolvers must have arguments. The system does not currently support defining field resolvers without arguments. This invariant ensures that the `parentCallID` increment in `enterFieldResolver` is always matched by a decrement in `LeaveField` (which checks `r.operation.FieldHasArguments(ref)`).

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go

📚 Learning: 2025-11-19T10:53:06.342Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1039-1097
Timestamp: 2025-11-19T10:53:06.342Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling, the `resolveRequiredFields` function intentionally uses two distinct approaches: for simple GraphQL object types it populates `message.Fields`, while for composite types (interface/union) it exclusively uses `message.FieldSelectionSet` with fragment-based selections. This differs from `buildFieldMessage` (regular queries) because field resolver responses returning composite types must align with protobuf oneOf structure, where all selections—including common interface fields—are handled through fragment selections built by `buildCompositeField`. The two approaches cannot be mixed in field resolver responses.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go

📚 Learning: 2025-12-02T08:25:26.682Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan.go:1053-1118
Timestamp: 2025-12-02T08:25:26.682Z
Learning: In v2/pkg/engine/datasource/grpc_datasource field resolver response handling for composite types (interface/union), buildFieldResolverTypeMessage correctly combines both message.Fields and message.FieldSelectionSet: message.Fields contains interface-level fields that can be selected directly on the interface (such as __typename or fields defined in the interface itself), while message.FieldSelectionSet contains type-specific fields from inline fragments. This mixing is intentional and correct for GraphQL interfaces, as interface-level fields exist outside inline fragment selections and must be handled separately from type-specific fragment selections.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go

📚 Learning: 2025-11-19T09:38:25.112Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1341
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go:1418-1526
Timestamp: 2025-11-19T09:38:25.112Z
Learning: In v2/pkg/engine/datasource/grpc_datasource test files, ResolvePath fields use snake_case (e.g., "test_containers.id") because they reference proto message field names, while JSONPath fields use camelCase (e.g., "testContainers") because they reference GraphQL/JSON response field names. Both casing styles are intentional and correct.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go

📚 Learning: 2025-08-08T09:43:07.433Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1262
File: v2/pkg/engine/datasource/grpc_datasource/json_builder.go:0-0
Timestamp: 2025-08-08T09:43:07.433Z
Learning: In v2/pkg/engine/datasource/grpc_datasource/json_builder.go, mergeEntities intentionally uses the loop index when calling indexMap.getResultIndex because the index map is type-aware, making per-type counters unnecessary under the current assumptions. Avoid suggesting per-type ordinal counters for this path in future reviews.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go

📚 Learning: 2025-10-16T13:05:19.838Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1323
File: v2/pkg/engine/datasource/grpc_datasource/compiler.go:683-702
Timestamp: 2025-10-16T13:05:19.838Z
Learning: In GraphQL field resolver context resolution (v2/pkg/engine/datasource/grpc_datasource/compiler.go), when traversing paths in resolveContextDataForPath, the code can safely assume that intermediate path segments will only be messages or lists, never scalars. This is because field resolvers are only defined on GraphQL object types, not scalar types, so the parent function must return either a message or a list. This invariant is enforced by the GraphQL type system design.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go

📚 Learning: 2025-07-28T12:44:56.405Z

Learnt from: Noroth
Repo: wundergraph/graphql-go-tools PR: 1246
File: v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go:457-457
Timestamp: 2025-07-28T12:44:56.405Z
Learning: In the graphql-go-tools gRPC datasource, only non-null lists use protobuf's repeated field syntax directly. For nullable or nested lists, wrapper types are used because protobuf repeated fields cannot be nullable. The TypeIsNonNullList check ensures only appropriate list types are marked as repeated in the protobuf message structure.

Applied to files:

• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go

🧬 Code graph analysis (4)

v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go (4)

v2/pkg/ast/ast_field_definition.go (1)

• FieldDefinition (17-26)

v2/pkg/operationreport/operationreport.go (1)

• Report (9-12)

v2/pkg/operationreport/externalerror.go (1)

• ExternalError (34-42)

v2/pkg/engine/datasource/grpc_datasource/compiler.go (1)

• Message (182-186)

v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go (3)

v2/pkg/ast/ast_field_definition.go (1)

• FieldDefinition (17-26)

v2/pkg/operationreport/operationreport.go (1)

• Report (9-12)

v2/pkg/operationreport/externalerror.go (1)

• ExternalError (34-42)

v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go (3)

v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (8)

• RPCExecutionPlan (57-62)
• RPCCall (78-94)
• RPCMessage (98-112)
• RPCField (183-212)
• RPCFields (265-265)
• CallKindResolve (73-73)
• OneOfType (29-29)
• OneOfTypeInterface (36-36)

v2/pkg/engine/datasource/grpc_datasource/compiler.go (3)

• DataTypeMessage (38-38)
• Message (182-186)
• DataTypeString (29-29)

v2/pkg/engine/datasource/grpc_datasource/configuration.go (2)

• FieldMapData (81-84)
• FieldArgumentMap (87-87)

v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (2)

v2/pkg/engine/datasource/grpc_datasource/compiler.go (2)

• InvalidRef (21-21)
• DataTypeMessage (38-38)

v2/pkg/ast/ast.go (1)

• InvalidRef (8-8)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)
• GitHub Check: Build and test (go 1.25 / ubuntu-latest)
• GitHub Check: Build and test (go 1.25 / windows-latest)

🔇 Additional comments (15)

v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_test.go (1)

4542-4568: LGTM! Test validates field resolvers without arguments.

This test case correctly validates the new feature allowing field resolvers to be called without parentheses when parameters are nullable. The assertions confirm that popularityScore returns the expected default value of 50 when no threshold is provided.

Based on learnings, this PR explicitly removes the previous invariant that field resolvers must have arguments.

v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor.go (2)

361-361: LGTM! Correctly uses field definition reference for resolver detection.

The change from ref to fieldDefRef aligns with the broader refactoring to use field definition references throughout the execution plan. The fieldDefRef is already validated at lines 351-357, ensuring this check is performed on a valid field definition.

---

421-429: LGTM! Defensive error handling with consistent resolver detection.

The defensive retrieval of fieldDefRef with external error reporting is appropriate when the field definition cannot be found. Using fieldDefRef for the resolver check at line 429 is consistent with the change in EnterField (line 361), ensuring that the parentCallID increment/decrement remains balanced.

v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go (2)

334-334: LGTM! Consistent with non-federation visitor implementation.

The change from ref to fieldDefRef mirrors the implementation in execution_plan_visitor.go, maintaining consistency across both visitor implementations. The fieldDefRef is validated at lines 324-330 before this check.

---

390-398: LGTM! Defensive error handling consistent with non-federation visitor.

The defensive retrieval of fieldDefRef with external error reporting mirrors the implementation in execution_plan_visitor.go (lines 421-429), maintaining consistency across both visitor implementations. The resolver check at line 398 uses fieldDefRef, ensuring balanced parentCallID increment/decrement.

v2/pkg/engine/datasource/grpc_datasource/execution_plan_field_resolvers_test.go (3)

2621-2728: Test case correctly validates optional argument handling for field resolvers.

The test properly validates the new behavior where a field resolver with an optional argument (foo: String) can be called without providing that argument. The expected execution plan shows:

1. The resolver call is still created (Kind: CallKindResolve)
2. The field_args message has only a Name with no Fields populated
3. Dependency on the parent call is correctly set

This aligns with the PR objective of allowing empty arguments for field resolvers.

---

2762-2762: Schema field correctly defines optional argument for testing.

The field fooResolverOptionalArgument(foo: String): Bar! with the @connect__fieldResolver directive properly tests the scenario where a resolver has an optional argument (foo: String without !), enabling the test to call it without providing the argument.

---

2839-2849: Mapping correctly defines argument mappings for the new resolver field.

The mapping includes ArgumentMappings even though the test query doesn't provide arguments. This is correct as it defines the schema-level mapping that would be used if arguments were provided. The implementation correctly only processes arguments that are actually present in the query.

v2/pkg/engine/datasource/grpc_datasource/execution_plan.go (7)

861-867: Correctly updated isFieldResolver to use field definition reference.

The function now uses fieldDefRef to check if a field is a resolver based on whether its definition has argument definitions. This is a key behavioral change from the previous approach:

• Previously: A field was considered a resolver if the query provided arguments
• Now: A field is a resolver if the schema definition declares arguments (optional or required)

This enables calling resolvers without providing arguments when they're all optional.

Based on learnings, the invariant "field resolvers must have arguments" has been relaxed to "field resolvers must have argument definitions, but arguments don't need to be provided at call time."

---

782-795: Refactor to fieldDefRef-based resolution looks correct.

The pattern of looking up fieldDefRef first, then using it for both isFieldResolver check and buildRequiredField call is consistent and correct. The error message appropriately uses the field name from the operation for clarity.

---

1105-1118: Consistent refactor pattern in buildFieldResolverTypeMessage.

The changes follow the same pattern as buildFieldMessage: lookup field definition reference first, check if it's a resolver (skip if so), then build the required field. Error handling is consistent.

---

1130-1151: Signature update to accept fieldDefinitionRef parameter is correct.

This eliminates redundant lookups since callers already have the field definition reference. The function correctly uses this parameter for both buildField and nested message construction.

---

1161-1191: Correct refactor of buildCompositeFields to use field definition references.

The function properly uses the new fieldDefinitionRefForType helper for looking up field definitions by type name, handles invalid references with an error, and consistently skips resolver fields. The pattern matches other refactored functions.

---

1194-1206: Helper function fieldDefinitionRefForType is well-designed.

The function encapsulates the two-step lookup pattern (find type node, then find field definition) and returns ast.InvalidRef when either step fails. This is consistent with the codebase's pattern for reference lookup functions.

---

1263-1274: Key change: Conditional argument handling correctly enables empty arguments.

This is the core change that allows field resolvers to be called without providing arguments. When resolvedField.fieldArguments is empty (no arguments provided in the query), fieldArgsMessage.Fields remains nil instead of being populated with an empty slice. This produces the expected RPC message structure where field_args has only a name but no fields.

The error handling within the loop correctly propagates any errors encountered during field argument creation.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}