Errors from confusing whether to use `let` or `bind`

[Abastro]

I am quite sure I've seen more cryptic error messages, but for now I can only found simpler ones.

Problem

I noticed that there are many errors coming from confusion between:

do
  foo <- bar baz
  useFoo foo

vs.

do
  let foo = bar baz
  useFoo foo

It would be great if we could give a suggestion.

Cases and Errors

StackOverflow search gave a few such cases.

Code:

parseDnsMessage :: BG.BitGet DnsMessage

recQuery :: BS.ByteString -> String -> IO BS.ByteString

resolveName :: [Word8] -> [Word8] -> BS.ByteString -> String
resolveName qname name bstr = do
  let newbstr = BSL.toStrict $ replace (BS.pack qname) (BS.pack name) bstr
  retbstr <- recQuery newbstr (head rootServers4)
  let msg = BG.runBitGet retbstr parseDnsMessage
  case msg of
    Right m -> (intercalate "." $ map show (rdata $ head $ answer $ m))

Error message:

Couldn't match expected type ‘[BSI.ByteString]’
            with actual type ‘IO BSI.ByteString’
In a stmt of a 'do' block:
  retbstr <- recQuery newbstr (head rootServers4)
In the expression:
  do { let newbstr
             = BSL.toStrict $ replace (BS.pack qname) (BS.pack name) bstr;
       retbstr <- recQuery newbstr (head rootServers4);
       let msg = BG.runBitGet retbstr parseDnsMessage;
       case msg of {
         Right m
           -> (intercalate "." $ map show (rdata $ head $ answer $ m)) } }

Code:

title :: IO Html
title = do
    y <- getCurrentYear
    return $ toHtml $ "Registration " ++ y

getRootR :: Handler RepHtml
getRootR = do
    (widget, enctype) <- generateFormPost personForm -- not important for the problem at hand, comes from the example in the yesod book
    defaultLayout $ do
        setTitle title -- this is where I get the type error
[...]

Error:

Couldn't match expected type `Html' with actual type `IO Html'
In the first argument of `setTitle', namely `title'
[...]

(You know there is no suggestion in ... part)

This one is special in that they are trying to use top-level IO action. still, likely a similar fix.

Code:

displayList :: [Int] -> IO()
displayList [] = putStrLn ""
displayList (firstUnit:theRest) =  putStrLn (show firstUnit ++ "\n" ++ 
                                   displayList theRest)

Error:

• Couldn't match expected type ‘[Char]’ with actual type ‘IO ()’
• In the second argument of ‘(++)’, namely ‘(displayList theRest)’
  In the first argument of ‘putStrLn’, namely
    ‘((show firstUnit) ++ (displayList theRest))’
  In the expression:
    putStrLn ((show firstUnit) ++ (displayList theRest))

Suggestion

Let me consider the last one.

Couldn't match expected type ‘[Char]’ with actual type ‘IO ()’
  In the second argument of ‘(++)’, namely ‘(displayList theRest)’
  In the first argument of ‘putStrLn’, namely
    ‘((show firstUnit) ++ (displayList theRest))’
  In the expression:
    putStrLn ((show firstUnit) ++ (displayList theRest))

Perhaps add suggestion like this:

Couldn't match expected type ‘[Char]’ with actual type ‘IO ()’
Perhaps you would like to bind `displayList theRest` in the do notation.
  In the second argument of ‘(++)’, namely ‘(displayList theRest)’
[...]

(Though, I do not like the word bind here)

or this:

Couldn't match expected type ‘[Char]’ with actual type ‘IO ()’
Suggested fix: use `<-` in do notation.
  In the second argument of ‘(++)’, namely ‘(displayList theRest)’
[...]
Perhaps you want to use:
  do
    d <- displayList theRest
    -- use d here
instead of:
  (displayList theRest)
in the expression.

[goldfirere]

Thanks for bringing this up.

I like your last suggestion the most, but I don't think I'd put the code repair there. There are too many opportunities for it to be wrong and lead the user down a blind alley. (Your suggestion might get the user to nest dos unnecessarily, or d might already be bound, or the user might replace (displayList theRest) with the do block mentioned, or something else.) I think use `<-` in do notation is informative enough.

Slightly larger context: I think suggesting repairs is properly the job of HLS, not GHC. HLS has the power to, say, analyze the user's code in the area and find the right fix. (One could even argue that use `<-` in do notation is something HLS should come up with, but that's a mismatch for current practice.)

[ketzacoatl]

Perhaps you would like to bind displayList theRest in the do notation.

I like this suggestion a lot, though as a user I would like some clarification around what we mean by "bind", or how to do it (eg in this case it's "use <-", and that's not obvious the first time around).

[Abastro]

I see, so we would better not have fully-fledged suggestion from GHC. At least giving hint to the users & HLS about let vs. <- would be nice, tho the wording is p hard. Meh, the naming bind is not explicit in code..

[MorrowM]

As far as terminology goes, I would phrase it

Perhaps you meant to bind the result of foo with <-, instead of binding foo itself with let?

It is a bit wordy, though.

[Abastro]

Wish I were better at building a comprehensible sentence :<