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

hide internal types/traits from std docs via new #[doc(masked)] attribute #44026

Merged
merged 4 commits into from
Sep 19, 2017

Conversation

QuietMisdreavus
Copy link
Member

@QuietMisdreavus QuietMisdreavus commented Aug 22, 2017

Fixes #43701 (hopefully for good this time)

This PR introduces a new parameter to the #[doc] attribute that rustdoc looks for on extern crate statements. When it sees #[doc(masked)] on such a statement, it hides traits and types from that crate from appearing in either the "Trait Implementations" section of many type pages, or the "Implementors" section of trait pages. This is then applied to the libc/rand/compiler_builtins imports in libstd to prevent those crates from creating broken links in the std docs.

Like in #43348, this also introduces a feature gate, doc_masked, that controls the use of this parameter.

To view the std docs generated with this change, head to https://tonberry.quietmisdreavus.net/std-43701/std/index.html.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

@QuietMisdreavus
Copy link
Member Author

Tracking issue: #44027

@QuietMisdreavus QuietMisdreavus changed the title [WIP] hide internal types/traits from std docs via new #{doc(masked)] attribute hide internal types/traits from std docs via new #{doc(masked)] attribute Aug 22, 2017
@steveklabnik steveklabnik changed the title hide internal types/traits from std docs via new #{doc(masked)] attribute hide internal types/traits from std docs via new #[doc(masked)] attribute Aug 22, 2017

// @has doc_masked/struct.LocalStruct.html
// @has - '//*[@class="impl"]//code' 'impl LocalTrait for LocalStruct'
// @!has - '//*[@class="impl"]//code' 'impl Copy for LocalStruct'
Copy link
Member

Choose a reason for hiding this comment

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

Masking failed. I guess it's because Copy and usize are from libcore instead of libstd?

[00:50:50] ---- [rustdoc] rustdoc/doc-masked.rs stdout ----
[00:50:50] 	
[00:50:50] error: htmldocck failed!
[00:50:50] status: exit code: 1
[00:50:50] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/doc-masked.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/doc-masked.rs"
[00:50:50] stdout:
[00:50:50] ------------------------------------------
[00:50:50] 
[00:50:50] ------------------------------------------
[00:50:50] stderr:
[00:50:50] ------------------------------------------
[00:50:50] 18: @!has check failed
[00:50:50] 	`XPATH PATTERN` did not match
[00:50:50] 	// @!has - '//*[@class="impl"]//code' 'impl Copy for LocalStruct'
[00:50:50] 24: @!has check failed
[00:50:50] 	`XPATH PATTERN` did not match
[00:50:50] 	// @!has - '//*[@id="implementors-list"]//code' 'impl LocalTrait for usize'
[00:50:50] 
[00:50:50] Encountered 2 errors
[00:50:50] 
[00:50:50] ------------------------------------------
[00:50:50] 
[00:50:50] thread '[rustdoc] rustdoc/doc-masked.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2511:8
[00:50:50] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:50:50] 
[00:50:50] 
[00:50:50] failures:
[00:50:50]     [rustdoc] rustdoc/doc-masked.rs
[00:50:50] 
[00:50:50] test result: FAILED. 162 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that makes more sense. I saw the travis failure last night, but chalked it up to getting the htmldocck rules wrong. I'll poke at this more and see what can be done about it.

#[macro_use]
#[macro_reexport(vec, format)]
extern crate alloc;
extern crate alloc_system;
extern crate std_unicode;
#[doc(masked)]
extern crate libc;

// We always need an unwinder currently for backtraces
extern crate unwind;
Copy link
Member

Choose a reason for hiding this comment

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

unwind would also need to be doc(masked) to get rid of the impls for _Unwind_Reason_Code and _Unwind_Action on the Clone page for example.

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 23, 2017
@kennytm
Copy link
Member

kennytm commented Aug 25, 2017

4 failures now 😒

[00:53:35] ---- [rustdoc] rustdoc/doc-masked.rs stdout ----
[00:53:35] 	
[00:53:35] error: htmldocck failed!
[00:53:35] status: exit code: 1
[00:53:35] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/doc-masked.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/doc-masked.rs"
[00:53:35] stdout:
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] stderr:
[00:53:35] ------------------------------------------
[00:53:35] 18: @!has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @!has - '//*[@class="impl"]//code' 'impl Copy for LocalStruct'
[00:53:35] 
[00:53:35] Encountered 1 errors
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] thread '[rustdoc] rustdoc/doc-masked.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2435:8
[00:53:35] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:53:35] 
[00:53:35] ---- [rustdoc] rustdoc/extern-impl.rs stdout ----
[00:53:35] 	
[00:53:35] error: htmldocck failed!
[00:53:35] status: exit code: 1
[00:53:35] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/extern-impl.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/extern-impl.rs"
[00:53:35] stdout:
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] stderr:
[00:53:35] ------------------------------------------
[00:53:35] 32: @has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @has - '//code' 'impl Bar for fn()'
[00:53:35] 34: @has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @has - '//code' 'impl Bar for extern "C" fn()'
[00:53:35] 36: @has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @has - '//code' 'impl Bar for extern "system" fn()'
[00:53:35] 
[00:53:35] Encountered 3 errors
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] thread '[rustdoc] rustdoc/extern-impl.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2435:8
[00:53:35] 
[00:53:35] ---- [rustdoc] rustdoc/inline_cross/issue-33113.rs stdout ----
[00:53:35] 	
[00:53:35] error: htmldocck failed!
[00:53:35] status: exit code: 1
[00:53:35] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/inline_cross/issue-33113.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/inline_cross/issue-33113.rs"
[00:53:35] stdout:
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] stderr:
[00:53:35] ------------------------------------------
[00:53:35] 18: @has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @has - '//code' "for &'a char"
[00:53:35] 
[00:53:35] Encountered 1 errors
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] thread '[rustdoc] rustdoc/inline_cross/issue-33113.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2435:8
[00:53:35] 
[00:53:35] ---- [rustdoc] rustdoc/issue-29503.rs stdout ----
[00:53:35] 	
[00:53:35] error: htmldocck failed!
[00:53:35] status: exit code: 1
[00:53:35] command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-29503.stage2-x86_64-unknown-linux-gnu" "/checkout/src/test/rustdoc/issue-29503.rs"
[00:53:35] stdout:
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] ------------------------------------------
[00:53:35] stderr:
[00:53:35] ------------------------------------------
[00:53:35] 18: @has check failed
[00:53:35] 	`XPATH PATTERN` did not match
[00:53:35] 	// @has - "//ul[@id='implementors-list']/li" "impl<T> MyTrait for T where T: Debug"
[00:53:35] 
[00:53:35] Encountered 1 errors
[00:53:35] 
[00:53:35] thread 'main' panicked at 'Some tests failed', /checkout/src/tools/compiletest/src/main.rs:322:21
[00:53:35] ------------------------------------------
[00:53:35] 
[00:53:35] thread '[rustdoc] rustdoc/issue-29503.rs' panicked at 'explicit panic', /checkout/src/tools/compiletest/src/runtest.rs:2435:8
[00:53:35] 
[00:53:35] 
[00:53:35] failures:
[00:53:35]     [rustdoc] rustdoc/doc-masked.rs
[00:53:35]     [rustdoc] rustdoc/extern-impl.rs
[00:53:35]     [rustdoc] rustdoc/inline_cross/issue-33113.rs
[00:53:35]     [rustdoc] rustdoc/issue-29503.rs
[00:53:35] 
[00:53:35] test result: FAILED. 159 passed; 4 failed; 1 ignored; 0 measured; 0 filtered out

@QuietMisdreavus
Copy link
Member Author

@kennytm Yeah, I'm needing to push to be able to test, since htmldocck.py doesn't work on python 3. >_>

It seems I was overzealous in my masking. Regardless, even with that change, Clone is still left out because its DefId still points to libcore instead of libstd.

@kennytm
Copy link
Member

kennytm commented Aug 25, 2017

