Auto merge of #6308 - matthiaskrgr:internal_lints, r=<try>

WIP: add internal-lints feature to enable clippys internal lints (off by default)

I'm getting liker errors (but not compiler warnings) when building locally, checking if this affects CI as well... :/

changelog: put internal lints behind a feature

Author: bors

.github/workflows/clippy_bors.yml

@@ -131,11 +131,18 @@ jobs:
     - name: Build
       run: cargo build --features deny-warnings
 
-    - name: Test
-      run: cargo test --features deny-warnings
+    # compiletest would panic due to "Found multiple rlibs for crate `clippy_lints`"
+    - name: clean rlibs
+      run: rm -f ./target/debug/deps/libclippy_lints*
 
-    - name: Test clippy_lints
-      run: cargo test --features deny-warnings
+    - name: Build with internal lints
+      run: cargo build --features deny-warnings internal-lints
+
+    - name: Test with internal lints
+      run: cargo test --features deny-warnings  --features internal-lints
+
+    - name: Test clippy_lints with internal lints
+      run: cargo test --features deny-warnings --features internal-lints
       working-directory: clippy_lints
 
     - name: Test rustc_tools_util

Cargo.toml

@@ -32,7 +32,7 @@ path = "src/driver.rs"
 clippy_lints = { version = "0.0.212", path = "clippy_lints" }
 # end automatic update
 semver = "0.11"
-rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
+rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }
 tempfile = { version = "3.1.0", optional = true }
 
 [dev-dependencies]
@@ -49,8 +49,9 @@ derive-new = "0.5"
 rustc-workspace-hack = "1.0.0"
 
 [build-dependencies]
-rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util"}
+rustc_tools_util = { version = "0.2.0", path = "rustc_tools_util" }
 
 [features]
 deny-warnings = []
 integration = ["tempfile"]
+internal-lints = ["clippy_lints/internal-lints"]

clippy_dev/src/lib.rs

@@ -146,16 +146,32 @@ pub fn gen_deprecated<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String>
 }
 
 #[must_use]
-pub fn gen_register_lint_list<'a>(lints: impl Iterator<Item = &'a Lint>) -> Vec<String> {
-    let pre = "    store.register_lints(&[".to_string();
-    let post = "    ]);".to_string();
-    let mut inner = lints
+pub fn gen_register_lint_list<'a>(
+    internal_lints: impl Iterator<Item = &'a Lint>,
+    usable_lints: impl Iterator<Item = &'a Lint>,
+) -> Vec<String> {
+    let header = "    store.register_lints(&[".to_string();
+    let footer = "    ]);".to_string();
+    let internal_lints = internal_lints
+        .sorted_by_key(|l| format!("        &{}::{},", l.module, l.name.to_uppercase()))
+        .map(|l| {
+            format!(
+                "        #[cfg(feature = \"internal-lints\")]\n        &{}::{},",
+                l.module,
+                l.name.to_uppercase()
+            )
+        })
+        .collect::<Vec<String>>();
+    let other_lints = usable_lints
+        .sorted_by_key(|l| format!("        &{}::{},", l.module, l.name.to_uppercase()))
         .map(|l| format!("        &{}::{},", l.module, l.name.to_uppercase()))
         .sorted()
         .collect::<Vec<String>>();
-    inner.insert(0, pre);
-    inner.push(post);
-    inner
+    let mut lint_list = vec![header];
+    lint_list.extend(internal_lints);
+    lint_list.extend(other_lints);
+    lint_list.push(footer);
+    lint_list
 }
 
 /// Gathers all files in `src/clippy_lints` and gathers all lints inside

clippy_dev/src/update_lints.rs

@@ -81,7 +81,7 @@ pub fn run(update_mode: UpdateMode) {
         "end register lints",
         false,
         update_mode == UpdateMode::Change,
-        || gen_register_lint_list(usable_lints.iter().chain(internal_lints.iter())),
+        || gen_register_lint_list(internal_lints.iter(), usable_lints.iter()),
     )
     .changed;
 

