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

Use spans pointing at the inside of a rustdoc attribute #51391

Merged
merged 3 commits into from
Jun 9, 2018

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Jun 6, 2018

Follow up to #51111.

Point to the link in a rustdoc attribute where intralink resolution failed, instead of the full rustdoc attribute's span.

r? @GuillaumeGomez cc @kennytm

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 6, 2018
@estebank

This comment has been minimized.

@rust-highfive

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

Awesome! Once ci fixed, r=me

@kennytm
Copy link
Member

kennytm commented Jun 6, 2018

@estebank Could you verify how this works for all kinds of doc comments:

#![feature(external_doc)]

/// Foo
/// bar [BarA] bar
/// baz
pub fn a() {}

/**
 * Foo
 * bar [BarB] bar
 * baz
 */
pub fn b() {}

/** Foo

bar [BarC] bar
baz

    let bar_c_1 = 0;
    let bar_c_2 = 0;
    let g = [bar_c_1];
    let h = g[bar_c_2];

*/
pub fn c() {}

#[doc = "Foo\nbar [BarD] bar\nbaz"]
pub fn d() {}

#[doc(include = "file.md")]
pub fn e() {}

macro_rules! f {
    ($f:expr) => {
        #[doc = $f]
        pub fn f() {}
    }
}
f!("Foo\nbar [BarF] bar\nbaz");

Here the content of file.md is

Foo
bar [BarE] bar
baz
For reference, the existing output post-51111
warning: [BarA] cannot be resolved, ignoring it...
 --> 1.rs:3:1
  |
3 | / /// Foo
4 | | /// bar [BarA] bar
5 | | /// baz
  | |_______^
  |
  = note: the link appears in this line:
          
           bar [BarA] bar
                ^^^^

warning: [BarB] cannot be resolved, ignoring it...
  --> 1.rs:8:1
   |
8  | / /**
9  | |  * Foo
10 | |  * bar [BarB] bar
11 | |  * baz
12 | |  */
   | |___^
   |
   = note: the link appears in this line:
           
            bar [BarB] bar
                 ^^^^

warning: [BarC] cannot be resolved, ignoring it...
  --> 1.rs:15:1
   |
15 | / /** Foo
16 | |
17 | | bar [BarC] bar
18 | | baz
...  |
24 | |
25 | | */
   | |__^
   |
   = note: the link appears in this line:
           
           bar [BarC] bar
                ^^^^

warning: [BarD] cannot be resolved, ignoring it...
  --> 1.rs:28:1
   |
28 | #[doc = "Foo\nbar [BarD] bar\nbaz"]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the link appears in this line:
           
           bar [BarD] bar
                ^^^^

warning: [BarE] cannot be resolved, ignoring it...
  --> 1.rs:31:1
   |
31 | #[doc(include = "file.md")]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: the link appears in this line:
           
           bar [BarE] bar
                ^^^^

warning: [BarF] cannot be resolved, ignoring it...
  --> 1.rs:36:9
   |
36 |         #[doc = $f]
   |         ^^^^^^^^^^^
...
40 | f!("Foo\nbar [BarF] bar\nbaz");
   | ------------------------------- in this macro invocation
   |
   = note: the link appears in this line:
           
           bar [BarF] bar
                ^^^^

@estebank
Copy link
Contributor Author

estebank commented Jun 6, 2018

@kennytm modified the code to fallback to the current behavior if the line count of the docs don't match the line count of the code. This uses the new spans for the /// doc comments but keeps the current behavior for all others. I also fixed the problem of different leading whitespace on different lines messing up the spans.

@rust-highfive

This comment has been minimized.

// Extract the specific span
let lo = sp.lo() + syntax_pos::BytePos((link_range.start + code_dox_len) as u32);
let hi = lo + syntax_pos::BytePos(link_range.len() as u32);
let sp = sp.with_lo(lo).with_hi(hi);
Copy link
Member

@kennytm kennytm Jun 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use

let sp = sp.from_inner_byte_pos(
    link_range.start + code_dox_len, 
    link_range.end + code_dox_len,
);

here 😃

https://doc.rust-lang.org/nightly/nightly-rustc/syntax_pos/struct.Span.html#method.from_inner_byte_pos

@estebank
Copy link
Contributor Author

estebank commented Jun 7, 2018

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jun 7, 2018

📌 Commit 31bb50b has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented Jun 7, 2018

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

@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 7, 2018
@bors
Copy link
Contributor

bors commented Jun 8, 2018

⌛ Testing commit 31bb50b with merge bdc515addf0bed215a5581c6d2d5fe90aca612fd...

@bors
Copy link
Contributor

bors commented Jun 8, 2018

💔 Test failed - status-appveyor

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 8, 2018
@kennytm
Copy link
Member

kennytm commented Jun 8, 2018

@bors retry

Why closing the tree would cancel the existing PR? 😕

@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 8, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 8, 2018
Use spans pointing at the inside of a rustdoc attribute

Follow up to rust-lang#51111.

Point to the link in a rustdoc attribute where intralink resolution failed, instead of the full rustdoc attribute's span.

r? @GuillaumeGomez cc @kennytm
bors added a commit that referenced this pull request Jun 8, 2018
Rollup of 13 pull requests

Successful merges:

 - #50143 (Add deprecation lint for duplicated `macro_export`s)
 - #51099 (Fix Issue 38777)
 - #51276 (Dedup auto traits in trait objects.)
 - #51298 (Stabilize unit tests with non-`()` return type)
 - #51360 (Suggest parentheses when a struct literal needs them)
 - #51391 (Use spans pointing at the inside of a rustdoc attribute)
 - #51394 (Use scope tree depths to speed up `nearest_common_ancestor`.)
 - #51396 (Make the size of Option<NonZero*> a documented guarantee.)
 - #51401 (Warn on `repr` without hints)
 - #51412 (Avoid useless Vec clones in pending_obligations().)
 - #51427 (compiletest: autoremove duplicate .nll.* files (#51204))
 - #51436 (Do not require stage 2 compiler for rustdoc)
 - #51437 (rustbuild: generate full list of dependencies for metadata)

Failed merges:
@bors bors merged commit 31bb50b into rust-lang:master Jun 9, 2018
@estebank estebank deleted the docspan branch November 9, 2023 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants