Remove the RawConfig type

[jcazevedo]

This PR removes the RawConfig type, modifying ConfigConverters to operate on ConfigValue directly.

My original use case was to be able to deserialize a HOCON such as the following:

{
    list = [{ a = 1 }, { a = 2 }, { a = 3 }]
}

into something like the following classes:

case class Foo(a: Int)
case class ListOfFoo(list: List[Foo])

Using loadConfig[ListOfFoo](config) wouldn't work, as the expected HOCON format was the following:

{
    0.a = 1
    1.a = 2
    2.a = 3
}

I find that format a bit harder to read and create, and prefer the former. The conversion to RawConfig would make config lists to lose their types and make them harder to work with, so I decided to have a go and try to operate directly on ConfigValues. Besides enabling the original use case, I believe this has simplified some things:

• The ConfigConvert type class no longer has to know about namespaces, as the ConfigValue they receive already is the one they should know how to deserialize;
• The StringConvert and FieldConvert classes were removed, as everything could fit into a ConfigConvert;
• It is no longer necessary to have derivations for traversables of "simple" and "complex" types, as every type that has the ConfigConvert type class is able to be derived for a traversable.

I understand this is a big change, so I'm open to rework things more to your liking. I would really like for pureconfig to support the first use case I mentioned, though. The README and the example project should also be updated (since the StringConvert class no longer exists), but I'd rather do that once we settle on the implementation.

[melrief]

This is interesting because I was working on something similar but I didn't have time to complete. Mine is also a bit more complex than this but your solution of working with ConfigValue could be a nice candidate because it is simple to understand. I like your idea and I'm willing to do this change in the design, if @leifwickland agrees, but I need some time to read the PR, understand it, try it and understand what kind of implications it has. In particular, I need to understand how compatible with the current system this is. I'll be at the scala.world this week and I should have sometime to work on this.

@leifwickland maybe you are interested in this PR, it's quite the improvement compared to what we have right now in pureconfig but it's also a big change.

[jcazevedo]

I like your idea and I'm willing to do this change in the design, if @leifwickland agrees, but I need some time to read the PR, understand it, try it and understand what kind of implications it has.

Sure! No problem!

I need to understand how compatible with the current system this is.

Current implementations of ConfigConvert are obviously not compatible, but the change should be transparent to users who rely on pureconfig's derived converters to load configurations. The new serialization into HOCON is compatible with the current one for some (simple) types but mostly incompatible.

I did a quick test, and this seems to fix #29.

[leifwickland]

@melrief I really like the idea of this change. It fits with the initial expectation I had when I walked into the project. I think using typesafe's ConfigValue is less surprising than a Map.
As to the specifics of the change, I don't see any problems, but I've only taken 15 minutes trying to work my way through it.

@jcazevedo Thanks for doing this!

[melrief]

I think it could make sense to provide helper methods like this one for each of the ConfigValueType available to help creating ConfigConverts that work only with one of the value types. For String it makes sense to render the parsed String from the ConfigValue and then try to parse it while for the other value types it could make sense to just return error if the type doesn't match.

[jcazevedo]

I agree with this in theory, but there are some specifics I'm not sure about:

• For ConfigValueType.LIST, should the interface require a function with type signature List[ConfigValue] => T, or List[U] => T, provided that there is a ConfigConvert for U?
• For ConfigValueType.OBJECT, should the interface require a function with type signature ConfigObject => T?
• At least for ConfigValueType.LIST and ConfigValueType.OBJECT, the interface should allow defining a method to render the type as a ConfigValue. Taking this into account, and considering the effort of defining a ConfigConvert, do these helpers still make sense?

[melrief]

I think we can postpone this discussion for now, this PR is big enough :-). Let's just keep this idea in consideration for future releases, I think it could be useful.

[melrief]

@jcazevedo I did a first full review of the code and I think it is excellent. I made a few comments but the general structure is what I was wishing for. I would like to go through the code again another day, if you don't mind and then I think we can consider to merge. Thanks for your effort!

