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

Omit more parens for wildcard type signature #2929

Merged
merged 2 commits into from
May 27, 2022
Merged

Conversation

sergv
Copy link
Contributor

@sergv sergv commented May 26, 2022

This is a followup to #2764

I noticed that for cases like foo :: _ the GHC always prints • In the type signature:, but when part of a subtype it will print a stack of contexts ending with In the type signature: but without . Also for the 'toplevel' case the type signature in the In the type signature bit will be normalized into foo :: _\n which can be checked for.

For instance this is the message when hole is not 'toplevel' - there's a stack of where the hole was located ending with In the type signature

source/library/Language/Haskell/Brittany/Internal.hs:130:20: error:
    • Found type wildcard ‘_’ standing for ‘GhcPs’
      To use the inferred type, enable PartialTypeSignatures
    • In the first argument of ‘HsDecl’, namely ‘_’
      In the type ‘HsDecl _’
      In the type signature: decl :: HsDecl _
    • Relevant bindings include
        hsmodDecls :: [LHsDecl GhcPs]
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:34)
        hsmodAnn :: EpAnn AnnsModule
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:24)
        m :: HsModule
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:13)
        splitAnnots :: HsModule -> ([LEpaComment], [LEpaComment])
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:1)
    |
130 |     decl :: HsDecl _
    |                    ^

And this is the error message for the 'toplevel' case - the line starting with • In the type signature also ends with :: _ which is easy to check and that's what this PR does.

source/library/Language/Haskell/Brittany/Internal.hs:131:13: error:
    • Found type wildcard ‘_’ standing for ‘HsDecl GhcPs’
      To use the inferred type, enable PartialTypeSignatures
    • In the type signature: decl :: _
      In an equation for ‘splitAnnots’:
          splitAnnots m@HsModule {hsmodAnn, hsmodDecls}
            = undefined
            where
                ann :: SrcSpanAnnA
                decl :: _
                L ann decl = head hsmodDecls
    • Relevant bindings include
        hsmodDecls :: [LHsDecl GhcPs]
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:34)
        hsmodAnn :: EpAnn AnnsModule
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:24)
        m :: HsModule
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:13)
        splitAnnots :: HsModule -> ([LEpaComment], [LEpaComment])
          (bound at source/library/Language/Haskell/Brittany/Internal.hs:127:1)
    |
131 |     decl :: _
    |             ^

Still not a general-purpose solution and a somewhat fragile heuristic that may break with newer GHC releases (but could be solved when GHC introduces something like structured errors). Yet it addresses a, what appears to me, pretty common case and sems to be reasonably simple.

@sergv sergv requested a review from pepeiborra as a code owner May 26, 2022 00:39
sergv added a commit to sergv/haskell-language-server that referenced this pull request May 26, 2022
Copy link
Collaborator

@michaelpj michaelpj left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding tests! Just a couple of doc comments.

bracket = ("(" `T.append`) . (`T.append` ")")
msgSigPart = snd $ T.breakOnEnd "standing for " msg
(sig, rest) = T.span (/='’') . T.dropWhile (=='‘') . T.dropWhile (/='‘') $ msgSigPart
(prefix, rest') = T.breakOn "• In the type signature:" rest
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you add a comment explaining what you explained in the PR description? Otherwise all that knowledge will get lost!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in a new commit after rebase.

(sig, rest) = T.span (/='’') . T.dropWhile (=='‘') . T.dropWhile (/='‘') $ msgSigPart
(prefix, rest') = T.breakOn "• In the type signature:" rest
-- If we're completing something like ‘foo :: _’ parens can be safely omitted.
isToplevelSig = not (T.null prefix) && " :: _" `T.isSuffixOf` T.takeWhile (/= '\n') rest'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does the prefix being empty here matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea is that breakOn didn't return any prefix if its pattern was not found. So the check is that the error message really contained • In the type signature: as a substring with the .

@michaelpj michaelpj merged commit 0769f23 into haskell:master May 27, 2022
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
* Omit more parens for wildcard type signature (haskell#2929)

This is a followup to haskell#2764.

* Review suggestions
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.

None yet

2 participants