Skip to content

Commit

Permalink
upper_case_acronyms: add optional aggressive mode and relax default
Browse files Browse the repository at this point in the history
Moves the lint back from pedantic to style group.
The lint default now only warns on names that are completely capitalized, like "WORD"
and only if the name is longer than 2 chars (so that names where each of the letter represents a word are still distinguishable).
For example: FP (false positive) would still be "valid" and not warned about (but EOF would warn).

A "upper_case_acronyms_aggressive: true/false" config option was added that restores the original lint behaviour to warn
on any kind of camel case name that had more than one capital letter following another capital letter.
  • Loading branch information
matthiaskrgr committed Feb 24, 2021
1 parent 4545150 commit 59750dc
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 10 deletions.
5 changes: 3 additions & 2 deletions clippy_lints/src/lib.rs
Expand Up @@ -1216,7 +1216,8 @@ 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);
let upper_case_acronyms_aggressive = conf.upper_case_acronyms_aggressive;
store.register_early_pass(move || box upper_case_acronyms::UpperCaseAcronyms::new(upper_case_acronyms_aggressive));
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 @@ -1416,7 +1417,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&unnecessary_wraps::UNNECESSARY_WRAPS),
LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(&unused_self::UNUSED_SELF),
LintId::of(&upper_case_acronyms::UPPER_CASE_ACRONYMS),
LintId::of(&wildcard_imports::ENUM_GLOB_USE),
LintId::of(&wildcard_imports::WILDCARD_IMPORTS),
LintId::of(&zero_sized_map_values::ZERO_SIZED_MAP_VALUES),
Expand Down Expand Up @@ -1835,6 +1835,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
40 changes: 32 additions & 8 deletions clippy_lints/src/upper_case_acronyms.rs
Expand Up @@ -5,16 +5,20 @@ 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_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::Ident;

declare_clippy_lint! {
/// **What it does:** Checks for camel case name containing a capitalized acronym.
/// **What it does:** Checks for fully capitalized names and optionally names 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.
///
/// By default, the lint only triggers on fully-capitalized names.
/// You can use the `upper_case_acronyms_aggressive: true` config option to enable linting
/// on all camel case names
///
/// **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.
Expand All @@ -29,11 +33,24 @@ declare_clippy_lint! {
/// struct HttpResponse;
/// ```
pub UPPER_CASE_ACRONYMS,
pedantic,
style,
"capitalized acronyms are against the naming convention"
}

declare_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);
#[derive(Default)]
pub struct UpperCaseAcronyms {
upper_case_acronyms_aggressive: bool,
}

impl UpperCaseAcronyms {
pub fn new(aggressive: bool) -> Self {
Self {
upper_case_acronyms_aggressive: aggressive,
}
}
}

impl_lint_pass!(UpperCaseAcronyms => [UPPER_CASE_ACRONYMS]);

fn correct_ident(ident: &str) -> String {
let ident = ident.chars().rev().collect::<String>();
Expand All @@ -56,11 +73,18 @@ fn correct_ident(ident: &str) -> String {
ident
}

fn check_ident(cx: &EarlyContext<'_>, ident: &Ident) {
fn check_ident(cx: &EarlyContext<'_>, ident: &Ident, be_aggressive: bool) {
let span = ident.span;
let ident = &ident.as_str();
let corrected = correct_ident(ident);
if ident != &corrected {
// warn if we have pure-uppercase idents
// assume that two-letter words are some kind of valid abbreviation like FP for false positive
// (and don't warn)
if (ident.chars().all(|c| c.is_ascii_uppercase()) && ident.len() > 2)
// otherwise, warn if we have SOmeTHING lIKE THIs but only warn with the aggressive
// upper_case_acronyms_aggressive config option enabled
|| (be_aggressive && ident != &corrected)
{
span_lint_and_sugg(
cx,
UPPER_CASE_ACRONYMS,
Expand All @@ -82,12 +106,12 @@ impl EarlyLintPass for UpperCaseAcronyms {
ItemKind::TyAlias(..) | ItemKind::Enum(..) | ItemKind::Struct(..) | ItemKind::Trait(..)
);
then {
check_ident(cx, &it.ident);
check_ident(cx, &it.ident, self.upper_case_acronyms_aggressive);
}
}
}

fn check_variant(&mut self, cx: &EarlyContext<'_>, v: &Variant) {
check_ident(cx, &v.ident);
check_ident(cx, &v.ident, self.upper_case_acronyms_aggressive);
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/utils/conf.rs
Expand Up @@ -173,6 +173,8 @@ define_Conf! {
(disallowed_methods, "disallowed_methods": Vec<String>, Vec::<String>::new()),
/// Lint: UNREADABLE_LITERAL. Should the fraction of a decimal be linted to include separators.
(unreadable_literal_lint_fractions, "unreadable_literal_lint_fractions": bool, true),
/// Lint: UPPER_CASE_ACRONYMS. Enabler verbose mode triggers if there is more than one uppercase char next to each other
(upper_case_acronyms_aggressive, "upper_case_acronyms_aggressive": bool, false),
/// Lint: _CARGO_COMMON_METADATA. For internal testing only, ignores the current `publish` settings in the Cargo manifest.
(cargo_ignore_publish, "cargo_ignore_publish": bool, false),
}
Expand Down

0 comments on commit 59750dc

Please sign in to comment.