Skip to content

Stuttter lint #994

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 8 commits into from
Jun 16, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ All notable changes to this project will be documented in this file.
[`string_add_assign`]: https://github.com/Manishearth/rust-clippy/wiki#string_add_assign
[`string_lit_as_bytes`]: https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes
[`string_to_string`]: https://github.com/Manishearth/rust-clippy/wiki#string_to_string
[`stutter`]: https://github.com/Manishearth/rust-clippy/wiki#stutter
[`suspicious_assignment_formatting`]: https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting
[`suspicious_else_formatting`]: https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting
[`temporary_assignment`]: https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Table of contents:

## Lints

There are 153 lints included in this crate:
There are 154 lints included in this crate:

name | default | meaning
---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -144,6 +144,7 @@ name
[string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead
[string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead
[string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead
[stutter](https://github.com/Manishearth/rust-clippy/wiki#stutter) | allow | finds type names prefixed/postfixed with their containing module's name
[suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn | suspicious formatting of `*=`, `-=` or `!=`
[suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting) | warn | suspicious formatting of `else if`
[temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/approx_const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,15 @@ const KNOWN_CONSTS: &'static [(f64, &'static str, usize)] = &[(f64::E, "E", 4),
(f64::SQRT_2, "SQRT_2", 5)];

#[derive(Copy,Clone)]
pub struct ApproxConstant;
pub struct Pass;

impl LintPass for ApproxConstant {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(APPROX_CONSTANT)
}
}

impl LateLintPass for ApproxConstant {
impl LateLintPass for Pass {
fn check_expr(&mut self, cx: &LateContext, e: &Expr) {
if let ExprLit(ref lit) = e.node {
check_lit(cx, lit, e);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub fn check_attrs<'a>(cx: &EarlyContext, valid_idents: &[String], attrs: &'a [a
}

#[allow(while_let_loop)] // #362
pub fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) -> Result<(), ()> {
fn check_doc(cx: &EarlyContext, valid_idents: &[String], docs: &[(&str, Span)]) -> Result<(), ()> {
// In markdown, `_` can be used to emphasize something, or, is a raw `_` depending on context.
// There really is no markdown specification that would disambiguate this properly. This is
// what GitHub and Rustdoc do:
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/drop_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ declare_lint! {
}

#[allow(missing_copy_implementations)]
pub struct DropRefPass;
pub struct Pass;

impl LintPass for DropRefPass {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(DROP_REF)
}
}

impl LateLintPass for DropRefPass {
impl LateLintPass for Pass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let ExprCall(ref path, ref args) = expr.node {
if let ExprPath(None, _) = path.node {
Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/enum_clike.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,15 @@ declare_lint! {
"finds C-like enums that are `repr(isize/usize)` and have values that don't fit into an `i32`"
}

pub struct EnumClikeUnportableVariant;
pub struct UnportableVariant;

impl LintPass for EnumClikeUnportableVariant {
impl LintPass for UnportableVariant {
fn get_lints(&self) -> LintArray {
lint_array!(ENUM_CLIKE_UNPORTABLE_VARIANT)
}
}

impl LateLintPass for EnumClikeUnportableVariant {
impl LateLintPass for UnportableVariant {
#[allow(cast_possible_truncation, cast_sign_loss)]
fn check_item(&mut self, cx: &LateContext, item: &Item) {
if let ItemEnum(ref def, _) = item.node {
Expand Down
179 changes: 123 additions & 56 deletions clippy_lints/src/enum_variants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

use rustc::lint::*;
use syntax::ast::*;
use syntax::codemap::Span;
use syntax::parse::token::InternedString;
use utils::span_help_and_lint;
use utils::{camel_case_from, camel_case_until};
use utils::{span_help_and_lint, span_lint};
use utils::{camel_case_from, camel_case_until, in_macro};

/// **What it does:** Warns on enum variants that are prefixed or suffixed by the same characters
///
Expand All @@ -18,87 +19,153 @@ declare_lint! {
"finds enums where all variants share a prefix/postfix"
}

pub struct EnumVariantNames;
/// **What it does:** Warns on type names that are prefixed or suffixed by the containing module's name
///
/// **Why is this bad?** It requires the user to type the module name twice
///
/// **Known problems:** None
///
/// **Example:** mod cake { struct BlackForestCake; }
declare_lint! {
pub STUTTER, Allow,
"finds type names prefixed/postfixed with their containing module's name"
}

#[derive(Default)]
pub struct EnumVariantNames {
modules: Vec<String>,
}

impl LintPass for EnumVariantNames {
fn get_lints(&self) -> LintArray {
lint_array!(ENUM_VARIANT_NAMES)
lint_array!(ENUM_VARIANT_NAMES, STUTTER)
}
}

fn var2str(var: &Variant) -> InternedString {
var.node.name.name.as_str()
}

// FIXME: waiting for https://github.com/rust-lang/rust/pull/31700
// fn partial_match(pre: &str, name: &str) -> usize {
// // skip(1) to ensure that the prefix never takes the whole variant name
// pre.chars().zip(name.chars().rev().skip(1).rev()).take_while(|&(l, r)| l == r).count()
// }
//
// fn partial_rmatch(post: &str, name: &str) -> usize {
// // skip(1) to ensure that the postfix never takes the whole variant name
// post.chars().rev().zip(name.chars().skip(1).rev()).take_while(|&(l, r)| l == r).count()
// }

/// Returns the number of chars that match from the start
fn partial_match(pre: &str, name: &str) -> usize {
let mut name_iter = name.chars();
let _ = name_iter.next_back(); // make sure the name is never fully matched
pre.chars().zip(name_iter).take_while(|&(l, r)| l == r).count()
}

/// Returns the number of chars that match from the end
fn partial_rmatch(post: &str, name: &str) -> usize {
let mut name_iter = name.chars();
let _ = name_iter.next(); // make sure the name is never fully matched
post.chars().rev().zip(name_iter.rev()).take_while(|&(l, r)| l == r).count()
}

impl EarlyLintPass for EnumVariantNames {
// FIXME: #600
#[allow(while_let_on_iterator)]
fn check_item(&mut self, cx: &EarlyContext, item: &Item) {
if let ItemKind::Enum(ref def, _) = item.node {
if def.variants.len() < 2 {
return;
// FIXME: #600
#[allow(while_let_on_iterator)]
fn check_variant(cx: &EarlyContext, def: &EnumDef, item_name: &str, item_name_chars: usize, span: Span) {
for var in &def.variants {
let name = var2str(var);
if partial_match(item_name, &name) == item_name_chars {
span_lint(cx, ENUM_VARIANT_NAMES, var.span, "Variant name starts with the enum's name");
}
if partial_rmatch(item_name, &name) == item_name_chars {
span_lint(cx, ENUM_VARIANT_NAMES, var.span, "Variant name ends with the enum's name");
}
}
if def.variants.len() < 2 {
return;
}
let first = var2str(&def.variants[0]);
let mut pre = &first[..camel_case_until(&*first)];
let mut post = &first[camel_case_from(&*first)..];
for var in &def.variants {
let name = var2str(var);

let pre_match = partial_match(pre, &name);
pre = &pre[..pre_match];
let pre_camel = camel_case_until(pre);
pre = &pre[..pre_camel];
while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() {
if next.is_lowercase() {
let last = pre.len() - last.len_utf8();
let last_camel = camel_case_until(&pre[..last]);
pre = &pre[..last_camel];
} else {
break;
}
let first = var2str(&def.variants[0]);
let mut pre = &first[..camel_case_until(&*first)];
let mut post = &first[camel_case_from(&*first)..];
for var in &def.variants {
let name = var2str(var);
}

let pre_match = partial_match(pre, &name);
pre = &pre[..pre_match];
let pre_camel = camel_case_until(pre);
pre = &pre[..pre_camel];
while let Some((next, last)) = name[pre.len()..].chars().zip(pre.chars().rev()).next() {
if next.is_lowercase() {
let last = pre.len() - last.len_utf8();
let last_camel = camel_case_until(&pre[..last]);
pre = &pre[..last_camel];
} else {
break;
let post_match = partial_rmatch(post, &name);
let post_end = post.len() - post_match;
post = &post[post_end..];
let post_camel = camel_case_from(post);
post = &post[post_camel..];
}
let (what, value) = match (pre.is_empty(), post.is_empty()) {
(true, true) => return,
(false, _) => ("pre", pre),
(true, false) => ("post", post),
};
span_help_and_lint(cx,
ENUM_VARIANT_NAMES,
span,
&format!("All variants have the same {}fix: `{}`", what, value),
&format!("remove the {}fixes and use full paths to \
the variants instead of glob imports",
what));
}

fn to_camel_case(item_name: &str) -> String {
let mut s = String::new();
let mut up = true;
for c in item_name.chars() {
if c.is_uppercase() {
// we only turn snake case text into CamelCase
return item_name.to_string();
}
if c == '_' {
up = true;
continue;
}
if up {
up = false;
s.extend(c.to_uppercase());
} else {
s.push(c);
}
}
s
}

impl EarlyLintPass for EnumVariantNames {
fn check_item_post(&mut self, _cx: &EarlyContext, _item: &Item) {
let last = self.modules.pop();
assert!(last.is_some());
}

fn check_item(&mut self, cx: &EarlyContext, item: &Item) {
let item_name = item.ident.name.as_str();
let item_name_chars = item_name.chars().count();
let item_camel = to_camel_case(&item_name);
if item.vis == Visibility::Public && !in_macro(cx, item.span) {
if let Some(mod_camel) = self.modules.last() {
// constants don't have surrounding modules
if !mod_camel.is_empty() {
let matching = partial_match(mod_camel, &item_camel);
let rmatching = partial_rmatch(mod_camel, &item_camel);
let nchars = mod_camel.chars().count();
if matching == nchars {
span_lint(cx, STUTTER, item.span, &format!("Item name ({}) starts with its containing module's name ({})", item_camel, mod_camel));
}
if rmatching == nchars {
span_lint(cx, STUTTER, item.span, &format!("Item name ({}) ends with its containing module's name ({})", item_camel, mod_camel));
}
}

let post_match = partial_rmatch(post, &name);
let post_end = post.len() - post_match;
post = &post[post_end..];
let post_camel = camel_case_from(post);
post = &post[post_camel..];
}
let (what, value) = match (pre.is_empty(), post.is_empty()) {
(true, true) => return,
(false, _) => ("pre", pre),
(true, false) => ("post", post),
};
span_help_and_lint(cx,
ENUM_VARIANT_NAMES,
item.span,
&format!("All variants have the same {}fix: `{}`", what, value),
&format!("remove the {}fixes and use full paths to \
the variants instead of glob imports",
what));
}
if let ItemKind::Enum(ref def, _) = item.node {
check_variant(cx, def, &item_name, item_name_chars, item.span);
}
self.modules.push(item_camel);
}
}
6 changes: 3 additions & 3 deletions clippy_lints/src/escape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use syntax::ast::NodeId;
use syntax::codemap::Span;
use utils::span_lint;

pub struct EscapePass;
pub struct Pass;

/// **What it does:** This lint checks for usage of `Box<T>` where an unboxed `T` would work fine.
///
Expand Down Expand Up @@ -44,13 +44,13 @@ struct EscapeDelegate<'a, 'tcx: 'a> {
set: NodeSet,
}

impl LintPass for EscapePass {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(BOXED_LOCAL)
}
}

impl LateLintPass for EscapePass {
impl LateLintPass for Pass {
fn check_fn(&mut self, cx: &LateContext, _: visit::FnKind, decl: &FnDecl, body: &Block, _: Span, id: NodeId) {
let param_env = ty::ParameterEnvironment::for_item(cx.tcx, id);

Expand Down
6 changes: 3 additions & 3 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ declare_lint! {
}

#[derive(Copy, Clone, Debug)]
pub struct FormatMacLint;
pub struct Pass;

impl LintPass for FormatMacLint {
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array![USELESS_FORMAT]
}
}

impl LateLintPass for FormatMacLint {
impl LateLintPass for Pass {
fn check_expr(&mut self, cx: &LateContext, expr: &Expr) {
if let Some(span) = is_expn_of(cx, expr.span, "format") {
match expr.node {
Expand Down
Loading