mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Implement checks for unnumbered TODOs and FIXMEs

Browse Source
This commit is contained in:
Marcus Klaas 2015-06-09 01:42:29 +02:00
parent 9da7be929f
commit d335d04575
8 changed files with 435 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/changes.rs
View File

@ -21,6 +21,7 @@ use std::fs::File;
use std::io::{Write, stdout};
use WriteMode;
use NewlineStyle;
use utils::round_up_to_power_of_two;
// This is basically a wrapper around a bunch of Ropes which makes it convenient
// to work with libsyntax. It is badly named.
@ -41,11 +42,10 @@ impl<'a> ChangeSet<'a> {
for f in codemap.files.borrow().iter() {
// Use the length of the file as a heuristic for how much space we
// need. I hope that at some stage someone rounds this up to the next
// power of two. TODO check that or do it here.
result.file_map.insert(f.name.clone(),
StringBuffer::with_capacity(f.src.as_ref().unwrap().len()));
// need. Round to the next power of two.
let buffer_cap = round_up_to_power_of_two(f.src.as_ref().unwrap().len());
result.file_map.insert(f.name.clone(), StringBuffer::with_capacity(buffer_cap));
result.file_spans.push((f.start_pos.0, f.end_pos.0));
}

14
src/config.rs
View File

@ -10,19 +10,25 @@
extern crate toml;
use {NewlineStyle, BraceStyle, ReturnIndent};
use lists::SeparatorTactic;
use issues::ReportTactic;
#[derive(RustcDecodable)]
pub struct Config {
pub max_width: usize,
pub ideal_width: usize,
pub leeway: usize,
pub tab_spaces: usize,
pub newline_style: ::NewlineStyle,
pub fn_brace_style: ::BraceStyle,
pub fn_return_indent: ::ReturnIndent,
pub newline_style: NewlineStyle,
pub fn_brace_style: BraceStyle,
pub fn_return_indent: ReturnIndent,
pub fn_args_paren_newline: bool,
pub struct_trailing_comma: bool,
pub struct_lit_trailing_comma: ::lists::SeparatorTactic,
pub struct_lit_trailing_comma: SeparatorTactic,
pub enum_trailing_comma: bool,
pub report_todo: ReportTactic,
pub report_fixme: ReportTactic,
}
impl Config {

2
src/default.toml
View File

@ -9,3 +9,5 @@ fn_args_paren_newline = true
struct_trailing_comma = true
struct_lit_trailing_comma = "Vertical"
enum_trailing_comma = true
report_todo = "Always"
report_fixme = "Never"

298
src/issues.rs Normal file
View File

@ -0,0 +1,298 @@
// Copyright 2015 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.
// Objects for seeking through a char stream for occurences of TODO and FIXME.
// Depending on the loaded configuration, may also check that these have an
// associated issue number.
use std::fmt;
static TO_DO_CHARS: &'static [char] = &['T', 'O', 'D', 'O'];
static FIX_ME_CHARS: &'static [char] = &['F', 'I', 'X', 'M', 'E'];
#[derive(Clone, Copy)]
pub enum ReportTactic {
Always,
Unnumbered,
Never
}
impl ReportTactic {
fn is_enabled(&self) -> bool {
match *self {
ReportTactic::Always => true,
ReportTactic::Unnumbered => true,
ReportTactic::Never => false
}
}
}
impl_enum_decodable!(ReportTactic, Always, Unnumbered, Never);
#[derive(Clone, Copy)]
enum Seeking {
Issue {
todo_idx: usize,
fixme_idx: usize
},
Number {
issue: Issue,
part: NumberPart
}
}
#[derive(Clone, Copy)]
enum NumberPart {
OpenParen,
Pound,
Number,
CloseParen
}
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
pub struct Issue {
issue_type: IssueType,
// Indicates whether we're looking for issues with missing numbers, or
// all issues of this type.
missing_number: bool,
}
impl fmt::Display for Issue {
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
let msg = match self.issue_type {
IssueType::Todo => "TODO",
IssueType::Fixme => "FIXME",
};
let details = if self.missing_number { " without issue number" } else { "" };
write!(fmt, "{}{}", msg, details)
}
}
#[derive(PartialEq, Eq, Debug, Clone, Copy)]
enum IssueType {
Todo,
Fixme
}
enum IssueClassification {
Good,
Bad(Issue),
None
}
pub struct BadIssueSeeker {
state: Seeking,
report_todo: ReportTactic,
report_fixme: ReportTactic,
}
impl BadIssueSeeker {
pub fn new(report_todo: ReportTactic, report_fixme: ReportTactic) -> BadIssueSeeker {
BadIssueSeeker {
state: Seeking::Issue { todo_idx: 0, fixme_idx: 0 },
report_todo: report_todo,
report_fixme: report_fixme,
}
}
// Check whether or not the current char is conclusive evidence for an
// unnumbered TO-DO or FIX-ME.
pub fn inspect(&mut self, c: char) -> Option<Issue> {
match self.state {
Seeking::Issue { todo_idx, fixme_idx } => {
self.state = self.inspect_issue(c, todo_idx, fixme_idx);
},
Seeking::Number { issue, part } => {
let result = self.inspect_number(c, issue, part);
if let IssueClassification::None = result {
return None;
}
self.state = Seeking::Issue { todo_idx: 0, fixme_idx: 0 };
if let IssueClassification::Bad(issue) = result {
return Some(issue);
}
}
}
None
}
fn inspect_issue(&mut self, c: char, mut todo_idx: usize, mut fixme_idx: usize) -> Seeking {
// FIXME: Should we also check for lower case characters?
if self.report_todo.is_enabled() && c == TO_DO_CHARS[todo_idx] {
todo_idx += 1;
if todo_idx == TO_DO_CHARS.len() {
return Seeking::Number {
issue: Issue {
issue_type: IssueType::Todo,
missing_number: if let ReportTactic::Unnumbered = self.report_todo {
true
} else {
false
}
},
part: NumberPart::OpenParen
};
}
fixme_idx = 0;
} else if self.report_fixme.is_enabled() && c == FIX_ME_CHARS[fixme_idx] {
// Exploit the fact that the character sets of todo and fixme
// are disjoint by adding else.
fixme_idx += 1;
if fixme_idx == FIX_ME_CHARS.len() {
return Seeking::Number {
issue: Issue {
issue_type: IssueType::Fixme,
missing_number: if let ReportTactic::Unnumbered = self.report_fixme {
true
} else {
false
}
},
part: NumberPart::OpenParen
};
}
todo_idx = 0;
} else {
todo_idx = 0;
fixme_idx = 0;
}
Seeking::Issue { todo_idx: todo_idx, fixme_idx: fixme_idx }
}
fn inspect_number(&mut self,
c: char,
issue: Issue,
mut part: NumberPart)
-> IssueClassification
{
if ! issue.missing_number || c == '\n' {
return IssueClassification::Bad(issue);
} else if c == ')' {
return if let NumberPart::CloseParen = part {
IssueClassification::Good
} else {
IssueClassification::Bad(issue)
};
}
match part {
NumberPart::OpenParen => {
if c != '(' {
return IssueClassification::Bad(issue);
} else {
part = NumberPart::Pound;
}
},
NumberPart::Pound => {
if c == '#' {
part = NumberPart::Number;
}
},
NumberPart::Number => {
if c >= '0' && c <= '9' {
part = NumberPart::CloseParen;
} else {
return IssueClassification::Bad(issue);
}
},
NumberPart::CloseParen => {}
}
self.state = Seeking::Number {
part: part,
issue: issue
};
IssueClassification::None
}
}
#[test]
fn find_unnumbered_issue() {
fn check_fail(text: &str, failing_pos: usize) {
println!("{:?}", text);
let mut seeker = BadIssueSeeker::new(ReportTactic::Unnumbered, ReportTactic::Unnumbered);
assert_eq!(Some(failing_pos), text.chars().position(|c| seeker.inspect(c).is_some()));
}
fn check_pass(text: &str) {
let mut seeker = BadIssueSeeker::new(ReportTactic::Unnumbered, ReportTactic::Unnumbered);
assert_eq!(None, text.chars().position(|c| seeker.inspect(c).is_some()));
}
check_fail("TODO\n", 4);
check_pass(" TO FIX DOME\n");
check_fail(" \n FIXME\n", 8);
check_fail("FIXME(\n", 6);
check_fail("FIXME(#\n", 7);
check_fail("FIXME(#1\n", 8);
check_fail("FIXME(#)1\n", 7);
check_pass("FIXME(#1222)\n");
check_fail("FIXME(#12\n22)\n", 9);
check_pass("FIXME(@maintainer, #1222, hello)\n");
check_fail("TODO(#22) FIXME\n", 15);
}
#[test]
fn find_issue() {
fn is_bad_issue(text: &str, report_todo: ReportTactic, report_fixme: ReportTactic) -> bool {
let mut seeker = BadIssueSeeker::new(report_todo, report_fixme);
text.chars().any(|c| seeker.inspect(c).is_some())
}
assert!(is_bad_issue("TODO(@maintainer, #1222, hello)\n",
ReportTactic::Always,
ReportTactic::Never));
assert!(! is_bad_issue("TODO: no number\n",
ReportTactic::Never,
ReportTactic::Always));
assert!(is_bad_issue("This is a FIXME(#1)\n",
ReportTactic::Never,
ReportTactic::Always));
assert!(! is_bad_issue("bad FIXME\n",
ReportTactic::Always,
ReportTactic::Never));
}
#[test]
fn issue_type() {
let mut seeker = BadIssueSeeker::new(ReportTactic::Always, ReportTactic::Never);
let expected = Some(Issue {
issue_type: IssueType::Todo,
missing_number: false
});
assert_eq!(expected,
"TODO(#100): more awesomeness".chars()
.map(|c| seeker.inspect(c))
.find(Option::is_some)
.unwrap());
let mut seeker = BadIssueSeeker::new(ReportTactic::Never, ReportTactic::Unnumbered);
let expected = Some(Issue {
issue_type: IssueType::Fixme,
missing_number: true
});
assert_eq!(expected,
"Test. FIXME: bad, bad, not good".chars()
.map(|c| seeker.inspect(c))
.find(Option::is_some)
.unwrap());
}

95
src/lib.rs
View File

@ -33,8 +33,6 @@ use rustc::session::config as rustc_config;
use rustc::session::config::Input;
use rustc_driver::{driver, CompilerCalls, Compilation};
use rustc_serialize::{Decodable, Decoder};
use syntax::ast;
use syntax::codemap::CodeMap;
use syntax::diagnostics;
@ -42,7 +40,9 @@ use syntax::visit;
use std::path::PathBuf;
use std::collections::HashMap;
use std::fmt;
use issues::{BadIssueSeeker, Issue};
use changes::ChangeSet;
use visitor::FmtVisitor;
@ -58,6 +58,7 @@ mod lists;
mod types;
mod expr;
mod imports;
mod issues;
const MIN_STRING: usize = 10;
// When we get scoped annotations, we should have rustfmt::skip.
@ -106,6 +107,58 @@ pub enum ReturnIndent {
impl_enum_decodable!(ReturnIndent, WithArgs, WithWhereClause);
enum ErrorKind {
// Line has more than config!(max_width) characters
LineOverflow,
// Line ends in whitespace
TrailingWhitespace,
// TO-DO or FIX-ME item without an issue number
BadIssue(Issue),
}
impl fmt::Display for ErrorKind {
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
match *self {
ErrorKind::LineOverflow => {
write!(fmt, "line exceeded maximum length")
},
ErrorKind::TrailingWhitespace => {
write!(fmt, "left behind trailing whitespace")
},
ErrorKind::BadIssue(issue) => {
write!(fmt, "found {}", issue)
},
}
}
}
// Formatting errors that are identified *after* rustfmt has run
struct FormattingError {
line: u32,
kind: ErrorKind,
}
struct FormatReport {
// Maps stringified file paths to their associated formatting errors
file_error_map: HashMap<String, Vec<FormattingError>>,
}
impl fmt::Display for FormatReport {
// Prints all the formatting errors.
fn fmt(&self, fmt: &mut fmt::Formatter) -> Result<(), fmt::Error> {
for (file, errors) in self.file_error_map.iter() {
for error in errors {
try!(write!(fmt,
"Rustfmt failed at {}:{}: {} (sorry)\n",
file,
error.line,
error.kind));
}
}
Ok(())
}
}
// Formatting which depends on the AST.
fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> {
let mut visitor = FmtVisitor::from_codemap(codemap);
@ -119,11 +172,11 @@ fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> {
}
// Formatting done on a char by char or line by line basis.
// TODO warn on TODOs and FIXMEs without an issue number
// TODO warn on bad license
// TODO other stuff for parity with make tidy
fn fmt_lines(changes: &mut ChangeSet) {
fn fmt_lines(changes: &mut ChangeSet) -> FormatReport {
let mut truncate_todo = Vec::new();
let mut report = FormatReport { file_error_map: HashMap::new() };
// Iterate over the chars in the change set.
for (f, text) in changes.text() {
@ -132,8 +185,21 @@ fn fmt_lines(changes: &mut ChangeSet) {
let mut line_len = 0;
let mut cur_line = 1;
let mut newline_count = 0;
let mut errors = vec![];
let mut issue_seeker = BadIssueSeeker::new(config!(report_todo),
config!(report_fixme));
for (c, b) in text.chars() {
if c == '\r' { continue; }
// Add warnings for bad todos/ fixmes
if let Some(issue) = issue_seeker.inspect(c) {
errors.push(FormattingError {
line: cur_line,
kind: ErrorKind::BadIssue(issue)
});
}
if c == '\n' {
// Check for (and record) trailing whitespace.
if let Some(lw) = last_wspace {
@ -142,9 +208,10 @@ fn fmt_lines(changes: &mut ChangeSet) {
}
// Check for any line width errors we couldn't correct.
if line_len > config!(max_width) {
// TODO store the error rather than reporting immediately.
println!("Rustfmt couldn't fix (sorry). {}:{}: line longer than {} characters",
f, cur_line, config!(max_width));
errors.push(FormattingError {
line: cur_line,
kind: ErrorKind::LineOverflow
});
}
line_len = 0;
cur_line += 1;
@ -165,18 +232,24 @@ fn fmt_lines(changes: &mut ChangeSet) {
if newline_count > 1 {
debug!("track truncate: {} {} {}", f, text.len, newline_count);
truncate_todo.push((f.to_string(), text.len - newline_count + 1))
truncate_todo.push((f.to_owned(), text.len - newline_count + 1))
}
for &(l, _, _) in trims.iter() {
// TODO store the error rather than reporting immediately.
println!("Rustfmt left trailing whitespace at {}:{} (sorry)", f, l);
errors.push(FormattingError {
line: l,
kind: ErrorKind::TrailingWhitespace
});
}
report.file_error_map.insert(f.to_owned(), errors);
}
for (f, l) in truncate_todo {
changes.get_mut(&f).truncate(l);
}
report
}
struct RustFmtCalls {
@ -237,7 +310,7 @@ impl<'a> CompilerCalls<'a> for RustFmtCalls {
// For some reason, the codemap does not include terminating newlines
// so we must add one on for each file. This is sad.
changes.append_newlines();
fmt_lines(&mut changes);
println!("{}", fmt_lines(&mut changes));
let result = changes.write_all_files(write_mode);

1
src/lists.rs
View File

@ -9,7 +9,6 @@
// except according to those terms.
use utils::make_indent;
use rustc_serialize::{Decodable, Decoder};
#[derive(Eq, PartialEq, Debug, Copy, Clone)]
pub enum ListTactic {

37
src/utils.rs
View File

@ -48,12 +48,38 @@ pub fn format_visibility(vis: Visibility) -> &'static str {
}
}
#[inline]
#[cfg(target_pointer_width="64")]
// Based on the trick layed out at
// http://graphics.stanford.edu/~seander/bithacks.html#RoundUpPowerOf2
pub fn round_up_to_power_of_two(mut x: usize) -> usize {
x -= 1;
x |= x >> 1;
x |= x >> 2;
x |= x >> 4;
x |= x >> 8;
x |= x >> 16;
x |= x >> 32;
x + 1
}
#[cfg(target_pointer_width="32")]
pub fn round_up_to_power_of_two(mut x: usize) -> usize {
x -= 1;
x |= x >> 1;
x |= x >> 2;
x |= x >> 4;
x |= x >> 8;
x |= x >> 16;
x + 1
}
// Macro for deriving implementations of Decodable for enums
#[macro_export]
macro_rules! impl_enum_decodable {
( $e:ident, $( $x:ident ),* ) => {
impl Decodable for $e {
fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
impl ::rustc_serialize::Decodable for $e {
fn decode<D: ::rustc_serialize::Decoder>(d: &mut D) -> Result<Self, D::Error> {
let s = try!(d.read_str());
match &*s {
$(
@ -65,3 +91,10 @@ macro_rules! impl_enum_decodable {
}
};
}
#[test]
fn power_rounding() {
assert_eq!(1, round_up_to_power_of_two(1));
assert_eq!(64, round_up_to_power_of_two(33));
assert_eq!(256, round_up_to_power_of_two(256));
}

2
tests/config/small_tabs.toml
View File

@ -9,3 +9,5 @@ fn_args_paren_newline = true
struct_trailing_comma = true
struct_lit_trailing_comma = "Vertical"
enum_trailing_comma = true
report_todo = "Always"
report_fixme = "Never"