Skip to content

surfer: Refactor builder to use dispatch pattern for arguments #3412

@quirogas

Description

@quirogas

Currently, newArguments in builder.go uses a mix of ad-hoc checks (if name == "parent") and function calls (IsPrimaryResource) to determine how to process fields. This leads to "mutation soup" and confusion about why certain fields are skipped while others are processed.

Context & Rationale

  • Schema Alignment: The gcloud schema defines arguments as oneOf: arg, resource_arg, arg_group. Our builder should reflect this structure.
  • Clarity: Explicit predicates (IsResourceArg, IsIgnored) will make the logic for skipping redundant fields (like parent) self-documenting and consistent.
  • Maintainability: This refactoring paves the way for properly supporting Argument Groups (tracked in issue surfer: add arg groups in gcloud commands #3287) by providing a dedicated IsArgGroup branch.

Implementation Guide & Predicate Logic

The refactoring should implement the following predicates to classify fields:

1. IsIgnored(field, method)

Determines if a field should be skipped entirely.

  • Criteria:
    • field.Name == "parent": Redundant because parent context is handled by the Primary Resource argument's attributes.
    • field.Behavior contains OUTPUT_ONLY: These fields are not user-settable.
    • field.Name == "name" AND (IsCreate(method) OR IsUpdate(method)): The resource name in the body is redundant because the resource identity is established via positional arguments (ID) or flags.

2. IsResourceArg(field, method)

Determines if a field represents a gcloud resource_arg.

  • Criteria:
    • utils.IsPrimaryResource(field, method) is true: This covers the {resource}_id field for Create methods and the name field for Get/Delete/Update methods. These become the "Anchor" argument.
    • field.ResourceReference != nil: Fields annotated with google.api.resource_reference point to other resources and should generate resource argument flags.

3. IsArgGroup(field) (Future / Partial Support)

Determines if a field represents a nested structure that should be grouped.

  • Criteria:
    • field.Typez == api.MESSAGE_TYPE: It is a nested message.
    • AND !field.Map: Maps are handled as ArgDict or repeated flags, not groups.
    • AND !IsResourceArg(field): It is not a reference.
  • Note: See issue surfer: add arg groups in gcloud commands #3287 for full arg_group implementation. For now, this branch might handle flattening.

4. IsArg(field)

Determines if a field represents a standard gcloud arg (flag).

  • Criteria:
    • Default fallback. Covers STRING, INT, BOOL, ENUM, BYTES, and MAP types.

Proposed Structure

Refactor newArguments to simple dispatch logic:

for _, f := range method.InputType.Fields {
    if IsIgnored(f, method) { continue }
    
    if IsResourceArg(f, method) {
        args.Params = append(args.Params, newResourceArgParam(...))
        continue
    }
    
    // Future: if IsArgGroup(f) { ... } // See #3287
    
    // Default
    args.Params = append(args.Params, newArgParam(...))
}

Relevant files

  • internal/surfer/gcloud/builder.go
  • internal/surfer/gcloud/utils/resource.go (For IsPrimaryResource)

Metadata

Metadata

Assignees

No one assigned

    Labels

    surferIssues related to the surfer project (https://github.com/googleapis/librarian/issues/2375)

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions