Merge pull request #698 from mcarton/conf

Add a configuration file and a POC `BLACKLISTED_NAME` lint
This commit is contained in:
Manish Goregaokar 2016-03-13 19:33:03 +05:30
commit eed9baa4fb
25 changed files with 590 additions and 56 deletions

46
src/blacklisted_name.rs Normal file
View file

@ -0,0 +1,46 @@
use rustc::lint::*;
use rustc_front::hir::*;
use utils::span_lint;
/// **What it does:** This lints about usage of blacklisted names.
///
/// **Why is this bad?** These names are usually placeholder names and should be avoided.
///
/// **Known problems:** None.
///
/// **Example:** `let foo = 3.14;`
declare_lint! {
pub BLACKLISTED_NAME,
Warn,
"usage of a blacklisted/placeholder name"
}
#[derive(Clone, Debug)]
pub struct BlackListedName {
blacklist: Vec<String>,
}
impl BlackListedName {
pub fn new(blacklist: Vec<String>) -> BlackListedName {
BlackListedName { blacklist: blacklist }
}
}
impl LintPass for BlackListedName {
fn get_lints(&self) -> LintArray {
lint_array!(BLACKLISTED_NAME)
}
}
impl LateLintPass for BlackListedName {
fn check_pat(&mut self, cx: &LateContext, pat: &Pat) {
if let PatKind::Ident(_, ref ident, _) = pat.node {
if self.blacklist.iter().any(|s| s == &*ident.node.name.as_str()) {
span_lint(cx,
BLACKLISTED_NAME,
pat.span,
&format!("use of a blacklisted/placeholder name `{}`", ident.node.name));
}
}
}
}

75
src/functions.rs Normal file
View file

@ -0,0 +1,75 @@
use rustc::lint::*;
use rustc_front::hir;
use rustc_front::intravisit;
use syntax::ast;
use syntax::codemap::Span;
use utils::span_lint;
/// **What it does:** Check for functions with too many parameters.
///
/// **Why is this bad?** Functions with lots of parameters are considered bad style and reduce
/// readability (“what does the 5th parameter means?”). Consider grouping some parameters into a
/// new type.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```
/// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) { .. }
/// ```
declare_lint! {
pub TOO_MANY_ARGUMENTS,
Warn,
"functions with too many arguments"
}
#[derive(Copy,Clone)]
pub struct Functions {
threshold: u64,
}
impl Functions {
pub fn new(threshold: u64) -> Functions {
Functions {
threshold: threshold
}
}
}
impl LintPass for Functions {
fn get_lints(&self) -> LintArray {
lint_array!(TOO_MANY_ARGUMENTS)
}
}
impl LateLintPass for Functions {
fn check_fn(&mut self, cx: &LateContext, _: intravisit::FnKind, decl: &hir::FnDecl, _: &hir::Block, span: Span, nodeid: ast::NodeId) {
use rustc::front::map::Node::*;
if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent_node(nodeid)) {
match item.node {
hir::ItemImpl(_, _, _, Some(_), _, _) | hir::ItemDefaultImpl(..) => return,
_ => (),
}
}
self.check_arg_number(cx, decl, span);
}
fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) {
if let hir::MethodTraitItem(ref sig, _) = item.node {
self.check_arg_number(cx, &sig.decl, item.span);
}
}
}
impl Functions {
fn check_arg_number(&self, cx: &LateContext, decl: &hir::FnDecl, span: Span) {
let args = decl.inputs.len() as u64;
if args > self.threshold {
span_lint(cx, TOO_MANY_ARGUMENTS, span,
&format!("this function has to many arguments ({}/{})", args, self.threshold));
}
}
}

View file

