Auto merge of rust-lang#7187 - camsteffen:avoid-break-exported, r=fli…

…p1995,phansch

Add avoid_breaking_exported_api config option

changelog: Add `avoid_breaking_exported_api` config option for [`enum_variant_names`], [`large_types_passed_by_value`], [`trivially_copy_pass_by_ref`], [`unnecessary_wraps`], [`upper_case_acronyms`] and [`wrong_self_convention`].

changelog: Deprecates [`pub_enum_variant_names`] and [`wrong_pub_self_convention`] as the non-pub variants are now configurable.

changelog: Fix various false negatives for `pub` items that are not exported from the crate.

A couple changes to late passes in order to use `cx.access_levels.is_exported` rather than `item.vis.kind.is_pub`.

I'm not sure how to better document the config option or lints that are (not) affected (see comments in rust-lang#6806). Suggestions are welcome. cc `@rust-lang/clippy`

I added `/clippy.toml` to use the config internally and `/tests/clippy.toml` to maintain a default config in ui tests.

Closes rust-lang#6806
Closes rust-lang#4504

README.md

@@ -147,6 +147,7 @@ Some lints can be configured in a TOML file named `clippy.toml` or `.clippy.toml
 value` mapping eg.
 
 ```toml
+avoid-breaking-exported-api = false
 blacklisted-names = ["toto", "tata", "titi"]
 cognitive-complexity-threshold = 30
 ```

build.rs

@@ -14,6 +14,6 @@ fn main() {
  );
  println!(
  "cargo:rustc-env=RUSTC_RELEASE_CHANNEL={}",
- rustc_tools_util::get_channel().unwrap_or_default()
+ rustc_tools_util::get_channel()
  );
 }

clippy.toml

@@ -0,0 +1 @@
+avoid-breaking-exported-api = false

clippy_lints/src/deprecated_lints.rs

@@ -141,3 +141,22 @@ declare_deprecated_lint! {
  pub FILTER_MAP,
  "this lint has been replaced by `manual_filter_map`, a more specific lint"
 }
+
+declare_deprecated_lint! {
+ /// **What it does:** Nothing. This lint has been deprecated.
+ ///
+ /// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which
+ /// enables the `enum_variant_names` lint for public items.
+ /// ```
+ pub PUB_ENUM_VARIANT_NAMES,
+ "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items"
+}
+
+declare_deprecated_lint! {
+ /// **What it does:** Nothing. This lint has been deprecated.
+ ///
+ /// **Deprecation reason:** The `avoid_breaking_exported_api` config option was added, which
+ /// enables the `wrong_self_conversion` lint for public items.
+ pub WRONG_PUB_SELF_CONVENTION,
+ "set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items"
+}

clippy_lints/src/enum_variants.rs

