Hlint "Expected type not found at the location specified in the refact file." error in certain circumstances when refactoring

[FreyrDev]

Your environment

Which OS do you use:
- Windows
Which LSP client (editor/plugin) do you use:
- VS Code
Describe your project (alternative: link to the project):
- GHC 8.10.7, Cabal 3.6.2.0

Steps to reproduce

Create a new .hs file with this code:

import Data.List (sortOn)
import Control.Arrow ((&&&))
import Data.Ord (Down(Down))

functionA :: [String] -> [(Char,Int)]
functionA = reverse . sortOn snd . map (head &&& length)

functionB :: [String] -> [(Char,Int)]
functionB = reverse . sortOn snd . map (head &&& length) . id

Both functions will be highlighted by hlint telling you to avoid using the reverse
Use the quick fix feature (Ctrl+. in VS Code) to apply the refactor on each function

Expected behaviour

The error should not appear, and the avoid reverse refactor should produce the same code with either function:

functionB = sortOn (Down . snd) . map (head &&& length) . id

Actual behaviour

Using the quick fix while the cursor is on the body of functionA will refactor the code as expected
However using the Quick Fix while the cursor is on the body of functionB will produce the follwing error:

hlint:applyOne: "Expected type not found at the location specified in the refact file.\n Expected type: HsExpr (GhcPass 'Parsed)\n Location: C:\\Users\\<username>\\Personal\\Hobbies\\Programming\\test.hs:9:36-61: inappropriate type"

Additonally the redundant id suggested refactor doesn't do anything at all in this circumstance.

Include debug information

Extended error message from the logs:

[2022-01-20 02:12:51.997] [exthost] [error] Error: hlint:applyOne: "Expected type not found at the location specified in the refact file.\n  Expected type: HsExpr (GhcPass 'Parsed)\n  Location: C:\\Users\\<username>\\Personal\\Hobbies\\Programming\\test.hs:9:36-61: inappropriate type"
	at c:\Users\<username>\.vscode\extensions\haskell.haskell-1.8.0\dist\extension.js:2:1005073
	at c:\Users\<username>\.vscode\extensions\haskell.haskell-1.8.0\dist\extension.js:2:1005367
	at Immediate.<anonymous> (c:\Users\<username>\.vscode\extensions\haskell.haskell-1.8.0\dist\extension.js:2:1005732)
	at processImmediate (internal/timers.js:461:21) 7804:hlint:applyOne undefined

[jneira]

Hi, thanks for the bug report, have you got the chance of checking if the standalone hlint executable (version 3.2.8 if possible) works?

[FreyrDev]

So running hlint test.hs in windows terminal gives an error:

