Fix: preserve type applications in record patterns#1680
Open
mgajda wants to merge 1 commit intondmitchell:masterfrom
Open
Fix: preserve type applications in record patterns#1680mgajda wants to merge 1 commit intondmitchell:masterfrom
mgajda wants to merge 1 commit intondmitchell:masterfrom
Conversation
mgajda
added a commit
to mgajda/hlint
that referenced
this pull request
Apr 19, 2026
…untrusted input
data/hlint.yaml: new group 'security', enabled: true.
Rules:
read x -> Text.Read.readMaybe x side: not (isLitString x) CWE-502
reads x -> Text.Read.readMaybe x side: not (isLitString x) CWE-502
Crypto.Hash.MD5.hash -> Crypto.Hash.SHA256.hash CWE-327
Crypto.Hash.MD5.hashlazy -> Crypto.Hash.SHA256.hashlazy CWE-327
Crypto.Hash.SHA1.hash -> Crypto.Hash.SHA256.hash CWE-327
Crypto.Hash.SHA1.hashlazy -> Crypto.Hash.SHA256.hashlazy CWE-327
Data.Digest.Pure.MD5.md5 -> Crypto.Hash.SHA256.hashlazy CWE-327
Data.Digest.Pure.SHA.sha1 -> Data.Digest.Pure.SHA.sha256 CWE-327
Disable per-rule via `-i "<name>"` or suppress the whole group with
`- group: {name: security, enabled: false}` in .hlint.yaml.
tests/security.test: four cases.
1. security-hash.hs : all six hash rules fire.
2. security-read-nonliteral.hs: `read x` fires for variable `x`.
3. security-read-literal.hs : `read "42"` and `read "3.14"` do not fire.
4. security-ignored.hs : per-rule -i suppression yields no hints.
Self-test: 974 tests, 3 failures (all pre-existing on master: issue ndmitchell#1674
intercalate/OverloadedStrings and two record-pattern type-application
cases from PR ndmitchell#1680).
Running hlint with the new group on hlint's own source produces one
finding: src/Test/InputOutput.hs:57 `read code` (parsing an EXIT line
from a .test file).
No HSEC advisory on github.com/haskell/security-advisories maps to
CWE-327 or CWE-502 for these function classes.
…1679) When suggesting record patterns, check if the constructor has meaningful type applications (e.g. @int, @type). Type applications can be removed only if there are no type arguments, or all type arguments are wildcards (@_). This prevents HLint from suggesting invalid transformations that would remove necessary type information. Examples: Ast.AstScatterS @_ @Shn1 @shp1 _ _ _ _ _ -> Suggests: Ast.AstScatterS {} (wildcards can be omitted) Ast.AstScatterS @A @Shn1 @shp1 _ _ _ _ _ -> No suggestion (meaningful type apps must be preserved) Test cases: - foo (Bar _ _ _ _) = x -> suggests Bar{} - foo (Bar @_ _ _ _) = x -> suggests Bar{} (@_ is redundant) - foo (Bar @int _ _ _) = x -> no suggestion (type app preserved)
824637e to
972b0d4
Compare
mgajda
added a commit
to mgajda/hlint
that referenced
this pull request
Apr 19, 2026
…untrusted input
data/hlint.yaml: new group 'security', enabled: true.
Rules:
read x -> Text.Read.readMaybe x side: not (isLitString x) CWE-502
reads x -> Text.Read.readMaybe x side: not (isLitString x) CWE-502
Crypto.Hash.MD5.hash -> Crypto.Hash.SHA256.hash CWE-327
Crypto.Hash.MD5.hashlazy -> Crypto.Hash.SHA256.hashlazy CWE-327
Crypto.Hash.SHA1.hash -> Crypto.Hash.SHA256.hash CWE-327
Crypto.Hash.SHA1.hashlazy -> Crypto.Hash.SHA256.hashlazy CWE-327
Data.Digest.Pure.MD5.md5 -> Crypto.Hash.SHA256.hashlazy CWE-327
Data.Digest.Pure.SHA.sha1 -> Data.Digest.Pure.SHA.sha256 CWE-327
Disable per-rule via `-i "<name>"` or suppress the whole group with
`- group: {name: security, enabled: false}` in .hlint.yaml.
tests/security.test: four cases.
1. security-hash.hs : all six hash rules fire.
2. security-read-nonliteral.hs: `read x` fires for variable `x`.
3. security-read-literal.hs : `read "42"` and `read "3.14"` do not fire.
4. security-ignored.hs : per-rule -i suppression yields no hints.
Self-test: 974 tests, 3 failures (all pre-existing on master: issue ndmitchell#1674
intercalate/OverloadedStrings and two record-pattern type-application
cases from PR ndmitchell#1680).
Running hlint with the new group on hlint's own source produces one
finding: src/Test/InputOutput.hs:57 `read code` (parsing an EXIT line
from a .test file).
No HSEC advisory on github.com/haskell/security-advisories maps to
CWE-327 or CWE-502 for these function classes.
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.
Fixes #1679
Problem
HLint was suggesting record pattern syntax that removes type applications, producing invalid code.
Example
Result: Type applications are lost, code doesn't parse!
Root Cause
The record pattern suggestion in
patHintdidn't check if the constructor had type arguments. It would suggest the record syntax{}even when type applications were present, which would strip them away.Solution
Check if the
PrefixConpattern has any type arguments before suggesting record pattern conversion. If type arguments exist, skip the suggestion.Changes
patHintinsrc/Hint/Pattern.hsto check for empty type argument listTesting
Test cases verify:
foo (Bar _ _ _ _) = x→ suggestsBar{}(normal case works)foo (Bar @A @B _ _ _) = x→ no suggestion (multiple type apps preserved)All existing tests pass.