Use syn in needless_doctest_main lint #4729
Conversation
|
PS.: My toolchain doesn't work right now, so I pushed without ui-test stderr. Will fix later. |
|
Once #4733 is merged |
flip1995
added
the
S-waiting-on-author
llogiq
force-pushed
the
needless-doc-main-syn
branch
from
270a46c
to
cdbb1e5
Compare
|
That failure is sure strange. Anyone have an idea why it happens? |
|
This didn't happen in other builds, I restarted the build, let's see if this was a temporary failure. |
|
Now that my toolchain works again, I get the same failure, so I doubt it's an intermittent CI problem. |
|
Yeah CI failed also with the same error. This is probably the syn dep? |
|
$!@&/ my toolchain broke again, but with nightly clippy url lints all right. |
|
I got my toolchain to work again, but I'm still stumped with the error. Somehow linting the |
llogiq
force-pushed
the
needless-doc-main-syn
branch
from
cdbb1e5
to
94f63a3
Compare
|
When I remove the However, that error seems to originate from a UI test, so it's likely not the error to look for. |
|
☔ The latest upstream changes (presumably #4777) made this pull request unmergeable. Please resolve the merge conflicts. |
llogiq
force-pushed
the
needless-doc-main-syn
branch
from
94f63a3
to
0bce0e2
Compare
|
This gets weirder and weirder. Now our linux build fails and the others work. Btw. I've debugged locally and my local build failed because of a full disk; I've tried on a bigger machine and it worked. |
llogiq
force-pushed
the
needless-doc-main-syn
branch
from
0bce0e2
to
1077c23
Compare
|
@bors try |
Use syn in needless_doctest_main lint changelog: none This fixes #4698 by only linting non-empty `fn main()`s. This is not a perfect solution, but I don't want to omit linting if any macro call or attribute is detected, as that would result in many false negatives.
|
💔 Test failed - status-appveyor |
clippy_lints/Cargo.toml
Outdated
| # cargo requires serde feat in its url dep | ||
| # see https://github.com/rust-lang/rust/pull/63587#issuecomment-522343864 | ||
| url = { version = "2.1.0", features = ["serde"] } |
There was a problem hiding this comment.
I think features could be dropped entirely because it's handled by rustc-workspace-hack: https://github.com/rust-lang/rust/blob/4752c05af4a5f392de026f9ae1877eae030be359/src/tools/rustc-workspace-hack/Cargo.toml#L66
Dogfood has hidden the error but maybe this change will help (it failed to build url).
|
I think you will have to rebase to make PR CI green. |
|
That should be enough. |
|
Now |
|
☔ The latest upstream changes (presumably #4866) made this pull request unmergeable. Please resolve the merge conflicts. |
llogiq
force-pushed
the
needless-doc-main-syn
branch
from
37edd26
to
92a6015
Compare
|
I've rebased, hopefully this will now work. It does so locally. |
|
We likely overflow some space quota with this. In my local tests, I build with As the benefits of doing this are not too big, I'll just close this for now. |
…nishearth,flip1995 Parse doctests in needless_doctest_main This switches from text-based search to running the parser to avoid false positives. Inspired by how [rustdoc](https://github.com/rust-lang/rust/blob/3f3250500fe152b5759c21453ba9a9129808d0d8/src/librustdoc/test.rs#L366) handles this and by #4729. cc @llogiq changelog: Fix multiple false positives in [`needless_doctest_main`]. Fixes #5879 Fixes #4906 Fixes #5103 Fixes #4698
changelog: none
This fixes #4698 by only linting non-empty
fn main()s. This is not a perfect solution, but I don't want to omit linting if any macro call or attribute is detected, as that would result in many false negatives.