Skip to content
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.

Sign up for GitHub

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

Improve error messages for unexpected target types #247

Merged
merged 1 commit into from
Mar 26, 2023
Merged

Improve error messages for unexpected target types #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions helper/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,16 +203,26 @@ var errRefTy = reflect.TypeOf((*error)(nil)).Elem()
// EvaluateExpr returns a value of the passed expression.
// Note that some features are limited
func (r *Runner) EvaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error {
var callback bool
rval := reflect.ValueOf(target)
rty := rval.Type()
// Callback must meet the following requirements:
// - It must be a function
// - It must take an argument
// - It must return an error
if rty.Kind() == reflect.Func && rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy) {

var callback bool
switch rty.Kind() {
case reflect.Func:
// Callback must meet the following requirements:
// - It must be a function
// - It must take an argument
// - It must return an error
if !(rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy)) {
panic(`callback must be of type "func (v T) error"`)
}
callback = true
target = reflect.New(rty.In(0)).Interface()

case reflect.Pointer:
// ok
default:
panic("target value is not a pointer or function")
}

err := r.evaluateExpr(expr, target, opts)
Expand Down Expand Up @@ -246,19 +256,19 @@ func (r *Runner) evaluateExpr(expr hcl.Expression, target interface{}, opts *tfl
ty = *opts.WantType
} else {
switch target.(type) {
case *string, string:
case *string:
ty = cty.String
case *int, int:
case *int:
ty = cty.Number
case *[]string, []string:
case *[]string:
ty = cty.List(cty.String)
case *[]int, []int:
case *[]int:
ty = cty.List(cty.Number)
case *map[string]string, map[string]string:
case *map[string]string:
ty = cty.Map(cty.String)
case *map[string]int, map[string]int:
case *map[string]int:
ty = cty.Map(cty.Number)
case cty.Value, *cty.Value:
case *cty.Value:
ty = cty.DynamicPseudoType
default:
return fmt.Errorf("unsupported target type: %T", target)
Expand Down
36 changes: 23 additions & 13 deletions plugin/plugin2host/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,16 +281,26 @@ var errRefTy = reflect.TypeOf((*error)(nil)).Elem()
// Passing a callback function instead of a value as the target will invoke the callback,
// passing the evaluated value to the argument.
func (c *GRPCClient) EvaluateExpr(expr hcl.Expression, target interface{}, opts *tflint.EvaluateExprOption) error {
var callback bool
rval := reflect.ValueOf(target)
rty := rval.Type()
// Callback must meet the following requirements:
// - It must be a function
// - It must take an argument
// - It must return an error
if rty.Kind() == reflect.Func && rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy) {

var callback bool
switch rty.Kind() {
case reflect.Func:
// Callback must meet the following requirements:
// - It must be a function
// - It must take an argument
// - It must return an error
if !(rty.NumIn() == 1 && rty.NumOut() == 1 && rty.Out(0).Implements(errRefTy)) {
panic(`callback must be of type "func (v T) error"`)
}
callback = true
target = reflect.New(rty.In(0)).Interface()

case reflect.Pointer:
// ok
default:
panic("target value is not a pointer or function")
}

err := c.evaluateExpr(expr, target, opts)
Expand Down Expand Up @@ -324,19 +334,19 @@ func (c *GRPCClient) evaluateExpr(expr hcl.Expression, target interface{}, opts
ty = *opts.WantType
} else {
switch target.(type) {
case *string, string:
case *string:
ty = cty.String
case *int, int:
case *int:
ty = cty.Number
case *[]string, []string:
case *[]string:
ty = cty.List(cty.String)
case *[]int, []int:
case *[]int:
ty = cty.List(cty.Number)
case *map[string]string, map[string]string:
case *map[string]string:
ty = cty.Map(cty.String)
case *map[string]int, map[string]int:
case *map[string]int:
ty = cty.Map(cty.Number)
case cty.Value, *cty.Value:
case *cty.Value:
ty = cty.DynamicPseudoType
default:
panic(fmt.Sprintf("unsupported target type: %T", target))
Expand Down
55 changes: 23 additions & 32 deletions tflint/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,18 +159,14 @@ type Runner interface {
// See the hclext.DecodeBody documentation and examples for more details.
DecodeRuleConfig(ruleName string, ret interface{}) error

// EvaluateExpr evaluates the given expression and assigns the value to the Go value target.
// The target must be a pointer. Otherwise, it will cause a panic.
// EvaluateExpr evaluates an expression and assigns its value to a Go value target,
// which must be a pointer or a function. Any other type of target will trigger a panic.
//
// If the value cannot be assigned to the target, it returns an error.
// There are particularly examples such as:
// For pointers, if the expression value cannot be assigned to the target, an error is returned.
// Some examples of this include unknown values (like variables without defaults or
// aws_instance.foo.arn), null values, and sensitive values (for variables with sensitive = true).
//
// 1. Unknown value (e.g. variables without defaults, `aws_instance.foo.arn`)
// 2. NULL
// 3. Sensitive value (variables with `sensitive = true`)
//
// However, if the target is cty.Value, these errors will not be returned.
// These errors can be handled with errors.Is().
// These errors be handled with errors.Is():
//
// ```
// var val string
Expand All @@ -192,29 +188,11 @@ type Runner interface {
// }
// ```
//
// The following are the types that can be passed as the target:
//
// 1. string
// 2. int
// 3. []string
// 4. []int
// 5. map[string]string
// 6. map[string]int
// 7. cty.Value
// 8. func (v T) error
//
// Passing any other type will cause a panic. If you pass a function, the assigned value
// will be used as an argument to execute the function. In this case, if a value cannot be
// assigned to the argument type, instead of returning an error, the execution is skipped.
// This is useful when it is always acceptable to ignore exceptional values.
//
// ```
// runner.EvaluateExpr(expr, func (val string) error {
// // Test value
// }, nil)
// ```
// However, if the target is cty.Value, these errors will not be returned.
//
// Besides this, you can pass a structure. In that case, you need to explicitly pass wantType.
// Here are the types that can be passed as the target: string, int, []string, []int,
// map[string]string, map[string]int, and cty.Value. Passing any other type will
// result in a panic, but you can make an exception by passing wantType as an option.
//
// ```
// type complexVal struct {
Expand All @@ -230,6 +208,19 @@ type Runner interface {
// var complexVals []complexVal
// runner.EvaluateExpr(expr, &compleVals, &tflint.EvaluateExprOption{WantType: &wantType})
// ```
//
// For functions (callbacks), the assigned value is used as an argument to execute
// the function. If a value cannot be assigned to the argument type, the execution
// is skipped instead of returning an error. This is useful when it's always acceptable
// to ignore exceptional values.
//
// Here's an example of how you can pass a function to EvaluateExpr:
//
// ```
// runner.EvaluateExpr(expr, func (val string) error {
// // Test value
// }, nil)
// ```
EvaluateExpr(expr hcl.Expression, target interface{}, option *EvaluateExprOption) error

// EmitIssue sends an issue to TFLint. You need to pass the message of the issue and the range.
Expand Down