diff --git a/.agents/skills/bump-sdk/SKILL.md b/.agents/skills/bump-sdk/SKILL.md index c9cc0e6aa0d..34a0ff97f4e 100644 --- a/.agents/skills/bump-sdk/SKILL.md +++ b/.agents/skills/bump-sdk/SKILL.md @@ -52,7 +52,7 @@ Decide with the user whether to keep the tagging-producer drift or restore those The task ends with step 4 (`generate-clijson-post` runs `bump-sdk-post`) and ignores its error, so read the end of its output, or go on with step 4 directly. **4. Regenerate everything downstream: `./task bump-sdk-post`.** -This is the whole mechanical part of the bump: `generate-check` (command stubs, refschema, schema, validation, direct-engine YAML, pydabs), then `test-update` for the acceptance goldens. +This is the whole mechanical part of the bump: `fix-sdk-field-copies` (complete reviewed direct-resource mappings), `generate-check` (command stubs, refschema, schema, validation, direct-engine YAML, pydabs), then `test-update` for the acceptance goldens. Everything after it needs judgment. Run directly, it fails on errors: `generate-refschema` and `test-update` build the CLI, so it fails while the bump still breaks compilation; fix that (step 5) and rerun the task. @@ -67,7 +67,7 @@ Revert any golden change you can't attribute to the new SDK or spec. **5. Handle SDK breaking changes.** Read the SDK's `CHANGELOG.md` at the target version (in the module cache) to enumerate breaking changes before chasing compile errors. A removed struct field that the CLI used (e.g. `jobs.AiRuntimeTask.CodeSourcePath`) should have its usage temporarily disabled with a comment noting it returns in a later SDK bump, not deleted outright. -A new struct field triggers an `exhaustruct` lint failure in `bundle/direct/dresources/*`. Run `./task lint` and (in case of issues) wire the field through `PrepareState` and `RemapState` when it exists on both the input and remote types. +A new struct field can trigger an `exhaustruct` lint failure in `bundle/direct/dresources/*`. `bump-sdk-post` first runs `fix-sdk-field-copies`, which adds missing same-name, same-JSON-name, identical-type copies only in literals explicitly annotated with `// sdk:copy-from source`. Existing assignments are preserved. Read `tools/sdkfieldcopies/README.md` for the reviewed scope and exclusions. Remaining fields still need judgment: run `./task lint`, review the fixer diagnostics, and wire the field through the appropriate mappings. Do not add zero values or annotations just to silence lint. Where a remote is rebuilt by hand from a GET response (e.g. `makeJobRunRemote` maps `jobs.Run` into the `jobs.RunNow`-shaped `JobRunRemote`), copy the new field from the response if the response has it, and list it with the request-only zero values otherwise. `job_runs.yml` must list every `jobs.RunNow` field under `ignore_remote_changes`; `TestJobRunIgnoresEveryRequestField` fails on a new one. A field the new spec now annotates as output-only may already be emitted into `.generated.yml`, making the manual entry in `.yml` redundant; `TestResourcesYMLNoRedundantRules` catches this, so remove the manual entry. diff --git a/Taskfile.yml b/Taskfile.yml index 8d59145d8a9..a6b087e116b 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -962,14 +962,20 @@ tasks: # then refresh the acceptance goldens. generate-clijson-post runs it last; # run it directly to redo it, e.g. after hand fixes. generate-refschema and # test-update build the CLI, so it fails while the bump breaks compilation - # (code using a removed SDK field). Those fixes, exhaustruct wiring and the + # (code using a removed SDK field). Unresolved field mappings and the # changelog PR link stay manual (.agents/skills/bump-sdk). bump-sdk-post: desc: Regenerate everything downstream of an SDK bump and refresh acceptance goldens cmds: + - task: fix-sdk-field-copies - task: generate-check - task: test-update + fix-sdk-field-copies: + desc: Complete explicitly annotated SDK field copies in direct resources + cmds: + - go run -C tools ./sdkfieldcopies -root .. + # Refreshes out.fields.txt, which records the field paths / types emitted by # `bundle debug refschema` (see cmd/bundle/debug/refschema.go). It reflects # over types reachable from bundle/, so any bundle change or SDK bump (go.mod diff --git a/bundle/direct/dresources/catalog.go b/bundle/direct/dresources/catalog.go index 638138bfbda..236623dc31b 100644 --- a/bundle/direct/dresources/catalog.go +++ b/bundle/direct/dresources/catalog.go @@ -23,7 +23,7 @@ func (*ResourceCatalog) PrepareState(input *resources.Catalog) *catalog.CreateCa } func (*ResourceCatalog) RemapState(info *catalog.CatalogInfo) *catalog.CreateCatalog { - return &catalog.CreateCatalog{ + return &catalog.CreateCatalog{ // sdk:copy-from info Comment: info.Comment, ConnectionName: info.ConnectionName, CustomMaxRetentionHours: info.CustomMaxRetentionHours, diff --git a/bundle/direct/dresources/cluster.go b/bundle/direct/dresources/cluster.go index 82167080c4d..7f7cf841099 100644 --- a/bundle/direct/dresources/cluster.go +++ b/bundle/direct/dresources/cluster.go @@ -105,7 +105,7 @@ func (r *ResourceCluster) PrepareState(input *resources.Cluster) *ClusterState { // RemapState maps the remote ClusterRemote to ClusterState for diff comparison. // Started is derived from cluster state so the planner can detect start/stop changes. func (r *ResourceCluster) RemapState(input *ClusterRemote) *ClusterState { - spec := &ClusterState{ + spec := &ClusterState{ // sdk:copy-from input ApplyPolicyDefaultValues: input.ApplyPolicyDefaultValues, Autoscale: input.Autoscale, AutoterminationMinutes: input.AutoterminationMinutes, @@ -428,7 +428,7 @@ func remoteClusterIsRunning(entry *PlanEntry) bool { } func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { - create := compute.CreateCluster{ + create := compute.CreateCluster{ // sdk:copy-from config ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, Autoscale: config.Autoscale, AutoterminationMinutes: config.AutoterminationMinutes, @@ -478,7 +478,7 @@ func makeCreateCluster(config *compute.ClusterSpec) compute.CreateCluster { } func makeEditCluster(id string, config *compute.ClusterSpec) compute.EditCluster { - edit := compute.EditCluster{ + edit := compute.EditCluster{ // sdk:copy-from config ClusterId: id, ApplyPolicyDefaultValues: config.ApplyPolicyDefaultValues, Autoscale: config.Autoscale, diff --git a/bundle/direct/dresources/cluster_policy.go b/bundle/direct/dresources/cluster_policy.go index ae69390b9cc..5ce573914bf 100644 --- a/bundle/direct/dresources/cluster_policy.go +++ b/bundle/direct/dresources/cluster_policy.go @@ -32,7 +32,7 @@ func (*ResourceClusterPolicy) PrepareState(input *resources.ClusterPolicy) *comp // RemapState copies the config fields shared by Policy and CreatePolicy; // output-only fields (policy_id, created_at_timestamp, creator_user_name, is_default) are not in the state. func (*ResourceClusterPolicy) RemapState(remote *compute.Policy) *compute.CreatePolicy { - return &compute.CreatePolicy{ + return &compute.CreatePolicy{ // sdk:copy-from remote Definition: remote.Definition, Description: remote.Description, Libraries: remote.Libraries, @@ -58,7 +58,7 @@ func (r *ResourceClusterPolicy) DoCreate(ctx context.Context, config *compute.Cr } func (r *ResourceClusterPolicy) DoUpdate(ctx context.Context, id string, config *compute.CreatePolicy, _ *PlanEntry) (*compute.Policy, error) { - return nil, r.client.ClusterPolicies.Edit(ctx, compute.EditPolicy{ + return nil, r.client.ClusterPolicies.Edit(ctx, compute.EditPolicy{ // sdk:copy-from config PolicyId: id, Name: config.Name, Definition: config.Definition, diff --git a/bundle/direct/dresources/experiment.go b/bundle/direct/dresources/experiment.go index e4f2e8ebbd7..16c0f78d6cc 100644 --- a/bundle/direct/dresources/experiment.go +++ b/bundle/direct/dresources/experiment.go @@ -23,7 +23,7 @@ func (*ResourceExperiment) New(client *databricks.WorkspaceClient) *ResourceExpe } func (*ResourceExperiment) PrepareState(input *resources.MlflowExperiment) *ml.CreateExperiment { - return &ml.CreateExperiment{ + return &ml.CreateExperiment{ // sdk:copy-from input Name: input.Name, ArtifactLocation: input.ArtifactLocation, Tags: input.Tags, @@ -33,7 +33,7 @@ func (*ResourceExperiment) PrepareState(input *resources.MlflowExperiment) *ml.C } func (*ResourceExperiment) RemapState(experiment *ml.Experiment) *ml.CreateExperiment { - return &ml.CreateExperiment{ + return &ml.CreateExperiment{ // sdk:copy-from experiment Name: experiment.Name, ArtifactLocation: experiment.ArtifactLocation, Tags: experiment.Tags, diff --git a/bundle/direct/dresources/external_location.go b/bundle/direct/dresources/external_location.go index 26c957020ab..0111bfe5dae 100644 --- a/bundle/direct/dresources/external_location.go +++ b/bundle/direct/dresources/external_location.go @@ -23,7 +23,7 @@ func (*ResourceExternalLocation) PrepareState(input *resources.ExternalLocation) } func (*ResourceExternalLocation) RemapState(info *catalog.ExternalLocationInfo) *catalog.CreateExternalLocation { - return &catalog.CreateExternalLocation{ + return &catalog.CreateExternalLocation{ // sdk:copy-from info Comment: info.Comment, CredentialName: info.CredentialName, // Output-only fields mirrored into state to avoid churn in remapped config. diff --git a/bundle/direct/dresources/instance_pool.go b/bundle/direct/dresources/instance_pool.go index 37a1aeab447..ff68c00065a 100644 --- a/bundle/direct/dresources/instance_pool.go +++ b/bundle/direct/dresources/instance_pool.go @@ -24,7 +24,7 @@ func (*ResourceInstancePool) PrepareState(input *resources.InstancePool) *comput // RemapState copies the config fields shared by GetInstancePool and CreateInstancePool; // output-only fields (state, stats, default_tags, instance_pool_id) are not in the state. func (*ResourceInstancePool) RemapState(remote *compute.GetInstancePool) *compute.CreateInstancePool { - return &compute.CreateInstancePool{ + return &compute.CreateInstancePool{ // sdk:copy-from remote AwsAttributes: remote.AwsAttributes, AzureAttributes: remote.AzureAttributes, CustomTags: remote.CustomTags, @@ -58,7 +58,7 @@ func (r *ResourceInstancePool) DoCreate(ctx context.Context, config *compute.Cre } func (r *ResourceInstancePool) DoUpdate(ctx context.Context, id string, config *compute.CreateInstancePool, _ *PlanEntry) (*compute.GetInstancePool, error) { - return nil, r.client.InstancePools.Edit(ctx, compute.EditInstancePool{ + return nil, r.client.InstancePools.Edit(ctx, compute.EditInstancePool{ // sdk:copy-from config InstancePoolId: id, InstancePoolName: config.InstancePoolName, NodeTypeId: config.NodeTypeId, diff --git a/bundle/direct/dresources/job.go b/bundle/direct/dresources/job.go index a19ecf4dae2..d73c0eb9a1c 100644 --- a/bundle/direct/dresources/job.go +++ b/bundle/direct/dresources/job.go @@ -182,7 +182,7 @@ func (r *ResourceJob) DoDelete(ctx context.Context, id string, _ *jobs.JobSettin func makeCreateJob(config jobs.JobSettings) (jobs.CreateJob, error) { // Note, exhaustruct linter validates that all off CreateJob fields are initialized. // We don't have linter that validates that all of config fields are used. - result := jobs.CreateJob{ + result := jobs.CreateJob{ // sdk:copy-from config AccessControlList: nil, // Not supported by DABs BudgetPolicyId: config.BudgetPolicyId, Continuous: config.Continuous, diff --git a/bundle/direct/dresources/job_run.go b/bundle/direct/dresources/job_run.go index 90a628b0486..c992808b691 100644 --- a/bundle/direct/dresources/job_run.go +++ b/bundle/direct/dresources/job_run.go @@ -121,7 +121,7 @@ func makeJobRunRemote(run *jobs.Run) *JobRunRemote { jobParameters[p.Name] = p.Value } } - return &JobRunRemote{ + return &JobRunRemote{ // sdk:copy-skip Parameter flattening and result-state reconstruction need manual mapping. JobId: run.JobId, JobParameters: jobParameters, DurationTarget: run.DurationTarget, diff --git a/bundle/direct/dresources/model.go b/bundle/direct/dresources/model.go index 83a4f8832aa..8ecacb0d147 100644 --- a/bundle/direct/dresources/model.go +++ b/bundle/direct/dresources/model.go @@ -43,7 +43,7 @@ func (*ResourceMlflowModel) PrepareState(input *resources.MlflowModel) *ml.Creat } func (*ResourceMlflowModel) RemapState(output *MlflowModelRemote) *ml.CreateModelRequest { - return &ml.CreateModelRequest{ + return &ml.CreateModelRequest{ // sdk:copy-from output Name: output.Name, Tags: output.Tags, Description: output.Description, diff --git a/bundle/direct/dresources/model_serving_endpoint.go b/bundle/direct/dresources/model_serving_endpoint.go index f9c0a2314ac..9ccc297539d 100644 --- a/bundle/direct/dresources/model_serving_endpoint.go +++ b/bundle/direct/dresources/model_serving_endpoint.go @@ -45,7 +45,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se if output == nil { return nil } - return &serving.AutoCaptureConfigInput{ + return &serving.AutoCaptureConfigInput{ // sdk:copy-from output CatalogName: output.CatalogName, SchemaName: output.SchemaName, TableNamePrefix: output.TableNamePrefix, @@ -57,7 +57,7 @@ func autoCaptureConfigOutputToInput(output *serving.AutoCaptureConfigOutput) *se func servedEntitiesOutputToInput(output []serving.ServedEntityOutput) []serving.ServedEntityInput { entities := make([]serving.ServedEntityInput, len(output)) for i, entity := range output { - entities[i] = serving.ServedEntityInput{ + entities[i] = serving.ServedEntityInput{ // sdk:copy-from entity EntityName: entity.EntityName, EntityVersion: entity.EntityVersion, EnvironmentVars: entity.EnvironmentVars, @@ -84,7 +84,7 @@ func configOutputToInput(output *serving.EndpointCoreConfigOutput) *serving.Endp if output == nil { return nil } - return &serving.EndpointCoreConfigInput{ + return &serving.EndpointCoreConfigInput{ // sdk:copy-skip Nested conversions and deprecated fields need manual review. AutoCaptureConfig: autoCaptureConfigOutputToInput(output.AutoCaptureConfig), ServedEntities: servedEntitiesOutputToInput(output.ServedEntities), ServedModels: nil, diff --git a/bundle/direct/dresources/pipeline.go b/bundle/direct/dresources/pipeline.go index 6f367d4e498..7ecb268d3c2 100644 --- a/bundle/direct/dresources/pipeline.go +++ b/bundle/direct/dresources/pipeline.go @@ -101,7 +101,7 @@ func makePipelineRemote(p *pipelines.GetPipelineResponse) *PipelineRemote { var createPipeline pipelines.CreatePipeline if p.Spec != nil { spec := p.Spec - createPipeline = pipelines.CreatePipeline{ + createPipeline = pipelines.CreatePipeline{ // sdk:copy-skip New fields need review to choose Spec or response-level sources. // Note: AllowDuplicateNames and DryRun are not in PipelineSpec, // they are request-only fields, so they stay at their zero values. AllowDuplicateNames: false, @@ -173,7 +173,7 @@ func (r *ResourcePipeline) DoUpdate(ctx context.Context, id string, config *Pipe return nil, nil } - request := pipelines.EditPipeline{ + request := pipelines.EditPipeline{ // sdk:copy-from config AllowDuplicateNames: config.AllowDuplicateNames, BudgetPolicyId: config.BudgetPolicyId, Catalog: config.Catalog, diff --git a/bundle/direct/dresources/quality_monitor.go b/bundle/direct/dresources/quality_monitor.go index c66fed4e0bb..02efa97eb0d 100644 --- a/bundle/direct/dresources/quality_monitor.go +++ b/bundle/direct/dresources/quality_monitor.go @@ -43,7 +43,7 @@ func (*ResourceQualityMonitor) PrepareState(input *resources.QualityMonitor) *Qu func (*ResourceQualityMonitor) RemapState(info *catalog.MonitorInfo) *QualityMonitorState { return &QualityMonitorState{ - CreateMonitor: catalog.CreateMonitor{ + CreateMonitor: catalog.CreateMonitor{ // sdk:copy-from info AssetsDir: info.AssetsDir, BaselineTableName: info.BaselineTableName, CustomMetrics: info.CustomMetrics, diff --git a/bundle/direct/dresources/registered_model.go b/bundle/direct/dresources/registered_model.go index 1a2fd468ebb..49f295b5a1b 100644 --- a/bundle/direct/dresources/registered_model.go +++ b/bundle/direct/dresources/registered_model.go @@ -25,7 +25,7 @@ func (*ResourceRegisteredModel) PrepareState(input *resources.RegisteredModel) * } func (*ResourceRegisteredModel) RemapState(model *catalog.RegisteredModelInfo) *catalog.CreateRegisteredModelRequest { - return &catalog.CreateRegisteredModelRequest{ + return &catalog.CreateRegisteredModelRequest{ // sdk:copy-from model CatalogName: model.CatalogName, Comment: model.Comment, Name: model.Name, diff --git a/bundle/direct/dresources/schema.go b/bundle/direct/dresources/schema.go index d80dac584bc..443497ce689 100644 --- a/bundle/direct/dresources/schema.go +++ b/bundle/direct/dresources/schema.go @@ -24,7 +24,7 @@ func (*ResourceSchema) PrepareState(input *resources.Schema) *catalog.CreateSche } func (*ResourceSchema) RemapState(info *catalog.SchemaInfo) *catalog.CreateSchema { - return &catalog.CreateSchema{ + return &catalog.CreateSchema{ // sdk:copy-from info CatalogName: info.CatalogName, Comment: info.Comment, CustomMaxRetentionHours: info.CustomMaxRetentionHours, diff --git a/bundle/direct/dresources/secret_scope.go b/bundle/direct/dresources/secret_scope.go index 7ad0b0d469a..60699477ee9 100644 --- a/bundle/direct/dresources/secret_scope.go +++ b/bundle/direct/dresources/secret_scope.go @@ -50,7 +50,7 @@ func (*ResourceSecretScope) New(client *databricks.WorkspaceClient) *ResourceSec } func (*ResourceSecretScope) PrepareState(input *resources.SecretScope) *SecretScopeConfig { - return &SecretScopeConfig{ + return &SecretScopeConfig{ // sdk:copy-skip Response-to-request field renames need manual mapping. Scope: input.Name, ScopeBackendType: input.BackendType, BackendAzureKeyvault: input.KeyvaultMetadata, @@ -60,7 +60,7 @@ func (*ResourceSecretScope) PrepareState(input *resources.SecretScope) *SecretSc } func (*ResourceSecretScope) RemapState(remote *SecretScopeRemote) *SecretScopeConfig { - return &SecretScopeConfig{ + return &SecretScopeConfig{ // sdk:copy-from remote Scope: remote.Scope, ScopeBackendType: remote.ScopeBackendType, BackendAzureKeyvault: remote.BackendAzureKeyvault, diff --git a/bundle/direct/dresources/sql_warehouse.go b/bundle/direct/dresources/sql_warehouse.go index f7b9ca09fbe..1d7e8218ec8 100644 --- a/bundle/direct/dresources/sql_warehouse.go +++ b/bundle/direct/dresources/sql_warehouse.go @@ -71,7 +71,7 @@ func (*ResourceSqlWarehouse) PrepareState(input *resources.SqlWarehouse) *SqlWar // RemapState maps the remote SqlWarehouseRemote to SqlWarehouseState for diff comparison. // Started is derived from warehouse state so the planner can detect start/stop changes. func (*ResourceSqlWarehouse) RemapState(warehouse *SqlWarehouseRemote) *SqlWarehouseState { - return &SqlWarehouseState{ + return &SqlWarehouseState{ // sdk:copy-from warehouse AutoStopMins: warehouse.AutoStopMins, Channel: warehouse.Channel, ClusterSize: warehouse.ClusterSize, @@ -137,7 +137,7 @@ func hasWarehouseChanges(entry *PlanEntry) bool { func (r *ResourceSqlWarehouse) DoUpdate(ctx context.Context, id string, config *SqlWarehouseState, entry *PlanEntry) (*SqlWarehouseRemote, error) { edited := hasWarehouseChanges(entry) if edited { - request := sql.EditWarehouseRequest{ + request := sql.EditWarehouseRequest{ // sdk:copy-from config AutoStopMins: config.AutoStopMins, Channel: config.Channel, ClusterSize: config.ClusterSize, diff --git a/bundle/direct/dresources/vector_search_endpoint.go b/bundle/direct/dresources/vector_search_endpoint.go index 12470872b62..54b9498161e 100644 --- a/bundle/direct/dresources/vector_search_endpoint.go +++ b/bundle/direct/dresources/vector_search_endpoint.go @@ -62,7 +62,7 @@ func (*ResourceVectorSearchEndpoint) PrepareState(input *resources.VectorSearchE } func (*ResourceVectorSearchEndpoint) RemapState(remote *VectorSearchEndpointRemote) *vectorsearch.CreateEndpoint { - return &vectorsearch.CreateEndpoint{ + return &vectorsearch.CreateEndpoint{ // sdk:copy-from remote Name: remote.Name, EndpointType: remote.EndpointType, BudgetPolicyId: remote.BudgetPolicyId, diff --git a/bundle/direct/dresources/vector_search_index.go b/bundle/direct/dresources/vector_search_index.go index 857e3fc44db..3757b5e997e 100644 --- a/bundle/direct/dresources/vector_search_index.go +++ b/bundle/direct/dresources/vector_search_index.go @@ -82,7 +82,7 @@ func (*ResourceVectorSearchIndex) PrepareState(input *resources.VectorSearchInde } func (*ResourceVectorSearchIndex) RemapState(remote *VectorSearchIndexRemote) *VectorSearchIndexState { - state := &VectorSearchIndexState{ + state := &VectorSearchIndexState{ // sdk:copy-from remote DeltaSyncIndexSpec: nil, // need to remap below DirectAccessIndexSpec: remote.DirectAccessIndexSpec, IndexSubtype: remote.IndexSubtype, @@ -93,7 +93,7 @@ func (*ResourceVectorSearchIndex) RemapState(remote *VectorSearchIndexRemote) *V EndpointUuid: remote.EndpointUuid, } if remote.DeltaSyncIndexSpec != nil { - state.DeltaSyncIndexSpec = &vectorsearch.DeltaSyncVectorIndexSpecRequest{ + state.DeltaSyncIndexSpec = &vectorsearch.DeltaSyncVectorIndexSpecRequest{ // sdk:copy-from remote.DeltaSyncIndexSpec ColumnsToIndex: remote.DeltaSyncIndexSpec.ColumnsToIndex, ColumnsToSync: remote.DeltaSyncIndexSpec.ColumnsToSync, EmbeddingSourceColumns: remote.DeltaSyncIndexSpec.EmbeddingSourceColumns, diff --git a/bundle/direct/dresources/volume.go b/bundle/direct/dresources/volume.go index 7cd1eaba1ce..fd38b8541e6 100644 --- a/bundle/direct/dresources/volume.go +++ b/bundle/direct/dresources/volume.go @@ -26,7 +26,7 @@ func (*ResourceVolume) PrepareState(input *resources.Volume) *catalog.CreateVolu } func (*ResourceVolume) RemapState(info *catalog.VolumeInfo) *catalog.CreateVolumeRequestContent { - return &catalog.CreateVolumeRequestContent{ + return &catalog.CreateVolumeRequestContent{ // sdk:copy-from info CatalogName: info.CatalogName, Comment: info.Comment, Name: info.Name, diff --git a/tools/go.mod b/tools/go.mod index 908ed616b92..d16d16d9335 100644 --- a/tools/go.mod +++ b/tools/go.mod @@ -7,6 +7,7 @@ toolchain go1.27.3 require ( github.com/stretchr/testify v1.12.1 golang.org/x/mod v0.41.0 + golang.org/x/tools v0.50.0 // BSD-3-Clause gopkg.in/yaml.v3 v3.0.1 ) @@ -220,7 +221,6 @@ require ( golang.org/x/telemetry v0.0.0-20260908163034-4bcc4b2ee518 // indirect golang.org/x/term v0.39.0 // indirect golang.org/x/text v0.41.0 // indirect - golang.org/x/tools v0.50.0 // indirect golang.org/x/vuln v1.3.0 // indirect google.golang.org/protobuf v1.36.12 // indirect gopkg.in/ini.v1 v1.67.0 // indirect diff --git a/tools/sdkfieldcopies/NOTICE b/tools/sdkfieldcopies/NOTICE new file mode 100644 index 00000000000..b2efd7b8bb3 --- /dev/null +++ b/tools/sdkfieldcopies/NOTICE @@ -0,0 +1,5 @@ +This development tool uses the following open source project, licensed under the BSD (3-clause) license: + +golang.org/x/tools - https://github.com/golang/tools +Copyright 2009 The Go Authors. +License - https://github.com/golang/tools/blob/master/LICENSE diff --git a/tools/sdkfieldcopies/README.md b/tools/sdkfieldcopies/README.md new file mode 100644 index 00000000000..4a6ceba1aa9 --- /dev/null +++ b/tools/sdkfieldcopies/README.md @@ -0,0 +1,102 @@ +# SDK field-copy fixer + +Run `./task fix-sdk-field-copies` from the repository root after an SDK bump. +`bump-sdk-post` runs it first, before generation checks and acceptance-output +updates. The fixer completes reviewed field-copy literals in +`bundle/direct/dresources`; other exhaustruct failures still require review. + +## What a verified copy means + +The annotation supplies the semantic intent: newly added compatible fields +belong to the existing copy from this particular source. For example: + +```go +request := sql.EditWarehouseRequest{ // sdk:copy-from config + // Existing assignments, including exceptions, remain here. +} +``` + +Place the directive on the literal's opening-brace line. The source must be an +identifier or selector chain, such as `config` or `remote.DeltaSyncIndexSpec`. +Each nested literal needs its own annotation; the tool does not infer sources. + +Use `// sdk:copy-skip ` on the opening-brace line to document an +intentionally manual mapping. This marker is explanatory: only `sdk:copy-from` +opts a literal into rewriting, and exhaustruct still checks skipped literals. + +For each missing destination field, the tool requires: + +- The same exported Go field name on an accessible, unambiguous source field. +- The same explicit, nonempty JSON field name, excluding `json:"-"` metadata. +- Identical Go types, without adding conversions or pointer dereferences. +- No source access through pointer embeddings that could introduce a nil panic. + +Existing source pointers and selector chains must already be valid at the copy +site. This is a reviewed precondition: the vector-index spec mapping, for +example, is inside an existing nil guard. Type matching does not prove nonnilness. + +Go 1.27 promoted destination keys are supported for partially populated value +embeddings. An assignment of the whole embedded value already carries its fields +and needs no promoted-key additions. Source lookup respects promotion and shadowing. + +Existing expressions stay intact: explicit zeros and nils, ID overrides, renamed +fields, enum casts, conversions, filters, and metadata handling. The tool never +invents `ForceSendFields`, update masks, zero defaults, or resource field policies. +It reports missing fields it cannot copy at annotated sites and leaves them for +lint and manual review. Unannotated literals remain manual too. Compilation or +directive errors stop the run before writes; proposed contents are type-checked +again before writing. + +These checks establish structural compatibility, not backend semantics. They do +not prove that an API accepts a field on update, permits clearing it, or treats +it as input rather than output. An annotation therefore requires a review of the +whole mapping, including how future fields should behave. + +## Audit and initial opt-ins + +The initial audit covered every registered direct resource and its population +paths. The 26 annotated literals below follow reviewed copy contracts. The +[resource guidelines](../../bundle/direct/dresources/README.md) give `RemapState` +a particularly clear contract: copy matching remote fields into the state subset; +API-specific reconstruction belongs in `DoRead`. + +| Resource or family | Annotated copies | Other population paths and exclusions | +| --- | --- | --- | +| Clusters | Remap; create and edit requests | Whole embedded preparation; read combines libraries, lifecycle, and lifted policy defaults. Resize/start/install operations remain selective. | +| Cluster policies | Remap; edit request | Preparation normalizes shadowing definition fields. Create forwards the whole SDK value. | +| Instance pools | Remap; edit request | Preparation/create use the whole SDK value. Planner rules still govern fields requiring recreation. | +| Jobs | Create request | Preparation/remap/reset use whole settings; read assembles settings and metadata. Existing nil ACL and its filter remain explicit. | +| Job runs | None | Mixed run/override/parameter sources, synthetic result and lifecycle, token replacement, and deliberate drift exclusions. | +| Pipelines | Edit request | Read combines spec with response-level Parameters/RunAs; whole embedded state also carries local cascade behavior. | +| SQL warehouses | Remap; edit request | Whole embedded preparation/create; read derives lifecycle. Existing WarehouseType enum conversions remain explicit. These two sites cover the mapping shape in PR #7155. | +| Model serving endpoints | Auto-capture and served-entity input helpers | Aggregate config uses conversions and deprecated fields; update requires paired set/unset behavior. Tags use deltas. | +| Catalogs, schemas, volumes | Each remap | Whole create config; update selects supported fields, identity/rename behavior, and verified clearable fields. | +| External locations | Remap | Update deliberately zeros output-only Effective* fields even though matching config fields exist. Rename and unsupported owner/isolation behavior stay manual. | +| Registered models | Remap | Update deliberately omits Owner despite its matching config field; rename is separate. Remap retains output metadata for drift rules. | +| MLflow models | Remap | Read wraps SDK output plus numeric ID; update response combines SDK fields, explicit zeros, and carried-forward ModelId. | +| Experiments | Prepare; remap | Create forwards config; update renames Name to NewName. Workspace-prefix normalization is separate. | +| Quality monitors | Nested SDK literal in remap | State shadows TableName for serialization. Update deliberately blanks failure metadata despite matching config fields. | +| Features | None | Whole SDK feature body; update overrides ID and forces Description with a description-only mask. | +| Database instances, database catalogs, synced database tables | None | Whole SDK objects inside request envelopes. Instance update separately overrides UID; deletion flags and waiters are operation-specific. | +| Postgres projects, branches, endpoints, databases, roles, catalogs, synced tables | None | Whole embedded specs already carry additions. Envelopes and bodies have different ID/parent responsibilities; optional specs, derived IDs, lifecycle flags, and masks are explicit. | +| Postgres snapshot schedules | None | Local state; derived branch/name; one update API handles create/update/delete using a fixed schedule-only mask. | +| Apps | None | Whole embedded App plus local config/lifecycle. Request builder clears SourceCodePath/GitSource because deployment uses a separate API; update has an explicit mask. | +| Alerts | None | Whole AlertV2 body inside request wrappers with explicit identity, mask, and purge behavior. | +| Dashboards | None | Draft/published sources, normalized parent, content conversion, publication state, selected request fields, and separate dataset selectors. | +| Genie spaces | None | Local config and converted content; update deliberately omits matching Etag to disable that concurrency precondition. | +| Secrets | None | PrepareState selectively copies the same SDK type while clearing output/effective fields; read normalizes effective values. Same name/type alone would copy unwanted output fields. | +| Secret scopes | Remap | Prepare/read rename scope/backend fields. Request-only InitialManagePrincipal remains explicitly empty on remap. | +| Secret-scope ACLs | None | Principal selection, permission conversion, parent references, and desired/current reconciliation. | +| Grants | None | Parsed identity, pagination, normalization, and privilege additions/removals assembled from desired and remote state. | +| Permissions | None | Typed/reflected input, parent references, inherited filtering, multiple sources, and Level-to-PermissionLevel rename. | +| Vector search endpoints | Remap | Remote ID/QPS reconstruction; update uses separate policy and target-QPS APIs. Request-only UsagePolicyId stays empty on remap. | +| Vector search indexes | Remap; guarded delta-sync spec conversion | Whole create config; separately resolved endpoint UUID and response/request spec conversion. The nested source remains behind its nil guard. | +| MCP services, model provider services, model services | None | Derived parent/ID and deliberately limited Comment/Config bodies; explicit output zeros and omitted etag preconditions. | +| Internal immutable snapshots | None | Entirely local state, derived paths, local-only fields, and upload results; no SDK subset requiring completion. | +| Alert-file loader outside direct resources | None | Also covered by exhaustruct, but YAML and file supply different authoritative fields; line-to-text transformations and source policy stay manual. | + +The exclusions are deliberate. Source assignment counts, matching struct types, +or a large number of existing same-name copies cannot establish a copy contract: +external locations, secrets, apps, and Genie spaces all contain same-name fields +that their write paths intentionally omit. Existing masks and resource YAML also +remain independent requirements when adding support for a new SDK field. diff --git a/tools/sdkfieldcopies/fix/fix.go b/tools/sdkfieldcopies/fix/fix.go new file mode 100644 index 00000000000..da9de4b0f29 --- /dev/null +++ b/tools/sdkfieldcopies/fix/fix.go @@ -0,0 +1,305 @@ +// Package fix completes reviewed field-copy mappings after an SDK update. +package fix + +import ( + "bytes" + "context" + "errors" + "fmt" + "go/ast" + "go/format" + "go/parser" + "go/types" + "io" + "os" + "path/filepath" + "reflect" + "slices" + "strings" + + "golang.org/x/tools/go/packages" +) + +const directive = "// sdk:copy-from " + +// Run fills missing fields in annotated direct-resource literals. Unmatched +// fields are reported for review and remain exhaustruct errors; they do not +// prevent the rest of bump-sdk-post from regenerating artifacts. Invalid Go or +// directives fail before any file is written. +func Run(ctx context.Context, root string, out io.Writer) error { + cfg := &packages.Config{Context: ctx, Dir: root, Mode: packages.LoadSyntax} + pkgs, err := load(cfg) + if err != nil { + return err + } + changes := make(map[string][]byte) + var report bytes.Buffer + for _, pkg := range pkgs { + for _, file := range pkg.Syntax { + name := pkg.Fset.PositionFor(file.Pos(), false).Filename + original, err := os.ReadFile(name) + if err != nil { + return fmt.Errorf("read %s: %w", filepath.ToSlash(name), err) + } + updated, err := rewrite(pkg, file, original, &report) + if err != nil { + return fmt.Errorf("%s: %w", filepath.ToSlash(name), err) + } + if !bytes.Equal(updated, original) { + changes[name] = updated + } + } + } + if len(changes) > 0 { + // Validate the entire proposed package, including interactions between + // nested literals, before touching any source file. + cfg.Overlay = changes + if _, err := load(cfg); err != nil { + return fmt.Errorf("validate proposed field copies: %w", err) + } + } + var names []string + for name := range changes { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + if err := os.WriteFile(name, changes[name], 0); err != nil { + return fmt.Errorf("write %s: %w", filepath.ToSlash(name), err) + } + } + _, err = report.WriteTo(out) + return err +} + +func load(cfg *packages.Config) ([]*packages.Package, error) { + pkgs, err := packages.Load(cfg, "./bundle/direct/dresources") + if err != nil { + return nil, fmt.Errorf("load direct resources: %w", err) + } + var problems []string + packages.Visit(pkgs, nil, func(pkg *packages.Package) { + for _, err := range pkg.Errors { + problems = append(problems, err.Error()) + } + }) + if len(problems) > 0 { + slices.Sort(problems) + return nil, fmt.Errorf("type-check direct resources:\n%s", strings.Join(problems, "\n")) + } + return pkgs, nil +} + +type insertion struct { + offset int + text string +} + +func rewrite(pkg *packages.Package, file *ast.File, original []byte, report io.Writer) ([]byte, error) { + markers := make(map[int]*ast.Comment) + for _, group := range file.Comments { + for _, comment := range group.List { + if strings.HasPrefix(comment.Text, strings.TrimSpace(directive)) { + markers[pkg.Fset.PositionFor(comment.Pos(), false).Line] = comment + } + } + } + if len(markers) == 0 { + return original, nil + } + var edits []insertion + var rewriteErr error + ast.Inspect(file, func(node ast.Node) bool { + if rewriteErr != nil { + return false + } + lit, ok := node.(*ast.CompositeLit) + if !ok { + return true + } + line := pkg.Fset.PositionFor(lit.Lbrace, false).Line + marker, ok := markers[line] + if !ok || marker.Pos() < lit.Lbrace || marker.End() > lit.Rbrace { + return true + } + brace := pkg.Fset.PositionFor(lit.Lbrace, false).Offset + comment := pkg.Fset.PositionFor(marker.Pos(), false).Offset + if len(bytes.TrimSpace(original[brace+1:comment])) != 0 { + return true + } + delete(markers, line) + lines, err := copies(pkg, lit, marker, report) + if err != nil { + rewriteErr = fmt.Errorf("line %d: %w", line, err) + return false + } + if len(lines) > 0 { + // Insert after the directive line. Existing comments retain their + // position relative to the original fields, including zero-value groups. + end := pkg.Fset.PositionFor(marker.End(), false).Offset + offset := end + bytes.IndexByte(original[end:], '\n') + 1 + edits = append(edits, insertion{offset: offset, text: strings.Join(lines, "\n") + "\n"}) + } + return true + }) + if rewriteErr != nil { + return nil, rewriteErr + } + if len(markers) != 0 { + return nil, fmt.Errorf("%s directive must follow a struct literal's opening brace on the same line", strings.TrimSpace(directive)) + } + if len(edits) == 0 { + return original, nil + } + slices.SortFunc(edits, func(a, b insertion) int { return b.offset - a.offset }) + updated := bytes.Clone(original) + for _, edit := range edits { + updated = slices.Insert(updated, edit.offset, []byte(edit.text)...) + } + formatted, err := format.Source(updated) + if err != nil { + return nil, fmt.Errorf("format field copies: %w", err) + } + return formatted, nil +} + +func copies(pkg *packages.Package, lit *ast.CompositeLit, marker *ast.Comment, report io.Writer) ([]string, error) { + source := strings.TrimSpace(strings.TrimPrefix(marker.Text, directive)) + expr, err := parser.ParseExpr(source) + if err != nil || !isSelector(expr) { + return nil, fmt.Errorf("copy source must be an identifier or selector chain, got %q", source) + } + value, err := types.Eval(pkg.Fset, pkg.Types, lit.Pos(), source) + if err != nil { + return nil, fmt.Errorf("resolve copy source %q: %w", source, err) + } + if !value.IsValue() || asStruct(value.Type) == nil { + return nil, fmt.Errorf("copy source %q must be a struct or pointer to a struct", source) + } + destination := pkg.TypesInfo.TypeOf(lit) + st, ok := destination.Underlying().(*types.Struct) + if !ok { + return nil, errors.New("copy directive requires a struct literal") + } + var present [][]int + for _, element := range lit.Elts { + kv, ok := element.(*ast.KeyValueExpr) + if !ok { + return nil, errors.New("copy directive requires a keyed struct literal") + } + key := kv.Key.(*ast.Ident) + _, index, _ := types.LookupFieldOrMethod(destination, false, pkg.Types, key.Name) + present = append(present, index) + } + fields := missingFields(st, nil, present) + slices.SortFunc(fields, func(a, b field) int { return strings.Compare(a.variable.Name(), b.variable.Name()) }) + var lines []string + for _, field := range fields { + name := field.variable.Name() + obj, _, _ := types.LookupFieldOrMethod(destination, false, pkg.Types, name) + reason := "destination field is shadowed or ambiguous" + if obj == field.variable { + reason = copyReason(value.Type, field, pkg.Types) + } + position := pkg.Fset.PositionFor(lit.Lbrace, false) + if reason != "" { + fmt.Fprintf(report, "%s:%d: leave %s for review: %s\n", filepath.Base(position.Filename), position.Line, name, reason) + continue + } + lines = append(lines, fmt.Sprintf("%s: %s.%s,", name, source, name)) + fmt.Fprintf(report, "%s:%d: add %s: %s.%s\n", filepath.Base(position.Filename), position.Line, name, source, name) + } + return lines, nil +} + +// isSelector excludes calls, indexes and other expressions with evaluation +// behavior beyond the reviewed source variable or selector chain. +func isSelector(expr ast.Expr) bool { + switch expr := expr.(type) { + case *ast.Ident: + return true + case *ast.SelectorExpr: + return isSelector(expr.X) + default: + return false + } +} + +type field struct { + variable *types.Var + tag string +} + +// missingFields follows only value embeddings already partially populated by +// promoted keys. Assigning the embedded field itself initializes all its fields. +func missingFields(st *types.Struct, prefix []int, present [][]int) []field { + var missing []field + for i := range st.NumFields() { + path := append(slices.Clone(prefix), i) + initialized, partial := false, false + for _, key := range present { + if slices.Equal(key, path) { + initialized = true + } + if len(key) > len(path) && slices.Equal(key[:len(path)], path) { + partial = true + } + } + if initialized { + continue + } + f := st.Field(i) + if partial { + if embedded, ok := f.Type().Underlying().(*types.Struct); ok { + missing = append(missing, missingFields(embedded, path, present)...) + continue + } + } + missing = append(missing, field{variable: f, tag: st.Tag(i)}) + } + return missing +} + +func copyReason(source types.Type, destination field, pkg *types.Package) string { + name := destination.variable.Name() + if !destination.variable.Exported() || name == "ForceSendFields" || name == "UpdateMask" { + return "metadata or unexported field requires an explicit value" + } + wireName := jsonName(destination.tag) + if wireName == "" || wireName == "-" { + return "destination has no JSON payload field name" + } + obj, index, _ := types.LookupFieldOrMethod(source, true, pkg, name) + f, ok := obj.(*types.Var) + if !ok || !f.IsField() { + return "source field is absent or ambiguous" + } + if !types.Identical(f.Type(), destination.variable.Type()) { + return "source and destination types differ" + } + st := asStruct(source) + for _, i := range index[:len(index)-1] { + embedded, ok := st.Field(i).Type().Underlying().(*types.Struct) + if !ok { + return "source field crosses a pointer embedding" + } + st = embedded + } + if jsonName(st.Tag(index[len(index)-1])) != wireName { + return "source and destination JSON field names differ" + } + return "" +} + +func asStruct(t types.Type) *types.Struct { + if p, ok := t.Underlying().(*types.Pointer); ok { + t = p.Elem() + } + st, _ := t.Underlying().(*types.Struct) + return st +} + +func jsonName(tag string) string { + name, _, _ := strings.Cut(reflect.StructTag(tag).Get("json"), ",") + return name +} diff --git a/tools/sdkfieldcopies/fix/fix_test.go b/tools/sdkfieldcopies/fix/fix_test.go new file mode 100644 index 00000000000..f5930d868dc --- /dev/null +++ b/tools/sdkfieldcopies/fix/fix_test.go @@ -0,0 +1,461 @@ +package fix_test + +import ( + "bytes" + "go/format" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/databricks/cli/tools/sdkfieldcopies/fix" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const fixtureFile = "bundle/direct/dresources/resource.go" + +func formatFixture(t *testing.T, source string) string { + t.Helper() + formatted, err := format.Source([]byte(source)) + require.NoError(t, err) + return string(formatted) +} + +func writeFixture(t *testing.T, source string) string { + t.Helper() + root := t.TempDir() + filename := filepath.Join(root, filepath.FromSlash(fixtureFile)) + require.NoError(t, os.MkdirAll(filepath.Dir(filename), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "go.mod"), []byte("module example.test/sdk-copy-fixture\n\ngo 1.27.0\n"), 0o644)) + require.NoError(t, os.WriteFile(filename, []byte(formatFixture(t, source)), 0o644)) + return root +} + +func readFixture(t *testing.T, root string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(fixtureFile))) + require.NoError(t, err) + return string(data) +} + +func TestRunWarehouseFieldsAndIdempotency(t *testing.T) { + const source = `package dresources + +type Warehouse struct { + Name string "json:\"name\"" + ReydenClusterSizeMultiplier float64 "json:\"reyden_cluster_size_multiplier,omitempty\"" + MaxDbu int "json:\"max_dbu,omitempty\"" + CustomerVersionLabel string "json:\"customer_version_label,omitempty\"" + ForceSendFields []string "json:\"-\"" +} +type EditWarehouse Warehouse + +func remap(remote *Warehouse) *Warehouse { + return &Warehouse{ // sdk:copy-from remote + // Preserve remote name and the explicit transport policy. + Name: remote.Name, + ForceSendFields: nil, + } +} + +func update(config *Warehouse) EditWarehouse { + return EditWarehouse{ // sdk:copy-from config + Name: config.Name, + ForceSendFields: config.ForceSendFields, + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + + expected := source + for _, name := range []string{"remote", "config"} { + marker := "// sdk:copy-from " + name + "\n" + expected = strings.Replace(expected, marker, marker+ + "CustomerVersionLabel: "+name+".CustomerVersionLabel,\n"+ + "MaxDbu: "+name+".MaxDbu,\n"+ + "ReydenClusterSizeMultiplier: "+name+".ReydenClusterSizeMultiplier,\n", 1) + } + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) + + first := readFixture(t, root) + output.Reset() + require.NoError(t, fix.Run(t.Context(), root, &output)) + assert.Equal(t, first, readFixture(t, root)) +} + +func TestRunPreservesExplicitMappingsAndUnmarkedLiterals(t *testing.T) { + //nolint:dupword // Go fixture fields intentionally share names with their types. + const source = `package dresources + +type Kind string +type Options struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} +type Input struct { + Name string "json:\"name\"" + Added string "json:\"added\"" + Disabled bool "json:\"disabled\"" + Count int "json:\"count\"" + Pointer *Options "json:\"pointer\"" + Options Options "json:\"options\"" + Kind string "json:\"kind\"" + ForceSendFields []string "json:\"-\"" +} +type Request struct { + Name string "json:\"name\"" + Added string "json:\"added\"" + Disabled bool "json:\"disabled\"" + Count int "json:\"count\"" + Pointer *Options "json:\"pointer\"" + Options Options "json:\"options\"" + Kind Kind "json:\"kind\"" + ForceSendFields []string "json:\"-\"" +} +func filter(fields []string) []string { return fields } + +func mapped(config, remote *Input) Request { + return Request{ // sdk:copy-from config + Name: remote.Name, // This existing field has a different source. + Disabled: false, + Count: 0, + Pointer: nil, + Options: Options{Name: "deliberate"}, // Nested literal has no directive. + Kind: Kind(remote.Kind), + ForceSendFields: filter(remote.ForceSendFields), + } +} + +func selective(config *Input) Request { + return Request{ + Name: config.Name, + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from config\n", "// sdk:copy-from config\nAdded: config.Added,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) +} + +func TestRunRequiresMatchingNamesTypesAndWireFields(t *testing.T) { + //nolint:dupword // Go fixture fields intentionally share names with their types. + const source = `package dresources + +type First string +type Second string +type Alias = string +type Source struct { + Name string "json:\"name\"" + Eligible string "json:\"eligible\"" + Alias string "json:\"alias\"" + Mismatch string "json:\"mismatch\"" + Assignable string "json:\"assignable\"" + Named First "json:\"named\"" + WireMismatch string "json:\"source_name\"" + NoTag string + EmptyTag string "json:\",omitempty\"" + Ignored string "json:\"-\"" + ForceSendFields []string "json:\"force_send_fields\"" + UpdateMask string "json:\"update_mask\"" + hidden string "json:\"hidden\"" +} +type Destination struct { + Name string "json:\"name\"" + Eligible string "json:\"eligible,omitempty\"" + Alias Alias "json:\"alias\"" + Mismatch int "json:\"mismatch\"" + Assignable any "json:\"assignable\"" + Named Second "json:\"named\"" + WireMismatch string "json:\"destination_name\"" + NoTag string + EmptyTag string "json:\",omitempty\"" + Ignored string "json:\"-\"" + Missing string "json:\"missing\"" + ForceSendFields []string "json:\"force_send_fields\"" + UpdateMask string "json:\"update_mask\"" + hidden string "json:\"hidden\"" +} + +func remap(src *Source) Destination { + return Destination{ // sdk:copy-from src + Name: src.Name, + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from src\n", "// sdk:copy-from src\nAlias: src.Alias,\nEligible: src.Eligible,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) + for _, field := range []string{"Mismatch", "Assignable", "Named", "WireMismatch", "NoTag", "EmptyTag", "Missing"} { + assert.Contains(t, output.String(), field) + } +} + +func TestRunPromotedFieldsAndWholeEmbeddings(t *testing.T) { + const source = `package dresources + +type Common struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} +type Source struct { Common } +type Destination struct { + Common + Id string "json:\"id\"" +} + +func partial(src *Source) Destination { + return Destination{ // sdk:copy-from src + Name: src.Name, + Id: "", + } +} + +func whole(src *Source) Destination { + return Destination{ // sdk:copy-from src + Common: src.Common, + Id: "", + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from src\n", "// sdk:copy-from src\nAdded: src.Added,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) +} + +func TestRunRejectsUnsafeSourcePromotions(t *testing.T) { + for _, tc := range []struct { + name string + source string + }{ + { + name: "shadowed field", + source: `type Source struct { + Common + Name string "json:\"name\"" + Added int "json:\"added\"" + }`, + }, + { + name: "shadowed by method", + source: `type Source struct { + Common + Name string "json:\"name\"" + } + func (Source) Added() string { return "" }`, + }, + { + name: "ambiguous field", + source: `type Other Common + type Source struct { + Common + Other + Name string "json:\"name\"" + }`, + }, + { + name: "embedded pointer", + source: `type Source struct { + *Common + Name string "json:\"name\"" + }`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + source := `package dresources + +type Common struct { Added string "json:\"added\"" } +type Destination struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} +` + tc.source + ` + +func remap(src *Source) Destination { + return Destination{ // sdk:copy-from src + Name: src.Name, + } +} +` + root := writeFixture(t, source) + before := readFixture(t, root) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + assert.Equal(t, before, readFixture(t, root)) + assert.Contains(t, output.String(), "Added") + }) + } +} + +func TestRunGuardedSelectorSource(t *testing.T) { + const source = `package dresources + +type Spec struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} +type Response struct { Spec *Spec } +type Request Spec + +func remap(remote *Response) *Request { + if remote.Spec == nil { + return nil + } + return &Request{ // sdk:copy-from remote.Spec + Name: remote.Spec.Name, + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from remote.Spec\n", "// sdk:copy-from remote.Spec\nAdded: remote.Spec.Added,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) +} + +func TestRunErrorsLeaveAllFilesUnchanged(t *testing.T) { + const source = `package dresources + +type Request struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} + +func remap(src *Request) Request { + return Request{ // sdk:copy-from src + Name: src.Name, + } +} +` + for _, tc := range []struct { + name string + invalid string + }{ + { + name: "missing source", + invalid: `func broken(src *Request) Request { + return Request{ // sdk:copy-from + Name: src.Name, + } + }`, + }, + { + name: "source call", + invalid: `func broken(src func() *Request) Request { + return Request{ // sdk:copy-from src() + Name: "", + } + }`, + }, + { + name: "dangling directive", + invalid: `// sdk:copy-from src + var unrelated = 1`, + }, + { + name: "package error", + invalid: `var broken = undefined`, + }, + { + name: "proposed edit introduces initialization cycle", + invalid: `var src = Request{ // sdk:copy-from src + Name: "", + }`, + }, + } { + t.Run(tc.name, func(t *testing.T) { + root := writeFixture(t, source) + before := readFixture(t, root) + invalid := formatFixture(t, "package dresources\n\n"+tc.invalid) + filename := filepath.Join(root, "bundle", "direct", "dresources", "invalid.go") + require.NoError(t, os.WriteFile(filename, []byte(invalid), 0o644)) + var output bytes.Buffer + require.Error(t, fix.Run(t.Context(), root, &output)) + assert.Equal(t, before, readFixture(t, root)) + after, err := os.ReadFile(filename) + require.NoError(t, err) + assert.Equal(t, invalid, string(after)) + }) + } +} + +func TestRunBindsDirectiveToInnerLiteralOnSameLine(t *testing.T) { + //nolint:dupword // Go fixture fields intentionally share names with their types. + const source = `package dresources + +type Inner struct { + Name string "json:\"name\"" + Added string "json:\"inner_added\"" +} +type Outer struct { + Inner Inner "json:\"inner\"" + Added string "json:\"added\"" +} + +func remap(src *Inner) Outer { + return Outer{Inner: Inner{ // sdk:copy-from src + Name: src.Name, + }} +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from src\n", "// sdk:copy-from src\nAdded: src.Added,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) +} + +func TestRunUsesPhysicalPositionsWithLineDirective(t *testing.T) { + const source = `package dresources + +type Request struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} + +//line elsewhere.go:900 +func remap(src *Request) Request { + return Request{ // sdk:copy-from src + Name: src.Name, + } +} +` + root := writeFixture(t, source) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + expected := strings.Replace(source, "// sdk:copy-from src\n", "// sdk:copy-from src\nAdded: src.Added,\n", 1) + assert.Equal(t, formatFixture(t, expected), readFixture(t, root)) + assert.Contains(t, output.String(), "resource.go:") + assert.NotContains(t, output.String(), "elsewhere.go:") +} + +func TestRunLeavesDestinationMethodCollisionForReview(t *testing.T) { + const source = `package dresources + +type Fields struct { + Name string "json:\"name\"" + Added string "json:\"added\"" +} +type Destination struct { Fields } +func (Destination) Added() string { return "" } + +func remap(src *Fields) Destination { + return Destination{ // sdk:copy-from src + Name: src.Name, + } +} +` + root := writeFixture(t, source) + before := readFixture(t, root) + var output bytes.Buffer + require.NoError(t, fix.Run(t.Context(), root, &output)) + assert.Equal(t, before, readFixture(t, root)) +} diff --git a/tools/sdkfieldcopies/main.go b/tools/sdkfieldcopies/main.go new file mode 100644 index 00000000000..2d804c7e507 --- /dev/null +++ b/tools/sdkfieldcopies/main.go @@ -0,0 +1,20 @@ +// Command sdkfieldcopies completes explicitly opted-in SDK field mappings. +package main + +import ( + "context" + "flag" + "fmt" + "os" + + "github.com/databricks/cli/tools/sdkfieldcopies/fix" +) + +func main() { + root := flag.String("root", "..", "CLI repository root") + flag.Parse() + if err := fix.Run(context.Background(), *root, os.Stdout); err != nil { + fmt.Fprintln(os.Stderr, "sdkfieldcopies:", err) + os.Exit(1) + } +}