Skip to content

Define Function Code Action should respect Haddock Comments #2715

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

Closed
drsooch opened this issue Feb 15, 2022 · 1 comment · Fixed by #2740
Closed

Define Function Code Action should respect Haddock Comments #2715

drsooch opened this issue Feb 15, 2022 · 1 comment · Fixed by #2740
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@drsooch
Copy link
Collaborator

drsooch commented Feb 15, 2022

Your environment

Which OS do you use:
Ubuntu

Which LSP client (editor/plugin) do you use:
emacs+lsp-mode

Describe your project (alternative: link to the project):
HLS :)

Steps to reproduce

  1. Use a function that has yet to be defined ex: filter isValidMessage messages (where isValidMessage is not yet created)
  2. Apply code action to define isValidMessage

Expected behaviour

The function should be added, without disrupting another binding and it's Haddock Comment.

Actual behaviour

The function is defined underneath an existing Haddock Comment

Include debug information

Before:

-- | If a diagnostic has the proper message create a ChangeSignature from it
matchingDiagnostic :: Uri -> Diagnostic -> Maybe ChangeSignature
matchingDiagnostic uri diag@Diagnostic{_message} = case map (unwrapMatch . (=~) _message) errorMessageRegexes of
                                                       []  -> Nothing
                                                       css -> join $ find isJust $ filter isValidMessage css
    where
        unwrapMatch :: (Text, Text, Text, [Text]) -> Maybe ChangeSignature
        -- due to using (.|\n) in regex we have to drop the erroneous, but necessary ("." doesn't match newlines), match
        unwrapMatch (_, _, _, [exp, act, _, name]) = Just $ ChangeSignature exp act name Nothing diag uri
        unwrapMatch _                           = Nothing

-- | List of regexes that match various Error Messages
errorMessageRegexes :: [Text]
errorMessageRegexes = [ -- be sure to add new Error Messages Regexes at the bottom to not fail any existing tests
    "Expected type: (" <> typeSigRegex <> ")\n +Actual type: (" <> typeSigRegex <> ")\n(.|\n)+In an equation for ‘(.+)’"
    , "Couldn't match expected type ‘(" <> typeSigRegex <> ")’ with actual type ‘(" <> typeSigRegex <> ")’\n(.|\n)+In an equation for ‘(.+)’"
    ]
    where
        typeSigRegex = "[a-zA-Z0-9 ->\\(\\)]+"

After:

-- | If a diagnostic has the proper message create a ChangeSignature from it
matchingDiagnostic :: Uri -> Diagnostic -> Maybe ChangeSignature
matchingDiagnostic uri diag@Diagnostic{_message} = case map (unwrapMatch . (=~) _message) errorMessageRegexes of
                                                       []  -> Nothing
                                                       css -> join $ find isJust $ filter isValidMessage css
    where
        unwrapMatch :: (Text, Text, Text, [Text]) -> Maybe ChangeSignature
        -- due to using (.|\n) in regex we have to drop the erroneous, but necessary ("." doesn't match newlines), match
        unwrapMatch (_, _, _, [exp, act, _, name]) = Just $ ChangeSignature exp act name Nothing diag uri
        unwrapMatch _                           = Nothing

-- | List of regexes that match various Error Messages

isValidMessage :: Maybe ChangeSignature -> Maybe ChangeSignature
isValidMessage = error "not implemented"
errorMessageRegexes :: [Text]
errorMessageRegexes = [ -- be sure to add new Error Messages Regexes at the bottom to not fail any existing tests
    "Expected type: (" <> typeSigRegex <> ")\n +Actual type: (" <> typeSigRegex <> ")\n(.|\n)+In an equation for ‘(.+)’"
    , "Couldn't match expected type ‘(" <> typeSigRegex <> ")’ with actual type ‘(" <> typeSigRegex <> ")’\n(.|\n)+In an equation for ‘(.+)’"
    ]
    where
        typeSigRegex = "[a-zA-Z0-9 ->\\(\\)]+"
@drsooch drsooch added type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. component: ghcide status: needs triage labels Feb 15, 2022
@pepeiborra
Copy link
Collaborator

Agreed. Also, the default implementation should be a hole instead of an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ghcide type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants