Apply Hlint doesn't work reliably

[AlexeyRaga]

It looks like Apply Hlint is broken now. According to the log the diff part is always empty.
Here is the relevant part of the log from HIE:

2018-04-22 00:01:27.351504 [ThreadId 7] - applyHint:ideas=[/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod65311/Main65310-0.hs:90:54: Warning: Redundant bracket
Found:
  "  " <> (show inputTopic)
Why not:
  "  " <> show inputTopic
]

2018-04-22 00:01:27.385353 [ThreadId 7] - applyHint:commands=[("/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod65311/Main65310-0.hs:90:54: Warning: Redundant bracket\nFound:\n  \"  \" <> (show inputTopic)\nWhy not:\n  \"  \" <> show inputTopic\n",[Replace {rtype = Expr, pos = SrcSpan {startLine = 90, startCol = 62, endLine = 90, endCol = 79}, subts = [("x",SrcSpan {startLine = 90, startCol = 63, endLine = 90, endCol = 78})], orig = "x"}])]

2018-04-22 00:01:27.395631 [ThreadId 7] - applyHint:diff=WorkspaceEdit {_changes = Just (fromList [(Uri {getUri = "file:///Users/araga/src/arbor/topictap/app/Main.hs"},List [])]), _documentChanges = Nothing}

2018-04-22 00:01:27.464431 [ThreadId 7] - applyOneCmd:file="/Users/araga/src/arbor/topictap/app/Main.hs"

2018-04-22 00:01:27.464508 [ThreadId 7] - applyOneCmd:res=Right (WorkspaceEdit {_changes = Just (fromList [(Uri {getUri = "file:///Users/araga/src/arbor/topictap/app/Main.hs"},List [])]), _documentChanges = Nothing}

Examples:

This doesn't work (log above)

forM_ inputTopics $ \inputTopic -> logInfo $ "  " <> (show inputTopic)

But this works fine

forM_ inputTopics $ \inputTopic -> logInfo $ "  " <> show (inputTopic)

I made some tests and it looks like in a number of examples suggestions that impact code after <> are not getting applied.

[AlexeyRaga]

Another thing that I've found is that when there is more than one hlint suggestion then only one gets applied:

2018-04-22 00:39:39.420003 [ThreadId 7] - applyHint:ideas=[/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:9: Warning: Redundant bracket
Found:
  (logInfo)
Why not:
  logInfo
,/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:9: Warning: Redundant $
Found:
  (logInfo) $
    ("Preparing staging directory: " <> (show stagingDirectory))
Why not:
  (logInfo)
    ("Preparing staging directory: " <> (show stagingDirectory))
,/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:22: Warning: Redundant bracket
Found:
  "Preparing staging directory: " <> (show stagingDirectory)
Why not:
  "Preparing staging directory: " <> show stagingDirectory
]

2018-04-22 00:39:39.449404 [ThreadId 7] - applyHint:commadns=[("/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:9: Warning: Redundant bracket\nFound:\n  (logInfo)\nWhy not:\n  logInfo\n",[Replace {rtype = Expr, pos = SrcSpan {startLine = 92, startCol = 9, endLine = 92, endCol = 18}, subts = [("x",SrcSpan {startLine = 92, startCol = 10, endLine = 92, endCol = 17})], orig = "x"}]),("/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:9: Warning: Redundant $\nFound:\n  (logInfo) $\n    (\"Preparing staging directory: \" <> (show stagingDirectory))\nWhy not:\n  (logInfo)\n    (\"Preparing staging directory: \" <> (show stagingDirectory))\n",[Replace {rtype = Expr, pos = SrcSpan {startLine = 92, startCol = 9, endLine = 92, endCol = 81}, subts = [("a",SrcSpan {startLine = 92, startCol = 9, endLine = 92, endCol = 18}),("b",SrcSpan {startLine = 92, startCol = 21, endLine = 92, endCol = 81})], orig = "a b"}]),("/private/var/folders/3x/l50y6rts50j_9jqyp0fl7lqh0000gn/T/ghc-mod74468/Main74467-25.hs:92:22: Warning: Redundant bracket\nFound:\n  \"Preparing staging directory: \" <> (show stagingDirectory)\nWhy not:\n  \"Preparing staging directory: \" <> show stagingDirectory\n",[Replace {rtype = Expr, pos = SrcSpan {startLine = 92, startCol = 57, endLine = 92, endCol = 80}, subts = [("x",SrcSpan {startLine = 92, startCol = 58, endLine = 92, endCol = 79})], orig = "x"}])]

2018-04-22 00:39:39.466565 [ThreadId 7] - applyHint:diff=WorkspaceEdit {_changes = Just (fromList [(Uri {getUri = "file:///Users/araga/src/arbor/topictap/app/Main.hs"},List [TextEdit {_range = Range {_start = Position {_line = 91, _character = 0}, _end = Position {_line = 91, _character = 80}}, _newText = "        (logInfo) (\"Preparing staging directory: \" <> (show stagingDirectory))"}])]), _documentChanges = Nothing}

An interesting thing is that the 2nd (out of 3) one gets applied for some reason.

[AlexeyRaga]

This is what is being sent from Server to Client:
https://github.com/haskell/haskell-ide-engine/blob/master/src/Haskell/Ide/Engine/Transport/LspStdio.hs#L559

Note that it only contains file and start position, but does not indicate the refactor itself.
Probably HIE then gets confused when the command is returned from the UI in case if more than one refactoring is available for the same line/position.

{ _jsonrpc = "2.0"
, _id = IdRspInt 6
, _result =
    Just
      (List
         [ Command
           { _title = "Apply hint:Redundant bracket"
           , _command = "applyrefact:applyOne"
           , _arguments =
               Just
                 (Array
                    [ Object
                        (fromList
                           [ ( "start_pos"
                             , Object
                                 (fromList
                                    [ ("line", Number 55.0)
                                    , ("character", Number 7.0)
                                    ]))
                           , ( "file"
                             , String
                                 "file:///Users/araga/src/arbor/topictap/src/App/AWS/S3.hs")
                           ])
                    ])
           }
         ])
, _error = Nothing
}

[AlexeyRaga]

That's it!
ApplyRefact only considers a file name and a position:
https://github.com/haskell/haskell-ide-engine/blob/master/src/Haskell/Ide/Engine/Plugin/ApplyRefact.hs#L58

And hlint can give more than one suggestion for a given line:

./src/App/AWS/S3.hs:56:8: Warning: Redundant bracket
Found:
  (coMetadataDirective)
Why not:
  coMetadataDirective

./src/App/AWS/S3.hs:56:8: Warning: Use ?~
Found:
  (coMetadataDirective) .~ Just MDCopy
Why not:
  (coMetadataDirective ?~ MDCopy)

We need a way to distinguish between those.
I think that adding an hlint title (like Redundant bracket) would make it better but still won't be enough.

Maybe an HLint suggestion number per file will do?
I'll try experimenting with it.

[alanz]

Thanks