search

ndmitchell / hlint

Haskell source code suggestions
Other
1.48k stars 195 forks source link

Fix no warn on (x.y) by treating x.y as atom #1478

Closed benbellick closed 1 year ago

benbellick commented 1 year ago

This is to resolve issue #1471

shayne-fletcher commented 1 year ago

the fix looks good but the test doesn't show it. OverloadedRecordDot is not default enabled so in issue1471 = (x.y), (x.y) is parsed to a parenthesized function composition and will generate a redundant brackets hint both before your fix and after it.

i suggest these tests from #1471:

-- no record dot syntax
referenceDesignator = ReferenceDesignator (p.placementReferenceDesignator)

-- record dot syntax
{-# LANGUAGE OverloadedRecordDot #-} \
referenceDesignator = ReferenceDesignator (p.placementReferenceDesignator) -- p.placementReferenceDesignator

as reported in the ticket, for the second test, before your fix it won't generate a redundant bracket hint but after your fix it will!

benbellick commented 1 year ago

Hey Shayne, thanks for the comments! I have updated the tests. Just to be clear, in the first case (w/o record dot syntax), the expression p.placementReferenceDesignator is parsed as p . placementReferenceDesignator, i.e. as two functions combined with the composition operator .. Is that correct?

shayne-fletcher commented 1 year ago

Just to be clear, in the first case (w/o record dot syntax), the expression p.placementReferenceDesignator is parsed as p . placementReferenceDesignator, i.e. as two functions combined with the composition operator .. Is that correct?

yes. if OverloadedRecordDot is not in effect the . in the expression above is parsed as an ITdot token which denotes function composition.

benbellick commented 1 year ago

It seems that OverloadedRecordDot was added in version 9.2, so version 9.0 is perhaps causing a test failure. Any suggestion on how to deal with this? Perhaps a way to ignore certain tests based off of version?

shayne-fletcher commented 1 year ago

@benbellick if the second test were updated to

-- record dot syntax
{-# LANGUAGE OverloadedRecordDot #-} \
referenceDesignator = ReferenceDesignator (p.placementReferenceDesignator) -- @Warning p.placementReferenceDesignator @NoRefactor: refactor requires GHC >= 9.2.1

i think it will pass the currently failing 9.0 test.

benbellick commented 1 year ago

@shayne-fletcher Thanks for the help. I have updated the test!

shayne-fletcher commented 1 year ago

Thanks for contributing to hlint!

benbellick commented 1 year ago

My pleasure! I love Haskell and hope to keep contributing to the community however I can. Thanks for the help!