Skip to content

Change ConfigFactory/SimpleConfigObject def to no paren#488

Merged
ekrich merged 1 commit intoekrich:mainfrom
mdedetrich:change-config-empty-no-paren
Sep 11, 2025
Merged

Change ConfigFactory/SimpleConfigObject def to no paren#488
ekrich merged 1 commit intoekrich:mainfrom
mdedetrich:change-config-empty-no-paren

Conversation

@mdedetrich
Copy link
Contributor

@mdedetrich mdedetrich commented Sep 11, 2025

While working on apache/pekko#2187, as part of doing this migration I noticed that I had to change ConfigFactory.empty to ConfigFactory.empty(). In Scala its idiomatic to have empty parens only on methods that create side effects and ConfigFactor.empty has no side effects (its a pure value). Other parts of sconfig already do this (i.e. see ValueType which is correctly missing parens) so I think that having the parens on ConfigFactory/SimpleConfigObject may have been an oversight.

This change may be source breaking (newer versions of Scala have deprecated the ability to automatically remove empty parens if the method definition has empty parens and vice versa depending on deprecation flags) but its not breaking binary compatibility in any way. The worst that users may have to experience is they would have to remove the parens from ConfigFactory.empty() in their Scala source code when updating sconfig to a release that his this fix. Java users are entirely unaffected (in Java you always have these parens for functions, regardless of how its defined in Scala).

Copy link
Owner

@ekrich ekrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. It may be possible that we missed other sites as well. Do you mind adding a scalafix rule for this as well?

@mdedetrich
Copy link
Contributor Author

mdedetrich commented Sep 11, 2025

This looks good. It may be possible that we missed other sites as well. Do you mind adding a scalafix rule for this as well?

So the ScalaFix rule is actually correct, it translated com.typesafe.config.ConfigObject.empty() to the parenless org.ekrich.config.ConfigObject.empty (see https://github.com/mdedetrich/sconfig/blob/be3e46adf497298b0bfa7c603d4fd924f2543d4a/scalafix/rules/src/main/scala/org/ekrich/sconfig/fix/ReplaceTypesafeConfig.scala#L44-L52). When I ran the migration tool on Pekko, I actually had to manually afterwards change org.ekrich.config.ConfigObject.empty to org.ekrich.config.ConfigObject.empty() because the actual definition had parens (which this PR fixes).

Given this I don't think there is anything that needs to be done unless I misunderstood something?

@ekrich
Copy link
Owner

ekrich commented Sep 11, 2025

I didn't think to look. Thanks.

Copy link
Owner

@ekrich ekrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ekrich ekrich merged commit 871bf5e into ekrich:main Sep 11, 2025
3 checks passed
@mdedetrich mdedetrich deleted the change-config-empty-no-paren branch September 11, 2025 15:16
@He-Pin
Copy link

He-Pin commented Sep 11, 2025

There are some pending pr in the lightbend configuration, can we have them.here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants