Wildcards for TemplateHaskellQuotes

[yairchu]

Suppose one would like a rule that will suggest replacing varE 'mempty with [|mempty|], or in a more general fashion:

- hint: {lhs: "varE 'a", rhs: "[|a|]"}

This doesn't work currently because a in the rule above isn't treated as a wildcard.

[ndmitchell]

I think its possible - I'm just not convinced how many rules of that form people are likely to want? If it's only one or two, they could be hardcoded as built in rules (if that's a general substitution property? I have no idea).

[yairchu]

It would probably be a bunch of rules, at the very least:

- hint: {lhs: "varE 'a", rhs: "[|a|]"}
- hint: {lhs: "conE ''a", rhs: "[|a|]"}
- hint: {lhs: "conT ''a", rhs: "[t|a|]"}
- hint: {lhs: "conP ''a", rhs: "[p|a|]"}

- hint: {lhs: "varE ''a `appE` b", res: "[|a $b|]"}
- hint: {lhs: "conE ''a `appE` b", res: "[|a $b|]"}
- hint: {lhs: "conT ''a `appT` b", res: "[t|a $b|]"}
- hint: {lhs: "conP ''a `appP` b", res: "[p|a $b|]"}

This could serve as a useful teaching tool pointing people to write things more easily using the [||] Template Haskell syntax.

[ndmitchell]

I'm not averse to a PR - I expect its just a few additional rules in the matching engine and wouldn't add much complexity.

[yairchu]

Do you have a recommended method to quickly see which LHsExpr GhcPs a given expression becomes? (from just reviewing the constructors of HsExpr I can't come with a good guess)

[ndmitchell]

See https://github.com/ndmitchell/hlint/blob/master/README.md#hacking-hlint, in particular https://github.com/ndmitchell/hlint/blob/master/README.md#printing-abstract-syntax

[yairchu]

Awesome thanks! Apparently what I was looking for was HsBracket with a VarBr inside. Didn't expect ' to be called a bracket!

[yairchu]

I've tried to make the changes, and they successfully produce the Subst, but when I tried to make substitute support substituting in these positions, even if I only try to trigger an error at these positions, that doesn't seem to happen (see https://github.com/yairchu/hlint/commit/5771c3e3ecb84dd653772155ad62f482347453c6), and I have difficulties following transformBracketOld to understand why, do you happen to have any idea?

[ndmitchell]

See https://github.com/yairchu/hlint/blob/5771c3e3ecb84dd653772155ad62f482347453c6/src/GHC/Util/Unify.hs#L69-L97 - which knows where variables are that can be substituted. My guess is you need to add a new case there for template haskell variables that can be substituted.

[yairchu]

I'm afraid I'm dumbfounded, even without fixing substitute, the rules trigger if I don't include the substitution variables in the rhs. So

- hint: {lhs: "varE 'a", rhs: "17"}

Successfully trigger, even though 17 isn't correct there :) Once the rhs includes a it won't trigger.

[ndmitchell]

After a substitution, we check the free variables of the term, and if additional fresh variables have been introduced, we assume that was because something got lifted out from under a lambda, and thus the hint doesn't really apply here. The code that does it is at https://github.com/ndmitchell/hlint/blob/master/src/Hint/Match.hs#L155-L156

You might need to tweak freeVars to note that quoted variables are still valid. Are they free variables in the Haskell sense?

[yairchu]

Now with the apply-refact changes the test passes.

Note that I also changed the PR to only add one rule for now, because it appears that additional changes might be needed to support matching for names which start with capital letters (types and data constructors).

[ndmitchell]

Is this all done now? Or are there more things that need doing?

[yairchu]

There are still some things that need doing. For example, the hint

- hint: {lhs: "conE 'a", rhs: "[|a|]"}

Is not there, because it is supposed to matched on things like conE 'Just, but if we were to test that, we would see that it won't work. I haven't yet looked into why (whether it's the substitution part or free-vars or something else).

[parsonsmatt]

This is unrelated to the issue at hand, but you should know that this hint isn't equivalent. varE 'a requires that a is in scope, while [|a|] will make an UnboundVarE (mkName "a") if a is not in scope. This can cause bugs for folks, even if it's not a huge deal.

[ndmitchell]

This sounds like suggesting varE 'a ==> [|a|] is a bad thing, given the inherent surprise footgun. @yairchu, it sounds like we should be removing that hint? Or maybe putting it in a group which is disabled by default?

[yairchu]

It's an interesting case, because when the rule applies it doesn't affect the behavior, rather it only affects the behavior upon future changes. In cases where the TH code is tested it would result in worse error messages, and in cases where it's not the error would be deferred to the future like in Python.

Or maybe putting it in a group which is disabled by default?

This seems to be the best approach. I'll send a PR.

[yairchu]

Should the group for it be generalise-for-conciseness? It generalises the code to also work if the name wasn't in scope..

[ndmitchell]

I think the generalise for conciseness is quite a different type of generalisation, so I'd create a new group - perhaps use-th-quotes?