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

Fix whitespace handling after where clause #98256

Merged
merged 2 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ impl Buffer {
pub(crate) fn reserve(&mut self, additional: usize) {
self.buffer.reserve(additional)
}

pub(crate) fn len(&self) -> usize {
self.buffer.len()
}
}

fn comma_sep<T: fmt::Display>(
Expand Down
55 changes: 41 additions & 14 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,17 @@ struct ItemVars<'a> {
src_href: Option<&'a str>,
}

/// Calls `print_where_clause` and returns `true` if a `where` clause was generated.
fn print_where_clause_and_check<'a, 'tcx: 'a>(
buffer: &mut Buffer,
gens: &'a clean::Generics,
cx: &'a Context<'tcx>,
) -> bool {
let len_before = buffer.len();
write!(buffer, "{}", print_where_clause(gens, cx, 0, true));
len_before != buffer.len()
}

pub(super) fn print_item(
cx: &mut Context<'_>,
item: &clean::Item,
Expand Down Expand Up @@ -1152,17 +1163,21 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::
render_attributes_in_pre(w, it, "");
write!(
w,
"{}enum {}{}{}",
"{}enum {}{}",
it.visibility.print_with_space(it.item_id, cx),
it.name.unwrap(),
e.generics.print(cx),
print_where_clause(&e.generics, cx, 0, true),
);
if !print_where_clause_and_check(w, &e.generics, cx) {
// If there wasn't a `where` clause, we add a whitespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be better to make this whitespace emitted by print_where_clause. Then the role of print_where_clause is simple to explain and self contained. It either (a) prints a newline, a where clause, and another newline, or (b) prints a whitespace.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me give it a try then!

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately it cannot work for cases like:

trait A {
    fn foo<T>(a: T)
    where T: Clone;
}

type F<T> where T: Clone = Vec<T>;
trait G<T> where T: Clone = A<T>;

If we handle the whitespace directly into print_where_clause, it'll need to be handled manually in these cases to "counter" the effect.

w.write_str(" ");
}

let variants_stripped = e.has_stripped_entries();
if count_variants == 0 && !variants_stripped {
w.write_str(" {}");
w.write_str("{}");
} else {
w.write_str(" {\n");
w.write_str("{\n");
let toggle = should_hide_fields(count_variants);
if toggle {
toggle_open(w, format_args!("{} variants", count_variants));
Expand Down Expand Up @@ -1643,13 +1658,21 @@ fn render_union(
tab: &str,
cx: &Context<'_>,
) {
write!(w, "{}union {}", it.visibility.print_with_space(it.item_id, cx), it.name.unwrap());
if let Some(g) = g {
write!(w, "{}", g.print(cx));
write!(w, "{}", print_where_clause(g, cx, 0, true));
write!(w, "{}union {}", it.visibility.print_with_space(it.item_id, cx), it.name.unwrap(),);

let where_displayed = g
.map(|g| {
write!(w, "{}", g.print(cx));
print_where_clause_and_check(w, g, cx)
})
.unwrap_or(false);

// If there wasn't a `where` clause, we add a whitespace.
if !where_displayed {
w.write_str(" ");
}

write!(w, " {{\n{}", tab);
write!(w, "{{\n{}", tab);
let count_fields =
fields.iter().filter(|f| matches!(*f.kind, clean::StructFieldItem(..))).count();
let toggle = should_hide_fields(count_fields);
Expand Down Expand Up @@ -1701,10 +1724,14 @@ fn render_struct(
}
match ty {
CtorKind::Fictive => {
if let Some(g) = g {
write!(w, "{}", print_where_clause(g, cx, 0, true),)
let where_diplayed = g.map(|g| print_where_clause_and_check(w, g, cx)).unwrap_or(false);

// If there wasn't a `where` clause, we add a whitespace.
if !where_diplayed {
w.write_str(" {");
} else {
w.write_str("{");
}
w.write_str(" {");
let count_fields =
fields.iter().filter(|f| matches!(*f.kind, clean::StructFieldItem(..))).count();
let has_visible_fields = count_fields > 0;
Expand Down Expand Up @@ -1759,7 +1786,7 @@ fn render_struct(
}
w.write_str(")");
if let Some(g) = g {
write!(w, "{}", print_where_clause(g, cx, 0, false),)
write!(w, "{}", print_where_clause(g, cx, 0, false));
}
// We only want a ";" when we are displaying a tuple struct, not a variant tuple struct.
if structhead {
Expand All @@ -1769,7 +1796,7 @@ fn render_struct(
CtorKind::Const => {
// Needed for PhantomData.
if let Some(g) = g {
write!(w, "{}", print_where_clause(g, cx, 0, false),)
write!(w, "{}", print_where_clause(g, cx, 0, false));
}
w.write_str(";");
}
Expand Down
4 changes: 4 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.enum.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock item-decl"><pre class="rust enum"><code>pub enum Cow&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt; <span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,&#160;</span>{
Borrowed(<a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B),
Whatever(<a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>),
}</code></pre></div>
4 changes: 4 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.enum2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock item-decl"><pre class="rust enum"><code>pub enum Cow2&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + 'a&gt; {
Borrowed(<a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B),
Whatever(<a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>),
}</code></pre></div>
77 changes: 77 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// This test ensures there is no whitespace before the first brace of
// trait, enum, struct and union items when they have a where clause.

#![crate_name = "foo"]

// @has 'foo/trait.ToOwned.html'
// @snapshot trait - '//*[@class="docblock item-decl"]'
pub trait ToOwned<T>
where T: Clone
{
type Owned;
fn to_owned(&self) -> Self::Owned;
fn whatever(&self) -> T;
}

// @has 'foo/trait.ToOwned2.html'
// @snapshot trait2 - '//*[@class="docblock item-decl"]'
// There should be a whitespace before `{` in this case!
pub trait ToOwned2<T: Clone> {
type Owned;
fn to_owned(&self) -> Self::Owned;
fn whatever(&self) -> T;
}

// @has 'foo/enum.Cow.html'
// @snapshot enum - '//*[@class="docblock item-decl"]'
pub enum Cow<'a, B: ?Sized + 'a>
where
B: ToOwned<Clone>,
{
Borrowed(&'a B),
Whatever(u32),
}

// @has 'foo/enum.Cow2.html'
// @snapshot enum2 - '//*[@class="docblock item-decl"]'
// There should be a whitespace before `{` in this case!
pub enum Cow2<'a, B: ?Sized + ToOwned<Clone> + 'a> {
Borrowed(&'a B),
Whatever(u32),
}

// @has 'foo/struct.Struct.html'
// @snapshot struct - '//*[@class="docblock item-decl"]'
pub struct Struct<'a, B: ?Sized + 'a>
where
B: ToOwned<Clone>,
{
pub a: &'a B,
pub b: u32,
}

// @has 'foo/struct.Struct2.html'
// @snapshot struct2 - '//*[@class="docblock item-decl"]'
// There should be a whitespace before `{` in this case!
pub struct Struct2<'a, B: ?Sized + ToOwned<Clone> + 'a> {
pub a: &'a B,
pub b: u32,
}

// @has 'foo/union.Union.html'
// @snapshot union - '//*[@class="docblock item-decl"]'
pub union Union<'a, B: ?Sized + 'a>
where
B: ToOwned<Clone>,
{
a: &'a B,
b: u32,
}

// @has 'foo/union.Union2.html'
// @snapshot union2 - '//*[@class="docblock item-decl"]'
// There should be a whitespace before `{` in this case!
pub union Union2<'a, B: ?Sized + ToOwned<Clone> + 'a> {
a: &'a B,
b: u32,
}
4 changes: 4 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.struct.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock item-decl"><pre class="rust struct"><code>pub struct Struct&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt; <span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,&#160;</span>{
pub a: <a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B,
pub b: <a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>,
}</code></pre></div>
4 changes: 4 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.struct2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="docblock item-decl"><pre class="rust struct"><code>pub struct Struct2&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + 'a&gt; {
pub a: <a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B,
pub b: <a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>,
}</code></pre></div>
6 changes: 6 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.trait.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<div class="docblock item-decl"><pre class="rust trait"><code>pub trait ToOwned&lt;T&gt; <span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;T: <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>,&#160;</span>{
type <a href="#associatedtype.Owned" class="associatedtype">Owned</a>;

fn <a href="#tymethod.to_owned" class="fnname">to_owned</a>(&amp;self) -&gt; Self::<a class="associatedtype" href="trait.ToOwned.html#associatedtype.Owned" title="type foo::ToOwned::Owned">Owned</a>;
<span class="item-spacer" /> fn <a href="#tymethod.whatever" class="fnname">whatever</a>(&amp;self) -&gt; T;
}</code></pre></div>
6 changes: 6 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.trait2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<div class="docblock item-decl"><pre class="rust trait"><code>pub trait ToOwned2&lt;T:&#160;<a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; {
type <a href="#associatedtype.Owned" class="associatedtype">Owned</a>;

fn <a href="#tymethod.to_owned" class="fnname">to_owned</a>(&amp;self) -&gt; Self::<a class="associatedtype" href="trait.ToOwned2.html#associatedtype.Owned" title="type foo::ToOwned2::Owned">Owned</a>;
<span class="item-spacer" /> fn <a href="#tymethod.whatever" class="fnname">whatever</a>(&amp;self) -&gt; T;
}</code></pre></div>
3 changes: 3 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.union.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="docblock item-decl"><pre class="rust union"><code>pub union Union&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt; <span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,&#160;</span>{
/* private fields */
}</code></pre></div>
3 changes: 3 additions & 0 deletions src/test/rustdoc/whitespace-after-where-clause.union2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div class="docblock item-decl"><pre class="rust union"><code>pub union Union2&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + 'a&gt; {
/* private fields */
}</code></pre></div>