Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't warn about redundant parentheses around record pattern matching #1282

Closed
anka-213 opened this issue Aug 20, 2021 · 5 comments
Closed

Comments

@anka-213
Copy link
Contributor

anka-213 commented Aug 20, 2021

The rule that A {x = y} binds more tightly than function application f x is counter-intuitive and confusing.

f A {x = y}

looks like f is applied to two arguments rather than one.

What I would like is a rule that suggests adding parentheses around A {x = y} and disables the warning about redundant parentheses for that case.

@ndmitchell
Copy link
Owner

This is something people disagree on, so I think HLint should remain neutral - in particular the lack of brackets can turn out to be very handy for certain DSLs. That said, given people do disagree, I don't think HLint should suggest removing the bracket. Using the test case:

f (A{x=1}) = g (B{x=1})

I see that it does suggest removing the brackets on the LHS of the =. I think that would be something that should be fixed

@anka-213
Copy link
Contributor Author

To clarify, I intended this whole issue to be opt-in, especially the suggestion to insert the extra parentheses, since people could disagree.
But yes, I would be very happy about not suggesting to remove the brackets on LHS by default and would be very satisfied with just that and no suggestion to insert parentheses and no configuration option about it. That would be better than I hoped for.

@anka-213 anka-213 changed the title Feature request: Mandate redundant parentheses around record pattern matching Feature request: <s>Mandate</s> Don't warn about redundant parentheses around record pattern matching Aug 31, 2021
@anka-213 anka-213 changed the title Feature request: <s>Mandate</s> Don't warn about redundant parentheses around record pattern matching Feature request: Don't warn about redundant parentheses around record pattern matching Aug 31, 2021
@anka-213 anka-213 changed the title Feature request: Don't warn about redundant parentheses around record pattern matching Don't warn about redundant parentheses around record pattern matching Aug 31, 2021
@ndmitchell
Copy link
Owner

Actually, trying a recent HLint it doesn't warn on either side with brackets. I'm not a massive fan of opt-in hints because they don't usually get used by very many people at all, so are rarely worth the cost, especially for something like inserting brackets which is fiddly to do right

georgefst added a commit to hackworthltd/primer that referenced this issue Oct 20, 2021
The comment about HLint running over the output of `tasty-discover` is only actually true within HLS and not through a local command line or on CI. See ndmitchell/hlint#1245 (comment).

Until haskell/haskell-language-server#638 is fixed, we're going to have to deal with spurious HLint warnings from HLS anyway. It should be easy to silence them in all LSP clients.

And even without that issue, we'll sometimes see extra hints in HLS due to version mismatches, since the HLint in HLS is statically linked, so we have no control over the version. For example, HLS 1.4 alerts to some redundant parentheses at line 140 in `Tests/Gen/Core/Typed.hs`, but these hints have been disabled in a recent HLint version - see ndmitchell/hlint#1282.
georgefst added a commit to hackworthltd/primer that referenced this issue Oct 20, 2021
The comment about HLint running over the output of `tasty-discover` is only actually true within HLS and not through a local command line or on CI. See ndmitchell/hlint#1245 (comment).

Until haskell/haskell-language-server#638 is fixed, we're going to have to deal with spurious HLint warnings from HLS anyway. It should be easy to silence them in all LSP clients.

And even without that issue, we'll sometimes see extra hints in HLS due to version mismatches, since the HLint in HLS is statically linked, so we have no control over the version. For example, HLS 1.4 alerts to some redundant parentheses at line 140 in `Tests/Gen/Core/Typed.hs`, but these hints have been disabled in a recent HLint version - see ndmitchell/hlint#1282.
@tysonzero
Copy link

@ndmitchell In that case I guess this issue is good to close? Do you happen to know what version of hlint changed this? I'm still seeing the redundant brackets suggestion in 3.2.7.

@ndmitchell
Copy link
Owner

@tysonzero - probably v3.3.1 as part of #1225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants