Skip to content

Commit

Permalink
Move 'use' to Rewrite
Browse files Browse the repository at this point in the history 
Implements Rewrite for ViewPath

Behavior change: always use max_width instead of ideal_width for use
list rewrite. I think it looks better, was also suggested by @nrc in
https://github.com/nrc/rustfmt/issues/82#issuecomment-105314265
  • Loading branch information
@cassiersg
cassiersg committed Jul 25, 2015
1 parent ff301ef commit 30b16bc
Show file tree
Hide file tree
Showing 4 changed files with 136 additions and 128 deletions.
205 changes: 112 additions & 93 deletions src/imports.rs
View file
Expand Up @@ -8,126 +8,145 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use visitor::FmtVisitor;
use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
use utils::{span_after, format_visibility};
use utils::span_after;
use rewrite::{Rewrite, RewriteContext};
use config::Config;

use syntax::ast;
use syntax::parse::token;
use syntax::print::pprust;
use syntax::codemap::Span;
use syntax::codemap::{CodeMap, Span};

// TODO (some day) remove unused imports, expand globs, compress many single imports into a list import

fn rewrite_single_use_list(path_str: String, vpi: ast::PathListItem, vis: &str) -> String {
impl Rewrite for ast::ViewPath {
// Returns an empty string when the ViewPath is empty (like foo::bar::{})
fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
match self.node {
ast::ViewPath_::ViewPathList(ref path, ref path_list) => {
Some(rewrite_use_list(width,
offset,
path,
path_list,
self.span,
context.codemap,
context.config).unwrap_or("".to_owned()))
}
ast::ViewPath_::ViewPathGlob(_) => {
// FIXME convert to list?
None
}
ast::ViewPath_::ViewPathSimple(_,_) => {
None
}
}
}
}

fn rewrite_single_use_list(path_str: String, vpi: ast::PathListItem) -> String {
if let ast::PathListItem_::PathListIdent{ name, .. } = vpi.node {
let name_str = token::get_ident(name).to_string();
if path_str.len() == 0 {
format!("{}use {};", vis, name_str)
name_str
} else {
format!("{}use {}::{};", vis, path_str, name_str)
format!("{}::{}", path_str, name_str)
}
} else {
if path_str.len() != 0 {
format!("{}use {};", vis, path_str)
path_str
} else {
// This catches the import: use {self}, which is a compiler error, so we just
// leave it alone.
format!("{}use {{self}};", vis)
"{self}".to_owned()
}
}
}

