Merge branch 'main' into ambiguous_step_report_fixes

# Conflicts:
#	internal/formatters/fmt_output_test.go

CHANGELOG.md

@@ -8,6 +8,7 @@ This document is formatted according to the principles of [Keep A CHANGELOG](htt
 
 ## Unreleased
 
+- Improved the type checking of step return types and improved the error messages - ([647](https://github.com/cucumber/godog/pull/647) - [johnlon](https://github.com/johnlon))
 - Ambiguous step definitions will now be detected when strict mode is activated - ([636](https://github.com/cucumber/godog/pull/636) - [johnlon](https://github.com/johnlon))
 - Provide support for attachments / embeddings including a new example in the examples dir - ([623](https://github.com/cucumber/godog/pull/623) - [johnlon](https://github.com/johnlon))
 

internal/models/stepdef.go

@@ -16,9 +16,9 @@ var typeOfBytes = reflect.TypeOf([]byte(nil))
 
 // matchable errors
 var (
- ErrUnmatchedStepArgumentNumber = errors.New("func received more arguments than expected")
+ ErrUnmatchedStepArgumentNumber = errors.New("func expected more arguments than given")
  ErrCannotConvert = errors.New("cannot convert argument")
- ErrUnsupportedArgumentType  = errors.New("unsupported argument type")
+ ErrUnsupportedParameterType = errors.New("func has unsupported parameter type")
 )
 
 // StepDefinition ...
@@ -36,6 +36,9 @@ type StepDefinition struct {
 var typeOfContext = reflect.TypeOf((*context.Context)(nil)).Elem()
 
 // Run a step with the matched arguments using reflect
+// Returns one of ...
+// (context, error)
+// (context, godog.Steps)
 func (sd *StepDefinition) Run(ctx context.Context) (context.Context, interface{}) {
  var values []reflect.Value
 
@@ -161,7 +164,8 @@ func (sd *StepDefinition) Run(ctx context.Context) (context.Context, interface{}
 
  return ctx, fmt.Errorf(`%w %d: "%v" of type "%T" to *messages.PickleTable`, ErrCannotConvert, i, arg, arg)
  default:
- return ctx, fmt.Errorf("%w: the argument %d type %T is not supported %s", ErrUnsupportedArgumentType, i, arg, param.Elem().String())
+ // the error here is that the declared function has an unsupported param type - really this ought to be trapped at registration ti,e
+ return ctx, fmt.Errorf("%w: the data type of parameter %d type *%s is not supported", ErrUnsupportedParameterType, i, param.Elem().String())
  }
  case reflect.Slice:
  switch param {
@@ -172,10 +176,13 @@ func (sd *StepDefinition) Run(ctx context.Context) (context.Context, interface{}
  }
  values = append(values, reflect.ValueOf([]byte(s)))
  default:
- return ctx, fmt.Errorf("%w: the slice argument %d type %s is not supported", ErrUnsupportedArgumentType, i, param.Kind())
+ // the problem is the function decl is not using a support slice type as the param
+ return ctx, fmt.Errorf("%w: the slice parameter %d type []%s is not supported", ErrUnsupportedParameterType, i, param.Elem().Kind())
  }
+ case reflect.Struct:
+ return ctx, fmt.Errorf("%w: the struct parameter %d type %s is not supported", ErrUnsupportedParameterType, i, param.String())
  default:
- return ctx, fmt.Errorf("%w: the argument %d type %s is not supported", ErrUnsupportedArgumentType, i, param.Kind())
+ return ctx, fmt.Errorf("%w: the parameter %d type %s is not supported", ErrUnsupportedParameterType, i, param.Kind())
  }
  }
 
@@ -184,17 +191,43 @@ func (sd *StepDefinition) Run(ctx context.Context) (context.Context, interface{}
  return ctx, nil
  }
 
+ // Note that the step fn return types were validated at Initialise in test_context.go stepWithKeyword()
+
+ // single return value may be one of ...
+ // error
+ // context.Context
+ // godog.Steps
+ result0 := res[0].Interface()
  if len(res) == 1 {
- r := res[0].Interface()
 
- if ctx, ok := r.(context.Context); ok {
+ // if the single return value is a context then just return it
+ if ctx, ok := result0.(context.Context); ok {
  return ctx, nil
  }
 
- return ctx, res[0].Interface()
+ // return type is presumably one of nil, "error" or "Steps" so place it into second return position
+ return ctx, result0
+ }
+
+ // multi-value value return must be
+ // (context, error) and the context value must not be nil
+ if ctx, ok := result0.(context.Context); ok {
+ return ctx, res[1].Interface()
+ }
+
+ result1 := res[1].Interface()
+ errMsg := ""
+ if result1 != nil {
+ errMsg = fmt.Sprintf(", step def also returned an error: %v", result1)
+ }
+
+ text := sd.StepDefinition.Expr.String()
+
+ if result0 == nil {
+ panic(fmt.Sprintf("step definition '%v' with return type (context.Context, error) must not return <nil> for the context.Context value%s", text, errMsg))
  }
 
- return res[0].Interface().(context.Context), res[1].Interface()
+ panic(fmt.Errorf("step definition '%v' has return type (context.Context, error), but found %v rather than a context.Context value%s", text, result0, errMsg))
 }
 
 func (sd *StepDefinition) shouldBeString(idx int) (string, error) {