Add "Redundant $ with block argument" hint

[rhendric]

Unusually, this hint has two default severities:

• Suggestion if BlockArguments is enabled in the module
• Ignore otherwise

Either way, users can override the severity of this hint as usual.

Fixes #1547.

[ysangkok]

Thanks for the PR, I like BlockArguments a lot.

However, I have two issues with this PR.

• comments after the dollar are not skipped
• Blocks after the dollar shouldn't be moved up. It fair to assume that if people have newlines after the dollar, they would also want that when they use BlockArguments.

For example, save this to Main.hs:

{-# LANGUAGE BlockArguments #-}
import Data.Maybe

main =
  maybe
    (fail "Failed, there are no elements")
    (putStrLn . ("Average: " <>) . show) $
      -- This block is pure, IO is dispatched above
      let
        elems = [4.0, 49.0]
      in
        case elems of
          [] -> Nothing
          _ -> Just $ sum elems / realToFrac (length elems)

hlint Main.hs --refactor emits:

{-# LANGUAGE BlockArguments #-}
import Data.Maybe

main =
  maybe
    (fail "Failed, there are no elements")
    (putStrLn . ("Average: " <>) . show)
      -- This block is pure, IO is dispatched above let
                                                      elems = [4.0, 49.0]
                                                    in
                                                      case elems of
                                                        [] -> Nothing
                                                        _ -> Just $ sum elems / realToFrac (length elems)

This is an invalid program, the let became part of the comment. Hlint shouldn't produce invalid programs.

(tested with GHC-9.12.2, cabal-install 3.14 and a Cabal index from today)

[rhendric]

HLint should indeed not produce invalid programs, but what you point out is a preexisting bug with HLint refactoring. This program is incorrectly refactored in the same way by HLint 3.6.1:

import Data.Maybe

main =
  maybe
    (fail "Failed, there are no elements")
    (putStrLn . ("Average: " <>) . show) $
      -- This comment causes the same issue
      Nothing

I can try to tackle it if that's a blocker for this PR, but maybe it should be its own PR?

[ysangkok]

Ok, thanks for investigating, I will make a separate bug report. I am not a maintainer, so I can't say what is a blocker or not.

[rhendric]

After poking around a bit, I suspect the issue would have to be reported and fixed in mpickering/apply-refact.

[ysangkok]

Ok, but what about the second bullet point above? If you remove the comment, the let gets indented too far to the right. Is that an issue with this PR or with apply-refact?

[rhendric]

That's purely a formatting issue, and so has to be implemented in apply-refact. But someone would have to spec out the logic of when to join lines together and when to leave them separate, which may not have been necessary before this PR.

[ysangkok]

The 'Avoid lambda' suggestion seems to be triggered by the use of BlockArguments.

If I remove the extension, and insert the dollar, and run hlint, I don't get the following bad hint.

Is this PR aiming to address this issue, or should it be left for an upcoming PR?

% git switch rhendric/blockarguments
Switched to branch 'rhendric/blockarguments'
% cabal install --overwrite-policy=always
Symlinking 'hlint' to '/home/janus/.local/bin/hlint'
% hlint /tmp/Main.hs
Main.hs:6:14-22: Suggestion: Avoid lambda
Found:
  \ () -> 42
Perhaps:
  (\ () -> 42)

1 hint

Input file:

{-# LANGUAGE BlockArguments #-}

import Control.Monad

main =
  forM_ [()] \() -> 42

[rhendric]

That's an issue with #1634. I'd say fix that in a separate PR, since it doesn't interact with this new hint.