impl<'a> FmtVisitor<'a> {
// Basically just pretty prints a multi-item import.
// Returns None when the import can be removed.
pub fn rewrite_use_list(&self,
block_indent: usize,
one_line_budget: usize, // excluding indentation
multi_line_budget: usize,
path: &ast::Path,
path_list: &[ast::PathListItem],
visibility: ast::Visibility,
span: Span)
-> Option<String> {
let path_str = pprust::path_to_string(path);
let vis = format_visibility(visibility);

match path_list.len() {
0 => return None,
1 => return Some(rewrite_single_use_list(path_str, path_list[0], vis)),
_ => ()
}

// 2 = ::
let path_separation_w = if path_str.len() > 0 {
2
} else {
0
};
// 5 = "use " + {
let indent = path_str.len() + 5 + path_separation_w + vis.len();

// 2 = } + ;
let used_width = indent + 2;
// Basically just pretty prints a multi-item import.
// Returns None when the import can be removed.
pub fn rewrite_use_list(width: usize,
offset: usize,
path: &ast::Path,
path_list: &[ast::PathListItem],
span: Span,
codemap: &CodeMap,
config: &Config)
-> Option<String> {
let path_str = pprust::path_to_string(path);

match path_list.len() {
0 => return None,
1 => return Some(rewrite_single_use_list(path_str, path_list[0])),
_ => ()
}

// Break as early as possible when we've blown our budget.
let remaining_line_budget = one_line_budget.checked_sub(used_width).unwrap_or(0);
let remaining_multi_budget = multi_line_budget.checked_sub(used_width).unwrap_or(0);
// 2 = ::
let path_separation_w = if path_str.len() > 0 {
2
} else {
0
};
// 1 = {
let supp_indent = path_str.len() + path_separation_w + 1;
// 1 = }
let remaining_width = width.checked_sub(supp_indent + 1).unwrap_or(0);

let fmt = ListFormatting {
tactic: ListTactic::Mixed,
separator: ",",
trailing_separator: SeparatorTactic::Never,
indent: offset + supp_indent,
h_width: remaining_width,
// FIXME This is too conservative, and will not use all width
// available
// (loose 1 column (";"))
v_width: remaining_width,
ends_with_newline: true,
};

let mut items = itemize_list(codemap,
vec![ListItem::from_str("")], /* Dummy value, explanation
* below */
path_list.iter(),
",",
"}",
|vpi| vpi.span.lo,
|vpi| vpi.span.hi,
|vpi| match vpi.node {
ast::PathListItem_::PathListIdent{ name, .. } => {
token::get_ident(name).to_string()
}
ast::PathListItem_::PathListMod{ .. } => {
"self".to_owned()
}
},
span_after(span, "{", codemap),
span.hi);

// We prefixed the item list with a dummy value so that we can
// potentially move "self" to the front of the vector without touching
// the rest of the items.
// FIXME: Make more efficient by using a linked list? That would
// require changes to the signatures of itemize_list and write_list.
let has_self = move_self_to_front(&mut items);
let first_index = if has_self {
0
} else {
1
};

let fmt = ListFormatting {
tactic: ListTactic::Mixed,
separator: ",",
trailing_separator: SeparatorTactic::Never,
indent: block_indent + indent,
h_width: remaining_line_budget,
v_width: remaining_multi_budget,
ends_with_newline: true,
};
if config.reorder_imports {
items[1..].sort_by(|a, b| a.item.cmp(&b.item));
}

let mut items = itemize_list(self.codemap,
vec![ListItem::from_str("")], /* Dummy value, explanation
* below */
path_list.iter(),
",",
"}",
|vpi| vpi.span.lo,
|vpi| vpi.span.hi,
|vpi| match vpi.node {
ast::PathListItem_::PathListIdent{ name, .. } => {
token::get_ident(name).to_string()
}
ast::PathListItem_::PathListMod{ .. } => {
"self".to_owned()
}
},
span_after(span, "{", self.codemap),
span.hi);
let list = write_list(&items[first_index..], &fmt);

// We prefixed the item list with a dummy value so that we can
// potentially move "self" to the front of the vector without touching
// the rest of the items.
// FIXME: Make more efficient by using a linked list? That would
// require changes to the signatures of itemize_list and write_list.
let has_self = move_self_to_front(&mut items);
let first_index = if has_self {
0
Some(if path_str.len() == 0 {
format!("{{{}}}", list)
} else {
1
};

if self.config.reorder_imports {
items[1..].sort_by(|a, b| a.item.cmp(&b.item));
}

let list = write_list(&items[first_index..], &fmt);

Some(if path_str.len() == 0 {
format!("{}use {{{}}};", vis, list)
} else {
format!("{}use {}::{{{}}};", vis, path_str, list)
})
}
format!("{}::{{{}}}", path_str, list)
})
}

// Returns true when self item was found.
Expand Down
48 changes: 19 additions & 29 deletions src/visitor.rs
View file
Expand Up @@ -164,39 +164,29 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {

match item.node {
ast::Item_::ItemUse(ref vp) => {
match vp.node {
ast::ViewPath_::ViewPathList(ref path, ref path_list) => {
let block_indent = self.block_indent;
let one_line_budget = self.config.max_width - block_indent;
let multi_line_budget = self.config.ideal_width - block_indent;
let formatted = self.rewrite_use_list(block_indent,
one_line_budget,
multi_line_budget,
path,
path_list,
item.vis,
item.span);

if let Some(new_str) = formatted {
self.format_missing_with_indent(item.span.lo);
self.changes.push_str_span(item.span, &new_str);
} else {
// Format up to last newline
let span = codemap::mk_sp(self.last_pos, item.span.lo);
let span_end = match self.snippet(span).rfind('\n') {
Some(offset) => self.last_pos + BytePos(offset as u32),
None => item.span.lo
};
self.format_missing(span_end);
}

let vis = utils::format_visibility(item.vis);
let offset = self.block_indent + vis.len() + "use ".len();
let context = RewriteContext {
codemap: self.codemap, config: self.config, block_indent: self.block_indent };
// 1 = ";"
match vp.rewrite(&context, self.config.max_width - offset - 1, offset) {
Some(ref s) if s.len() == 0 => {
// Format up to last newline
let span = codemap::mk_sp(self.last_pos, item.span.lo);
let span_end = match self.snippet(span).rfind('\n') {
Some(offset) => self.last_pos + BytePos(offset as u32),
None => item.span.lo
};
self.format_missing(span_end);
self.last_pos = item.span.hi;
}
ast::ViewPath_::ViewPathGlob(_) => {
Some(ref s) => {
let s = format!("{}use {};", vis, s);
self.format_missing_with_indent(item.span.lo);
// FIXME convert to list?
self.changes.push_str_span(item.span, &s);
self.last_pos = item.span.hi;
}
ast::ViewPath_::ViewPathSimple(_,_) => {
None => {
self.format_missing_with_indent(item.span.lo);
}
}
Expand Down
4 changes: 2 additions & 2 deletions tests/target/imports.rs
View file
Expand Up @@ -23,8 +23,8 @@ mod Foo {
pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};

mod Foo2 {
pub use syntax::ast::{self, ItemForeignMod, ItemImpl, ItemMac, ItemMod,
ItemStatic, ItemDefaultImpl};
pub use syntax::ast::{self, ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic,
ItemDefaultImpl};
}
}

Expand Down
7 changes: 3 additions & 4 deletions tests/target/multiple.rs
View file
Expand Up @@ -14,10 +14,9 @@ extern crate foo;
extern crate foo;

use std::cell::*;
use std::{self, any, ascii, borrow, boxed, char, borrow, boxed, char, borrow,
borrow, boxed, char, borrow, boxed, char, borrow, boxed, char,
borrow, boxed, char, borrow, boxed, char, borrow, boxed, char,
borrow, boxed, char, borrow, boxed, char};
use std::{self, any, ascii, borrow, boxed, char, borrow, boxed, char, borrow, borrow, boxed, char,
borrow, boxed, char, borrow, boxed, char, borrow, boxed, char, borrow, boxed, char,
borrow, boxed, char, borrow, boxed, char, borrow, boxed, char};

mod doc;
mod other;
Expand Down

0 comments on commit 30b16bc

Please sign in to comment.