search

zclconf / go-cty

A type system for dynamic values in Go applications
MIT License
338 stars 70 forks source link

cty: WithoutOptionalAttributesDeep does not panic on cty.NilType #170

Closed ryancragun closed 9 months ago

ryancragun commented 9 months ago

Fix an issue where WithoutOptionalAttributesDeep() on a cty.NilType would result in a panic. This was discovered when using gohcl to decode a block that has attributes that are set to null.

panic: WithoutOptionalAttributesDeep does not support the given type

goroutine 46 [running]:
github.com/zclconf/go-cty/cty.Type.WithoutOptionalAttributesDeep({{0x0?, 0x0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/type.go:152 +0x565
github.com/zclconf/go-cty/cty/convert.conversionObjectToObject.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion_object.go:86 +0x3a5
github.com/zclconf/go-cty/cty/convert.getConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {0x0, 0x0, 0x0})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:57 +0x2b3
github.com/zclconf/go-cty/cty/convert.GetConversionUnsafe.retConversion.func1({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/conversion.go:199 +0x34
github.com/zclconf/go-cty/cty/convert.Convert({{{0x197f548?, 0xc0006ff170?}}, {0x1766d40?, 0xc0005b9da0?}}, {{0x197f548?, 0xc0006288b0?}})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/zclconf/go-cty@v1.14.0/cty/convert/public.go:51 +0x151
github.com/hashicorp/hcl/v2/gohcl.DecodeExpression({0x197f698, 0xc0003b27e0}, 0x197f580?, {0x1720c40, 0xc0001cdcf8})
  ¦ ¦ ¦ /home/runner/go/pkg/mod/github.com/hashicorp/hcl/v2@v2.18.0/gohcl/decode.go:299 +0xb0
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decodeAndValidateTerraformCLIAttribute(0xc0001cdce0, 0xc0000cac00, 0xc001ac7998)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:349 +0x610
github.com/hashicorp/enos/internal/flightplan.(*Scenario).decode(...)
  ¦ ¦ ¦ /home/runner/actions-runner/_work/enos/enos/internal/flightplan/scenario.go:187
github.com/hashicorp/enos/internal/flightplan.(*ScenarioDecoder).decodeScenario(0xc0005b36c8, 0xc000bc8bd0, 0xc00085cf70)
panic: WithoutOptionalAttributesDeep does not support the given type
apparentlymart commented 9 months ago

Hi @ryancragun! Thanks for working on this.

cty.NilType is not a valid type -- it represents the absence of one -- and so I think it's reasonable for this to be a panic although I would agree that the panic message is not communicating that well.

The name NilType is intended to communicate that it's similar to Go's native nil for types like interfaces, where it literally represents "nothing" and most interactions with it will panic. The same is true for NilValue which is the analogous concept for values. These "nothing" values exist for situations such as in functions that return cty.Type, error where the error and the type are mutually exclusive. No "real" type should ever include cty.NilType anywhere in it.

ryancragun commented 9 months ago

Sounds good. Here's another attempt of fixing the panic https://github.com/zclconf/go-cty/pull/171