hlint : The term 'hlint' is not recognized as the name of a cmdlet, function, script file, or operable program. Check
the spelling of the name, or if a path was included, verify that the path is correct and try again.
At line:1 char:1
+ hlint test.hs
+ ~~~~~
    + CategoryInfo          : ObjectNotFound: (hlint:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CommandNotFoundException

This implies that hlint isn't installed properly (though the fact that it was working most of the time in VSC is a bit weird if that was the case), I tried to reinstall it with cabal install hlint, and the same errors still occur after installation

[jneira]

oh, sorry I did not mention vscode does not use the hint executable but hlint as a library so probably it was not available in your env
but cabal install hlint-3.2.8 should install it, maybe you don't have the path where cabal put the executables in you global PATH. iirc cabal tells you where is installing it

[FreyrDev]

Ok so that fails after it's installed most of the required libs with this error

Failed to build assoc-1.0.2.
Build log (
C:\cabal\logs\ghc-8.10.7\assoc-1.0.2-d53dd1e0077569ecd82e3d688e4dac257a2715f1.log
):
Preprocessing library for assoc-1.0.2..
Building library for assoc-1.0.2..
[1 of 2] Compiling Data.Bifunctor.Assoc ( src\Data\Bifunctor\Assoc.hs, dist\build\Data\Bifunctor\Assoc.o )
[2 of 2] Compiling Data.Bifunctor.Swap ( src\Data\Bifunctor\Swap.hs, dist\build\Data\Bifunctor\Swap.o )
dist\build\objs-50224: removeDirectoryRecursive:removeContentsRecursive:RemoveDirectory "\\\\?\\C:\\Users\\<username>\\AppData\\Local\\Temp\\cabal-install.-39132\\dist-newstyle\\tmp\\src-39140\\assoc-1.0.2\\dist\\build\\objs-50224": unsatisfied constraints (The directory is not empty.)
cabal-3.6.2.0.exe: Failed to build assoc-1.0.2 (which is required by exe:hlint
from hlint-3.2.8). See the build log above for details.
Failed to build indexed-traversable-instances-0.1.1 (which is required by
exe:hlint from hlint-3.2.8). The failure occurred during the final install
step. The exception was:
C:\Users\<username>\AppData\Local\Temp\cabal-install.-39132\dist-newstyle\tmp\package-registration--39132:
removeDirectoryRecursive:removeContentsRecursive:RemoveDirectory
"\\\\?\\C:\\Users\\<username>\\AppData\\Local\\Temp\\cabal-install.-39132\\dist-newstyle\\tmp\\package-registration--39132":
unsatisfied constraints (The directory is not empty.)

As far as I can tell cabal is installing to C:\cabal\bin as that folder contains an hlint.exe (from before I ran the install 3.2.8 command), and my PATH has a CABAL_DIR variable pointing to C:\cabal but not to the bin folder, could that be the problem?

[jneira]

to make available executables installed with cabal in the shell the PATH env var must include C:\cabal\bin

[FreyrDev]

Both refactor suggestions show up correctly when using hlint in the command line, however the error still appears in VSC, so it might be a problem with how hlint is implemented in VSC or just a problem with VSC itself.

[jneira]

Thanks for trying, yeah, the main goal was ensure the bug is in our side and it seems so

[eddiemundo]

It looks like the right refactorings are being sent to apply-refact in both cases except one doesn't work.

The "f" is the snd and according to examples endCol should be one past the last column so things look OK. "x" is the rest of the line.

Works:

[
    [Replace {
        rtype = Expr, pos = SrcSpan {
            startLine = 24, startCol = 13, endLine = 24, endCol = 57
        }, subts = [("f", SrcSpan {
            startLine = 24, startCol = 30, endLine = 24, endCol = 33
        }), ("x", SrcSpan {
            startLine = 24, startCol = 36, endLine = 24, endCol = 57
        })], orig = "sortOn (Down . f) . x"
    }]
]

Doesn't work:

[
    [Replace {
        rtype = Expr, pos = SrcSpan {
            startLine = 27, startCol = 13, endLine = 27, endCol = 62
        }, subts = [("f", SrcSpan {
            startLine = 27, startCol = 30, endLine = 27, endCol = 33
        }), ("x", SrcSpan {
            startLine = 27, startCol = 36, endLine = 27, endCol = 62
        })], orig = "sortOn (Down . f) . x"
    }]
]

the difference being the id.

These examples also don't work

-- same error
functionC = reverse . sortOn snd . id . map (head &&& length)
-- does nothing even though `id` should be removed
functionD = sortOn (Down . snd) . map (head &&& length) . id
-- does nothing even though `id` should be removed
functionE a = (a +) . (a +) . id

This example does work

functionF = reverse . sortOn snd . (map (head &&& length) . id)

So it seems to have something to do with not recognizing expr . id as an expression. This seems to happen when apply-refact calls syb.
https://github.com/mpickering/apply-refact/blob/0414ce677e7144a3048b01fffc2724eef21d5bf3/src/Refact/Internal.hs#L699-L719

Don't really have an idea of why it works for hlint and not HLS. hlint seems to call the apply-refact executable, but didn't really look closely.

[eddiemundo]

It might have to do with application of fixities because the error is due to the fact that the AST looks like

op
  op
    arg1
    arg2
  id

but we're trying to substitute op arg2 id which no src span matches so syb fails. I'll try figuring it out later.