clippy_lints/Cargo.toml

@@ -36,3 +36,5 @@ syn = { version = "1", features = ["full"] }
 
 [features]
 deny-warnings = []
+# build clippy with internal lints enabled, off by default
+internal-lints = []

clippy_lints/src/lib.rs

@@ -499,6 +499,24 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
 
     // begin register lints, do not remove this comment, it’s used in `update_lints`
     store.register_lints(&[
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::CLIPPY_LINTS_INTERNAL,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::COMPILER_LINT_FUNCTIONS,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::DEFAULT_LINT,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::INVALID_PATHS,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::OUTER_EXPN_EXPN_DATA,
+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::PRODUCE_ICE,
         &approx_const::APPROX_CONSTANT,
         &arithmetic::FLOAT_ARITHMETIC,
         &arithmetic::INTEGER_ARITHMETIC,
@@ -902,15 +920,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &unwrap_in_result::UNWRAP_IN_RESULT,
         &use_self::USE_SELF,
         &useless_conversion::USELESS_CONVERSION,
-        &utils::internal_lints::CLIPPY_LINTS_INTERNAL,
-        &utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS,
-        &utils::internal_lints::COMPILER_LINT_FUNCTIONS,
-        &utils::internal_lints::DEFAULT_LINT,
-        &utils::internal_lints::INVALID_PATHS,
-        &utils::internal_lints::LINT_WITHOUT_LINT_PASS,
-        &utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM,
-        &utils::internal_lints::OUTER_EXPN_EXPN_DATA,
-        &utils::internal_lints::PRODUCE_ICE,
         &vec::USELESS_VEC,
         &vec_resize_to_zero::VEC_RESIZE_TO_ZERO,
         &verbose_file_reads::VERBOSE_FILE_READS,
@@ -931,11 +940,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
 
     store.register_late_pass(|| box await_holding_invalid::AwaitHolding);
     store.register_late_pass(|| box serde_api::SerdeAPI);
-    store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
-    store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
-    store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass);
-    store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
-    store.register_late_pass(|| box utils::inspector::DeepCodeInspector);
+    #[cfg(feature = "internal-lints")]
+    {
+        store.register_late_pass(|| box utils::internal_lints::CompilerLintFunctions::new());
+        store.register_late_pass(|| box utils::internal_lints::LintWithoutLintPass::default());
+        store.register_late_pass(|| box utils::internal_lints::OuterExpnDataPass);
+        store.register_late_pass(|| box utils::internal_lints::InvalidPaths);
+        store.register_late_pass(|| box utils::inspector::DeepCodeInspector);
+    }
     store.register_late_pass(|| box utils::author::Author);
     let vec_box_size_threshold = conf.vec_box_size_threshold;
     store.register_late_pass(move || box types::Types::new(vec_box_size_threshold));
@@ -1104,6 +1116,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| box literal_representation::LiteralDigitGrouping);
     let literal_representation_threshold = conf.literal_representation_threshold;
     store.register_early_pass(move || box literal_representation::DecimalLiteralRepresentation::new(literal_representation_threshold));
+    #[cfg(feature = "internal-lints")]
     store.register_early_pass(|| box utils::internal_lints::ClippyLintsInternal);
     let enum_variant_name_threshold = conf.enum_variant_name_threshold;
     store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
@@ -1118,6 +1131,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || box large_const_arrays::LargeConstArrays::new(array_size_threshold));
     store.register_late_pass(|| box floating_point_arithmetic::FloatingPointArithmetic);
     store.register_early_pass(|| box as_conversions::AsConversions);
+    #[cfg(feature = "internal-lints")]
     store.register_early_pass(|| box utils::internal_lints::ProduceIce);
     store.register_late_pass(|| box let_underscore::LetUnderscore);
     store.register_late_pass(|| box atomic_ordering::AtomicOrdering);
@@ -1134,6 +1148,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box dereference::Dereferencing);
     store.register_late_pass(|| box option_if_let_else::OptionIfLetElse);
     store.register_late_pass(|| box future_not_send::FutureNotSend);
