Allow parsing scalars and mappings into the same type

[vpanteleev-sym]

Some YAML schemas seem to support specifying values either as a single string, or as a dictionary.

One practical example is the image option in .gitlab-ci.yml files:

https://docs.gitlab.com/ee/ci/yaml/#image

GitLab accepts either

test-job:
  image: ruby:3.0

or

test-job:
  image:
    name: ruby:3.0

This change enables us to support this functionality, by calling fromString only on scalar nodes; mappings are deserialized as usual.

[Geod24]

At first I thought it made a lot of sense to do this, but thinking again, if a configuration format supports a short and long form, the longer form (mapping) would probably allow more options, wouldn't it ? This is the case for GitLab at least.
Dub has such as use case, with its dependencies array. This is how it's implemented: https://github.com/dlang/dub/blob/82207b0b29613fd8ad4eed7265aee9bbd777ac24/source/dub/recipe/packagerecipe.d#L192-L211

The only case where this feature would make sense is if you only wanted to handle image but not entrypoint, which I don't think warrant its own feature (you can always drop to fromYAML).

[vpanteleev-sym]

At first I thought it made a lot of sense to do this, but thinking again, if a configuration format supports a short and long form, the longer form (mapping) would probably allow more options, wouldn't it ?

Not necessarily - the case I'm using this for is a restricted sum-type (a node can be e.g. integer or array_of: integer).

Dub has such as use case, with its dependencies array. This is how it's implemented: https://github.com/dlang/dub/blob/82207b0b29613fd8ad4eed7265aee9bbd777ac24/source/dub/recipe/packagerecipe.d#L192-L211

Ah, that works too. Though, I think we have enough evidence that this is a common pattern, also encountered when projects need to extend their YAML schemas, so I think it would make sense to support it directly.

The only case where this feature would make sense is if you only wanted to handle image but not entrypoint, which I don't think warrant its own feature (you can always drop to fromYAML).

Sorry, I'm not sure what you mean by this. As it is implemented in this PR, if the node is not a scalar, it deserialises into the struct ignoring fromString, so you can get e.g. entrypoint. The examples and tests don't demonstrate this, though.

[Geod24]

Not necessarily - the case I'm using this for is a restricted sum-type (a node can be e.g. integer or array_of: integer).

Can you give a code example ?

Sorry, I'm not sure what you mean by this. As it is implemented in this PR, if the node is not a scalar, it deserialises into the struct ignoring fromString, so you can get e.g. entrypoint. The examples and tests don't demonstrate this, though.

Before this, fromString was supposed to deserialize a whole record. Now, it is a convenience for scalar. I think the original use case wasn't very useful (it was supposed to be the escape hatch people could use, but in practice fromYAML was needed and later implemented). I would however suggest a few things:

1. fromString and a string constructor should behave the same;
2. Documentation needs to be updated to mention this short form;

I don't mind doing (2), as I've been thinking of revamping the FAQ to include such examples, but this PR should do (1).

[vpanteleev-sym]

Can you give a code example ?

Sure:

import std.conv;
import configy.attributes;

struct Type {
  enum BasicType {
    none, // Not a basic type - see other fields
    integer,
  }

  // Only one of these may be set:
  private @Optional BasicType _basicType;
  @Optional Type* array_of;

  /// A plain scalar indicates a built-in type.
  static Type fromString(string s) => Type(_basicType: s.to!BasicType);
  @property BasicType basicType() const => _basicType; /// ditto
}

Before this, fromString was supposed to deserialize a whole record. Now, it is a convenience for scalar.

I am a little confused by the wording here - here is how I see this:

• Before this, when configy saw a struct with fromString, it could only deserialize a YAML scalar into it.
• Now, when it sees a struct with fromString, it will accept either a scalar or a mapping.
• I guess by record you mean a D struct and not YAML mapping.
• Still don't understand what you mean by "convenience for scalar".

I think the original use case wasn't very useful (it was supposed to be the escape hatch people could use, but in practice fromYAML was needed and later implemented).

I think it's nice that you don't have to deal with DYAML. fromYAML and having to import DYAML seems like a layering violation, though I see that it is sometimes unavoidable.

fromString and a string constructor should behave the same;

We actually cannot do the same thing for the constructor here. The presence of the constructor blocks deserializing into struct fields (it causes the type to fail the "does not support field-wise (default) construction" check). (This asymmetry is perhaps not a bad thing - if the user wants to force configy to never deserialize into a struct, then they can declare a constructor instead of using fromString.)

I don't mind doing (2), as I've been thinking of revamping the FAQ to include such examples, but this PR should do (1).

I don't mind doing that either, where should that be done, the FAQ?

[Geod24]

Coming back to this, sorry for the delay. Consider the following struct:

struct MeetingSchedule {
    ushort day, month, year, hour, minutes;
    static string fromString (string data) {
       // Parsing logic here
    }
}
struct Config {
  MeetingSchedule[] appointments;
}

Without the fromString, Configy will of course expect the following (long form):

appointments:
  - day: 24
     month: 4
     year: 2025
     hour: 10
     minute: 42

And with the fromString we could have something like this short form:

appointments:
  - "2024-04-04T10:42:00Z"

However we do not want to accept the long form when the fromString is present. If I understand the example above properly, that would enable this.

[vpanteleev-sym]

However we do not want to accept the long form when the fromString is present.

So in essence we want to be able to allow application authors to decide how their structs are parseable:

• only as maps
• only as strings
• as either maps or strings.

Previously we supported only the first and second. This PR changes it to support only the first and third.

I think the second goal can be achieved by reducing the visibility of those fields (i.e. making them package). Would that work?

[Geod24]

Just a few nits.

[Geod24]

@CyberShadow Looks like this exposed a bug, will rebase on v3.x.x

[Geod24]

@CyberShadow : "Fixed" by re-implementing Only. However it triggers a compilation error when a struct has alias this, I'll need to look at this. Perhaps I should just remove the alias this functionality, it is dangerous anyways. I think it is a good example of something that would be affected. The more I think about this, the more I think it violates the principle of least surprise.

[CyberShadow]

Sure, whatever works 👍

By the way I realized that another useful use case for this feature is allowing extending the schema in a backwards compatible way (just like what GitLab did in the initial example).

[Geod24]

Why does this need to change ?

[Geod24]

Can you add a second unittest with both fromString and fromConfig and make sure that both path can be taken based on the input type ?