`within` changes the behavior of `modules: {as: }`

[parsonsmatt]

We want to gradually become consistent with our naming of Data.List.NonEmpty, so we create an hlint rule:

- modules:
    - name: [Data.List.NonEmpty] 
      as: NE

I run hlint on our codebase to get all the currently failing examples, and I create a within to ignore them:

- modules:
    - name: [Data.List.NonEmpty] 
      as: NE
      within: [ ... ]

But now it starts complaining about importing Data.List.NonEmpty at all. It seems like within is causing it to ignore the as: key.

[zliu41]

This is indeed very confusing, and I think there are two separate problems:

1. when within is used, everything else including as is ignored, like you said
2. To restrict importing a module itself, one must use within, otherwise the restriction won't apply (i.e., the import is allowed everywhere). But when restricting something else, such as module alias, within is not needed for the restriction to take effect (in fact you can't even use within because of the point above). The behaviors are inconsistent.

I'm marking this as a good first issue. The function that needs updating is Hint.Restrict.checkImports.

[parsonsmatt]

I traced this down a bit, and we're doing something a little odd here:

            restrictWithin <- parseFieldOpt "within" v >>= maybe (pure [("","")]) (parseArray >=> concatMapM parseWithin)

If within is not provided, then we assume [("","")]. When we call within, we do this:

within :: String -> String -> [(String, String)] -> Bool
within modu func = any (\(a,b) -> (a ~= modu || a == "") && (b ~= func || b == ""))
  where (~=) = wildcardMatch

This gives us the behavior of our code.

I think there are two basic approaches here:

1. In checkImports, special-case the riWithin = [("","")] to actually be [] - any _ [] = False, which should trigger a problem. This is the quick and easy solution.
2. Refactor the type of riWithin (and others) to use more explicit datatypes. Maybe to indicate presence/absence of the field, along with a record datatype instead of a tuple to make the meaning of the fields more apparent.

I'm leaning towards 2 - this'll require touching all of the use-sites of the within code, but it should also make it more apparent how the information is being used.

[zliu41]

I'm leaning towards 2

I agree - an explicit data type with meaningful names is better.