Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to the latest metaconfig #1145

Merged
merged 23 commits into from May 10, 2018
Merged

Upgrade to the latest metaconfig #1145

merged 23 commits into from May 10, 2018

Conversation

olafurpg
Copy link
Member

This PR removes the dependency on scalameta/paradise macro annotations and upgrades to the latest metaconfig version (https://olafurpg.github.io/metaconfig/). This is the first step towards having configuration work the same across JVM, JS and Native.

Most of the diff is replacing the @GenerateConfDecoder macro annotation with the semi-automatic metaconfig.generic.{deriveDecoder,deriveSurface} def macros. Amazingly, 1031 out of 1032 tests passed on first compile 馃挭

The surface macro is nice since it allows automatic generation of all configuration settings with explanation messages, default values and types. Example how we use it in scalafix https://scalacenter.github.io/scalafix/docs/rules/DisableSyntax

The only regression I can't avoid is the "Other" section on the website that did a best-effort to pretty-print all available configuration options as HOCON. The website links to the ScalafmtConfig sources instead now.

This was referenced Apr 22, 2018
@olafurpg
Copy link
Member Author

olafurpg commented Apr 23, 2018

Appears that we need to upgrade to scalameta v1.8 from v1.7 to complete this PR. The Scala.js linker failures indicate the current classpath is broken on the JVM potentially causing `MethodNotFoundExceptiond due to a binary incompatible change between fastparse 0.4.2 and 0.4.3. The JVM test suite didn't catch it because it is not as exhaustive as the Scala.js closed world optimizer + linker.

The changes for v1.7 to v1.8 are small but enough to cause ~40 test failures. I fixed one incompatibility which brings it down to ~5 test failures.

I'm gonna go ahead and publish a release with the current master before going ahead with this fix.

@olafurpg
Copy link
Member Author

olafurpg commented May 5, 2018

All tests are passing now after upgrading to v1.8 馃帀 scalafmt can now format code splices inside of xml literals :)

We are now blocked by the lack of a ConfEncoder[T] in metaconfig for the website to show configuration examples next to code listings. I started working on this in metaconfig, it doesn't seem too hard but stuck on

[error] (metaconfig-coreJVM/test:compileIncremental) scala.reflect.internal.FatalError: Unknown type: <error>, <error> [class scala.reflect.internal.Types$ErrorType$, class scala.reflect.internal.Types$ErrorType$] TypeRef? false

writing the encoder derivation macro

@olafurpg
Copy link
Member Author

olafurpg commented May 6, 2018

Opened https://github.com/olafurpg/metaconfig/pull/43 adding metaconfig.ConfEncoder typeclass to replace toHocon

@olafurpg
Copy link
Member Author

olafurpg commented May 7, 2018

The upgrade to scalameta v1.8 is complete, it took a while since the changes around lambdas were quite big. Overall I think it's a nice improvement, for example it can now force the body of a HOF to be on the same line as the parameter list

// before
(x: Int) =>
  x + 1
// after
(x: Int) => x + 1

Now we are only blocked by lack of toHocon.

@fommil
Copy link

fommil commented May 7, 2018

if you're using semi automatic derivation in scalameta, maybe there is an opportunity to use the compiler plugin (without the scalaz bits) from https://gitlab.com/fommil/scalaz-deriving/

@olafurpg
Copy link
Member Author

olafurpg commented May 7, 2018

I am indeed using semiauto but with a a twist where I need a base default value passed into the derivation macro. This is required to apply configuration on top of a base setting, for example // scalafmt: { maxColumn = 200} comments are applied on top of the active configuration.

I took a look at scalaz deriving but struggled to figure out what required scalaz and what didn't. It would be helpful to have a section on how to use it without bringing in scalaz.

@fommil
Copy link

fommil commented May 7, 2018

@olafurpg the entire https://gitlab.com/fommil/scalaz-deriving/#compiler-plugin does not require scalaz.

@mads-hartmann
Copy link
Contributor

Once this is merged I should be able to use metals on this code-base right?

@olafurpg
Copy link
Member Author

olafurpg commented May 9, 2018

@mads-hartmann I haven't tried, there might be other issues but removing scalameta/paradise should definitely improve the situation

This upgrade is quite brutal since we no longer use the paradise macro
annotations to generate readers.
There was a change in how Term.Function inside Term.Block are encoded,
the bodies used to be `Term.Block` with a single expression but now the body is unwrapped
Previously, in v1.7 the syntax `a.map { x => y }` produced a `Term.Block`
wrapping the `y` but in v1.8 there is no block. The previous commit
introduced a fix for one layer of curried lambdas and this commit
extends that to any layer of nested lambdas.
This fixes the last remaining failing test cases.
I thought there had been a regression when browsing the diff in
scala-repos but I suspect this was a bug from before upgrading.
Either way, it's fixed now.
The `case ClassTag.Double` was wrongly indented by 2 spaces.
Discovered running on scala-repos.
@olafurpg
Copy link
Member Author

olafurpg commented May 9, 2018

Almost done 馃樃 we now have a ConfEncoder typeclass that we can use to pretty print ScalafmtConfig as hocon

screen shot 2018-05-10 at 00 50 28

This is far cleaner than what we had before which used ugly runtime type matching. The show/hide buttons are still empty for some reason, that's the only remaining blocker to merge this.

This should likely be moved upstream to metaconfig
@olafurpg
Copy link
Member Author

olafurpg commented May 10, 2018

Released metaconfig v0.7.2 that adds a hocon printer and Conf.patch method to print out minimal configuration examples given two ScalafmtConfig

https://github.com/olafurpg/metaconfig/releases/tag/v0.7.2

@olafurpg
Copy link
Member Author

OK, this is ready for merge 馃挭 @mads-hartmann any chance you could take a look at the configuration examples to see if I messed up?

@olafurpg
Copy link
Member Author

Gonna go ahead and merge this to unblock a followup PR that upgrades scalameta to v3.7

@olafurpg olafurpg merged commit 295cee6 into scalameta:master May 10, 2018
@olafurpg olafurpg deleted the 3.7 branch May 10, 2018 14:19
@mads-hartmann
Copy link
Contributor

@olafurpg Awesome work. As far as I can tell the show/hide config options works like expected 馃憤

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.

None yet

3 participants