"ignore: {}" causes restricted functions and imports to be ignored

[langston-barrett]

In HLint 2.2.2, the following config:

- ignore: {}
- functions:
  - {name: putStrLn, within: []}
- modules:
  - {name: [Data.List], as: List}

causes no warning to be issued on the following module:

module Main where
import Data.List as L
main :: IO ()
main = putStrLn $ L.intercalate " " ["hlint", "is", "awesome"]

whereas the appropriate warnings are raised without the ignore: {}. This is counterintuitive to me, because the README says:

These directives are applied in the order they are given, with later hints overriding earlier ones.

[ndmitchell]

The logic is that when you type functions you are providing predicates for the banned function warning. However, you aren't enabling the banned function warning, just configuring it. So separately you ignored it, and haven't put in a corresponding unignore directive, so it remains ignored. Does that at least make sense why it happens that way?

[langston-barrett]

Yeah, that makes sense. Is there a way to enable them after configuring them?

It seems like in the more normal configuration (not ignoring everything), enabling and configuring go hand in hand. But when using ignore: {}, these become separate steps. That makes sense of #747 as well, the difference there being that I figured out a workaround for that.

[langston-barrett]

The workaround for modules is to additionally write the following:

- error: {name: "Avoid restricted alias"}

[neilmayhew]

There's a similar workaround for functions. Using this hints file, the behaviour is as desired:

- ignore: {}
- warn: {name: Avoid restricted function}
- warn: {name: Avoid restricted alias}
- functions:
  - {name: putStrLn, within: []}
- modules:
  - {name: [Data.List], as: List}

I therefore think this issue can be closed.

[langston-barrett]

Thank you for the workaround! I still find the behavior to be counter-intuitive. I can't imagine a use-case where someone wants to "define" a restricted function without actually checking that it isn't used. Therefore, I'd prefer that the ticket track a change in this behavior.

[neilmayhew]

Yes, it does seem a bit counter-intuitive. However, I see this as more of a documentation issue than a bug in the code. It depends on the user's understanding of what the functions hint means, and if there was a note in its documentation saying that it won't have any effect if Avoid restricted function is ignored, perhaps that would make things less counter-intuitive.

The same problem would arise if Avoid restricted function was disabled in the source file, so it's not merely an issue with the configuration file syntax (although in that situation it might be more obvious what's going on).

A compromise would be for hlint to issue a warning if functions hints exist and Avoid restricted function is disabled.

[zliu41]

I see no downside to enabling the restricting hints automatically after configuration. This is probably not hard to implement, so marking it a good first issue.