-
Notifications
You must be signed in to change notification settings - Fork 3
chore: remove all possible AsString panics #183
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -267,6 +267,7 @@ func optionalStringEnum[T ~string](block *terraform.Block, key string, def T, va | |
func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic) { | ||
tyAttr := block.GetAttribute(key) | ||
tyVal := tyAttr.Value() | ||
|
||
if tyVal.Type() != cty.String { | ||
typeName := "<nil>" | ||
if !tyVal.Type().Equals(cty.NilType) { | ||
|
@@ -297,8 +298,24 @@ func requiredString(block *terraform.Block, key string) (string, *hcl.Diagnostic | |
return "", diag | ||
} | ||
|
||
// nolint:gocritic // string type asserted | ||
return tyVal.AsString(), nil | ||
tyValStr, ok := hclext.AsString(tyVal) | ||
if !ok { | ||
// Either the val is unknown or null | ||
diag := &hcl.Diagnostic{ | ||
Severity: hcl.DiagError, | ||
Summary: fmt.Sprintf("Invalid %q attribute for block %s", key, block.Label()), | ||
Detail: "Expected a string, got an unknown or null value", | ||
EvalContext: block.Context().Inner(), | ||
} | ||
|
||
if tyAttr.IsNotNil() { | ||
diag.Subject = &(tyAttr.HCLAttribute().Range) | ||
// diag.Context = &(block.HCLBlock().DefRange) | ||
diag.Expression = tyAttr.HCLAttribute().Expr | ||
} | ||
return "", diag | ||
} | ||
return tyValStr, nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Returns an error now instead of a panic. |
||
} | ||
|
||
func optionalBoolean(block *terraform.Block, key string) bool { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,12 +11,12 @@ func MergeObjects(a, b cty.Value) cty.Value { | |
output[key] = val | ||
} | ||
b.ForEachElement(func(key, val cty.Value) (stop bool) { | ||
// TODO: Should this error be captured? | ||
if key.Type() != cty.String { | ||
return true | ||
} | ||
//nolint:gocritic // string type asserted above | ||
k := key.AsString() | ||
k, ok := AsString(key) | ||
if !ok { | ||
// TODO: Should this error be captured? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably no harm to drop a warn diag here, if possible? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I cannot really. This is all for merging context values. Which happens in a hook that cannot raise errors. The context is supposed to store variable (reference) values. So this failing just means a reference/var is skipped. Which leaves it's value as Which is the correct behavior at a high level. In reality though, I do not think this should happen. Maybe with some complex structs/types? I need to test more. |
||
return stop | ||
} | ||
Comment on lines
-14
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Invalid values were skipped before, so behavior is unchanged. Just no potential for a panic. |
||
old := output[k] | ||
if old.IsKnown() && isNotEmptyObject(old) && isNotEmptyObject(val) { | ||
output[k] = MergeObjects(old, val) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,7 +68,12 @@ func CreateDotReferenceFromTraversal(traversals ...hcl.Traversal) string { | |
switch { | ||
case part.Key.Type().Equals(cty.String): | ||
//nolint:gocritic // string type asserted above | ||
refParts = append(refParts, fmt.Sprintf("[%s]", part.Key.AsString())) | ||
stringName, ok := AsString(part.Key) | ||
if !ok { | ||
// Nothing we can do, just put a placeholder | ||
stringName = "??" | ||
} | ||
refParts = append(refParts, fmt.Sprintf("[%s]", stringName)) | ||
Comment on lines
-71
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
case part.Key.Type().Equals(cty.Number): | ||
idx, _ := part.Key.AsBigFloat().Int64() | ||
refParts = append(refParts, fmt.Sprintf("[%d]", idx)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,8 @@ import ( | |
"github.com/zclconf/go-cty/cty" | ||
"github.com/zclconf/go-cty/cty/function" | ||
"golang.org/x/xerrors" | ||
|
||
"github.com/coder/preview/hclext" | ||
) | ||
|
||
// init intends to override some of the default functions afforded by terraform. | ||
|
@@ -27,7 +29,11 @@ func init() { | |
Impl: func(args []cty.Value, retType cty.Type) (cty.Value, error) { | ||
// This code is taken directly from https://github.com/mitchellh/go-homedir/blob/af06845cf3004701891bf4fdb884bfe4920b3727/homedir.go#L58 | ||
// The only change is that instead of expanding the path, we return an error | ||
path := args[0].AsString() | ||
path, ok := hclext.AsString(args[0]) | ||
if !ok { | ||
return cty.NilVal, xerrors.Errorf("invalid path argument") | ||
} | ||
Comment on lines
+32
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Being safe |
||
|
||
if len(path) == 0 { | ||
return cty.StringVal(path), nil | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,8 +39,10 @@ func parameterContextsEvalHook(input Input) func(ctx *tfcontext.Context, blocks | |
continue | ||
} | ||
|
||
//nolint:gocritic // string type asserted | ||
name := nameVal.AsString() | ||
name, ok := hclext.AsString(nameVal) | ||
if !ok { | ||
continue | ||
} | ||
Comment on lines
+42
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code above does the same thing as this. |
||
var value cty.Value | ||
pv, ok := input.RichParameterValue(name) | ||
if ok { | ||
|
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.