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

Fix needless_doctest_main span #5241

Closed
wants to merge 0 commits into from
Closed

Fix needless_doctest_main span #5241

wants to merge 0 commits into from

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Feb 28, 2020

First, I extend the ranges, not just the text when I coalesce two events and second: I use this (and work around the removed prefixes by counting the newlines and multiplying by 4, which is admittedly a bit hacky) when finding the fn main().

This fixes #5236.


changelog: none

@llogiq
Copy link
Contributor Author

llogiq commented Feb 29, 2020

ICE seems unrelated. Probably #5238.

@JohnTitor
Copy link
Member

Yeah, the failure is unrelated. And I noticed AppVeyor still lives for branch CI, not only master :/ We can say goodbye to them completely by removing AppVeyor's project iirc.

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 2, 2020
clippy_lints/src/doc.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 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 Mar 3, 2020
@bors
Copy link
Collaborator

bors commented Mar 3, 2020

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

let lo = span.lo() + BytePos::from_usize(range.start - begin);
// we need a span that includes at least the code.
// however, because we have already stripped the comment
// prefixes, we need to add their size back. We currently
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// prefixes, we need to add their size back. We currently
// prefixes (`/// `), we need to add their size back. We currently

Took me way too long to figure out what you meant by "comment prefixes". Can you add this for people like me? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'm rebuilding the branch anyway, because somehow a rebase went wrong.

@@ -67,7 +67,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NoNegCompOpForPartialOrd {
};

let implements_partial_ord = {
if let Some(id) = utils::get_trait_def_id(cx, &paths::PARTIAL_ORD) {
if let Some(id) = cx.tcx.lang_items().partial_ord_trait() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like something gone wrong with the rebase?

@llogiq llogiq closed this Mar 4, 2020
@flip1995 flip1995 deleted the needless-doc-main-span branch June 5, 2020 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

needless_doctest_main reports wrong line if there are imports
4 participants