@QuietMisdreavus Seems only a problem of import (I haven't actually tried to run it after patching). These diffs should make it work for Python 3:

diff --git a/src/etc/htmldocck.py b/src/etc/htmldocck.py
index a5449b748d..c8246bf727 100644
--- a/src/etc/htmldocck.py
+++ b/src/etc/htmldocck.py
@@ -110,11 +110,17 @@ import os.path
 import re
 import shlex
 from collections import namedtuple
-from HTMLParser import HTMLParser
+try:
+    from html.parser import HTMLParser
+except ImportError:
+    from HTMLParser import HTMLParser
 from xml.etree import cElementTree as ET
 
 # &larrb;/&rarrb; are not in HTML 4 but are in HTML 5
-from htmlentitydefs import entitydefs
+try:
+    from html.entities import entitydefs
+except ImportError:
+    from htmlentitydefs import entitydefs
 entitydefs['larrb'] = u'\u21e4'
 entitydefs['rarrb'] = u'\u21e5'
 entitydefs['nbsp'] = ' '

@QuietMisdreavus
Copy link
Member Author

@kennytm Want to make a new PR for that? I'm sure many people would love you for it. :P

@kennytm
Copy link
Member

kennytm commented Aug 25, 2017

@QuietMisdreavus Let me verify if it actually works on Python 3 first 😄

@QuietMisdreavus
Copy link
Member Author

Thinking about it more, it seems that to make this work how the tests expect it to basically amounts to fixing #22083. Since the DefId for exported types points to the original crate, any reference to them later on, even if from the exported location, still points to that original crate. Add another thing to the pile of "reexport pains".

So... does that block this PR? It still works in the context of std, just not so well outside of it.

@QuietMisdreavus QuietMisdreavus changed the title hide internal types/traits from std docs via new #[doc(masked)] attribute [WIP] hide internal types/traits from std docs via new #[doc(masked)] attribute Aug 25, 2017
@QuietMisdreavus
Copy link
Member Author

To help anyone who wants to see this effect when run on std and not just on my toy (broken) examples, i have a rendering of libstd docs here.

@QuietMisdreavus QuietMisdreavus added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Aug 25, 2017
@bors
Copy link
Contributor

bors commented Aug 26, 2017

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

@QuietMisdreavus
Copy link
Member Author

QuietMisdreavus commented Aug 30, 2017

cc @rust-lang/dev-tools because i just realized i never properly pinged the team.

This PR hides some internal types from appearing in the std docs by adding a new flag to the doc attribute, #[doc(masked)]. The problem is that the tests i wrote for this applied it to std itself, which doesn't completely work, since re-exported types aren't marked as coming from their exported crate.

The question i'm posing to the team (which may need to be relayed to other teams, come to think of it >_>) is whether we still want to land this, and how i should update the tests/docs if so. I think it's still useful in the context of the std docs, but until i/we can handle re-exports with this it's less useful outside of it.

(EDIT: also i fixed the merge conflict and squashed the commits some)

@bors
Copy link
Contributor

bors commented Sep 3, 2017

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

@nrc
Copy link
Member

nrc commented Sep 5, 2017

We discussed this at today's dev-tools meetings. We should land this as is and iterate (since it is behind a feature flag). Better to fix the std links issue sooner and work towards something more generally useful.

bors added a commit that referenced this pull request Sep 6, 2017
rustdoc: add new "Implementations on Foreign Types" section to traits

Demo screenshot:

![image](https://user-images.githubusercontent.com/5217170/29281219-c547f758-80e3-11e7-808f-49f592c65c5b.png)

Full demo available at https://tonberry.quietmisdreavus.net/foreign-test/foreign_test/trait.CheckIt.html

This PR splits the "Implementors" section on trait pages into two: First, for impls on types local to the crate, their impls are kept as-is, printing one line for the impl line, and any additional lines for associated types. However, for types external to the crate, they are now pulled up over the others and are printed (almost) like the summary impl on the type page itself. This gives any doc comments on these impls or methods to be exposed in the documentation.

There's just one small problem, though: [libstd docs apparently surface impls for libc and rand, possibly among others](https://tonberry.quietmisdreavus.net/foreign-std/std/marker/trait.Copy.html#foreign-impls). This adds this section to pages in the std docs where we might not want them to show up in the first place. I think this is a bug distinct from this PR, but it does make it drastically apparent.

~~My question, then, is this: Do we want this here? Taking it out involves fixing which impls are visible to rustdoc, possibly specifically when rendering the std facade. I'm convinced this is fine to land as-is, since it adds a feature specifically for non-std crates (i'm thinking of things like `num` or related crates that implement things on primitives or std types as part of their functionality).~~ (EDIT: I have an open PR to fix this: #44026)
@QuietMisdreavus QuietMisdreavus changed the title [WIP] hide internal types/traits from std docs via new #[doc(masked)] attribute hide internal types/traits from std docs via new #[doc(masked)] attribute Sep 6, 2017
@QuietMisdreavus
Copy link
Member Author

I've fixed the merge conflict. The travis failure is due to a stalled macOS build.

@carols10cents carols10cents added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 11, 2017
@carols10cents
Copy link
Member

I restarted the one osx builder.

ping looks like this is ready for your review, @steveklabnik !

@carols10cents
Copy link
Member

ping @steveklabnik @rust-lang/docs @rust-lang/dev-tools, this needs a review please!

@QuietMisdreavus QuietMisdreavus removed the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label Sep 18, 2017
@steveklabnik
Copy link
Member

Looks good to me.

@steveklabnik
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 18, 2017

📌 Commit 64f6111 has been approved by steveklabnik

@bors
Copy link
Contributor

bors commented Sep 19, 2017

⌛ Testing commit 64f6111 with merge 9a00f3c...

bors added a commit that referenced this pull request Sep 19, 2017
hide internal types/traits from std docs via new #[doc(masked)] attribute

Fixes #43701 (hopefully for good this time)

This PR introduces a new parameter to the `#[doc]` attribute that rustdoc looks for on `extern crate` statements. When it sees `#[doc(masked)]` on such a statement, it hides traits and types from that crate from appearing in either the "Trait Implementations" section of many type pages, or the "Implementors" section of trait pages. This is then applied to the `libc`/`rand`/`compiler_builtins` imports in libstd to prevent those crates from creating broken links in the std docs.

Like in #43348, this also introduces a feature gate, `doc_masked`, that controls the use of this parameter.

To view the std docs generated with this change, head to https://tonberry.quietmisdreavus.net/std-43701/std/index.html.
@bors
Copy link
Contributor

bors commented Sep 19, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing 9a00f3c to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants