add `internal-lints` feature to enable clippys internal lints (off by default) #6308

matthiaskrgr · 2020-11-08T13:30:50Z

This PR moves the internal lint tests into a new subdirectory (I couldn't find a different way to compile-time-conditionally exclude them from compiletest) and only builds and tests internal lints if the internal-lints feature is enabled.

Fixes #6306

changelog: put internal lints behind a feature ("internal-lints")

rust-highfive · 2020-11-08T13:30:53Z

r? @llogiq

(rust_highfive has picked a reviewer for you, use r? to override)

flip1995 · 2020-11-08T14:42:44Z

This will also require updating the clippy_dev update_lints file updates.

llogiq

Just a small nit, and of course the clippy_dev update, but otherwise I like this change. Makes clippy smaller & faster to both build & run with no downside.

Cargo.toml

flip1995 · 2020-11-08T22:18:02Z

This will also require some CI changes to enable internal lints in CI. I would like to review those (or do them myself, if you want) before this gets merged, but I also like this change generally speaking.

matthiaskrgr · 2020-11-09T11:20:43Z

Hm, the current roadblock is that compiletest does not seem to provide a way to skip tests based on crate features, but I need to skip some of the internal tests during default compilation...
https://rustc-dev-guide.rust-lang.org/tests/adding.html#other-header-commands

matthiaskrgr · 2020-11-12T13:50:24Z

Had the showerthought to put all the internal tests into a separate directory, that might make it work.

matthiaskrgr · 2020-11-13T00:37:56Z

I moved the internal tests into a new directory and cargo test and cargo test --features internal-lints passes now! 🎉
cargo dev update_lints however tries to remove my #[cfg(feature = "internal-lints")] annotations again hmm.

matthiaskrgr · 2020-11-13T01:33:15Z

@bors 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

bors · 2020-11-13T01:33:23Z

⌛ Trying commit 4148bb3 with merge 008b71c...

bors · 2020-11-13T01:54:31Z

💔 Test failed - checks-action_test

matthiaskrgr · 2020-11-13T10:39:17Z

@bors 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

bors · 2020-11-13T10:39:25Z

⌛ Trying commit ba3f1ae with merge ab8552a...

bors · 2020-11-13T10:44:29Z

💔 Test failed - checks-action_test

matthiaskrgr · 2020-11-13T10:47:44Z

Ah this needs a rustup first... #6329

matthiaskrgr · 2020-11-13T11:14:10Z

@bors 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

bors · 2020-11-13T11:14:19Z

⌛ Trying commit bdb2fe4 with merge 7446bba...

bors · 2020-11-13T11:36:09Z

💔 Test failed - checks-action_test

matthiaskrgr · 2020-11-13T13:14:29Z

@bors try

bors · 2020-11-13T13:14:37Z

⌛ Trying commit e9d3b59 with merge 1d7d8ce...

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

…efault)

… for crate `clippy_lints`" compiletest error

clippy_lints/src/lib.rs

flip1995 · 2020-11-29T20:36:41Z

clippy_lints/src/lib.rs

+        #[cfg(feature = "internal-lints")]
+        &utils::internal_lints::PRODUCE_ICE,


Oh no, this'll get rid of our sole easteregg in Clippy :(

Mmh, is this bad?

No, just a bit sad 😄

matthiaskrgr · 2020-11-29T21:03:26Z

@bors try

bors · 2020-11-29T21:03:33Z

⌛ Trying commit c6f70f8 with merge 83ea8c8...