+    #[cfg(feature = "internal-lints")]
     store.register_late_pass(|| box utils::internal_lints::CollapsibleCalls);
     store.register_late_pass(|| box if_let_mutex::IfLetMutex);
     store.register_late_pass(|| box mut_mutex_lock::MutMutexLock);
@@ -1161,6 +1176,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| box float_equality_without_abs::FloatEqualityWithoutAbs);
     store.register_late_pass(|| box async_yields_async::AsyncYieldsAsync);
     store.register_late_pass(|| box manual_strip::ManualStrip);
+    #[cfg(feature = "internal-lints")]
     store.register_late_pass(|| box utils::internal_lints::MatchTypeOnDiagItem);
     let disallowed_methods = conf.disallowed_methods.iter().cloned().collect::<FxHashSet<_>>();
     store.register_late_pass(move || box disallowed_method::DisallowedMethod::new(&disallowed_methods));
@@ -1295,7 +1311,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&wildcard_imports::ENUM_GLOB_USE),
         LintId::of(&wildcard_imports::WILDCARD_IMPORTS),
     ]);
-
+    #[cfg(feature = "internal-lints")]
     store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![
         LintId::of(&utils::internal_lints::CLIPPY_LINTS_INTERNAL),
         LintId::of(&utils::internal_lints::COLLAPSIBLE_SPAN_LINT_CALLS),

clippy_lints/src/utils/diagnostics.rs

@@ -186,7 +186,9 @@ pub fn span_lint_hir_and_then(
 ///     |
 ///     = note: `-D fold-any` implied by `-D warnings`
 /// ```
-#[allow(clippy::collapsible_span_lint_calls)]
+
+#[allow(clippy::unknown_clippy_lints)]
+#[cfg_attr(feature = "internal-lints", allow(clippy::collapsible_span_lint_calls))]
 pub fn span_lint_and_sugg<'a, T: LintContext>(
     cx: &'a T,
     lint: &'static Lint,

clippy_lints/src/utils/mod.rs

@@ -14,6 +14,7 @@ pub mod eager_or_lazy;
 pub mod higher;
 mod hir_utils;
 pub mod inspector;
+#[cfg(feature = "internal-lints")]
 pub mod internal_lints;
 pub mod numeric_literal;
 pub mod paths;

clippy_lints/src/utils/paths.rs

@@ -31,6 +31,7 @@ pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"];
 pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"];
 pub const DROP: [&str; 3] = ["core", "mem", "drop"];
 pub const DURATION: [&str; 3] = ["core", "time", "Duration"];
+#[cfg(feature = "internal-lints")]
 pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"];
 pub const EXIT: [&str; 3] = ["std", "process", "exit"];
 pub const F32_EPSILON: [&str; 4] = ["core", "f32", "<impl f32>", "EPSILON"];
@@ -59,8 +60,10 @@ pub const INTO_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "Into
 pub const IO_READ: [&str; 3] = ["std", "io", "Read"];
 pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"];
 pub const ITERATOR: [&str; 5] = ["core", "iter", "traits", "iterator", "Iterator"];
+#[cfg(feature = "internal-lints")]
 pub const LATE_CONTEXT: [&str; 2] = ["rustc_lint", "LateContext"];
 pub const LINKED_LIST: [&str; 4] = ["alloc", "collections", "linked_list", "LinkedList"];
+#[cfg(feature = "internal-lints")]
 pub const LINT: [&str; 2] = ["rustc_lint_defs", "Lint"];
 pub const MEM_DISCRIMINANT: [&str; 3] = ["core", "mem", "discriminant"];
 pub const MEM_FORGET: [&str; 3] = ["core", "mem", "forget"];
@@ -125,6 +128,7 @@ pub const STR_ENDS_WITH: [&str; 4] = ["core", "str", "<impl str>", "ends_with"];
 pub const STR_FROM_UTF8: [&str; 4] = ["core", "str", "converts", "from_utf8"];
 pub const STR_LEN: [&str; 4] = ["core", "str", "<impl str>", "len"];
 pub const STR_STARTS_WITH: [&str; 4] = ["core", "str", "<impl str>", "starts_with"];
+#[cfg(feature = "internal-lints")]
 pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
 pub const TO_OWNED: [&str; 3] = ["alloc", "borrow", "ToOwned"];
 pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];

tests/compile-test.rs

@@ -12,6 +12,9 @@ use std::path::{Path, PathBuf};
 
 mod cargo;
 
+// whether to run internal tests or not
+const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal-lints");
+
 fn host_lib() -> PathBuf {
     option_env!("HOST_LIBS").map_or(cargo::CARGO_TARGET_DIR.join(env!("PROFILE")), PathBuf::from)
 }
@@ -96,6 +99,16 @@ fn run_mode(cfg: &mut compiletest::Config) {
     compiletest::run_tests(&cfg);
 }
 
+fn run_internal_tests(cfg: &mut compiletest::Config) {
+    // only run internal tests with the internal-tests feature
+    if !RUN_INTERNAL_TESTS {
+        return;
+    }
+    cfg.mode = TestMode::Ui;
+    cfg.src_base = Path::new("tests").join("ui-internal");
+    compiletest::run_tests(&cfg);
+}
+
 fn run_ui_toml(config: &mut compiletest::Config) {
     fn run_tests(config: &compiletest::Config, mut tests: Vec<tester::TestDescAndFn>) -> Result<bool, io::Error> {
         let mut result = true;
@@ -199,7 +212,6 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
                         Some("main.rs") => {},
                         _ => continue,
                     }
-
                     let paths = compiletest::common::TestPaths {
                         file: file_path,
                         base: config.src_base.clone(),
@@ -253,4 +265,5 @@ fn compile_test() {
     run_mode(&mut config);
     run_ui_toml(&mut config);
     run_ui_cargo(&mut config);
+    run_internal_tests(&mut config);
 }

tests/dogfood.rs

@@ -18,20 +18,39 @@ fn dogfood_clippy() {
     }
     let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
 
-    let output = Command::new(&*CLIPPY_PATH)
-        .current_dir(root_dir)
-        .env("CLIPPY_DOGFOOD", "1")
-        .env("CARGO_INCREMENTAL", "0")
-        .arg("clippy-preview")
-        .arg("--all-targets")
-        .arg("--all-features")
-        .arg("--")
-        .args(&["-D", "clippy::all"])
-        .args(&["-D", "clippy::internal"])
-        .args(&["-D", "clippy::pedantic"])
-        .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
-        .output()
-        .unwrap();
+    let output = if cfg!(feature = "internal-lints") {
+        // with internal lints and internal warnings
+        Command::new(&*CLIPPY_PATH)
+            .current_dir(root_dir)
+            .env("CLIPPY_DOGFOOD", "1")
+            .env("CARGO_INCREMENTAL", "0")
+            .arg("clippy-preview")
+            .arg("--all-targets")
+            .arg("--all-features")
+            .args(&["--features", "internal-lints"])
+            .arg("--")
+            .args(&["-D", "clippy::all"])
+            .args(&["-D", "clippy::pedantic"])
+            .args(&["-D", "clippy::internal"])
+            .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
+            .output()
+            .unwrap()
+    } else {
+        // without internal lints or warnings
+        Command::new(&*CLIPPY_PATH)
+            .current_dir(root_dir)
+            .env("CLIPPY_DOGFOOD", "1")
+            .env("CARGO_INCREMENTAL", "0")
+            .arg("clippy-preview")
+            .arg("--all-targets")
+            .arg("--all-features")
+            .arg("--")
+            .args(&["-D", "clippy::all"])
+            .args(&["-D", "clippy::pedantic"])
+            .arg("-Cdebuginfo=0") // disable debuginfo to generate less data in the target dir
+            .output()
+            .unwrap()
+    };
     println!("status: {}", output.status);
     println!("stdout: {}", String::from_utf8_lossy(&output.stdout));
     println!("stderr: {}", String::from_utf8_lossy(&output.stderr));