From 691bc3bbd70acb175ad2f954dd309d3a49dd358e Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 3 Nov 2017 22:25:30 +0900 Subject: [PATCH 1/5] Add a test for structs with visibility --- tests/source/structs.rs | 5 +++++ tests/target/structs.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/tests/source/structs.rs b/tests/source/structs.rs index e63de8ecdb4..daf3821c2d9 100644 --- a/tests/source/structs.rs +++ b/tests/source/structs.rs @@ -258,3 +258,8 @@ struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo struct Looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong {} struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong {} struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong { x: i32 } + +// structs with visibility, do not duplicate visibility (#2110). +pub(in self) struct Foo(); +pub(super) struct Foo(); +pub(crate) struct Foo(); diff --git a/tests/target/structs.rs b/tests/target/structs.rs index 5132162f3d5..37bc3bc218b 100644 --- a/tests/target/structs.rs +++ b/tests/target/structs.rs @@ -300,3 +300,8 @@ struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { x: i32, } + +// structs with visibility, do not duplicate visibility (#2110). +pub(in self) struct Foo(); +pub(super) struct Foo(); +pub(crate) struct Foo(); From af3d793e306482ade58ac95f59c068905c5739d3 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 3 Nov 2017 23:38:32 +0900 Subject: [PATCH 2/5] Add more tests --- tests/source/structs.rs | 3 +++ tests/target/structs.rs | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/source/structs.rs b/tests/source/structs.rs index daf3821c2d9..acae17befdd 100644 --- a/tests/source/structs.rs +++ b/tests/source/structs.rs @@ -260,6 +260,9 @@ struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong { x: i32 } // structs with visibility, do not duplicate visibility (#2110). +pub(in self) struct Foo{} +pub(super) struct Foo{} +pub(crate) struct Foo{} pub(in self) struct Foo(); pub(super) struct Foo(); pub(crate) struct Foo(); diff --git a/tests/target/structs.rs b/tests/target/structs.rs index 37bc3bc218b..e29647c71fb 100644 --- a/tests/target/structs.rs +++ b/tests/target/structs.rs @@ -302,6 +302,9 @@ struct Loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo } // structs with visibility, do not duplicate visibility (#2110). -pub(in self) struct Foo(); +pub(self) struct Foo {} +pub(super) struct Foo {} +pub(crate) struct Foo {} +pub(self) struct Foo(); pub(super) struct Foo(); pub(crate) struct Foo(); From 84526ab87eb367a968130b54acfd65479be4cd23 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 3 Nov 2017 23:51:19 +0900 Subject: [PATCH 3/5] Add get_bytepos_after_visibility() --- src/items.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/items.rs b/src/items.rs index 8adfe080a2c..27e46cb0874 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1154,6 +1154,25 @@ pub fn format_struct_struct( } } +/// Returns a bytepos that is after that of `(` in `pub(..)`. If the given visibility does not +/// contain `pub(..)`, then return the `lo` of the `defualt_span`. Yeah, but for what? Well, we need +/// to bypass the `(` in the visibility when creating a span of tuple's body or fn's args. +fn get_bytepos_after_visibility( + context: &RewriteContext, + vis: &ast::Visibility, + default_span: Span, + terminator: &str, +) -> BytePos { + match *vis { + ast::Visibility::Crate(s, CrateSugar::PubCrate) => context + .codemap + .span_after(mk_sp(s.hi(), default_span.hi()), terminator), + ast::Visibility::Crate(s, CrateSugar::JustCrate) => s.hi(), + ast::Visibility::Restricted { ref path, .. } => path.span.hi(), + _ => default_span.lo(), + } +} + fn format_tuple_struct( context: &RewriteContext, item_name: &str, From 3973cdd0a8dcccd986d38d7b3915e88ceeff1a8c Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 3 Nov 2017 23:53:07 +0900 Subject: [PATCH 4/5] Use correct span for tuple struct's body --- src/items.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/items.rs b/src/items.rs index 27e46cb0874..5151ec92ced 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1189,12 +1189,13 @@ fn format_tuple_struct( result.push_str(&header_str); let body_lo = if fields.is_empty() { - context.codemap.span_after(span, "(") + let lo = get_bytepos_after_visibility(context, vis, span, ")"); + context.codemap.span_after(mk_sp(lo, span.hi()), "(") } else { fields[0].span.lo() }; let body_hi = if fields.is_empty() { - context.codemap.span_after(span, ")") + context.codemap.span_after(mk_sp(body_lo, span.hi()), ")") } else { // This is a dirty hack to work around a missing `)` from the span of the last field. let last_arg_span = fields[fields.len() - 1].span; @@ -1242,7 +1243,10 @@ fn format_tuple_struct( .to_string(context.config)) } result.push('('); - let snippet = context.snippet(mk_sp(body_lo, context.codemap.span_before(span, ")"))); + let snippet = context.snippet(mk_sp( + body_lo, + context.codemap.span_before(mk_sp(body_lo, span.hi()), ")"), + )); if snippet.is_empty() { // `struct S ()` } else if snippet.trim_right_matches(&[' ', '\t'][..]).ends_with('\n') { From 16302d3578f4faa68051c90b153b581169bc240c Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 3 Nov 2017 23:53:38 +0900 Subject: [PATCH 5/5] Use get_bytepos_after_visibility() --- src/items.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/items.rs b/src/items.rs index 5151ec92ced..a0ae60b3c37 100644 --- a/src/items.rs +++ b/src/items.rs @@ -1788,13 +1788,7 @@ fn rewrite_fn_base( } // Skip `pub(crate)`. - let lo_after_visibility = match fn_sig.visibility { - ast::Visibility::Crate(s, CrateSugar::PubCrate) => { - context.codemap.span_after(mk_sp(s.hi(), span.hi()), ")") - } - ast::Visibility::Crate(s, CrateSugar::JustCrate) => s.hi(), - _ => span.lo(), - }; + let lo_after_visibility = get_bytepos_after_visibility(context, &fn_sig.visibility, span, ")"); // A conservative estimation, to goal is to be over all parens in generics let args_start = fn_sig .generics