Fixes issue 5095

[xiongmao86]

fixes #5095.

• Followed lint naming conventions
• Added passing UI tests (including committed .stderr file)
• cargo test passes locally
• Executed cargo dev update_lints
• Added lint documentation
• Run cargo dev fmt

changelog: (internal) warn about collapsible span_lint_and_then calls.

[xiongmao86]

@flip1995, Can I use author lint on internal lints, I have tried the playground tricks, but it is not working because of extern crate declaration.

[flip1995]

Yes you can. You could make the utils module public during developing the lint and then use clippy::author in the test file. And then just run the test and the author lint should print to stdout.

[xiongmao86]

As a self reminder, have not done db.note() and db.help().

[xiongmao86]

Oh, no. My dear old laptop have blackout. Some work have lost.

[flip1995]

After rebase, update_lints will also update the internal lints.

[xiongmao86]

Copy, thanks for the update.

[xiongmao86]

Still downloading toolchain, network has been slow.

[xiongmao86]

Oh, there are so many problem to set up a proper environment for development. I can't use my bluetooth keyboard in ubuntu, I don't have a proper connection to successfully download rustup. I think I should buy a new laptop that is compatible with linux like my broken laptop. But there are the nCov out there and I think I couldn't get a new laptop in recent time.

So I think maybe it is better to close this pr.

[flip1995]

No problem, internal lints are nothing urgent, so you can get back to this, whenever your ready.

So, I am wondering should I just close this pr, @flip1995?

Sure, you can open it again, when you want to continue working on this. I marked it as S-inactive-closed in the meantime.

[xiongmao86]

Ho, ho I am back.

[bors]

☔ The latest upstream changes (presumably #5319) made this pull request unmergeable. Please resolve the merge conflicts.

[flip1995]

Implementation looks really good overall. The findings in the Codebase need reformatting. Before you do this, it would be good to adapt the wrapper functions, like I described in my comment below.

[bors]

☔ The latest upstream changes (presumably #5470) made this pull request unmergeable. Please resolve the merge conflicts.

[xiongmao86]

I don't know why this build failed, do I need to wait another rustup?

[flip1995]

@xiongmao86 Thanks for all the work! There were only some formatting errors, which could be fixed in the GH UI, so I did this myself (don't want to bother you more, after all the work).

@bors r+

[bors]

📌 Commit 7aeb3a4 has been approved by flip1995

[bors]

⌛ Testing commit 7aeb3a4 with merge 6dcc8d5...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 6dcc8d5 to master...

[xiongmao86]

@flip1995, sorry I have appear to be urging to finish, I couldn't help, I try to inhabit the urge and after several pr it have been less intense.

And I really don't understand why my local branch build is successful and the ci build would fail.

I have pull upstream master, and then I passed cargo test, and after that I push to origin.

I think in this case I couldn't possible to fail to build unless due to updating of rust repo (in this case I have to wait for another rustup pr).

If it is due to the change of function signature with out corresponding change in function body. Why would my local build successful?

If it is due to the delay of upstream master update after my push to origin, then github should have warn about that recent change stop the code from merging.

Did I make any mistake? Please tell me, I appreciate any opportunity to learn.

[flip1995]

Recently every occurrence of |db| was renamed to |diag|. But you didn't have that change in your local checkout. When you then changed the wrapper functions you changed the old version with |db|. In one function you didn't have to change the last two lines, so they stayed untouched.

Upstream:

docs_link(&mut diag, lint);
diag.emit();

Local:

docs_link(&mut db, lint);
db.emit();

Now Git(Hub) calculates the diff locally only per commit on your local branch, so the diag change didn't show up. On PRs the diff is calculated to the upstream master branch. Since no commit touched these two lines on your branch, they didn't got changed, but CI always runs on the changes as if the PR was already merged. So both your changes (with |db|) and the upstream changes (with |diag|) were included in the code that got checked by the CI.

I hope I have explained this good enough. I don't think, that I ever saw that subtle of an interaction of non-conflicting git changes.

[flip1995]

If the CI fails, but your local build is successful, most of the time a rebase helps.

[xiongmao86]

I have the idea now. It's a long pr. Thank you very much for helping.