-
-
Notifications
You must be signed in to change notification settings - Fork 367
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
Implement 'Attempt to fill hole' code action #431
Conversation
What do you think about converting it in github "draft" until is ready to review (to enforce the WIP tag)? |
@pepeiborra this is ready for review when you have some time :) |
import SrcLoc | ||
|
||
|
||
-- TODO(sandy): Consolidate this with LocalBindings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if you made a PR on ghcide for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (I think)
case splitTyConApp_maybe $ unCType t of | ||
Nothing -> throwError $ GoalMismatch "destruct" g | ||
Just (tc, apps) -> do | ||
let dcs = tyConDataCons tc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these DataCons
in scope? Are they even known to be exported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Neither. I don't think the first is important; other plugins can suggest the import.
That being said, preventing unexported datacons from being generated is more interesting, Is there an easy way we can check that?
= take 1 | ||
. fmap toLower | ||
. filterReplace isSymbol 's' | ||
. filterReplace isPunctuation 'p' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replacing symbols and punctuation like this doesn't really need like it will lead to good names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but without it we run into problems giving names for type operators; it's really hard to come up with a name for a tycon called (:<!>)
, eg.
(tcmod, _) <- MaybeT $ runIde state $ useWithStale TypeCheck nfp | ||
let span = rangeToRealSrcSpan (fromNormalizedFilePath nfp) range' | ||
-- TODO(sandy): unclear if this span is correct; might be | ||
-- pointing to the wrong version of the file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix this TODO by applying the position mapping returned by useWithStale
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
{ ts_skolems :: [TyVar] | ||
, ts_unifier :: TCvSubst | ||
, ts_used_vals :: Set OccName | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should all probably be strict fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really qualified to review this, but the ghcide and perf bits look fine to me. Can't wait to play with the new features!
Merged! |
Thanks!! |
(Raised on IRC, but repeating) This PR makes the |
Submodules strike again! The reference should be pointed to haskell/ghcide#845 |
|
* Add ModLocation to Import type * Add ModuleNames to dependency information With @adamse * Clarify ModLocation assumption * Add a comment on use of rwhnf * newtype ArtifactsLocation Co-authored-by: Marcelo Lazaroni <[email protected]>
This PR brings in the new version of refinery in order to implement the
Attempt to fill hole
code action. This action will use the tactic machinery to try to synthesize a term that matches the type of the hole.