Make a bool conversion error more verbose

[kayrus]

In case, when a bool string contains True or False, the conversion doesn't work.

see https://github.com/terraform-providers/terraform-provider-openstack/issues/924

[codecov]

Codecov Report

Merging #29 into master will decrease coverage by 0.04%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master      #29      +/-   ##
==========================================
- Coverage   75.92%   75.88%   -0.05%     
==========================================
  Files          74       74              
  Lines        5758     5764       +6     
==========================================
+ Hits         4372     4374       +2     
- Misses        996     1000       +4     
  Partials      390      390

Impacted Files	Coverage Δ
cty/convert/conversion_primitive.go	85.71% <42.85%> (-14.29%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f21e35a...b429fbd. Read the comment docs.

[apparentlymart]

Hi @kayrus! Thanks for working on this.

It was intentional to have only a very small set of valid values for conversion from string to bool, because it's a common cause of confusion in other languages to have too many automatic conversions, and I'd rather have a small set of well-defined behavior than many different ways to say the same thing.

Perhaps as a compromise we could add some additional cases to this switch in order to give the user more direct feedback on how to fix the problem:

switch val.AsString() {
case "true", "1":
	return cty.True, nil
case "false", "0":
	return cty.False, nil
default:
	switch strings.ToLower(val.AsString()) {
	case "true":
		return cty.NilVal, path.NewErrorf("a bool is required; to convert from string, use lowercase \"true\"")
	case "false":
		return cty.NilVal, path.NewErrorf("a bool is required; to convert from string, use lowercase \"false\"")
	default:
		return cty.NilVal, path.NewErrorf("a bool is required")
	}
}

I think this would change the error message seen in Terraform like this:

Error: error setting default for "insecure": a bool is required; to convert from string, use lowercase "true"

In retrospect I regret permitting "1" and "0" in here too since it is inconsistent with the rule that 0 (the number) can't convert to false and 1 (the number) can't convert to true, and it means that a round trip from string to bool to string is lossy, but it's too late to change that now. We can at least respond to that mistake by not extending it any further, and instead just giving better feedback.

[kayrus]

@apparentlymart thanks for the explanation, fixed.

[apparentlymart]

Thanks @kayrus! I'm going to merge this now.