Fix: narrow "Parenthesize unary negation" to surprising precedence (fixes #1636)#1685
Open
mgajda wants to merge 1 commit into
Open
Fix: narrow "Parenthesize unary negation" to surprising precedence (fixes #1636)#1685mgajda wants to merge 1 commit into
mgajda wants to merge 1 commit into
Conversation
…ixes ndmitchell#1636) The hint was introduced in ndmitchell#1484 for cases where unary negation combines with higher-fixity operators in non-obvious ways: -1 ^ 2 -- parses as -(1^2) = -1, not (-1)^2 = 1 -5 `mod` 3 -- parses as -(5 `mod` 3), which returns -2 It also fired on fixity-7 symbolic operators (`*`, `+`, `/`), where the precedence matches the conventional mathematical reading and the extra parens are just noise — as flagged in ndmitchell#1636: addUTCTime (-365.25 * nominalDay * fromIntegral y) Narrow the rule to only fire on operators that are actually surprising: - alphanumeric functions infixed with backticks (fixity 9) - exponentiation: `^`, `^^`, `**` (fixity 8) This keeps the canonical ndmitchell#1484 cases firing while silencing the ndmitchell#1636 noise.
2a839c0 to
c4229fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1636. The "Parenthesize unary negation" hint (introduced by #1484)
was firing on conventional arithmetic operators where the precedence
matches the reader's mental model, producing noise like:
Narrow the rule to the operators where precedence around unary
-isgenuinely surprising:
-5 `mod` 3parses as-(5 \mod` 3) = -2` — the classic Suggest explicit brackets to mitigate counter-intuitive prefix negation precedence #1484 case.^/^^/**(fixity 8):-1 ^ 2 = -1, not1— also canonical in Suggest explicit brackets to mitigate counter-intuitive prefix negation precedence #1484.
Conventional arithmetic (
+,-,*,/, fixity 6–7) no longertriggers the hint.
Test plan
hlint --testpasses (969 tests)-1 ^ 2still triggers (canonical Suggest explicit brackets to mitigate counter-intuitive prefix negation precedence #1484 case preserved)-365.25 * nominalDay * fromIntegral yno longer triggers (Incorrect "Parenthesize unary negation" suggestion #1636)