ndmitchell / hlint

Haskell source code suggestions
Other
1.47k stars 196 forks source link

Functor law hint may be too opinionated #1606

Open pbrisbin opened 3 months ago

pbrisbin commented 3 months ago

I think with https://github.com/ndmitchell/hlint/pull/1540, we are now hitting Functor law hints more than before.

As an example, I was just prompted to change some code from:

    PlatformSettingsYaml
      <$> (Last <$> parseStackName o)
      <*> o .:? "stackDescription"
      <*> o .:? "docker"
      <*> o .:? "assets"

To:

    PlatformSettingsYaml . Last
      <$> parseStackName o
      <*> o .:? "stackDescription"
      <*> o .:? "docker"
      <*> o .:? "assets"

This has come up 5 or so times in one code base, all of this general form: an applicative record construction where the first field happens to need some wrapper over an applicative action.

I find the former code to be clearer because it keeps that wrapping (Last <$>) as part of the "field" of the applicative build. Moving the Last out and up to PlatformSettingsYaml . Last kind of just breaks my brain, to use (.) with a constructor function of many arguments diverges too far from the (f . g) x form that I can't grok it.

It does compile of course, so it's not "wrong" at all, I just wonder:

  1. Is this the most common kind of code this lint catches, and
  2. Am I alone in thinking it makes the code less readable?

If the answer is yes/yes, maybe it should be ignored by default?

ndmitchell commented 2 months ago

Fair. I guess maybe disable it for now? Better to not suggest, than suggest something that is confusing.