Add lint output to lint list #8947

Serial-ATA · 2022-06-04T16:03:13Z

changelog: Add the ability to show the lint output in the lint list

This just adds the logic to produce the output, it hasn't been added to any lints yet. It did help find some mistakes in some docs though 😄.

Screenshots

A single code block

A single code block with a "Use instead" section

Multiple code blocks

This is the last task in #7172 🎉.
r? @xFrednet (?)

xFrednet

The implementation looks good to me, a few smaller nits, but everything should be simple to fix.

With this change, I would like to ensure that the metadata collection works during the bors check and not on master when the deploy script is run. @flip1995 Would it be possible to run deploy.sh with bors and also push it to the gh-pages (if it's not @bors try). And would we want to have that?

clippy_lints/src/utils/internal_lints/metadata_collector.rs

xFrednet · 2022-06-06T13:54:41Z

clippy_lints/src/utils/internal_lints/metadata_collector.rs

+                    if line.eq_ignore_ascii_case("Use instead:") || line.eq_ignore_ascii_case("Could be written as:") {
+                        break 'outer;
+                    }


This means that a lint description like:

/// ```rs /// <bad-example 1> /// ``` /// /// Use instead: /// ```rs /// <good-example 1> /// ``` /// /// ### Example 2 /// /// ```rs /// <bad-example 2> /// ``` /// /// Use instead: /// ```rs /// <good-example 2> /// ```

would only lint the first example correct? I don't know of any lint that has such a documentation, but this would be good to know.

Yep, this is just an attempt to do as little work as possible. Normally when lints have multiple examples they use "or" anyway, so it shouldn't be an issue.

Okay, I'm debating if this actually has a huge impact on the runtime. 🤔

I also want to see how what effect this PR has on the runtime. But I doubt that we can really do any optimization here, since we need to compile every snippet separately. Therefore, it should be fine.

Here are a few timings:

Before the PR

Executed in 8.75 secs fish external usr time 15.40 secs 310.00 micros 15.40 secs sys time 0.62 secs 185.00 micros 0.62 secs

After the PR (no generated output)

Executed in 9.44 secs fish external usr time 16.87 secs 551.00 micros 16.87 secs sys time 0.62 secs 0.00 micros 0.62 secs

Generating the output of 5 lints

Executed in 30.93 secs fish external usr time 59.21 secs 0.00 micros 59.21 secs sys time 3.75 secs 554.00 micros 3.75 secs

That's quite a step up from the original run time, and just for 5 lints. Have you looked into parallelizing this step somehow? Maybe we could start the linting processes and not wait for its output?

If it's not possible, I'm also fine with this, as it will only be run, during for merge commits.

Before we merge this, I want to try adding this to the bors workflow, to ensure that the metadata collection always succeeds. Currently, it's only run on master after the merge and can therefore fail without us noticing. I have a good idea how this can be done, and I'll create a PR on the weekend, unless you want to give it a try 🙃

I'll work on parallelization, I've made slight improvements but it could probably be much better.

Also, I'll just wait for your PR. Workflows aren't my forte 😄.

clippy_lints/src/utils/internal_lints/metadata_collector.rs

[WIP] Test metadata collection in Bors CI workflow This PR adds a new check to bors CI workflows, which ensures that the metadata collection success, when it's run as part of the `deploy` script. I've only added it to bors workflows, as the runtime will be high while it'll also succeed most of the time. This is a preparation for #8947. --- changelog: none r? `@ghost`

flip1995 · 2022-06-13T08:00:10Z

Would it be possible to run deploy.sh with bors and also push it to the gh-pages (if it's not @bors try). And would we want to have that?

It would be possible, but we don't want to push from auto. If anything goes wrong in the bors CI and it already pushed to gh-pages, we have invalid docs until the next PR is merged successfully.

Test metadata collection in Bors CI workflow This PR adds a new check to bors CI workflows, which ensures that the metadata collection success, when it's run as part of the `deploy` script. I've only added it to bors workflows, as the runtime will be high while it'll also succeed most of the time. This is a preparation for #8947. --- changelog: none r? `@ghost`

xFrednet · 2022-06-13T17:21:24Z

Would it be possible to run deploy.sh with bors and also push it to the gh-pages (if it's not @bors try). And would we want to have that?

It would be possible, but we don't want to push from auto. If anything goes wrong in the bors CI and it already pushed to gh-pages, we have invalid docs until the next PR is merged successfully.

That's also what I thought. With #8988 we now test the metadata collection, which was the main goal with my comment.

I'm also interested how much this PR effects the CI time. So, let's give it a go:

@bors try

bors · 2022-06-13T17:21:30Z

⌛ Trying commit fdadebe with merge 7f85771...

Add lint output to lint list changelog: Add the ability to show the lint output in the lint list This just adds the logic to produce the output, it hasn't been added to any lints yet. It did help find some mistakes in some docs though 😄. ### Screenshots <details> <summary>A single code block</summary> ![single-code-block](https://user-images.githubusercontent.com/69764315/172013766-145b22b1-1d91-4fb8-9cd0-b967a52d6330.png) </details> <details> <summary>A single code block with a "Use instead" section</summary> ![with-usage](https://user-images.githubusercontent.com/69764315/172013792-d2dd6c9c-defa-41e0-8c27-8e8e311adb63.png) </details> <details> <summary>Multiple code blocks</summary> ![multi-code-block](https://user-images.githubusercontent.com/69764315/172013808-5328f59b-e7c5-4914-a396-253822a6d350.png) </details> This is the last task in #7172 🎉. r? `@xFrednet` (?)

bors · 2022-06-13T17:24:04Z

💔 Test failed - checks-action_dev_test

xFrednet · 2022-06-13T17:28:02Z

It looks like this requires a rebase. I'll leave that to @Serial-ATA as this is not time critical and has a low chance of having conflicts since it's an internal lint.

@rustbot author

xFrednet · 2022-06-14T09:25:22Z

The CI was just fixed on master, let's try this again.

@bors try

bors · 2022-06-14T09:25:28Z

⌛ Trying commit fdadebe with merge 6b1f2a6...

Add lint output to lint list changelog: Add the ability to show the lint output in the lint list This just adds the logic to produce the output, it hasn't been added to any lints yet. It did help find some mistakes in some docs though 😄. ### Screenshots <details> <summary>A single code block</summary> ![single-code-block](https://user-images.githubusercontent.com/69764315/172013766-145b22b1-1d91-4fb8-9cd0-b967a52d6330.png) </details> <details> <summary>A single code block with a "Use instead" section</summary> ![with-usage](https://user-images.githubusercontent.com/69764315/172013792-d2dd6c9c-defa-41e0-8c27-8e8e311adb63.png) </details> <details> <summary>Multiple code blocks</summary> ![multi-code-block](https://user-images.githubusercontent.com/69764315/172013808-5328f59b-e7c5-4914-a396-253822a6d350.png) </details> This is the last task in #7172 🎉. r? `@xFrednet` (?)

bors · 2022-06-14T09:43:15Z

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: 6b1f2a6 (6b1f2a6a5ae11bfe0f0d42e448808a1b4e41d899)

xFrednet · 2022-06-14T10:41:56Z

After, the changes on master this looks good to me. The collection is also still plenty past. It now took 3 min in the CI with compilation. In the future, it might be nice to have an environment value to disable the output production. But this is not necessary right now. Thank you very much for the addition. I love the change!

xFrednet · 2022-06-14T10:42:04Z

@bors r+

bors · 2022-06-14T10:42:05Z

📌 Commit fdadebe has been approved by xFrednet

bors · 2022-06-14T10:42:11Z

⌛ Testing commit fdadebe with merge 5a45805...

bors · 2022-06-14T11:08:15Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 5a45805 to master...

xFrednet · 2022-06-14T11:29:06Z

Just noticed that this PR doesn't add any {{produces}} markers to any lints. It has been a few days since I did the full review 😅. But this adds the infrastructure to add these markers 👍

flip1995

Please remove the clippy_dev dependency when next working on this.

flip1995 · 2022-06-16T14:49:43Z

clippy_lints/Cargo.toml

@@ -10,6 +10,7 @@ edition = "2021"

 [dependencies]
 cargo_metadata = "0.14"
+clippy_dev = { path = "../clippy_dev", optional = true }


Just noticed during the sync: depending on clippy_dev feels really wrong. clippy_dev isn't intended to be used as a library, but a standalone binary for clippy dev tools. Nothing should depend on clippy_dev IMO.

Ups yeah your right, slipped under my radar😅

flip1995 · 2022-06-16T14:50:44Z

clippy_lints/src/utils/internal_lints/metadata_collector.rs

@@ -188,6 +191,7 @@ impl MetadataCollector {
            lints: BinaryHeap::<LintMetadata>::default(),
            applicability_info: FxHashMap::<String, ApplicabilityInfo>::default(),
            config: collect_configs(),
+            clippy_project_root: clippy_dev::clippy_project_root(),


This is the only place clippy_dev is used. I'm sure we can just hardcode this path.

rust-highfive assigned xFrednet Jun 4, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 4, 2022

xFrednet reviewed Jun 6, 2022

View reviewed changes

Add lint output to lint list

fdadebe

Serial-ATA force-pushed the lint-produces-output branch from e5b69df to fdadebe Compare June 7, 2022 00:05

xFrednet mentioned this pull request Jun 12, 2022

Test metadata collection in Bors CI workflow #8988

Merged

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

xFrednet approved these changes Jun 14, 2022

View reviewed changes

bors merged commit 5a45805 into rust-lang:master Jun 14, 2022

flip1995 reviewed Jun 16, 2022

View reviewed changes

xFrednet mentioned this pull request Oct 12, 2022

Update Documentation to Focus on LateLintPass #9426

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lint output to lint list #8947

Add lint output to lint list #8947

Serial-ATA commented Jun 4, 2022

xFrednet left a comment •

edited

Loading

xFrednet Jun 6, 2022

Serial-ATA Jun 6, 2022

xFrednet Jun 7, 2022 •

edited

Loading

Serial-ATA Jun 7, 2022

xFrednet Jun 9, 2022 •

edited

Loading

Serial-ATA Jun 9, 2022 •

edited

Loading

flip1995 commented Jun 13, 2022

xFrednet commented Jun 13, 2022

bors commented Jun 13, 2022

bors commented Jun 13, 2022

xFrednet commented Jun 13, 2022

xFrednet commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

xFrednet commented Jun 14, 2022

xFrednet commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

xFrednet commented Jun 14, 2022

flip1995 left a comment

flip1995 Jun 16, 2022

xFrednet Jun 16, 2022

flip1995 Jun 16, 2022

Add lint output to lint list #8947

Add lint output to lint list #8947

Conversation

Serial-ATA commented Jun 4, 2022

Screenshots

xFrednet left a comment • edited Loading

Choose a reason for hiding this comment

xFrednet Jun 6, 2022

Choose a reason for hiding this comment

Serial-ATA Jun 6, 2022

Choose a reason for hiding this comment

xFrednet Jun 7, 2022 • edited Loading

Choose a reason for hiding this comment

Serial-ATA Jun 7, 2022

Choose a reason for hiding this comment

xFrednet Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

Serial-ATA Jun 9, 2022 • edited Loading

Choose a reason for hiding this comment

flip1995 commented Jun 13, 2022

xFrednet commented Jun 13, 2022

bors commented Jun 13, 2022

bors commented Jun 13, 2022

xFrednet commented Jun 13, 2022

xFrednet commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

xFrednet commented Jun 14, 2022

xFrednet commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

bors commented Jun 14, 2022

xFrednet commented Jun 14, 2022

flip1995 left a comment

Choose a reason for hiding this comment

flip1995 Jun 16, 2022

Choose a reason for hiding this comment

xFrednet Jun 16, 2022

Choose a reason for hiding this comment

flip1995 Jun 16, 2022

Choose a reason for hiding this comment

xFrednet left a comment •

edited

Loading

xFrednet Jun 7, 2022 •

edited

Loading

xFrednet Jun 9, 2022 •

edited

Loading

Serial-ATA Jun 9, 2022 •

edited

Loading