From d90a40fcec00692f1407248b217f855fbe8be44f Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 26 May 2021 10:27:16 +0100 Subject: [PATCH] Use custom Copy functions instead of reflection (#513) --- go.mod | 5 +- go.sum | 8 +- internal/state/copiers.go | 44 ---------- internal/state/module.go | 87 +++++++++++++++++-- internal/state/provider_schema.go | 30 ++++--- internal/state/provider_schema_test.go | 10 +-- internal/terraform/datadir/module_manifest.go | 18 ++++ 7 files changed, 119 insertions(+), 83 deletions(-) delete mode 100644 internal/state/copiers.go diff --git a/go.mod b/go.mod index 657a3d591..1263a40f8 100644 --- a/go.mod +++ b/go.mod @@ -11,15 +11,14 @@ require ( github.com/hashicorp/go-memdb v1.3.2 github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-version v1.3.0 - github.com/hashicorp/hcl-lang v0.0.0-20210514214009-15d3afdead68 + github.com/hashicorp/hcl-lang v0.0.0-20210522074354-f7480edf31b5 github.com/hashicorp/hcl/v2 v2.10.0 github.com/hashicorp/terraform-exec v0.13.3 github.com/hashicorp/terraform-json v0.11.0 github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 - github.com/hashicorp/terraform-schema v0.0.0-20210514214427-b947185f165d + github.com/hashicorp/terraform-schema v0.0.0-20210522075401-0f5e258f5e97 github.com/mh-cbon/go-fmt-fail v0.0.0-20160815164508-67765b3fbcb5 github.com/mitchellh/cli v1.1.2 - github.com/mitchellh/copystructure v1.2.0 github.com/mitchellh/go-homedir v1.1.0 github.com/mitchellh/mapstructure v1.4.1 github.com/pmezard/go-difflib v1.0.0 diff --git a/go.sum b/go.sum index c5ef7b156..c29e87073 100644 --- a/go.sum +++ b/go.sum @@ -189,8 +189,8 @@ github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uG github.com/hashicorp/hcl v0.0.0-20170504190234-a4b07c25de5f/go.mod h1:oZtUIOe8dh44I2q6ScRibXws4Ajl+d+nod3AaR9vL5w= github.com/hashicorp/hcl v1.0.0 h1:0Anlzjpi4vEasTeNFn2mLJgTSwt0+6sfsiTG8qcWGx4= github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ= -github.com/hashicorp/hcl-lang v0.0.0-20210514214009-15d3afdead68 h1:Y5HxVHg9bKiS8Q4Q3dHOdETtF4DEmKNVdBh93o5K1L0= -github.com/hashicorp/hcl-lang v0.0.0-20210514214009-15d3afdead68/go.mod h1:7BtIzYAy75UR501SFkNjL98xMZbvn5Vc7bj+dOgcH70= +github.com/hashicorp/hcl-lang v0.0.0-20210522074354-f7480edf31b5 h1:lgywSdFExtTcqjaenkU2xhnbmtYLJIW+Ch3qQW0z6Jg= +github.com/hashicorp/hcl-lang v0.0.0-20210522074354-f7480edf31b5/go.mod h1:yPc3ggegh0njWLfIBPbmTk6a5T/vJVsMm4z6IuEgePU= github.com/hashicorp/hcl/v2 v2.0.0/go.mod h1:oVVDG71tEinNGYCxinCYadcmKU9bglqW9pV3txagJ90= github.com/hashicorp/hcl/v2 v2.10.0 h1:1S1UnuhDGlv3gRFV4+0EdwB+znNP5HmcGbIqwnSCByg= github.com/hashicorp/hcl/v2 v2.10.0/go.mod h1:FwWsfWEjyV/CMj8s/gqAuiviY72rJ1/oayI9WftqcKg= @@ -207,8 +207,8 @@ github.com/hashicorp/terraform-json v0.11.0 h1:4zDqqW2F3kOysORIaYKFGgWDYIRA3hwqx github.com/hashicorp/terraform-json v0.11.0/go.mod h1:pmbq9o4EuL43db5+0ogX10Yofv1nozM+wskr/bGFJpI= github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896 h1:1FGtlkJw87UsTMg5s8jrekrHmUPUJaMcu6ELiVhQrNw= github.com/hashicorp/terraform-registry-address v0.0.0-20210412075316-9b2996cce896/go.mod h1:bzBPnUIkI0RxauU8Dqo+2KrZZ28Cf48s8V6IHt3p4co= -github.com/hashicorp/terraform-schema v0.0.0-20210514214427-b947185f165d h1:XFxb7PJ76yRdoo/IINuh2ASgujNL8jlhaeulXVbDVUA= -github.com/hashicorp/terraform-schema v0.0.0-20210514214427-b947185f165d/go.mod h1:qzSjebjYVlcz56pBd2It3XmkOywXHNjnC/420WHSdaU= +github.com/hashicorp/terraform-schema v0.0.0-20210522075401-0f5e258f5e97 h1:ajIMzYZuMyoTVuUvfeXR9pNCJ52Wk6W9ukxYPwtaV3c= +github.com/hashicorp/terraform-schema v0.0.0-20210522075401-0f5e258f5e97/go.mod h1:hXxoH61q0kSvc4vlCxHEmV47u9Gp/1mXnuI7Yhz7jsQ= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/huandu/xstrings v1.3.2 h1:L18LIDzqlW6xN2rEkpdV8+oL/IXWJ1APd+vsdYy4Wdw= diff --git a/internal/state/copiers.go b/internal/state/copiers.go deleted file mode 100644 index cceccf43e..000000000 --- a/internal/state/copiers.go +++ /dev/null @@ -1,44 +0,0 @@ -package state - -import ( - "reflect" - - "github.com/hashicorp/go-version" - "github.com/hashicorp/hcl/v2" - "github.com/hashicorp/terraform-ls/internal/terraform/datadir" - "github.com/mitchellh/copystructure" - "github.com/zclconf/go-cty/cty" -) - -var copiers = map[reflect.Type]copystructure.CopierFunc{ - reflect.TypeOf(cty.NilType): ctyTypeCopier, - reflect.TypeOf(cty.Value{}): ctyValueCopier, - reflect.TypeOf(version.Version{}): versionCopier, - reflect.TypeOf(version.Constraint{}): constraintCopier, - reflect.TypeOf(datadir.ModuleManifest{}): modManifestCopier, - reflect.TypeOf(hcl.File{}): hclFileCopier, -} - -func ctyTypeCopier(v interface{}) (interface{}, error) { - return v.(cty.Type), nil -} - -func ctyValueCopier(v interface{}) (interface{}, error) { - return v.(cty.Value), nil -} - -func versionCopier(v interface{}) (interface{}, error) { - return v.(version.Version), nil -} - -func constraintCopier(v interface{}) (interface{}, error) { - return v.(version.Constraint), nil -} - -func modManifestCopier(v interface{}) (interface{}, error) { - return v.(datadir.ModuleManifest), nil -} - -func hclFileCopier(v interface{}) (interface{}, error) { - return v.(hcl.File), nil -} diff --git a/internal/state/module.go b/internal/state/module.go index 2cbabe43e..6bae9a74e 100644 --- a/internal/state/module.go +++ b/internal/state/module.go @@ -7,7 +7,6 @@ import ( "github.com/hashicorp/hcl/v2" tfaddr "github.com/hashicorp/terraform-registry-address" tfmod "github.com/hashicorp/terraform-schema/module" - "github.com/mitchellh/copystructure" "github.com/hashicorp/terraform-ls/internal/terraform/datadir" op "github.com/hashicorp/terraform-ls/internal/terraform/module/operation" @@ -19,6 +18,30 @@ type ModuleMetadata struct { ProviderRequirements map[tfaddr.Provider]version.Constraints } +func (mm ModuleMetadata) Copy() ModuleMetadata { + newMm := ModuleMetadata{ + // version.Constraints is practically immutable once parsed + CoreRequirements: mm.CoreRequirements, + } + + if mm.ProviderReferences != nil { + newMm.ProviderReferences = make(map[tfmod.ProviderRef]tfaddr.Provider, len(mm.ProviderReferences)) + for ref, provider := range mm.ProviderReferences { + newMm.ProviderReferences[ref] = provider + } + } + + if mm.ProviderRequirements != nil { + newMm.ProviderRequirements = make(map[tfaddr.Provider]version.Constraints, len(mm.ProviderRequirements)) + for provider, vc := range mm.ProviderRequirements { + // version.Constraints is never mutated in this context + newMm.ProviderRequirements[provider] = vc + } + } + + return newMm +} + type Module struct { Path string @@ -48,6 +71,59 @@ type Module struct { Diagnostics map[string]hcl.Diagnostics } +func (m *Module) Copy() *Module { + if m == nil { + return nil + } + newMod := &Module{ + Path: m.Path, + + ModManifest: m.ModManifest.Copy(), + ModManifestErr: m.ModManifestErr, + ModManifestState: m.ModManifestState, + + // version.Version is practically immutable once parsed + TerraformVersion: m.TerraformVersion, + TerraformVersionErr: m.TerraformVersionErr, + TerraformVersionState: m.TerraformVersionState, + + ProviderSchemaErr: m.ProviderSchemaErr, + ProviderSchemaState: m.ProviderSchemaState, + + References: m.References.Copy(), + ReferencesErr: m.ReferencesErr, + ReferencesState: m.ReferencesState, + + ParsingErr: m.ParsingErr, + ParsingState: m.ParsingState, + + Meta: m.Meta.Copy(), + MetaErr: m.MetaErr, + MetaState: m.MetaState, + } + + if m.ParsedFiles != nil { + newMod.ParsedFiles = make(map[string]*hcl.File, len(m.ParsedFiles)) + for name, f := range m.ParsedFiles { + // hcl.File is practically immutable once it comes out of parser + newMod.ParsedFiles[name] = f + } + } + + if m.Diagnostics != nil { + newMod.Diagnostics = make(map[string]hcl.Diagnostics, len(m.Diagnostics)) + for name, diags := range m.Diagnostics { + newMod.Diagnostics[name] = make(hcl.Diagnostics, len(diags)) + for i, diag := range diags { + // hcl.Diagnostic is practically immutable once it comes out of parser + newMod.Diagnostics[name][i] = diag + } + } + } + + return newMod +} + func newModule(modPath string) *Module { return &Module{ Path: modPath, @@ -136,14 +212,7 @@ func moduleCopyByPath(txn *memdb.Txn, path string) (*Module, error) { return nil, err } - mCopy, err := copystructure.Config{ - Copiers: copiers, - }.Copy(mod) - if err != nil { - return nil, err - } - - return mCopy.(*Module), nil + return mod.Copy(), nil } func (s *ModuleStore) List() ([]*Module, error) { diff --git a/internal/state/provider_schema.go b/internal/state/provider_schema.go index b62cadb0f..7c5525fc0 100644 --- a/internal/state/provider_schema.go +++ b/internal/state/provider_schema.go @@ -8,7 +8,6 @@ import ( "github.com/hashicorp/go-version" "github.com/hashicorp/terraform-registry-address" tfschema "github.com/hashicorp/terraform-schema/schema" - "github.com/mitchellh/copystructure" ) type ProviderSchema struct { @@ -19,6 +18,19 @@ type ProviderSchema struct { Schema *tfschema.ProviderSchema } +func (ps *ProviderSchema) Copy() *ProviderSchema { + if ps == nil { + return nil + } + + return &ProviderSchema{ + Address: ps.Address, + Version: ps.Version, // version.Version is immutable by design + Source: ps.Source, + Schema: ps.Schema.Copy(), + } +} + type ProviderSchemaIterator struct { it memdb.ResultIterator } @@ -58,16 +70,12 @@ func updateProviderVersions(txn *memdb.Txn, modPath string, pv map[tfaddr.Provid versionedPs := obj.(*ProviderSchema) if versionedPs.Schema != nil { - psRawCopy, err := copystructure.Config{ - Copiers: copiers, - }.Copy(versionedPs) - psCopy := psRawCopy.(*ProviderSchema) - _, err = txn.DeleteAll(providerSchemaTableName, "id_prefix", pAddr, src) if err != nil { return fmt.Errorf("unable to delete provider schema: %w", err) } + psCopy := versionedPs.Copy() psCopy.Version = pVer psCopy.Schema.SetProviderVersion(psCopy.Address, pVer) @@ -113,10 +121,7 @@ func (s *ProviderSchemaStore) AddLocalSchema(modPath string, addr tfaddr.Provide Source: src, } - schemaRawCopy, err := copystructure.Config{ - Copiers: copiers, - }.Copy(schema) - schemaCopy := schemaRawCopy.(*tfschema.ProviderSchema) + schemaCopy := schema.Copy() if obj != nil { existingEntry, ok := obj.(*ProviderSchema) @@ -174,10 +179,7 @@ func (s *ProviderSchemaStore) AddPreloadedSchema(addr tfaddr.Provider, pv *versi } } - schemaRawCopy, err := copystructure.Config{ - Copiers: copiers, - }.Copy(schema) - schemaCopy := schemaRawCopy.(*tfschema.ProviderSchema) + schemaCopy := schema.Copy() ps.Schema = schemaCopy diff --git a/internal/state/provider_schema_test.go b/internal/state/provider_schema_test.go index a50a67eeb..c4f6f83df 100644 --- a/internal/state/provider_schema_test.go +++ b/internal/state/provider_schema_test.go @@ -11,7 +11,6 @@ import ( "github.com/hashicorp/hcl-lang/schema" tfaddr "github.com/hashicorp/terraform-registry-address" tfschema "github.com/hashicorp/terraform-schema/schema" - "github.com/mitchellh/copystructure" ) func TestStateStore_AddPreloadedSchema_duplicate(t *testing.T) { @@ -876,14 +875,7 @@ func BenchmarkProviderSchema(b *testing.B) { func schemaSliceFromIterator(it *ProviderSchemaIterator) []*ProviderSchema { schemas := make([]*ProviderSchema, 0) for ps := it.Next(); ps != nil; ps = it.Next() { - psCopy, err := copystructure.Config{ - Copiers: copiers, - }.Copy(ps) - if err != nil { - continue - } - schemaCopy := psCopy.(*ProviderSchema) - schemas = append(schemas, schemaCopy) + schemas = append(schemas, ps.Copy()) } return schemas } diff --git a/internal/terraform/datadir/module_manifest.go b/internal/terraform/datadir/module_manifest.go index 9710357d2..c707b9674 100644 --- a/internal/terraform/datadir/module_manifest.go +++ b/internal/terraform/datadir/module_manifest.go @@ -101,6 +101,24 @@ type ModuleManifest struct { Records []ModuleRecord `json:"Modules"` } +func (mm *ModuleManifest) Copy() *ModuleManifest { + if mm == nil { + return nil + } + + newMm := &ModuleManifest{ + rootDir: mm.rootDir, + Records: make([]ModuleRecord, len(mm.Records)), + } + + for i, record := range mm.Records { + // Individual records are immutable once parsed + newMm.Records[i] = record + } + + return newMm +} + func NewModuleManifest(rootDir string, records []ModuleRecord) *ModuleManifest { return &ModuleManifest{ rootDir: rootDir,