-
Notifications
You must be signed in to change notification settings - Fork 156
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
Detect type alias in implementation. #1243
Conversation
match typeDefnInfo with | ||
| None -> return [] | ||
| Some (typeName, mTypeDefn, implPath) -> | ||
|
||
match parseAndCheckResults.TryGetSymbolUseFromIdent sourceText typeName with | ||
| None -> return [] | ||
| Some typeSymbolUse -> |
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.
This makes me feel like we could use a short-circuit/guard style function to reduce the nesting, maybe something like
let guard input =
match input with
| None -> Ok []
| Some i -> Ok i
let! (typeName, mTypeDefn, implPath) = guard typeDefnInfo
thoughts? It could go in the CodeFix
module where some of our other filters/checks go.
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 think I get the concept but I can't get that example to work in the asyncResult
thing.
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.
This would be a bit more tricky this is mimicking early return which would be more of a 3 case DU. (Ok for continue, Done for stop, Error for stop, then match at the end to map Done -> Ok)
Only way to model currently without going down the new CE rabbit hole would be to use the Result.requireXXX
functions with an appropriate error message(s) and match on the message(s) after with AsyncResult.orElseWith
and return empty list in those cases.
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.
We can punt on making this nicer - though I could see some kind of codeFix
CE that includes guards and custom operations for 'summoning' the relevant pieces of data for a codefix easily :D
Added some tests, calling it a day, ready for review. |
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.
As always, thank you for this contribution @nojaf!
I'd like to add a codefix to include a type alias inside a signature file.
I'm quite specifically interested in this, and only this.