Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

needless_doctest_main yields false positive with proc macros #4698

Closed
Rantanen opened this issue Oct 19, 2019 · 2 comments · Fixed by #5912
Closed

needless_doctest_main yields false positive with proc macros #4698

Rantanen opened this issue Oct 19, 2019 · 2 comments · Fixed by #5912

Comments

@Rantanen
Copy link
Contributor

Clippy version: 0.0.212 (e979eb4 2019-10-17)

Clippy warns that the fn main() {} is not needed in the following doctest:

https://github.com/Rantanen/intercom/blob/45837a4d77c340c9cb8cf45142843198a80a82b6/intercom/src/lib.rs#L12-L36

However removing that will end up with doctest wrapping the whole thing in a function, which results in the com_library!(Calculator); becoming a statement - something that is not allowed for (at least that) proc macro.

I feel like this is enough of a niche case that there's no real need to fix it. A note could be added to the Known problems section of the lint.

@Rantanen
Copy link
Contributor Author

Rantanen commented Oct 19, 2019

Okay this was a bit more annoying than I thought. The fact that I had the issue on crate level documentation prevented me from using #[allow(clippy::needless_doctest_main)] on that specific doctest. The attribute inside the doctest did nothing while outside the only way I could have put it on the "item" was a crate-wide #![allow(...)].

In the end I found out that the check is a rather simple one:

fn check_code(cx: &LateContext<'_, '_>, text: &str, span: Span) {
if text.contains("fn main() {") {
span_lint(cx, NEEDLESS_DOCTEST_MAIN, span, "needless `fn main` in doctest");
}
}

My workaround involves turning the main function into fn main() -> () {}, which results in Clippy not discovering it.

Edit: I wanted to document this in the doctest itself so I wouldn't wonder why I'd include the -> () in the main declaration. While doing this, I made the mistake of writing fn main() {} in the comments within the doctest and suddenly clippy started yelling at me again... :)

@llogiq
Copy link
Contributor

llogiq commented Oct 19, 2019

Yeah the check is at the moment just text-based. And detecting proc macros that expand to items is somewhat harder.

A simple trick that would work is

///# fn main 
///# // keep this to quell `needless_doctest_main` warning
///#(){}

bors added a commit that referenced this issue Nov 28, 2019
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.
@bors bors closed this as completed in f8db258 Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants