apply-refact icon indicating copy to clipboard operation
apply-refact copied to clipboard

Apply-Refact not applying HLint suggestions.

Open SarthakKumar1997 opened this issue 6 years ago • 2 comments

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.

SarthakKumar1997 avatar Apr 12 '19 11:04 SarthakKumar1997

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 avatar Jun 06 '20 07:06 zliu41

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

zliu41 avatar Jun 06 '20 14:06 zliu41