Skip to content

Add infrastructure for formatting specific line ranges #1007

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

Merged
merged 11 commits into from
May 31, 2016
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
6 changes: 6 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,4 @@ log = "0.3"
env_logger = "0.3"
getopts = "0.2"
itertools = "0.4.15"
multimap = "0.3"
19 changes: 19 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,25 @@ the command line. For example `rustfmt --write-mode=display src/filename.rs`

`cargo fmt` uses `--write-mode=replace` by default.

If you want to restrict reformatting to specific sets of lines, you can
use the `--file-lines` option. Its argument is a JSON array of objects
with `file` and `range` properties, where `file` is a file name, and
`range` is an array representing a range of lines like `[7,13]`. Ranges
are 1-based and inclusive of both end points. Specifying an empty array
will result in no files being formatted. For example,

```
rustfmt --file-lines '[
{"file":"src/lib.rs","range":[7,13]},
{"file":"src/lib.rs","range":[21,29]},
{"file":"src/foo.rs","range":[10,11]},
{"file":"src/foo.rs","range":[15,15]}]'
```

would format lines `7-13` and `21-29` of `src/lib.rs`, and lines `10-11`,
and `15` of `src/foo.rs`. No other files would be formatted, even if they
are included as out of line modules from `src/lib.rs`.

If `rustfmt` successfully reformatted the code it will exit with `0` exit
status. Exit status `1` signals some unexpected error, like an unknown option or
a failure to read a file. Exit status `2` is returned if there are syntax errors
Expand Down
26 changes: 21 additions & 5 deletions src/bin/rustfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ extern crate env_logger;
extern crate getopts;

use rustfmt::{run, Input, Summary};
use rustfmt::file_lines::FileLines;
use rustfmt::config::{Config, WriteMode};

use std::{env, error};
Expand Down Expand Up @@ -57,6 +58,7 @@ struct CliOptions {
skip_children: bool,
verbose: bool,
write_mode: Option<WriteMode>,
file_lines: FileLines, // Default is all lines in all files.
}

impl CliOptions {
Expand All @@ -73,12 +75,17 @@ impl CliOptions {
}
}

if let Some(ref file_lines) = matches.opt_str("file-lines") {
options.file_lines = try!(file_lines.parse());
}

Ok(options)
}

fn apply_to(&self, config: &mut Config) {
fn apply_to(self, config: &mut Config) {
config.skip_children = self.skip_children;
config.verbose = self.verbose;
config.file_lines = self.file_lines;
if let Some(write_mode) = self.write_mode {
config.write_mode = write_mode;
}
Expand Down Expand Up @@ -168,6 +175,10 @@ fn make_opts() -> Options {
"Recursively searches the given path for the rustfmt.toml config file. If not \
found reverts to the input file path",
"[Path for the configuration file]");
opts.optopt("",
"file-lines",
"Format specified line ranges. See README for more detail on the JSON format.",
"JSON");

opts
}
Expand Down Expand Up @@ -198,8 +209,12 @@ fn execute(opts: &Options) -> FmtResult<Summary> {

Ok(run(Input::Text(input), &config))
}
Operation::Format { files, config_path } => {
Operation::Format { mut files, config_path } => {
let options = try!(CliOptions::from_matches(&matches));

// Add any additional files that were specified via `--file-lines`.
files.extend(options.file_lines.files().cloned().map(PathBuf::from));

let mut config = Config::default();
let mut path = None;
// Load the config path file if provided
Expand Down Expand Up @@ -227,7 +242,7 @@ fn execute(opts: &Options) -> FmtResult<Summary> {
config = config_tmp;
}

options.apply_to(&mut config);
options.clone().apply_to(&mut config);
error_summary.add(run(Input::File(file), &config));
}
Ok(error_summary)
Expand Down Expand Up @@ -306,8 +321,8 @@ fn determine_operation(matches: &Matches) -> FmtResult<Operation> {
Some(dir)
});

// if no file argument is supplied, read from stdin
if matches.free.is_empty() {
// if no file argument is supplied and `--file-lines` is not specified, read from stdin
if matches.free.is_empty() && !matches.opt_present("file-lines") {
Copy link
Member

Choose a reason for hiding this comment

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

Why do this? Seems like we could apply --file-lines to stdin if we wanted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason here is that rustfmt --file-lines='[{"file":"blah.rs", range:[1,15]}]' with no free arguments should format blah.rs not stdin. I could change it so that a string like <stdin> is accepted in the file part of JSON and treated specially. This might require some validation that we're not mixing <stdin> and file names to avoid confusing behaviour. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On this note, I'm tempted to make --file-lines mutually exclusive with specifying file names on the command line, since it's redundant and in the worst case results in unnecessary parsing. I'd leave that to a later PR though.

Copy link
Member

Choose a reason for hiding this comment

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

Accepting file: <stdin> sounds good. I would allow --file-lines and a file specified on the command line, as long as the names match up - seems silly to do, but no reason to error out. You can leave all of this till later though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 for leaving this until a later PR. I want to get the basic idea merged in so that I can build on it in various ways.


let mut buffer = String::new();
try!(io::stdin().read_to_string(&mut buffer));
Expand All @@ -318,6 +333,7 @@ fn determine_operation(matches: &Matches) -> FmtResult<Operation> {
});
}

// We append files from `--file-lines` later in `execute()`.
let files: Vec<_> = matches.free.iter().map(PathBuf::from).collect();

Ok(Operation::Format {
Expand Down
94 changes: 94 additions & 0 deletions src/codemap.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

//! This module contains utilities that work with the `CodeMap` from libsyntax / syntex_syntax.
//! This includes extension traits and methods for looking up spans and line ranges for AST nodes.

use std::rc::Rc;

use syntax::codemap::{BytePos, CodeMap, FileMap, Span};

use comment::FindUncommented;

/// A range of lines in a file, inclusive of both ends.
pub struct LineRange {
pub file: Rc<FileMap>,
pub lo: usize,
pub hi: usize,
}

impl LineRange {
pub fn file_name(&self) -> &str {
self.file.as_ref().name.as_str()
}
}

pub trait SpanUtils {
fn span_after(&self, original: Span, needle: &str) -> BytePos;
fn span_after_last(&self, original: Span, needle: &str) -> BytePos;
fn span_before(&self, original: Span, needle: &str) -> BytePos;
}

pub trait LineRangeUtils {
/// Returns the `LineRange` that corresponds to `span` in `self`.
///
/// # Panics
///
/// Panics if `span` crosses a file boundary, which shouldn't happen.
fn lookup_line_range(&self, span: Span) -> LineRange;
}

impl SpanUtils for CodeMap {
#[inline]
fn span_after(&self, original: Span, needle: &str) -> BytePos {
let snippet = self.span_to_snippet(original).unwrap();
let offset = snippet.find_uncommented(needle).unwrap() + needle.len();

original.lo + BytePos(offset as u32)
}

#[inline]
fn span_after_last(&self, original: Span, needle: &str) -> BytePos {
let snippet = self.span_to_snippet(original).unwrap();
let mut offset = 0;

while let Some(additional_offset) = snippet[offset..].find_uncommented(needle) {
offset += additional_offset + needle.len();
}

original.lo + BytePos(offset as u32)
}

#[inline]
fn span_before(&self, original: Span, needle: &str) -> BytePos {
let snippet = self.span_to_snippet(original).unwrap();
let offset = snippet.find_uncommented(needle).unwrap();

original.lo + BytePos(offset as u32)
}
}

impl LineRangeUtils for CodeMap {
fn lookup_line_range(&self, span: Span) -> LineRange {
let lo = self.lookup_char_pos(span.lo);
let hi = self.lookup_char_pos(span.hi);

assert!(lo.file.name == hi.file.name,
"span crossed file boundary: lo: {:?}, hi: {:?}",
lo,
hi);

LineRange {
file: lo.file.clone(),
lo: lo.line,
hi: hi.line,
}
}
}
10 changes: 10 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

extern crate toml;

use file_lines::FileLines;
use lists::{SeparatorTactic, ListTactic};
use std::io::Write;

Expand Down Expand Up @@ -200,6 +201,12 @@ impl ConfigType for String {
}
}

impl ConfigType for FileLines {
fn doc_hint() -> String {
String::from("<json>")
}
}

pub struct ConfigHelpItem {
option_name: &'static str,
doc_string: &'static str,
Expand Down Expand Up @@ -327,6 +334,9 @@ macro_rules! create_config {
create_config! {
verbose: bool, false, "Use verbose output";
skip_children: bool, false, "Don't reformat out of line modules";
file_lines: FileLines, FileLines::all(),
"Lines to format; this is not supported in rustfmt.toml, and can only be specified \
via the --file-lines option";
max_width: usize, 100, "Maximum width of each line";
ideal_width: usize, 80, "Ideal width of each line";
tab_spaces: usize, 4, "Number of spaces per tab";
Expand Down
5 changes: 3 additions & 2 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,13 @@ use std::iter::ExactSizeIterator;
use std::fmt::Write;

use {Indent, Spanned};
use codemap::SpanUtils;
use rewrite::{Rewrite, RewriteContext};
use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
DefinitiveListTactic, definitive_tactic, ListItem, format_item_list};
use string::{StringFormat, rewrite_string};
use utils::{CodeMapSpanUtils, extra_offset, last_line_width, wrap_str, binary_search,
first_line_width, semicolon_for_stmt, trimmed_last_line_width, left_most_sub_expr};
use utils::{extra_offset, last_line_width, wrap_str, binary_search, first_line_width,
semicolon_for_stmt, trimmed_last_line_width, left_most_sub_expr};
use visitor::FmtVisitor;
use config::{Config, StructLitStyle, MultilineStyle, ElseIfBraceStyle, ControlBraceStyle};
use comment::{FindUncommented, rewrite_comment, contains_comment, recover_comment_removed};
Expand Down
Loading