Skip to content
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

Add new lint upper_case_acronyms #6475

Merged
merged 3 commits into from
Jan 20, 2021
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 @@ -2274,6 +2274,7 @@ Released 2018-09-13
[`unusual_byte_groupings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unusual_byte_groupings
[`unwrap_in_result`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_in_result
[`unwrap_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
[`upper_case_acronyms`]: https://rust-lang.github.io/rust-clippy/master/index.html#upper_case_acronyms
[`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug
[`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self
[`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding
Expand Down
8 changes: 4 additions & 4 deletions clippy_lints/src/cognitive_complexity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ impl CognitiveComplexity {

let expr = &body.value;

let mut helper = CCHelper { cc: 1, returns: 0 };
let mut helper = CcHelper { cc: 1, returns: 0 };
helper.visit_expr(expr);
let CCHelper { cc, returns } = helper;
let CcHelper { cc, returns } = helper;
let ret_ty = cx.typeck_results().node_type(expr.hir_id);
let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::result_type) {
returns
Expand Down Expand Up @@ -136,12 +136,12 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity {
}
}

struct CCHelper {
struct CcHelper {
cc: u64,
returns: u64,
}

impl<'tcx> Visitor<'tcx> for CCHelper {
impl<'tcx> Visitor<'tcx> for CcHelper {
type Map = Map<'tcx>;

fn visit_expr(&mut self, e: &'tcx Expr<'_>) {
Expand Down
24 changes: 12 additions & 12 deletions clippy_lints/src/int_plus_one.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ declare_lint_pass!(IntPlusOne => [INT_PLUS_ONE]);

#[derive(Copy, Clone)]
enum Side {
LHS,
RHS,
Lhs,
Rhs,
}

impl IntPlusOne {
Expand All @@ -66,11 +66,11 @@ impl IntPlusOne {
match (lhskind.node, &lhslhs.kind, &lhsrhs.kind) {
// `-1 + x`
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS)
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::Lhs)
},
// `x - 1`
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS)
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::Lhs)
},
_ => None,
}
Expand All @@ -82,10 +82,10 @@ impl IntPlusOne {
match (&rhslhs.kind, &rhsrhs.kind) {
// `y + 1` and `1 + y`
(&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS)
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::Rhs)
},
(_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS)
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::Rhs)
},
_ => None,
}
Expand All @@ -97,10 +97,10 @@ impl IntPlusOne {
match (&lhslhs.kind, &lhsrhs.kind) {
// `1 + x` and `x + 1`
(&ExprKind::Lit(ref lit), _) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::LHS)
Self::generate_recommendation(cx, binop, lhsrhs, rhs, Side::Lhs)
},
(_, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::LHS)
Self::generate_recommendation(cx, binop, lhslhs, rhs, Side::Lhs)
},
_ => None,
}
Expand All @@ -110,11 +110,11 @@ impl IntPlusOne {
match (rhskind.node, &rhslhs.kind, &rhsrhs.kind) {
// `-1 + y`
(BinOpKind::Add, &ExprKind::Lit(ref lit), _) if Self::check_lit(lit, -1) => {
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::RHS)
Self::generate_recommendation(cx, binop, rhsrhs, lhs, Side::Rhs)
},
// `y - 1`
(BinOpKind::Sub, _, &ExprKind::Lit(ref lit)) if Self::check_lit(lit, 1) => {
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::RHS)
Self::generate_recommendation(cx, binop, rhslhs, lhs, Side::Rhs)
},
_ => None,
}
Expand All @@ -138,8 +138,8 @@ impl IntPlusOne {
if let Some(snippet) = snippet_opt(cx, node.span) {
if let Some(other_side_snippet) = snippet_opt(cx, other_side.span) {
let rec = match side {
Side::LHS => Some(format!("{} {} {}", snippet, binop_string, other_side_snippet)),
Side::RHS => Some(format!("{} {} {}", other_side_snippet, binop_string, snippet)),
Side::Lhs => Some(format!("{} {} {}", snippet, binop_string, other_side_snippet)),
Side::Rhs => Some(format!("{} {} {}", other_side_snippet, binop_string, snippet)),
};
return rec;
}
Expand Down
7 changes: 6 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ mod unused_self;
mod unused_unit;
mod unwrap;
mod unwrap_in_result;
mod upper_case_acronyms;
mod use_self;
mod useless_conversion;
mod vec;
Expand Down Expand Up @@ -944,6 +945,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&unwrap::PANICKING_UNWRAP,
&unwrap::UNNECESSARY_UNWRAP,
&unwrap_in_result::UNWRAP_IN_RESULT,
&upper_case_acronyms::UPPER_CASE_ACRONYMS,
&use_self::USE_SELF,
&useless_conversion::USELESS_CONVERSION,
&vec::USELESS_VEC,
Expand Down Expand Up @@ -983,7 +985,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
}
store.register_late_pass(|| box utils::author::Author);
store.register_late_pass(|| box await_holding_invalid::AwaitHolding);
store.register_late_pass(|| box serde_api::SerdeAPI);
store.register_late_pass(|| box serde_api::SerdeApi);
let vec_box_size_threshold = conf.vec_box_size_threshold;
store.register_late_pass(move || box types::Types::new(vec_box_size_threshold));
store.register_late_pass(|| box booleans::NonminimalBool);
Expand Down Expand Up @@ -1174,6 +1176,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
store.register_early_pass(|| box upper_case_acronyms::UpperCaseAcronyms);
store.register_late_pass(|| box default::Default::default());
store.register_late_pass(|| box unused_self::UnusedSelf);
store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
Expand Down Expand Up @@ -1659,6 +1662,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&unwrap::PANICKING_UNWRAP),
LintId::of(&unwrap::UNNECESSARY_UNWRAP),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&useless_conversion::USELESS_CONVERSION),
LintId::of(&vec::USELESS_VEC),
LintId::of(&vec_init_then_push::VEC_INIT_THEN_PUSH),
Expand Down Expand Up @@ -1776,6 +1780,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
LintId::of(&unused_unit::UNUSED_UNIT),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&write::PRINTLN_EMPTY_STRING),
LintId::of(&write::PRINT_LITERAL),
LintId::of(&write::PRINT_WITH_NEWLINE),
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/serde_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ declare_clippy_lint! {
"various things that will negatively affect your serde experience"
}

declare_lint_pass!(SerdeAPI => [SERDE_API_MISUSE]);
declare_lint_pass!(SerdeApi => [SERDE_API_MISUSE]);

impl<'tcx> LateLintPass<'tcx> for SerdeAPI {
impl<'tcx> LateLintPass<'tcx> for SerdeApi {
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
if let ItemKind::Impl(Impl {
of_trait: Some(ref trait_ref),
Expand Down
93 changes: 93 additions & 0 deletions clippy_lints/src/upper_case_acronyms.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
use crate::utils::span_lint_and_sugg;
use if_chain::if_chain;
use itertools::Itertools;
use rustc_ast::ast::{Item, ItemKind, Variant};
use rustc_errors::Applicability;
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::symbol::Ident;

declare_clippy_lint! {
/// **What it does:** Checks for camel case name containing a capitalized acronym.
///
/// **Why is this bad?** In CamelCase, acronyms count as one word.
/// See [naming conventions](https://rust-lang.github.io/api-guidelines/naming.html#casing-conforms-to-rfc-430-c-case)
/// for more.
///
/// **Known problems:** When two acronyms are contiguous, the lint can't tell where
/// the first acronym ends and the second starts, so it suggests to lowercase all of
/// the letters in the second acronym.
///
/// **Example:**
///
/// ```rust
/// struct HTTPResponse;
/// ```
/// Use instead:
/// ```rust
/// struct HttpResponse;
/// ```
pub UPPER_CASE_ACRONYMS,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub UPPER_CASE_ACRONYMS,
pub UPPERCASE_ACRONYMS,

Uppercase is usually written as one word. Like in the char::to_uppercase method

Copy link
Member Author

@hkmatsumoto hkmatsumoto Jan 7, 2021

Choose a reason for hiding this comment

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

There's non-upper-case-globals lint in rustc and I suppose it's reasonable to make the name of this lint similar to that. (It seems they wanted to go consistent with non-camel-case-types and non_snake_case when naming the rustc lint)

Copy link
Member

Choose a reason for hiding this comment

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

Well, I find that inconsistent. But lets be consistently inconsistent and keep the name as it is 👍

style,
"capitalized acronyms are against the naming convention"
}

declare_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);

fn correct_ident(ident: &str) -> String {
let ident = ident.chars().rev().collect::<String>();
let fragments = ident
.split_inclusive(|x: char| !x.is_ascii_lowercase())
.rev()
.map(|x| x.chars().rev().collect::<String>());

let mut ident = fragments.clone().next().unwrap();
for (ref prev, ref curr) in fragments.tuple_windows() {
if [prev, curr]
.iter()
.all(|s| s.len() == 1 && s.chars().next().unwrap().is_ascii_uppercase())
{
ident.push_str(&curr.to_ascii_lowercase());
} else {
ident.push_str(curr);
}
}
ident
}

fn check_ident(cx: &EarlyContext<'_>, ident: &Ident) {
let span = ident.span;
let ident = &ident.as_str();
let corrected = correct_ident(ident);
if ident != &corrected {
span_lint_and_sugg(
cx,
UPPER_CASE_ACRONYMS,
span,
&format!("name `{}` contains a capitalized acronym", ident),
"consider making the acronym lowercase, except the initial letter",
corrected,
Applicability::MaybeIncorrect,
)
}
}

impl EarlyLintPass for UpperCaseAcronyms {
fn check_item(&mut self, cx: &EarlyContext<'_>, it: &Item) {
if_chain! {
if !in_external_macro(cx.sess(), it.span);
if matches!(
it.kind,
ItemKind::TyAlias(..) | ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Trait(..)
);
then {
check_ident(cx, &it.ident);
}
}
}

fn check_variant(&mut self, cx: &EarlyContext<'_>, v: &Variant) {
check_ident(cx, &v.ident);
}
}
2 changes: 1 addition & 1 deletion tests/ui/complex_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ struct S {
f: Vec<Vec<Box<(u32, u32, u32, u32)>>>,
}

struct TS(Vec<Vec<Box<(u32, u32, u32, u32)>>>);
struct Ts(Vec<Vec<Box<(u32, u32, u32, u32)>>>);

enum E {
Tuple(Vec<Vec<Box<(u32, u32, u32, u32)>>>),
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/complex_types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ LL | f: Vec<Vec<Box<(u32, u32, u32, u32)>>>,
error: very complex type used. Consider factoring parts into `type` definitions
--> $DIR/complex_types.rs:14:11
|
LL | struct TS(Vec<Vec<Box<(u32, u32, u32, u32)>>>);
LL | struct Ts(Vec<Vec<Box<(u32, u32, u32, u32)>>>);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: very complex type used. Consider factoring parts into `type` definitions
Expand Down
1 change: 1 addition & 0 deletions tests/ui/crashes/ice-6256.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// originally from rustc ./src/test/ui/regions/issue-78262.rs
// ICE: to get the signature of a closure, use substs.as_closure().sig() not fn_sig()
#![allow(clippy::upper_case_acronyms)]

trait TT {}

Expand Down
6 changes: 3 additions & 3 deletions tests/ui/crashes/ice-6256.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error[E0308]: mismatched types
--> $DIR/ice-6256.rs:11:28
--> $DIR/ice-6256.rs:12:28
|
LL | let f = |x: &dyn TT| x.func(); //[default]~ ERROR: mismatched types
| ^^^^ lifetime mismatch
|
= note: expected reference `&(dyn TT + 'static)`
found reference `&dyn TT`
note: the anonymous lifetime #1 defined on the body at 11:13...
--> $DIR/ice-6256.rs:11:13
note: the anonymous lifetime #1 defined on the body at 12:13...
--> $DIR/ice-6256.rs:12:13
|
LL | let f = |x: &dyn TT| x.func(); //[default]~ ERROR: mismatched types
| ^^^^^^^^^^^^^^^^^^^^^
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/enum_variants.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![feature(non_ascii_idents)]
#![warn(clippy::enum_variant_names, clippy::pub_enum_variant_names)]
#![allow(non_camel_case_types)]
#![allow(non_camel_case_types, clippy::upper_case_acronyms)]

enum FakeCallType {
CALL,
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/needless_question_mark.fixed
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
// run-rustfix

#![warn(clippy::needless_question_mark)]
#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
#![allow(
clippy::needless_return,
clippy::unnecessary_unwrap,
clippy::upper_case_acronyms,
dead_code,
unused_must_use
)]
#![feature(custom_inner_attributes)]

struct TO {
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/needless_question_mark.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
// run-rustfix

#![warn(clippy::needless_question_mark)]
#![allow(clippy::needless_return, clippy::unnecessary_unwrap, dead_code, unused_must_use)]
#![allow(
clippy::needless_return,
clippy::unnecessary_unwrap,
clippy::upper_case_acronyms,
dead_code,
unused_must_use
)]
#![feature(custom_inner_attributes)]

struct TO {
Expand Down
Loading