@@ -3,8 +3,8 @@
 use clippy_utils::camel_case;
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::source::is_present_in_source;
-use rustc_ast::ast::{EnumDef, Item, ItemKind, VisibilityKind};
-use rustc_lint::{EarlyContext, EarlyLintPass, Lint};
+use rustc_hir::{EnumDef, Item, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::Symbol;
@@ -39,36 +39,6 @@ declare_clippy_lint! {
  "enums where all variants share a prefix/postfix"
 }
 
-declare_clippy_lint! {
- /// **What it does:** Detects public enumeration variants that are
- /// prefixed or suffixed by the same characters.
- ///
- /// **Why is this bad?** Public enumeration variant names should specify their variant,
- /// not repeat the enumeration name.
- ///
- /// **Known problems:** None.
- ///
- /// **Example:**
- /// ```rust
- /// pub enum Cake {
- /// BlackForestCake,
- /// HummingbirdCake,
- /// BattenbergCake,
- /// }
- /// ```
- /// Could be written as:
- /// ```rust
- /// pub enum Cake {
- /// BlackForest,
- /// Hummingbird,
- /// Battenberg,
- /// }
- /// ```
- pub PUB_ENUM_VARIANT_NAMES,
- pedantic,
- "public enums where all variants share a prefix/postfix"
-}
-
 declare_clippy_lint! {
  /// **What it does:** Detects type names that are prefixed or suffixed by the
  /// containing module's name.
@@ -127,21 +97,22 @@ declare_clippy_lint! {
 pub struct EnumVariantNames {
  modules: Vec<(Symbol, String)>,
  threshold: u64,
+ avoid_breaking_exported_api: bool,
 }
 
 impl EnumVariantNames {
  #[must_use]
- pub fn new(threshold: u64) -> Self {
+ pub fn new(threshold: u64, avoid_breaking_exported_api: bool) -> Self {
  Self {
  modules: Vec::new(),
  threshold,
+ avoid_breaking_exported_api,
  }
  }
 }
 
 impl_lint_pass!(EnumVariantNames => [
  ENUM_VARIANT_NAMES,
- PUB_ENUM_VARIANT_NAMES,
  MODULE_NAME_REPETITIONS,
  MODULE_INCEPTION
 ]);
@@ -167,33 +138,42 @@ fn partial_rmatch(post: &str, name: &str) -> usize {
 }
 
 fn check_variant(
- cx: &EarlyContext<'_>,
+ cx: &LateContext<'_>,
  threshold: u64,
- def: &EnumDef,
+ def: &EnumDef<'_>,
  item_name: &str,
  item_name_chars: usize,
  span: Span,
- lint: &'static Lint,
 ) {
  if (def.variants.len() as u64) < threshold {
  return;
  }
- for var in &def.variants {
+ for var in def.variants {
  let name = var.ident.name.as_str();
  if partial_match(item_name, &name) == item_name_chars
  && name.chars().nth(item_name_chars).map_or(false, |c| !c.is_lowercase())
  && name.chars().nth(item_name_chars + 1).map_or(false, |c| !c.is_numeric())
  {
- span_lint(cx, lint, var.span, "variant name starts with the enum's name");
+ 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, lint, var.span, "variant name ends with the enum's name");
+ span_lint(
+ cx,
+ ENUM_VARIANT_NAMES,
+ var.span,
+ "variant name ends with the enum's name",
+ );
  }
  }
  let first = &def.variants[0].ident.name.as_str();
  let mut pre = &first[..camel_case::until(&*first)];
  let mut post = &first[camel_case::from(&*first)..];
- for var in &def.variants {
+ for var in def.variants {
  let name = var.ident.name.as_str();
 
  let pre_match = partial_match(pre, &name);
@@ -226,7 +206,7 @@ fn check_variant(
  };
  span_lint_and_help(
  cx,
- lint,
+ ENUM_VARIANT_NAMES,
  span,
  &format!("all variants have the same {}fix: `{}`", what, value),
  None,
@@ -261,14 +241,14 @@ fn to_camel_case(item_name: &str) -> String {
  s
 }
 
-impl EarlyLintPass for EnumVariantNames {
- fn check_item_post(&mut self, _cx: &EarlyContext<'_>, _item: &Item) {
+impl LateLintPass<'_> for EnumVariantNames {
+ fn check_item_post(&mut self, _cx: &LateContext<'_>, _item: &Item<'_>) {
  let last = self.modules.pop();
  assert!(last.is_some());
  }
 
  #[allow(clippy::similar_names)]
- fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+ fn check_item(&mut self, cx: &LateContext<'_>, 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);
@@ -286,7 +266,7 @@ impl EarlyLintPass for EnumVariantNames {
  );
  }
  }
- if item.vis.kind.is_pub() {
+ if item.vis.node.is_pub() {
  let matching = partial_match(mod_camel, &item_camel);
  let rmatching = partial_rmatch(mod_camel, &item_camel);
  let nchars = mod_camel.chars().count();
@@ -317,11 +297,9 @@ impl EarlyLintPass for EnumVariantNames {
  }
  }
  if let ItemKind::Enum(ref def, _) = item.kind {
- let lint = match item.vis.kind {
- VisibilityKind::Public => PUB_ENUM_VARIANT_NAMES,
- _ => ENUM_VARIANT_NAMES,
- };
- check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span, lint);
+ if !(self.avoid_breaking_exported_api && cx.access_levels.is_exported(item.hir_id())) {
+ check_variant(cx, self.threshold, def, &item_name, item_name_chars, item.span);
+ }
  }
  self.modules.push((item.ident.name, item_camel));
  }

clippy_lints/src/lib.rs

@@ -393,7 +393,6 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) {
 
 #[doc(hidden)]
 pub fn read_conf(sess: &Session) -> Conf {
- use std::path::Path;
  let file_name = match utils::conf::lookup_conf_file() {
  Ok(Some(path)) => path,
  Ok(None) => return Conf::default(),
@@ -404,16 +403,6 @@ pub fn read_conf(sess: &Session) -> Conf {
  },
  };
 
- let file_name = if file_name.is_relative() {
- sess.local_crate_source_file
- .as_deref()
- .and_then(Path::parent)
- .unwrap_or_else(|| Path::new(""))
- .join(file_name)
- } else {
- file_name
- };
-
  let TryConf { conf, errors } = utils::conf::read(&file_name);
  // all conf errors are non-fatal, we just use the default conf in case of error
  for error in errors {
@@ -493,6 +482,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  "clippy::filter_map",
  "this lint has been replaced by `manual_filter_map`, a more specific lint",
  );
+ store.register_removed(
+ "clippy::pub_enum_variant_names",
+ "set the `avoid_breaking_exported_api` config option to `false` to enable the `enum_variant_names` lint for public items",
+ );
+ store.register_removed(
+ "clippy::wrong_pub_self_convention",
+ "set the `avoid_breaking_exported_api` config option to `false` to enable the `wrong_self_convention` lint for public items",
+ );
  // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
  // begin register lints, do not remove this comment, it’s used in `update_lints`
@@ -606,7 +603,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  enum_variants::ENUM_VARIANT_NAMES,
  enum_variants::MODULE_INCEPTION,
  enum_variants::MODULE_NAME_REPETITIONS,
- enum_variants::PUB_ENUM_VARIANT_NAMES,
  eq_op::EQ_OP,
  eq_op::OP_REF,
  erasing_op::ERASING_OP,
@@ -790,7 +786,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  methods::UNNECESSARY_LAZY_EVALUATIONS,
  methods::UNWRAP_USED,
  methods::USELESS_ASREF,
- methods::WRONG_PUB_SELF_CONVENTION,
  methods::WRONG_SELF_CONVENTION,
  methods::ZST_OFFSET,
  minmax::MIN_MAX,
@@ -1014,7 +1009,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(methods::FILETYPE_IS_FILE),
  LintId::of(methods::GET_UNWRAP),
  LintId::of(methods::UNWRAP_USED),
- LintId::of(methods::WRONG_PUB_SELF_CONVENTION),
  LintId::of(misc::FLOAT_CMP_CONST),
  LintId::of(misc_early::UNNEEDED_FIELD_PATTERN),
  LintId::of(missing_doc::MISSING_DOCS_IN_PRIVATE_ITEMS),
@@ -1066,7 +1060,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  LintId::of(doc::MISSING_PANICS_DOC),
  LintId::of(empty_enum::EMPTY_ENUM),
  LintId::of(enum_variants::MODULE_NAME_REPETITIONS),
- LintId::of(enum_variants::PUB_ENUM_VARIANT_NAMES),
  LintId::of(eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
  LintId::of(excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
  LintId::of(excessive_bools::STRUCT_EXCESSIVE_BOOLS),
@@ -1850,7 +1843,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  })
  });
 
- store.register_late_pass(move || box methods::Methods::new(msrv));
+ let avoid_breaking_exported_api = conf.avoid_breaking_exported_api;
+ store.register_late_pass(move || box methods::Methods::new(avoid_breaking_exported_api, msrv));
  store.register_late_pass(move || box matches::Matches::new(msrv));
  store.register_early_pass(move || box manual_non_exhaustive::ManualNonExhaustive::new(msrv));
  store.register_late_pass(move || box manual_strip::ManualStrip::new(msrv));
@@ -1932,6 +1926,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  let pass_by_ref_or_value = pass_by_ref_or_value::PassByRefOrValue::new(
  conf.trivial_copy_size_limit,
  conf.pass_by_value_size_limit,
+ conf.avoid_breaking_exported_api,
  &sess.target,
  );
  store.register_late_pass(move || box pass_by_ref_or_value);
@@ -1958,7 +1953,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  store.register_late_pass(|| box redundant_clone::RedundantClone);
  store.register_late_pass(|| box slow_vector_initialization::SlowVectorInit);
  store.register_late_pass(|| box unnecessary_sort_by::UnnecessarySortBy);
- store.register_late_pass(|| box unnecessary_wraps::UnnecessaryWraps);
+ store.register_late_pass(move || box unnecessary_wraps::UnnecessaryWraps::new(avoid_breaking_exported_api));
  store.register_late_pass(|| box assertions_on_constants::AssertionsOnConstants);
  store.register_late_pass(|| box transmuting_null::TransmutingNull);
  store.register_late_pass(|| box path_buf_push_overwrite::PathBufPushOverwrite);
@@ -1999,10 +1994,10 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
  let literal_representation_threshold = conf.literal_representation_threshold;
  store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
  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_late_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold, avoid_breaking_exported_api));
  store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
  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(move || box upper_case_acronyms::UpperCaseAcronyms::new(avoid_breaking_exported_api, 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);