mpickering / apply-refact

Refactor Haskell source files
BSD 3-Clause "New" or "Revised" License
147 stars 29 forks source link

Apply-Refact not applying HLint suggestions. #39

Open SarthakKumar1997 opened 5 years ago

SarthakKumar1997 commented 5 years ago

I recently added some refactorings to some HLint suggestions (https://github.com/ndmitchell/hlint/pull/614/files). The suggestions were related to list comprehensions, and function names adhering to the camelCase naming convention.

In the case of camelCase, while HLint provides the right refactorings and apply-refact, using the -s refactor option, detects that a change is available to be made, the apply-refact package refuses to make the necessary change.

For reference, here is a sample piece of code that I used to test this behaviour:

memoized_fib = (map fib [0 ..] !!)
  where
    fib 0 = 0
    fib 1 = 1
    fib n = memoized_fib (n - 2) + memoized_fib (n - 1)

fast_fib n = fib' 0 1 n
  where
    fib' a _ 0 = a
    fib' a b n = fib' b (a + b) (n - 1)

slow_fib 0 = 0
slow_fib 1 = 1
slow_fib n = slow_fib (n - 2) + slow_fib (n - 1)

Is this not working because this is a Bind type refactoring, or could there be an alternative reason?

Please let me know if you need any further information from my side.

zliu41 commented 4 years ago

Hmm, this one looks tricky - it seems there are bugs in both HLint and apply-refact related to this.

Besides, the current approach relies on unsafePrettyPrint to print the entire LHsDecl:

https://github.com/ndmitchell/hlint/blob/6657cbc299de19c26114ddfdbb706cdfa0019eb8/src/Hint/Naming.hs#L73

which can be quite large, e.g., the entire function definition. This is undesirable - it's better to shift as much printing work to apply-refact as possible.

zliu41 commented 4 years ago

I disabled refactoring for this hint in https://github.com/ndmitchell/hlint/pull/1036.