Merge pull request #1842 from emilio/rustfmt-format-diff

bin: Add a very simple rustfmt-format-diff.

Author: nrc

Cargo.toml

@@ -20,9 +20,13 @@ name = "rustfmt"
 [[bin]]
 name = "cargo-fmt"
 
+[[bin]]
+name = "rustfmt-format-diff"
+
 [features]
-default = ["cargo-fmt"]
+default = ["cargo-fmt", "rustfmt-format-diff"]
 cargo-fmt = []
+rustfmt-format-diff = []
 
 [dependencies]
 toml = "0.4"

src/bin/rustfmt-format-diff.rs

@@ -0,0 +1,268 @@
+// Copyright 2017 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.
+
+// Inspired by Clang's clang-format-diff:
+//
+// https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format-diff.py
+
+#![deny(warnings)]
+
+extern crate env_logger;
+extern crate getopts;
+#[macro_use]
+extern crate log;
+extern crate regex;
+extern crate serde;
+#[macro_use]
+extern crate serde_derive;
+extern crate serde_json as json;
+
+use std::{env, fmt, process};
+use std::collections::HashSet;
+use std::error::Error;
+use std::io::{self, BufRead};
+
+use regex::Regex;
+
+/// The default pattern of files to format.
+///
+/// We only want to format rust files by default.
+const DEFAULT_PATTERN: &'static str = r".*\.rs";
+
+#[derive(Debug)]
+enum FormatDiffError {
+    IncorrectOptions(getopts::Fail),
+    IncorrectFilter(regex::Error),
+    IoError(io::Error),
+}
+
+impl fmt::Display for FormatDiffError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        fmt::Display::fmt(self.cause().unwrap(), f)
+    }
+}
+
+impl Error for FormatDiffError {
+    fn description(&self) -> &str {
+        self.cause().unwrap().description()
+    }
+
+    fn cause(&self) -> Option<&Error> {
+        Some(match *self {
+            FormatDiffError::IoError(ref e) => e,
+            FormatDiffError::IncorrectFilter(ref e) => e,
+            FormatDiffError::IncorrectOptions(ref e) => e,
+        })
+    }
+}
+
+impl From<getopts::Fail> for FormatDiffError {
+    fn from(fail: getopts::Fail) -> Self {
+        FormatDiffError::IncorrectOptions(fail)
+    }
+}
+
+impl From<regex::Error> for FormatDiffError {
+    fn from(err: regex::Error) -> Self {
+        FormatDiffError::IncorrectFilter(err)
+    }
+}
+
+impl From<io::Error> for FormatDiffError {
+    fn from(fail: io::Error) -> Self {
+        FormatDiffError::IoError(fail)
+    }
+}
+
+fn main() {
+    let _ = env_logger::init();
+
+    let mut opts = getopts::Options::new();
+    opts.optflag("h", "help", "show this message");
+    opts.optopt(
+        "p",
+        "skip-prefix",
+        "skip the smallest prefix containing NUMBER slashes",
+        "NUMBER",
+    );
+    opts.optopt(
+        "f",
+        "filter",
+        "custom pattern selecting file paths to reformat",
+        "PATTERN",
+    );
+
+    if let Err(e) = run(&opts) {
+        println!("{}", opts.usage(e.description()));
+        process::exit(1);
+    }
+}
+
+#[derive(Debug, Eq, PartialEq, Serialize, Deserialize)]
+struct Range {
+    file: String,
+    range: [u32; 2],
+}
+
+fn run(opts: &getopts::Options) -> Result<(), FormatDiffError> {
+    let matches = opts.parse(env::args().skip(1))?;
+
+    if matches.opt_present("h") {
+        println!("{}", opts.usage("usage: "));
+        return Ok(());
+    }
+
+    let filter = matches
+        .opt_str("f")
+        .unwrap_or_else(|| DEFAULT_PATTERN.into());
+
+    let skip_prefix = matches
+        .opt_str("p")
+        .and_then(|p| p.parse::<u32>().ok())
+        .unwrap_or(0);
+
+    let (files, ranges) = scan_diff(io::stdin(), skip_prefix, &filter)?;
+
+    run_rustfmt(&files, &ranges)
+}
+
+fn run_rustfmt(files: &HashSet<String>, ranges: &[Range]) -> Result<(), FormatDiffError> {
+    if files.is_empty() || ranges.is_empty() {
+        debug!("No files to format found");
+        return Ok(());
+    }
+
+    let ranges_as_json = json::to_string(ranges).unwrap();
+
+    debug!("Files: {:?}", files);
+    debug!("Ranges: {:?}", ranges);
+
+    let exit_status = process::Command::new("rustfmt")
+        .args(files)
+        .arg("--file-lines")
+        .arg(ranges_as_json)
+        .status()?;
+
+    if !exit_status.success() {
+        return Err(FormatDiffError::IoError(io::Error::new(
+            io::ErrorKind::Other,
+            format!("rustfmt failed with {}", exit_status),
+        )));
+    }
+    Ok(())
+}
+
+/// Scans a diff from `from`, and returns the set of files found, and the ranges
+/// in those files.
+fn scan_diff<R>(
+    from: R,
+    skip_prefix: u32,
+    file_filter: &str,
+) -> Result<(HashSet<String>, Vec<Range>), FormatDiffError>
+where
+    R: io::Read,
+{
+    let diff_pattern = format!(r"^\+\+\+\s(?:.*?/){{{}}}(\S*)", skip_prefix);
+    let diff_pattern = Regex::new(&diff_pattern).unwrap();
+
+    let lines_pattern = Regex::new(r"^@@.*\+(\d+)(,(\d+))?").unwrap();
+
+    let file_filter = Regex::new(&format!("^{}$", file_filter))?;
+
+    let mut current_file = None;
+
+    let mut files = HashSet::new();
+    let mut ranges = vec![];
+    for line in io::BufReader::new(from).lines() {
+        let line = line.unwrap();
+
+        if let Some(captures) = diff_pattern.captures(&line) {
+            current_file = Some(captures.get(1).unwrap().as_str().to_owned());
+        }
+
+        let file = match current_file {
+            Some(ref f) => &**f,
+            None => continue,
+        };
+
+        // TODO(emilio): We could avoid this most of the time if needed, but
+        // it's not clear it's worth it.
+        if !file_filter.is_match(file) {
+            continue;
+        }
+
+        let lines_captures = match lines_pattern.captures(&line) {
+            Some(captures) => captures,
+            None => continue,
+        };
+
+        let start_line = lines_captures
+            .get(1)
+            .unwrap()
+            .as_str()
+            .parse::<u32>()
+            .unwrap();
+        let line_count = match lines_captures.get(3) {
+            Some(line_count) => line_count.as_str().parse::<u32>().unwrap(),
+            None => 1,
+        };
+
+        if line_count == 0 {
+            continue;
+        }
+
+        let end_line = start_line + line_count - 1;
+        files.insert(file.to_owned());
+        ranges.push(Range {
+            file: file.to_owned(),
+            range: [start_line, end_line],
+        });
+    }
+
+    Ok((files, ranges))
+}
+
+#[test]
+fn scan_simple_git_diff() {
+    const DIFF: &'static str = include_str!("test/bindgen.diff");
+    let (files, ranges) = scan_diff(DIFF.as_bytes(), 1, r".*\.rs").expect("scan_diff failed?");
+
+    assert!(
+        files.contains("src/ir/traversal.rs"),
+        "Should've matched the filter"
+    );
+
+    assert!(
+        !files.contains("tests/headers/anon_enum.hpp"),
+        "Shouldn't have matched the filter"
+    );
+
+    assert_eq!(
+        &ranges,
+        &[
+            Range {
+                file: "src/ir/item.rs".into(),
+                range: [148, 158],
+            },
+            Range {
+                file: "src/ir/item.rs".into(),
+                range: [160, 170],
+            },
+            Range {
+                file: "src/ir/traversal.rs".into(),
+                range: [9, 16],
+            },
+            Range {
+                file: "src/ir/traversal.rs".into(),
+                range: [35, 43],
+            }
+        ]
+    );
+}

src/bin/test/bindgen.diff

@@ -0,0 +1,67 @@
+diff --git a/src/ir/item.rs b/src/ir/item.rs
+index 7f3afefb..90d15e96 100644
+--- a/src/ir/item.rs
++++ b/src/ir/item.rs
+@@ -148,7 +148,11 @@ impl<'a, 'b> Iterator for ItemAncestorsIter<'a, 'b>
+ impl AsTemplateParam for ItemId {
+     type Extra = ();
+ 
+-    fn as_template_param(&self, ctx: &BindgenContext, _: &()) -> Option<ItemId> {
++    fn as_template_param(
++        &self,
++        ctx: &BindgenContext,
++        _: &(),
++    ) -> Option<ItemId> {
+         ctx.resolve_item(*self).as_template_param(ctx, &())
+     }
+ }
+@@ -156,7 +160,11 @@ impl AsTemplateParam for ItemId {
+ impl AsTemplateParam for Item {
+     type Extra = ();
+ 
+-    fn as_template_param(&self, ctx: &BindgenContext, _: &()) -> Option<ItemId> {
++    fn as_template_param(
++        &self,
++        ctx: &BindgenContext,
++        _: &(),
++    ) -> Option<ItemId> {
+         self.kind.as_template_param(ctx, self)
+     }
+ }
+diff --git a/src/ir/traversal.rs b/src/ir/traversal.rs
+index 762a3e2d..b9c9dd4e 100644
+--- a/src/ir/traversal.rs
++++ b/src/ir/traversal.rs
+@@ -9,6 +9,8 @@ use std::collections::{BTreeMap, VecDeque};
+ ///
+ ///   from --> to
+ ///
++/// Random content to generate a diff.
++///
+ /// The `from` is left implicit: it is the concrete `Trace` implementer which
+ /// yielded this outgoing edge.
+ #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+@@ -33,7 +35,9 @@ impl Into<ItemId> for Edge {
+     }
+ }
+ 
+-/// The kind of edge reference. This is useful when we wish to only consider
++/// The kind of edge reference.
++///
++/// This is useful when we wish to only consider
+ /// certain kinds of edges for a particular traversal or analysis.
+ #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+ pub enum EdgeKind {
+diff --git a/tests/headers/anon_enum.hpp b/tests/headers/anon_enum.hpp
+index 1961fe6c..34759df3 100644
+--- a/tests/headers/anon_enum.hpp
++++ b/tests/headers/anon_enum.hpp
+@@ -1,7 +1,7 @@
+ struct Test {
+   int foo;
+   float bar;
+-  enum { T_NONE };
++  enum { T_NONE, T_SOME };
+ };
+ 
+ typedef enum {