Skip to content
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

Add macro support in jump to definition feature #91264

Merged
merged 6 commits into from
Jun 24, 2022

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Nov 26, 2021

Fixes #91174.

To do so, I check if the span comes from an expansion, and if so, I infer the original macro DefId or Span depending if it's a defined in the current crate or not.

There is one limitation due to macro expansion though:

macro_rules! yolo { () => {}}

fn foo() {
    yolo!();
}

In foo, yolo! won't be linked because after expansion, it is replaced by nothing (which seems logical). So I can't get an item from the Visitor from which I could tell if its Span comes from an expansion.

I added a test for this specific limitation alongside others.

Demo: https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html

As for the empty macro issue that cannot create a jump to definition, you can see it here.

r? @jyn514

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Nov 26, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 26, 2021
@jyn514
Copy link
Member

jyn514 commented Jan 4, 2022

I won't have a chance to review this any time soon.

r? @camelid

@rust-highfive rust-highfive assigned camelid and unassigned jyn514 Jan 4, 2022
@camelid
Copy link
Member

camelid commented Jan 8, 2022

I don't have knowledge of this code. r? rust-lang/rustdoc

@rust-highfive rust-highfive assigned jsha and unassigned camelid Jan 8, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2022
@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2022
@Dylan-DPC
Copy link
Member

@jsha this is waiting for your review

@bors
Copy link
Contributor

bors commented Mar 14, 2022

☔ The latest upstream changes (presumably #94139) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Apr 30, 2022

☔ The latest upstream changes (presumably #93803) made this pull request unmergeable. Please resolve the merge conflicts.

@GuillaumeGomez
Copy link
Member Author

I'll fix the conflict once #96636 is merged.

@jsha
Copy link
Contributor

jsha commented May 10, 2022

#96636 is now merged. Can you put up a demo, including a demo of the "expands to empty" behavior you mentioned?

@GuillaumeGomez
Copy link
Member Author

Here you go: https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html

As for the empty macro issue that cannot create a jump to definition, you can see it here.

@Kobzol
Copy link
Contributor

Kobzol commented May 18, 2022

I wonder why eprintln! in the demo links to doc and not source?

@GuillaumeGomez
Copy link
Member Author

It's because the code tried to find it location based on its Span and failed, using then the documentation page (which we can generate pretty easily).

@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@GuillaumeGomez
Copy link
Member Author

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 22, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Jun 22, 2022
…=jsha

Add macro support in jump to definition feature

Fixes rust-lang#91174.

To do so, I check if the span comes from an expansion, and if so, I infer the original macro `DefId` or `Span` depending if it's a defined in the current crate or not.

There is one limitation due to macro expansion though:

```rust
macro_rules! yolo { () => {}}

fn foo() {
    yolo!();
}
```

In `foo`, `yolo!` won't be linked because after expansion, it is replaced by nothing (which seems logical). So I can't get an item from the `Visitor` from which I could tell if its `Span` comes from an expansion.

I added a test for this specific limitation alongside others.

Demo: https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html

As for the empty macro issue that cannot create a jump to definition, you can see it [here](https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html#35).

r? `@jyn514`
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 24, 2022
…=jsha

Add macro support in jump to definition feature

Fixes rust-lang#91174.

To do so, I check if the span comes from an expansion, and if so, I infer the original macro `DefId` or `Span` depending if it's a defined in the current crate or not.

There is one limitation due to macro expansion though:

```rust
macro_rules! yolo { () => {}}

fn foo() {
    yolo!();
}
```

In `foo`, `yolo!` won't be linked because after expansion, it is replaced by nothing (which seems logical). So I can't get an item from the `Visitor` from which I could tell if its `Span` comes from an expansion.

I added a test for this specific limitation alongside others.

Demo: https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html

As for the empty macro issue that cannot create a jump to definition, you can see it [here](https://rustdoc.crud.net/imperio/macro-jump-to-def/src/foo/check-source-code-urls-to-def-std.rs.html#35).

r? ``@jyn514``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2022
Rollup of 9 pull requests

Successful merges:

 - rust-lang#91264 (Add macro support in jump to definition feature)
 - rust-lang#96955 (Remove (transitive) reliance on sorting by DefId in pretty-printer)
 - rust-lang#97633 (Session object: Set OS/ABI)
 - rust-lang#98039 (Fix `panic` message for `BTreeSet`'s `range` API and document `panic` cases)
 - rust-lang#98214 (rustc_target: Remove some redundant target properties)
 - rust-lang#98280 (Improve suggestion for calling fn-like expr on type mismatch)
 - rust-lang#98394 (Fixup missing renames from `#[main]` to `#[rustc_main]`)
 - rust-lang#98411 (Update tendril)
 - rust-lang#98419 (Remove excess rib while resolving closures)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 97f4d7b into rust-lang:master Jun 24, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 24, 2022
@GuillaumeGomez GuillaumeGomez deleted the macro-jump-to-def branch June 24, 2022 13:03
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 10, 2022
…eGomez

rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang#91264 (comment)

r? `@GuillaumeGomez`
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 11, 2022
…eGomez

rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang#91264 (comment)

r? ``@GuillaumeGomez``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 11, 2022
…eGomez

rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang#91264 (comment)

r? ```@GuillaumeGomez```
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang/rust#91264 (comment)

r? ``@GuillaumeGomez``
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
rustdoc: simplify highlight.rs

Split render_with_highlighting, which took many optional parameters, into three functions for specific purposes, which each take a smaller number of mostly required parameters.

Remove some plumbing to pass through an "edition" parameter, which was used solely to avoid highlighting some 2021 Edition keywords in non-2021 code.

I've tested a build of std docs before and after, and this does not change the generated HTML at all.

Followup from rust-lang/rust#91264 (comment)

r? ``@GuillaumeGomez``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Jump to definition doesn't generate a link for macro calls