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

Fix -Wall in qualified imported names plugin #4070

Merged

Conversation

jhrcek
Copy link
Collaborator

@jhrcek jhrcek commented Feb 12, 2024

No description provided.

@jhrcek jhrcek requested a review from eddiemundo as a code owner February 12, 2024 13:24
@@ -111,7 +110,7 @@ data ImportedBy = ImportedBy {
}

isRangeWithinImportedBy :: Range -> ImportedBy -> Bool
isRangeWithinImportedBy range (ImportedBy _ srcSpan) = fromMaybe False $ spanContainsRange srcSpan range
isRangeWithinImportedBy range ImportedBy{importedBySrcSpan} = fromMaybe False $ spanContainsRange importedBySrcSpan range
Copy link
Collaborator Author

@jhrcek jhrcek Feb 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree that using named field puns could be controversial, I'm just fixing warnings of the type
Defined but not used: ‘importedBySrcSpan’

Other ways to make this go away:

  • prefix record selectors in data type definition with _
  • use the record selectors (e.g. importedBySrcSpan ib) in function body

Also agree that these are inconsequential warnings, but I'm really close to fixing all the warnings/enabling pedantic flag everywhere and don't want to disable warnings selectively just because these last few small things 😄

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine, and I don't think we have any policy regarding NamedFieldPuns.

@jhrcek jhrcek merged commit 409bf3b into haskell:master Feb 12, 2024
39 checks passed
@jhrcek jhrcek deleted the jhrcek/fix-Wall-qualify-imported-names branch February 22, 2024 05:44
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

Successfully merging this pull request may close these issues.

2 participants