upper_case_acronyms: add optional aggressive mode and relax default

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.
This commit is contained in:
Matthias Krüger 2021-02-24 23:20:49 +01:00
parent 454515040a
commit 59750dceb8
3 changed files with 37 additions and 10 deletions

View file

@ -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);
@ -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),
@ -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),

View file

@ -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.
@ -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>();
@ -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,
@ -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);
}
}

View file

@ -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),
}