add `internal-lints` feature to enable clippys internal lints (off by default) This PR moves the internal lint tests into a new subdirectory (I couldn't find a different way to compile-time-conditionally exclude them from compiletest) and only builds and tests internal lints if the `internal-lints` feature is enabled. Fixes #6306 changelog: put internal lints behind a feature ("internal-lints")

bors · 2020-11-29T21:25:42Z

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: 83ea8c8 (83ea8c820163c1384bfb1121af1fecb1cffff0f3)

clippy_dev/src/lib.rs

ci: always build with internal lints group up internal lints in lib.rs dogfood: we already pass --all-features, no need to enable internal-lints again

Outdated

flip1995 · 2020-11-30T09:24:00Z

@bors r+

Thanks!

(I dismissed @llogiq review, because it was about something already fixed)

bors · 2020-11-30T09:24:01Z

📌 Commit 252083f has been approved by flip1995

bors · 2020-11-30T09:41:36Z

⌛ Testing commit 252083f with merge 871f3ff...

add `internal-lints` feature to enable clippys internal lints (off by default) This PR moves the internal lint tests into a new subdirectory (I couldn't find a different way to compile-time-conditionally exclude them from compiletest) and only builds and tests internal lints if the `internal-lints` feature is enabled. Fixes #6306 changelog: put internal lints behind a feature ("internal-lints")

bors · 2020-11-30T10:01:53Z

💔 Test failed - checks-action_test

flip1995 · 2020-12-03T09:19:39Z

@bors treeclosed=10 retry rollup

Rollup of 4 pull requests Successful merges: - #6308 (add `internal-lints` feature to enable clippys internal lints (off by default)) - #6395 (switch Version/VersionReq usages to RustcVersion ) - #6402 (Add Collapsible match lint) - #6407 (CONTRIBUTING: update bors queue url from buildbot2.rlo to bors.rlo) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup changelog: rollup

rust-highfive assigned llogiq Nov 8, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 8, 2020

llogiq previously requested changes Nov 8, 2020

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

matthiaskrgr force-pushed the internal_lints branch from 37b4edf to 13f2619 Compare November 9, 2020 11:17

matthiaskrgr force-pushed the internal_lints branch from 13f2619 to 44908a2 Compare November 13, 2020 00:30

matthiaskrgr force-pushed the internal_lints branch from 197361d to 4148bb3 Compare November 13, 2020 01:21

matthiaskrgr force-pushed the internal_lints branch from ba3f1ae to bdb2fe4 Compare November 13, 2020 11:13

matthiaskrgr force-pushed the internal_lints branch from bdb2fe4 to e9d3b59 Compare November 13, 2020 13:14

matthiaskrgr added 3 commits November 29, 2020 21:07

add internal-lints feature to enable clippys internal lints (off by d…

2838b04

…efault)

fix clippy-dev update_lints

958e2e2

ci: partly clean build artifacts to work around "Found multiple rlibs…

b25a6df

… for crate `clippy_lints`" compiletest error

matthiaskrgr force-pushed the internal_lints branch 2 times, most recently from 6f328f9 to fa4f131 Compare November 29, 2020 20:19

flip1995 reviewed Nov 29, 2020

View reviewed changes

matthiaskrgr force-pushed the internal_lints branch 2 times, most recently from 603e1af to c6f70f8 Compare November 29, 2020 21:02

matthiaskrgr added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Nov 29, 2020

phansch reviewed Nov 29, 2020

View reviewed changes

clippy_dev/src/lib.rs Outdated Show resolved Hide resolved

address review comments and rebase

252083f

ci: always build with internal lints group up internal lints in lib.rs dogfood: we already pass --all-features, no need to enable internal-lints again

matthiaskrgr force-pushed the internal_lints branch from c6f70f8 to 252083f Compare November 29, 2020 22:44

flip1995 approved these changes Nov 29, 2020

View reviewed changes

flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 30, 2020

flip1995 mentioned this pull request Dec 3, 2020

Rollup of 4 pull requests #6415

Merged

bors merged commit c3db082 into rust-lang:master Dec 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `internal-lints` feature to enable clippys internal lints (off by default) #6308

add `internal-lints` feature to enable clippys internal lints (off by default) #6308

matthiaskrgr commented Nov 8, 2020 •

edited

Loading

rust-highfive commented Nov 8, 2020

flip1995 commented Nov 8, 2020

llogiq left a comment

flip1995 commented Nov 8, 2020

matthiaskrgr commented Nov 9, 2020

matthiaskrgr commented Nov 12, 2020

matthiaskrgr commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

flip1995 Nov 29, 2020 •

edited

Loading

matthiaskrgr Nov 29, 2020

flip1995 Nov 29, 2020

matthiaskrgr commented Nov 29, 2020

bors commented Nov 29, 2020

bors commented Nov 29, 2020

flip1995 commented Nov 30, 2020

bors commented Nov 30, 2020

bors commented Nov 30, 2020

bors commented Nov 30, 2020

flip1995 commented Dec 3, 2020

		#[cfg(feature = "internal-lints")]
		&utils::internal_lints::PRODUCE_ICE,

add internal-lints feature to enable clippys internal lints (off by default) #6308

add internal-lints feature to enable clippys internal lints (off by default) #6308

Conversation

matthiaskrgr commented Nov 8, 2020 • edited Loading

rust-highfive commented Nov 8, 2020

flip1995 commented Nov 8, 2020

llogiq left a comment

Choose a reason for hiding this comment

flip1995 commented Nov 8, 2020

matthiaskrgr commented Nov 9, 2020

matthiaskrgr commented Nov 12, 2020

matthiaskrgr commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

bors commented Nov 13, 2020

matthiaskrgr commented Nov 13, 2020

bors commented Nov 13, 2020

flip1995 Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

matthiaskrgr Nov 29, 2020

Choose a reason for hiding this comment

flip1995 Nov 29, 2020

Choose a reason for hiding this comment

matthiaskrgr commented Nov 29, 2020

bors commented Nov 29, 2020

bors commented Nov 29, 2020

flip1995 commented Nov 30, 2020

bors commented Nov 30, 2020

bors commented Nov 30, 2020

bors commented Nov 30, 2020

flip1995 commented Dec 3, 2020

add `internal-lints` feature to enable clippys internal lints (off by default) #6308

add `internal-lints` feature to enable clippys internal lints (off by default) #6308

matthiaskrgr commented Nov 8, 2020 •

edited

Loading

flip1995 Nov 29, 2020 •

edited

Loading