cabal format inlines and removes common stanzas

[m-renaud]

Overview

When running cabal format, all common stanzas are inlined into the blocks they're referenced from.

Example

Before:

cabal-version: 2.4
name: inittest
version: 0.1.0.0
build-type: Simple

common common-deps
    default-language: Haskell2010
    build-depends: base >=4.12 && <4.13

library
    import: common-deps
    exposed-modules:
        MyLib
    hs-source-dirs: src

executable inittest
    import: common-deps
    main-is: Main.hs
    hs-source-dirs: app
    build-depends:
        inittest -any

After:

cabal-version: 2.4
name: inittest
version: 0.1.0.0
build-type: Simple

library
    exposed-modules:
        MyLib
    hs-source-dirs: src
    default-language: Haskell2010
    build-depends:
        base >=4.12 && <4.13

executable inittest
    main-is: Main.hs
    hs-source-dirs: app
    default-language: Haskell2010
    build-depends:
        base >=4.12 && <4.13,
        inittest -any

/cc @phadej @hvr

[phadej]

This is by design. Parser produces a GPD to be consumed by solver/Setup.hs/...

Asking to preserve common stanzas is in the same spirit as asking GHC to be able to pretty-print Core into surface Haskell language (analogy: type class instances, there are no such things in Core).

That said, having more intermediate structures would be reasonable; so e.g. cabal check could work on more "surface syntax looking" data (and importantly preserve the source locations). But that should not affect performance of parsing. This is massive task to do, I don't see clear iterative path.

[phadej]

I label this as wontfix. Please don't be discouraged, If someone have time and willingness to pursuit this task, be my guest. But I don't feel this is important enough to have any priority.

[m-renaud]

Thanks for the info. I feel like this behaviour living under the command cabal format doesn't make any sense then, the name implies it should "format my cabal files" not "reprint my cabal files after reducing them to a GPD, then make me undo the change so that they're actually well structured again".

[phadej]

So what should we do, any suggestions?

[m-renaud]

My 2cents:

1. Rename the format command to something more specific/targeted to its use case, maybe something like simplify-cabal-files (bikeshed away) and keep it hidden. Update the docs to reflect that this is not meant for re-formatting your cabal files.

2. Introduce a cabal fix command which does a few things:
   i) Re-formats cabal files to a "standard" format (subject to configuration options), but leaves the structure intact.
   ii) Invokes a configured Haskell code formatter on the project (if set in cabal.project or global config)
   ii) (optional) Discover and add newly created modules to the cabal file (I have some ideas on how the most common cases can be handled automatically).

The idea would be that this could start out as something not very fancy, just enforce things like:

• Remove trailing space from lines
• Use name: value format for single valued fields.
• Always hang and indent by <n> spaces repeated values:

cabal-version: 2.4
version: 0.1.0.0
executable foo
  ghc-options:
    -opt-x
    -opt-y
  build-depends:
    a ^>= 1.2,
    b ^>= 4.4,

This could even start as a project outside of cabal-install, but I think it would be best to develop it inside since we could re-use some of the parsers and pretty printers that already exist.

[gbaz]

I think we do need to fix this, otherwise cabal format is not going to be useful. Also c.f. #5306 where it is noted that format deletes comments (and #2460 where it seems that other than character encodings, at least format is data preserving on roundtrip, and if that is solved could at least be listed in --help).

Why would anybody want to reformat their files if it messes up common stanzas and deletes comments?

I mention in one of the above tickets the ghc-exactprint approach and "reprinting". For comments, we don't need to mess with the existing nice parser. We just need to augment it with a "reprinter" in some fashion.

For common stanzas, I think its fair enough to toss in just enough intermediate structure in the parsed output to not "merge" them in the same pass, but preserve them at first, and then collapse them down before feeding them to Solver and friends.

As such, I'd like to remove the wontfix resolution, since it implies that this is something we're fine with, as opposed to something we're not fine with, but recognize is not a hot item in terms of core dev priorities at the moment.

[phadej]

I'd personally remove cabal format again for now. showGenericPackageDescription is nice debugging tool, but growing it into something ghc-exactprint-like is a big task.

[gbaz]

Without going all the way to exactprint, I think just resolving the inlined stanzas should be a minor refactor without big performance implications. I have a few (well more than a few) other things on my plate, but at some point I could maybe take a look.

[phadej]

FWIW cabal format as now, reduces a lot. it merges build-depends (and other fields), it reorders fields, it moves if conditionals around. Some maybe some people want, some not. So reduction of common stanzas is just easily seen (as without them cabal files are often almost in "normal form").

[m-renaud]

I have some more time to look at this now, I think in general we should separate the parsing of the cabal file to some "source representation", and then perform the common stanza inlining in a separate transformation from the "source representation" to GenericPackageDescription.

I have a very much WIP change that I'll make a PR tomorrow to get feedback on. Specifically I really have no idea what the difference between PackageDescription and GenericPackageDescription is supposed to be so I ended up forking some of the code to get a clearer separation. There's also some questions about if we want separate source representations for each of the section types, or if we should just add another field to the existing data types.

it merges build-depends (and other fields), it reorders fields, it moves if conditionals around

I think all of these are reasonable things to do to put the spec into some normal form. The things that should be preserved are a) common stanzas, b) comments (although at lower priority imo).

Would appreciate feedback on the PR when it's up.

[jneira]

I think the won't fix is misleading, as it contains an interesting discussion about the alternatives to actual cabal format (hide, rename, fix it, etc)
So i would remove it if nobody disagree.

[Mikolaj]

Right and this it related to the hot exact-print challenge.