feat: Add introspectable ConfigSpec

[Iltotore]

Add ConfigSpec, an higher-level equivalent of ConfigValue that can be compiled to ConfigValue and introspected. This is useful for deriving documentation or default configuration files/maps (e.g for Kubernetes). Example of MD documentation derivation:

import cats.syntax.all.*
import ciris.*

case class ProgramEnvironment(pwd: String, username: String)

val config = (
  ConfigSpec.prop("user.dir"),
  ConfigSpec.prop("does.not.exist").or(ConfigSpec.env("USERNAME"))
).imapN(ProgramEnvironment.apply)(Tuple.fromProductTyped)
  .default(ProgramEnvironment("/", "default_user"))

println(ConfigSpecMarkdownInterpreter(config))

ConfigSpecMarkdownInterpreter.scala

import ciris.ConfigSpec
import ciris.ConfigField

object ConfigSpecMarkdownInterpreter:

  private def spacesAfter(cell: String, length: Int): String = cell + " " * (length - cell.length())
  
  def apply(spec: ConfigSpec[?]): String =
    val entries = spec.fields

    val keyHeader = "Key"
    val defaultValueHeader = "Default Value"
    
    val maxKeyLength = entries.map(_.key.description.length()).maxOption.getOrElse(0).max(keyHeader.length())
    val maxDefaultValueLength = entries.map(_.defaultValue.fold(1)(_.length())).maxOption.getOrElse(0).max(defaultValueHeader.length())

    val rows = entries
      .map:
        case ConfigField(key, defaultValue) =>
          s"| ${spacesAfter(key.description, maxKeyLength)} | ${spacesAfter(defaultValue.getOrElse("-"), maxDefaultValueLength)} |"
      .mkString("\n|")

    val header = s"| ${spacesAfter(keyHeader, maxKeyLength)} | ${spacesAfter(defaultValueHeader, maxDefaultValueLength)} |"
    val separator = s"| ${"-" * maxKeyLength} | ${"-" * maxDefaultValueLength} |"
    
    s"""|$header
        |$separator
        |$rows""".stripMargin

Output:

| Key                            | Default Value |
| ------------------------------ | ------------- |
| system property user.dir       | /             |
| system property does.not.exist | default_user  |
| environment variable USERNAME  | default_user  |

Key	Default Value
system property user.dir	/
system property does.not.exist	default_user
environment variable USERNAME	default_user

[vlovgr]

Hey, thanks for looking into this. I know there is some interest in this already. My current thinking for the next major version is to change ConfigValue to allow for introspection. This will require some changes, most notably removing flatMap and replacing it with something from Selective (which I believe haven't yet made its way to a stable cats release).

I haven't started this work yet, so it's great to see this already. If you want to continue working on this, I would suggest you start looking into changing ConfigValue instead of adding a separate ConfigSpec construct.

[Iltotore]

Thank you for the insight. I originally did not touch ConfigValue to avoid breaking backward compatibility by removing flatMap but if it is ok then here we go 👍

I'm not familiar with Selective and it does not seem to be mentioned in the official doc (is it Alternative?). Do you have some documentation about it?

[Iltotore]

Should map be replaced by imap? It does not seem possible to propagate default value of a node to its leaves (like in the example above) with uni-directional map. Or should we keep it for backward compatibility reasons and discard default values in the following situation:

val config = (
  ConfigValue.prop("user.dir"),
  ConfigValue.prop("does.not.exist").or(ConfigValue.env("USERNAME"))
).mapN(ProgramEnvironment.apply)
  .default(ProgramEnvironment("/", "default_user")) //We cannot map the default value back to its tuple form

[Iltotore]

I pushed the beginning of my work on ConfigValue. I am a little confused by some ConfigValue methods (mostly effectful ones) so I'd like to know your opinion on it.

[vlovgr]

I'm not familiar with Selective and it does not seem to be mentioned in the official doc (is it Alternative?). Do you have some documentation about it?

There's a draft pull request for cats and the mentioned issue has more details.

Should map be replaced by imap?

I think it makes sense, yes. Similarly to how ConfigDecoder becomes ConfigCodec. Most decoding is done through as, which should continue to work.

I am a little confused by some ConfigValue methods (mostly effectful ones) so I'd like to know your opinion on it.

Not sure exactly what you mean, could you perhaps be a little more specific? Edit: if you mean what we should do about them, I guess we can’t really keep them, right?

[Iltotore]

Not sure exactly what you mean, could you perhaps be a little more specific?

Actually, only blocking's implementation confuses me. When reading the code, it looks like the computation marked as "blocking" is the ConfigValue parameter's definition instead of its evaluation.

Edit: if you mean what we should do about them, I guess we can’t really keep them, right?

They still can be evaluated but they cannot be introspected. Since most of them are "constructors"/"leaves", they contaminate their entire branch:

(
  blocking(...).default("default_value"),
  env("FOO_B").as[Int]
).tupled.fields

will return only the environment variable "FOO_B".

The only exception is the method ConfigValue#evalMap which can be introspected but without default values

def getUserId(txt: String): IO[UserId] = ???

env("USERNAME")
  .evalMap(getUserId)
  .default(defaultUserId)
  .fields //Environment variable "USERNAME" without default value

If map-like operations are turned to imaps, then default values after evalIMap' could be introspected if the "back" function is pure:

def getUserId(txt: String): IO[UserId] = ???
def getName(id: UserId): String = ???

env("USERNAME")
  .evalIMap(getUserId)(getName)
  .default(defaultUserId)
  .fieldsAsync[IO] //Environment variable "USERNAME" with default value `getName(defaultUserId)`

I don't know enough about how much these methods are used so I can't tell if we should keep them or not but note that despite being unable to be introspected, they would still work.

I think it makes sense, yes. Similarly to how ConfigDecoder becomes ConfigCodec. Most decoding is done through as, which should continue to work.

There would still be a little breaking change with as because custom ConfigDecoders will have to be rewritten to ConfigCodecs

[vlovgr]

Actually, only blocking's implementation confuses me. When reading the code, it looks like the computation marked as "blocking" is the ConfigValue parameter's definition instead of its evaluation.

It might help to look at an example.

I don't know enough about how much these methods are used so I can't tell if we should keep them or not but note that despite being unable to be introspected, they would still work.

I guess there is a decision to be made about flatMap and other operations which don't support introspection in their current form. We can choose to either keep them (with the caveat that they will break introspection), or we can change them so they support introspection. Is it worth breaking existing code that much to always have introspection? Maybe we can do it in steps, so the next major version keeps these operations (e.g. evalMap), but also add introspectable alternatives (e.g. evalImap). And then a later major version can consider removing the non-introspectable operations?

There would still be a little breaking change with as because custom ConfigDecoders will have to be rewritten to ConfigCodecs

Yes, but only for custom ConfigDecoders (the ones in the library will be rewritten as part of this work).

[Iltotore]

Maybe we can do it in steps, so the next major version keeps these operations (e.g. evalMap), but also add introspectable alternatives (e.g. evalImap) ...

I think this is the way. Deprecate old unidirectional map methods and remove them later.

[vlovgr]

@Iltotore I noticed you marked this as ready-for-review. I'll try to have a proper look this week. 👍

[vlovgr]

I would expect this to work similar to how default values currently work, where later defaults override earlier defaults. The option is treated as .map(_.some).default(None), so this is .map(_.some).default(None).default(Some(...)) and the default value is Some(...). Maybe I'm missing something here?

[Iltotore]

.map(_.some).default(None).default(Some(...))

So this code is supposed to return Some(...) if there is no value passed IIUC?

[vlovgr]

Yes, exactly.

[Iltotore]

Ok. So I indeed implemented the wrong behaviour. Currently, configA.default(b).default(x) will only return configA's value or b.

I cannot fix it at the moment but I might be able to work on it during the weekend

[vlovgr]

There's no rush from my side. I still have parts left to review, but great to confirm the behaviours.

[vlovgr]

If we're going to fit this into 3.7.0, we have to fix the following compatibility issues.

1. static method configValueNonEmptyParallel()cats.NonEmptyParallel in class ciris.ConfigValue does not have a correspondent in current version
2. static method configValueFlatMap()cats.FlatMap in class ciris.ConfigValue does not have a correspondent in current version
3. method as(ciris.ConfigDecoder)ciris.ConfigValue in class ciris.ConfigValue's type is different in current version, where it is (ciris.ConfigCodec)ciris.ConfigValue instead of (ciris.ConfigDecoder)ciris.ConfigValue
4. method evalFlatMap(scala.Function1)ciris.ConfigValue in class ciris.ConfigValue does not have a correspondent in current version
5. method flatMap(scala.Function1)ciris.ConfigValue in class ciris.ConfigValue does not have a correspondent in current version
6. abstract method fieldsRec(scala.Option)scala.collection.immutable.List in class ciris.ConfigValue is present only in current version
7. method configValueFlatMap()cats.FlatMap in object ciris.ConfigValue does not have a correspondent in current version
8. method configValueNonEmptyParallel()cats.NonEmptyParallel in object ciris.ConfigValue does not have a correspondent in current version

I believe (6) can be ignored since ConfigValue is sealed. Most of these are direct removals though, and I'm not sure how feasible you think it is to keep them?

[Iltotore]

We can keep them without any issue except not supporting introspection when flatMap and such are used

[Iltotore]

Launching mimaReportBinaryIssues in local, the only remaining "problem" is the new fieldRec method.

[vlovgr]

This seems unused, can be removed?

[vlovgr]

This should still hold from a end user perspective, no?

[vlovgr]

I've avoided providing Monad so far, because there are two theoretical candidates for pure (default and loaded), thus forcing the user to choose. But I also realise, one most likely want default as the choice for pure, which is also what we do in tailRecM. We should however add the law tests for Monad.

Edit: I also added back the removed law tests and seems some of them are failing.

[vlovgr]

I'm thinking we should rename ConfigValue#asIso to a different name to avoid as -> asIso -> as migration for end users. Maybe we can rename the internal to and then use at as the new name for asIso? So end users only migrate once from as -> to.

[vlovgr]

It would be nice to have roundtrip tests for the ConfigCodecs (so encode is also tested). Same for the codecs in the integration modules.

[vlovgr]

Do you think we should introduce a ConfigFields type or similar? So we can provide some rendering methods (and control toString) directly in the returned type?

Edit: I'm thinking we should provide some default renderers (e.g. markdown). But let's leave that for a separate pull request.

[Iltotore]

I'm not sure about the relevance of a ConfigFieldList or ConfigFields wrapper 🤔. What would be the advantage over List[ConfigField] with function taking it as a parameter e.g renderMarkdown(fields: List[ConfigField])?

[vlovgr]

I guess it has better discoverability (e.g. config.fields.renderMarkdown vs. discovering the renderMarkdown function). And as we add more renderers (or other functionality), it's convenient to find them all through autocomplete on fields.. Plus we can control ConfigFields#toString and do something better than List#toString.