Support go-to-definition for type aliases #5239
Conversation
|
Hello! Would you mind rebasing on main to resolve the conflicts? |
vshakitskiy
force-pushed
the
go-to-definition-for-type-aliases
branch
2 times, most recently
from
0630dd9 to
d0a133f
Compare
vshakitskiy
force-pushed
the
go-to-definition-for-type-aliases
branch
2 times, most recently
from
daa2053 to
cb47031
Compare
|
Done! |
giacomocavalieri
left a comment
giacomocavalieri
left a comment
There was a problem hiding this comment.
Hello! I've left a comment and a small stylistic note
|
This is looking great! I've left a final round of small comments |
vshakitskiy
force-pushed
the
go-to-definition-for-type-aliases
branch
from
8dea2f8 to
007b7c7
Compare
|
Thanks for the reviews! Should be looking good now. I need to do more documentation comments in my future commits. |
giacomocavalieri
left a comment
giacomocavalieri
left a comment
There was a problem hiding this comment.
Looks good to me, still will still need to be reviewed by Louis before it can be merged!
lpil
left a comment
lpil
left a comment
There was a problem hiding this comment.
Thank you! I've left some notes inline
| TestProject::for_source(code), | ||
| find_position_of("Two").nth_occurrence(2) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Could we have a tests for these please:
-
An alias that points to a custom type
-
Aliases within more complex type expressions:
fn wibble() -> List(Two) ^^^
fn wibble() -> #(One(Two), Int) ^^^
-
Aliases used in custom type definitions
-
Aliases used in type alias definitions
There was a problem hiding this comment.
Sorry, I am not sure what does "alias used in custom type/type alias definition" mean. Do you mean:
// Alias in custom type definition
type One(a) {
One(a)
}
type Two = Int
fn wibble() -> One(Two) {
// ^^^
todo
}// Alias in type alias definition
type One(a) = List(a)
type Two = Int
fn wibble() -> One(Two) {
// ^^^
todo
}
compiler-core/src/build.rs
Outdated
| module: Some(module_name.clone()), | ||
| span, | ||
| }) | ||
| }), |
There was a problem hiding this comment.
Sorry, I'm confused by this branch. Why is that all the importable modules are traversed when the type alias is referred to without a module qualifier?
Won't that return the definition location for the first type with the same name, regardless of where it was defined? Not sure I'm understanding it correctly.
There was a problem hiding this comment.
I misunderstood the importable_modules, it contains all modules in the project, for some reason I thought it is slightly different thing 🙏. This is not the logic I intended to have here.
I should probably pass module's imports to definition_location, then in None branch find which import brought in this type name, and then look up in that specific module.
|
When you are ready for a review please un-draft this PR and tag me for a review. Thank you! |
|
@lpil Sorry for keeping the PR drafted🙏. I was waiting for the response regardless of the test cases. Quoting the question again:
// Alias in custom type definition
type One(a) {
One(a)
}
type Two = Int
fn wibble() -> One(Two) {
// ^^^
todo
}// Alias in type alias definition
type One(a) = List(a)
type Two = Int
fn wibble() -> One(Two) {
// ^^^
todo
}I was more confused about type alias definition part. Am I thinking in the right way? |
3 participants
Closes #5227.