Allow maps to convert into objects when missing optional attributes are not compatible

[liamcervante]

Before this PR

When converting a map into an object, if any attributes/values were not compatible the whole conversion would fail.

After this PR

If an attribute is not convertible then we check if it's optional. If it is optional, then we don't error right away. We record a nil converter for that attribute leaving us with three cases for each attribute:

1. No entry in the converters map at all, this means no conversion necessary (eg. string to string)
2. Non-nil entry in the converters map, this means a conversion is required but possible (eg. string to number)
3. Nil entry in the converters map, this means a conversion is required but not possible (eg. string to object)

Case 1 and Case 2 are covered already, and case 3 is introduced by this PR. Case 3 is only possible when we have found an optional attribute that is not convertible (we fail early if an attribute is required but not convertible). If we encounter case 3 during the conversion, it means an optional attribute that is not convertible is actually present in the map, so we fail at this point. If case 3 is never encountered then it means the map didn't actually contain any of these optional attributes so we can just set null values for them and still have a successful conversion.

[apparentlymart]

This test case does seem reasonable to me. I would expect it to behave as if the map were first trivially converted to the closest analogous object type (cty.Object({"a": cty.String()}) in this case) and then converted to the target object type from there.

It doesn't actually need to be implemented exactly like that, of course... but the main point is that we must treat it like an object which has c unset first because otherwise there's no way for this map to meet the target type constraint: a map can't have both a string element and an object-typed element at the same time.

[liamcervante]

@apparentlymart - this is ready for review and contains a fix for the original test case.

[apparentlymart]

Thanks for working on this, @liamcervante!

The logic you implemented here looks correct to me. While I was reviewing this I noticed that the tests here only check for whether the conversion succeeded or failed and don't actually test the error messages, so I made a change elsewhere to improve that so we can make sure all of the failing conversions are failing in the way they ought to be failing. I then rebased what you did here onto that commit so that your new test cases would match the new structure.

Aside from rebasing to the new test structure, I've also made a couple other minor changes:

• While I was thinking about whether it's okay for the "get conversion" function to return a type conversion that might fail once given an actual value I noted the similarity to the rules for converting from string to number or string to bool: in that case we also optimistically assume that conversion can succeed and then fail dynamically if the final value doesn't match.

  However, that behavior for the primitive types is guarded behind the "unsafe" flag, so convert.GetConversion will not offer such a conversion while convert.GetConversionUnsafe will. Terraform and HCL always use "unsafe" conversions (that is: conversions that might fail dynamically) via convert.Convert, so this flag is a moot point for them but I do want to preserve that behavior for other callers that may wish to perform stricter type checking. (The backstory here is that I originally intended to use stricter type checking throughout, but later introduced the "unsafe" mode as a concession to how HCL 1 had previously behaved, so that it would be practical to use cty for HCL 2 without creating a migration nightmare for Terraform users.)

  All of this is to say: I added an extra check && unsafe to the check for whether the target attribute is optional, so "safe mode" will not report that it can convert from map to object in that case and so this preserves the assumption that a conversion should not dynamically fail in unsafe mode. This doesn't affect the behavior for HCL or Terraform, but preserves the idea of the "safe mode" for other callers.

• I also lightly tweaked the error message text for the dynamic conversion failure to be (subjectively) more like some of the other neighboring error messages. Specifically it says:

  map element type is incompatible with attribute "c": object required
  

  ...because error messages from this family of functions typically use the style of just stating that something is wrong rather than including language like "failed to..." or "could not...". (That sort of language is typically added by the calling application as additional context, so including that here tends to cause the overall sentence to look redundant in the end-user error message.)

I'm going to merge this now. Thanks again!