Fixity info from .ormolu overrides CLI flags

[brandonchinn178]

Describe the bug

Reading the code here:

ormolu/src/Ormolu.hs

Lines 193 to 207 in 49eb083

	cfgFixityOverrides =
	FixityOverrides $
	Map.unions
	[ unFixityOverrides fixityOverrides,
	unFixityOverrides (cfgFixityOverrides rawConfig),
	unFixityOverrides defaultFixityOverrides
	],
	cfgModuleReexports =
	ModuleReexports $
	Map.unionsWith
	(<>)
	[ unModuleReexports reexports,
	unModuleReexports (cfgModuleReexports rawConfig),
	unModuleReexports defaultModuleReexports
	],

It seems like configuration in .ormolu files take precedence over command line flags, when the help text (and standard behavior of flags vs config files) seems to indicate that flags take precedence. Specifically, Map.union is left-biased, so fixityOverrides (coming from mfixityOverrides, which comes from mDotOrmolu) is preferred over cfgFixityOverrides rawConfig (which is parsed from command line flags)

To Reproduce

1. Create some Haskell file with fixities
2. Specify the wrong fixity in .ormolu
3. Specify the right fixity with --fixity
4. Observe ormolu formatting with the wrong fixity

Expected behavior
Ormolu should format with the fixity specified on the command line

Environment

• OS name + version:
• Version of the code:

Additional context
Add any other context about the problem here.

[amesgen]

I don't immediately see a reason to use the CLI flags when one already has an .ormolu file; is there a concrete use case?

But indeed, this was changed in #994, this seems to be the relevant review comment: #994 (comment) (before that, the behavior you are expecting was preserved by that diff).

[brandonchinn178]

Perhaps this is where the expected behavior of refineConfig fights with the usage in Main.hs. From the perspective of refineConfig, it makes sense that the passed-in fixity info should override the config passed in.

I personally don't see a concrete use-case for specifying fixities and reexports on the command line. But given that the flags exist, this is surprising behavior. I would be more in favor of removing the flags altogether than keeping the current behavior.

[tbagrel1]

I think it is related to my remark here: #994 (comment)

I didn't have full context to understand the code when I wrote my review, and then @mrkkrp accepted my suggestion.

Now, I agree with @brandonchinn178 and I think CLI flags should have priority over config file ones. I suppose we could revert the change?

[mrkkrp]

It is true that there is a conflict between semantics of refineConfig and its usage in Main, it did not occur to me before.

I would be more in favor of removing the flags altogether than keeping the current behavior.

I think the flags are very useful in situations when you don't want to write to the filesystem, e.g. in scripts when you want to execute a one-off action without extra state.

I will see if I can adjust this...

[mrkkrp]

I think we should probably keep refineConfig as is (it is reasonable for the arguments to override the "raw" config, after all) and instead change the way it is used in Main.