[jcazevedo]

I would like to go through the code again another day, if you don't mind and then I think we can consider to merge.

No problem. Thanks!

I took the liberty of adding some more helper methods in this PR, so that we're able to operate directly on Config, ConfigValue and types which have a ConfigConvert:

import pureconfig._
import com.typesafe.config._

case class Conf(a: List[Int], b: Map[String, String])

val conf = ConfigFactory.parseString("""{ "a": [1, 2, 3, 4], "b": { "k1": "v1", "k2": "v2" } }""")
// conf: com.typesafe.config.Config = Config(SimpleConfigObject({"a":[1,2,3,4],"b":{"k1":"v1","k2":"v2"}}))

conf.to[Conf]
// Success(Conf(List(1, 2, 3, 4),Map(k1 -> v1, k2 -> v2)))

conf.to[Conf].get.toConfig
// SimpleConfigObject({"a":[1,2,3,4],"b":{"k1":"v1","k2":"v2"}})

I added these in commit 306b8ee. I'm not sure if this is something you want to see in the library, so let me know if you want it removed.

[melrief]

@jcazevedo I think we can merge this PR and think about releasing a new version of pureconfig. The PR is a substantial improvement of the library, it is tested, it is well written and also fixes some bugs.

Before I do this, I would like to ask you a favour : commit 306b8ee adds a few implicits that, while useful, in my opinion should not be imported automatically with import pureconfig._. The style of libraries like Cats is to have a package dedicated for syntactic extensions called syntax. I think we should do the same and have those extensions under the package pureconfig.syntax such that you get them when you do import pureconfig.syntax._. It's one more import but it gives more control to the developer.

[leifwickland]

I think the import pureconfig.syntax._ is probably a step in the right direction.

@jcazevedo While we're asking for favors, please update the README to show an example of how to use the syntax.

[jcazevedo]

Before I do this, I would like to ask you a favour : commit 306b8ee adds a few implicits that, while useful, in my opinion should not be imported automatically with import pureconfig.. The style of libraries like Cats is to have a package dedicated for syntactic extensions called syntax. I think we should do the same and have those extensions under the package pureconfig.syntax such that you get them when you do import pureconfig.syntax.. It's one more import but it gives more control to the developer.

I agree that it does give more control to the developer. I've moved the implicit conversions to the syntax package object.

@jcazevedo While we're asking for favors, please update the README to show an example of how to use the syntax.

I've updated both the README and the example project. However, I had to change the configuration on the example project from application.properties to application.conf. When loading a java properties file using the typesafe config library, all values are read as strings, and the example Config object has a set of recipients which need to be derived from a traversable. pureconfig doesn't provide a way to convert a ConfigValue with ConfigValueType of STRING to a traversable (I also don't think that it should), so I think it's better to provide an example using a more featureful format.

[melrief]

Nice, I'll review the last changes and then we can merge.

[melrief]

@jcazevedo thanks for the contribution, it's a great improvement over the previous implementation!

I will publish the next artifact soon, probably labelled 0.3.0 to highlight that this is a big step forward and also breaks compatibility with 0.2.x releases.

There is then a last thing to discuss: because you contributed so much to the library both in terms of ideas and of code, I would like to add you as author of the library. Being author means that the library is yours and you can vote on any PR and changes that will be done. We also will have soon a gitter channel where we help people and decide about the future of the library together with @leifwickland and any other brave developer that decides to contribute to pureconfig. What do you think, can I add you to the authors?

[jcazevedo]

There is then a last thing to discuss: because you contributed so much to the library both in terms of ideas and of code, I would like to add you as author of the library. Being author means that the library is yours and you can vote on any PR and changes that will be done. We also will have soon a gitter channel where we help people and decide about the future of the library together with @leifwickland and any other brave developer that decides to contribute to pureconfig. What do you think, can I add you to the authors?

Sure! Thanks, @melrief!