Change Type Signature Plugin #2660
Merged
Conversation
This file contains 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
This implementation only matches a single GHC Error message:
```
• Couldn't match type ‘Int’
with ‘Data.HashSet.Internal.HashSet Int’
Expected type: Int -> Int
Actual type: Data.HashSet.Internal.HashSet Int -> Int
• In the expression: head . toList
In an equation for ‘test’: test = head . toList
```
Specifically on `Expected type: ...`, `Actual type:...` and `In an equation ...`.
There are plenty of error messages that match this format but aren't actually valid.
jneira
reviewed
Feb 1, 2022
jneira
reviewed
Feb 1, 2022
…nguage-server into change-type-signature
…nguage-server into change-type-signature
|
Does anyone have a preference as to where this plugin should go (if the functionality expands out a bit more?) Separate module? Leave it in |
Move it outside please - standalone plugins have better isolation, error handling, run in parallel, and are much easier to upgrade gradually when there is a new GHC version |
pepeiborra
approved these changes
Feb 5, 2022
michaelpj
approved these changes
Feb 6, 2022
There was a problem hiding this comment.
Generally looks good!
Since Pepe mentioned the structured errors infrastructure, I wonder whether this is actually a good opportunity to make a feature request upstream! I know Richard has been looking for feedback from us about what would actually be useful in structured errors. Maybe we could open a GHC issue explaining what we're trying to do and asking if there would be a good way to include extra information in structured type-mismatch errors so we could always work out the "actual" type of the enclosing binding or something.
|
https://gitlab.haskell.org/ghc/ghc/-/issues/21063 -- Linking this here |
- Add new regex for matching extra errors message types - Revamp original regex to match more. - Add basic test suite. - Begin adding `tidyActualType` semantics to provide slightly prettier TyVars
- Add Pretty Printing for Types - Added a few test scenarios
|
Going to mark this as ready for review, there are a few things I'd like to clean up and I've left relevant comments. |
- Be able to tidy signatures with operators in it - Use T.words instead of regex matching to split tyVars
pepeiborra
reviewed
Feb 19, 2022
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
michaelpj
reviewed
Feb 19, 2022
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/src/Ide/Plugin/ChangeTypeSignature.hs
Outdated
Show resolved
Hide resolved
plugins/hls-change-type-signature-plugin/test/testdata/TErrorGivenPartialSignature.hs
Show resolved
Hide resolved
michaelpj
reviewed
Feb 22, 2022
michaelpj
approved these changes
Feb 22, 2022
* convert to contravariant logging style part 1, uses additional hardcoded log file to see it side by side with original logging * convert Session to contravariant logging style * convert Plugin/HLS and FireStore to contravariant logging style * convert Rules (and most of the universe) to contravariant logging style * fix tests, allow old style logging and contravariant logging to write to same log file * fix import inside wrong CPP * add CPP for LogTactic constructor * remove redundant import * fix ghcide tests * remove unused import * fix plugin tests * LSP_TEST_STDERR should apply to contra logger as well * fix tactic plugin test * use CPP for Log datatype plugin constructors, remove unused imports * add a few Pretty instances, add prettyprinter to haskell-language-sever and hls-plugin-api dependencies * add Pretty Log instances for Session, FileStore, Notifications * add remaining Pretty Log instances * add logToPriorities * fix slight interleaving issue with hslogger and logger both logging, have default logger be mutex stderr or file handle, use stderr if failing to open log file * forgot to add .cabal files with hslogger dep * dont use UnliftIO file IO helpers because they are too new * remove log helper comments, use Doc instead of Text as final console/file logger input, renaming, export Log constructors * remove accidentally added useless file, removed prettyprinter dep from hls-plugin-api because stack ghc8.6.5 doesnt have it? * use deprecated prettyprint modules import for the sake of circleci ghc-8.6.5 * use dummy stderr logger for plugin cli commands, use priorityToHsLoggerPriority function instead of manual mapping * remove old plugin detritus that somehow got committed * fix prettyprinter imports for 8.6.5 * try enforcing prettyprinter bounds? * enforcing bound makes no sense * maybe changing stack yamls does trick * filter out warnings when their diags are empty to more closely match original * add ability to select wanted logging columns, match prev ghcide exe logging behaviour * dont log anything when diags are empty in some defineEarlyCutoff versions * use non-deprecated prettyprinter imports * fix ghcide test module * change logWith to accept priority at call site, remove all logToPriority functions, add cmapWithPrio that contramaps through WithPriority * remove useless hiding import list, add comments to default recorder makers * make cradleToOptsAndLibDir take concrete cradle to remove existential type var in Log constructor * Types.Logger now re-exports prettyprinter, remove unused dependencies on prettyprinter and hslogger * existential type var to remove boilerplate in Plugins.hs, remove a few Show instances * add SourceLoc logging column, inline logToDoc functions, add comment explaining hslogger setup existence * qualify a name to match original source * fix -WError
It's a bit non-standard, and moreover it's entirely dead.
…re' into change-type-signature
|
Sorry for this marathon PR. I pushed a change to handling local signatures for better support. New tests aren't passing locally, just going to see if it'll work in CI. I'll revisit this tonight. |
|
This looks green but I wasn't sure if you still have work you want to do or not! |
|
Yes this is all set! |
|
Ooops, we forgot to add the test suite to CI |
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.
This plugin is designed to take care of #2436 . This a very rough first draft, with a basic outline targeting one specific GHC Type Error message (see
error.txtfor an example). There are certain instances of this error message that WILL NOT work in the current implementation. I just wanted to get a first look and any potential help for further expansion.It should be noted that this currently sits in
ghcideas when I started out I assumed there would be some code reuse between some similar ghcide internal plugins. Turns out that's not the case so far.