@ -1,3 +1,4 @@
#![feature(type_macros)]
#![feature(plugin_registrar, box_syntax)]
#![feature(rustc_private, collections)]
#![feature(iter_arith)]
@ -18,6 +19,8 @@ extern crate rustc;
#[macro_use]
extern crate rustc_front;
extern crate toml;
// Only for the compile time checking of paths
extern crate core;
extern crate collections;
@ -44,6 +47,7 @@ pub mod approx_const;
pub mod array_indexing;
pub mod attrs;
pub mod bit_mask;
pub mod blacklisted_name;
pub mod block_in_if_condition;
pub mod collapsible_if;
pub mod copies;
@ -59,6 +63,7 @@ pub mod escape;
pub mod eta_reduction;
pub mod format;
pub mod formatting;
pub mod functions;
pub mod identity_op;
pub mod if_not_else;
pub mod items_after_statements;
@ -107,6 +112,33 @@ mod reexport {
#[plugin_registrar]
#[cfg_attr(rustfmt, rustfmt_skip)]
pub fn plugin_registrar(reg: &mut Registry) {
let conf = match utils::conf::conf_file(reg.args()) {
Ok(file_name) => {
// if the user specified a file, it must exist, otherwise default to `clippy.toml` but
// do not require the file to exist
let (ref file_name, must_exist) = if let Some(ref file_name) = file_name {
(&**file_name, true)
} else {
("clippy.toml", false)
};
let (conf, errors) = utils::conf::read_conf(&file_name, must_exist);
// all conf errors are non-fatal, we just use the default conf in case of error
for error in errors {
reg.sess.struct_err(&format!("error reading Clippy's configuration file: {}", error)).emit();
}
conf
}
Err((err, span)) => {
reg.sess.struct_span_err(span, err)
.span_note(span, "Clippy will use defaulf configuration")
.emit();
utils::conf::Conf::default()
}
};
reg.register_late_lint_pass(box types::TypePass);
reg.register_late_lint_pass(box misc::TopLevelRefPass);
reg.register_late_lint_pass(box misc::CmpNan);
@ -144,7 +176,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box entry::HashMapLint);
reg.register_late_lint_pass(box ranges::StepByZero);
reg.register_late_lint_pass(box types::CastPass);
reg.register_late_lint_pass(box types::TypeComplexityPass);
reg.register_late_lint_pass(box types::TypeComplexityPass::new(conf.type_complexity_threshold));
reg.register_late_lint_pass(box matches::MatchPass);
reg.register_late_lint_pass(box misc::PatternPass);
reg.register_late_lint_pass(box minmax::MinMaxPass);
@ -157,7 +189,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box map_clone::MapClonePass);
reg.register_late_lint_pass(box temporary_assignment::TemporaryAssignmentPass);
reg.register_late_lint_pass(box transmute::UselessTransmute);
reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(25));
reg.register_late_lint_pass(box cyclomatic_complexity::CyclomaticComplexity::new(conf.cyclomatic_complexity_threshold));
reg.register_late_lint_pass(box escape::EscapePass);
reg.register_early_lint_pass(box misc_early::MiscEarly);
reg.register_late_lint_pass(box misc::UsedUnderscoreBinding);
@ -179,6 +211,8 @@ pub fn plugin_registrar(reg: &mut Registry) {
reg.register_late_lint_pass(box overflow_check_conditional::OverflowCheckConditional);
reg.register_late_lint_pass(box unused_label::UnusedLabel);
reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);
reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names));
reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
reg.register_lint_group("clippy_pedantic", vec![
array_indexing::INDEXING_SLICING,
@ -211,6 +245,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
attrs::INLINE_ALWAYS,
bit_mask::BAD_BIT_MASK,
bit_mask::INEFFECTIVE_BIT_MASK,
blacklisted_name::BLACKLISTED_NAME,
block_in_if_condition::BLOCK_IN_IF_CONDITION_EXPR,
block_in_if_condition::BLOCK_IN_IF_CONDITION_STMT,
collapsible_if::COLLAPSIBLE_IF,
@ -230,6 +265,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
format::USELESS_FORMAT,
formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
formatting::SUSPICIOUS_ELSE_FORMATTING,
functions::TOO_MANY_ARGUMENTS,
identity_op::IDENTITY_OP,
if_not_else::IF_NOT_ELSE,
items_after_statements::ITEMS_AFTER_STATEMENTS,

View file

@ -417,7 +417,15 @@ declare_lint! {
}
#[allow(missing_copy_implementations)]
pub struct TypeComplexityPass;
pub struct TypeComplexityPass {
threshold: u64,
}
impl TypeComplexityPass {
pub fn new(threshold: u64) -> Self {
TypeComplexityPass { threshold: threshold }
}
}
impl LintPass for TypeComplexityPass {
fn get_lints(&self) -> LintArray {
@ -427,18 +435,18 @@ impl LintPass for TypeComplexityPass {
impl LateLintPass for TypeComplexityPass {
fn check_fn(&mut self, cx: &LateContext, _: FnKind, decl: &FnDecl, _: &Block, _: Span, _: NodeId) {
check_fndecl(cx, decl);
self.check_fndecl(cx, decl);
}
fn check_struct_field(&mut self, cx: &LateContext, field: &StructField) {
// enum variants are also struct fields now
check_type(cx, &field.ty);
self.check_type(cx, &field.ty);
}
fn check_item(&mut self, cx: &LateContext, item: &Item) {
match item.node {
ItemStatic(ref ty, _, _) |
ItemConst(ref ty, _) => check_type(cx, ty),
ItemConst(ref ty, _) => self.check_type(cx, ty),
// functions, enums, structs, impls and traits are covered
_ => (),
}
@ -447,8 +455,8 @@ impl LateLintPass for TypeComplexityPass {
fn check_trait_item(&mut self, cx: &LateContext, item: &TraitItem) {
match item.node {
ConstTraitItem(ref ty, _) |
TypeTraitItem(_, Some(ref ty)) => check_type(cx, ty),
MethodTraitItem(MethodSig { ref decl, .. }, None) => check_fndecl(cx, decl),
TypeTraitItem(_, Some(ref ty)) => self.check_type(cx, ty),
MethodTraitItem(MethodSig { ref decl, .. }, None) => self.check_fndecl(cx, decl),
// methods with default impl are covered by check_fn
_ => (),
}
@ -457,7 +465,7 @@ impl LateLintPass for TypeComplexityPass {
fn check_impl_item(&mut self, cx: &LateContext, item: &ImplItem) {
match item.node {
ImplItemKind::Const(ref ty, _) |
ImplItemKind::Type(ref ty) => check_type(cx, ty),
ImplItemKind::Type(ref ty) => self.check_type(cx, ty),
// methods are covered by check_fn
_ => (),
}
@ -465,47 +473,49 @@ impl LateLintPass for TypeComplexityPass {
fn check_local(&mut self, cx: &LateContext, local: &Local) {
if let Some(ref ty) = local.ty {
check_type(cx, ty);
self.check_type(cx, ty);
}
}
}
fn check_fndecl(cx: &LateContext, decl: &FnDecl) {
for arg in &decl.inputs {
check_type(cx, &arg.ty);
impl TypeComplexityPass {
fn check_fndecl(&self, cx: &LateContext, decl: &FnDecl) {
for arg in &decl.inputs {
self.check_type(cx, &arg.ty);
}
if let Return(ref ty) = decl.output {
self.check_type(cx, ty);
}
}
if let Return(ref ty) = decl.output {
check_type(cx, ty);
}
}
fn check_type(cx: &LateContext, ty: &Ty) {
if in_macro(cx, ty.span) {
return;
}
let score = {
let mut visitor = TypeComplexityVisitor {
score: 0,
nest: 1,
fn check_type(&self, cx: &LateContext, ty: &Ty) {
if in_macro(cx, ty.span) {
return;
}
let score = {
let mut visitor = TypeComplexityVisitor {
score: 0,
nest: 1,
};
visitor.visit_ty(ty);
visitor.score
};
visitor.visit_ty(ty);
visitor.score
};
if score > 250 {
span_lint(cx,
TYPE_COMPLEXITY,
ty.span,
"very complex type used. Consider factoring parts into `type` definitions");
if score > self.threshold {
span_lint(cx,
TYPE_COMPLEXITY,
ty.span,
"very complex type used. Consider factoring parts into `type` definitions");
}
}
}
/// Walks a type and assigns a complexity score to it.
struct TypeComplexityVisitor {
/// total complexity score of the type
score: u32,
score: u64,
/// current nesting level
nest: u32,
nest: u64,
}
impl<'v> Visitor<'v> for TypeComplexityVisitor {

200
src/utils/conf.rs Normal file
View file

@ -0,0 +1,200 @@
use std::{fmt, fs, io};
use std::io::Read;
use syntax::{ast, codemap, ptr};
use syntax::parse::token;
use toml;
/// Get the configuration file from arguments.
pub fn conf_file(args: &[ptr::P<ast::MetaItem>]) -> Result<Option<token::InternedString>, (&'static str, codemap::Span)> {
for arg in args {
match arg.node {
ast::MetaItemKind::Word(ref name) | ast::MetaItemKind::List(ref name, _) => {
if name == &"conf_file" {
return Err(("`conf_file` must be a named value", arg.span));
}
}
ast::MetaItemKind::NameValue(ref name, ref value) => {
if name == &"conf_file" {
return if let ast::LitKind::Str(ref file, _) = value.node {
Ok(Some(file.clone()))
} else {
Err(("`conf_file` value must be a string", value.span))
};
}
}
}
}
Ok(None)
}
/// Error from reading a configuration file.
#[derive(Debug)]
pub enum ConfError {
IoError(io::Error),
TomlError(Vec<toml::ParserError>),
TypeError(&'static str, &'static str, &'static str),
UnknownKey(String),
}
impl fmt::Display for ConfError {
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
match *self {
ConfError::IoError(ref err) => err.fmt(f),
ConfError::TomlError(ref errs) => {
let mut first = true;
for err in errs {
if !first {
try!(", ".fmt(f));
first = false;
}
try!(err.fmt(f));
}
Ok(())
}
ConfError::TypeError(ref key, ref expected, ref got) => {
write!(f, "`{}` is expected to be a `{}` but is a `{}`", key, expected, got)
}
ConfError::UnknownKey(ref key) => write!(f, "unknown key `{}`", key),
}
}
}
impl From<io::Error> for ConfError {
fn from(e: io::Error) -> Self {
ConfError::IoError(e)
}
}
macro_rules! define_Conf {
($(#[$doc: meta] ($toml_name: tt, $rust_name: ident, $default: expr => $($ty: tt)+),)+) => {
/// Type used to store lint configuration.
pub struct Conf {
$(#[$doc] pub $rust_name: define_Conf!(TY $($ty)+),)+
}
impl Default for Conf {
fn default() -> Conf {
Conf {
$($rust_name: define_Conf!(DEFAULT $($ty)+, $default),)+
}
}
}
impl Conf {
/// Set the property `name` (which must be the `toml` name) to the given value
#[allow(cast_sign_loss)]
fn set(&mut self, name: String, value: toml::Value) -> Result<(), ConfError> {
match name.as_str() {
$(
define_Conf!(PAT $toml_name) => {
if let Some(value) = define_Conf!(CONV $($ty)+, value) {
self.$rust_name = value;
}
else {
return Err(ConfError::TypeError(define_Conf!(EXPR $toml_name),
stringify!($($ty)+),
value.type_str()));
}
},
)+
"third-party" => {
// for external tools such as clippy-service
return Ok(());
}
_ => {
return Err(ConfError::UnknownKey(name));
}
}
Ok(())
}
}
};
// hack to convert tts
(PAT $pat: pat) => { $pat };
(EXPR $e: expr) => { $e };
(TY $ty: ty) => { $ty };
// how to read the value?
(CONV i64, $value: expr) => { $value.as_integer() };
(CONV u64, $value: expr) => { $value.as_integer().iter().filter_map(|&i| if i >= 0 { Some(i as u64) } else { None }).next() };
(CONV String, $value: expr) => { $value.as_str().map(Into::into) };
(CONV Vec<String>, $value: expr) => {{
let slice = $value.as_slice();
if let Some(slice) = slice {
if slice.iter().any(|v| v.as_str().is_none()) {
None
}
else {
Some(slice.iter().map(|v| v.as_str().unwrap_or_else(|| unreachable!()).to_owned()).collect())
}
}
else {
None
}
}};
// provide a nicer syntax to declare the default value of `Vec<String>` variables
(DEFAULT Vec<String>, $e: expr) => { $e.iter().map(|&e| e.to_owned()).collect() };
(DEFAULT $ty: ty, $e: expr) => { $e };
}
define_Conf! {
/// Lint: BLACKLISTED_NAME. The list of blacklisted names to lint about
("blacklisted-names", blacklisted_names, ["foo", "bar", "baz"] => Vec<String>),
/// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have
("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64),
/// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have
("too-many-arguments-threshold", too_many_arguments_threshold, 7 => u64),
/// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have
("type-complexity-threshold", type_complexity_threshold, 250 => u64),
}
/// Read the `toml` configuration file. The function will ignore “File not found” errors iif
/// `!must_exist`, in which case, it will return the default configuration.
/// In case of error, the function tries to continue as much as possible.
pub fn read_conf(path: &str, must_exist: bool) -> (Conf, Vec<ConfError>) {
let mut conf = Conf::default();
let mut errors = Vec::new();
let file = match fs::File::open(path) {
Ok(mut file) => {
let mut buf = String::new();
if let Err(err) = file.read_to_string(&mut buf) {
errors.push(err.into());
return (conf, errors);
}
buf
}
Err(ref err) if !must_exist && err.kind() == io::ErrorKind::NotFound => {
return (conf, errors);
}
Err(err) => {
errors.push(err.into());
return (conf, errors);
}
};
let mut parser = toml::Parser::new(&file);
let toml = if let Some(toml) = parser.parse() {
toml
} else {
errors.push(ConfError::TomlError(parser.errors));
return (conf, errors);
};
for (key, value) in toml {
if let Err(err) = conf.set(key, value) {
errors.push(err);
}
}
(conf, errors)
}

View file

@ -14,6 +14,7 @@ use syntax::codemap::{ExpnInfo, Span, ExpnFormat};
use syntax::errors::DiagnosticBuilder;
use syntax::ptr::P;
pub mod conf;
mod hir;
pub use self::hir::{SpanlessEq, SpanlessHash};
pub type MethodArgs = HirVec<P<Expr>>;