Skip to content

Add Applicability to suggestion lints: Take 2 #3459

New issue

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

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

Already on GitHub? # to your account

Merged
merged 8 commits into from
Nov 27, 2018

Conversation

flip1995
Copy link
Member

This is #2943 completely redone, so that we finally have applicability levels for our suggestion lints and #2943 can finally be closed.

Resolves #2930

Implements #2943 (comment)

Still open: multispan_sugg (Should this also be done in this PR?)

PS: Sorry for some unrelated formatting changes, those just slipped in ¯\_(ツ)_/¯

@phansch phansch added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 27, 2018
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

just a nit.

Lets do this!

@@ -300,6 +301,7 @@ impl WarningType {
"mistyped literal suffix",
"did you mean to write",
grouping_hint.to_string(),
Applicability::MachineApplicable,
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this should be machine applicable. It's just a "looks wrong, needs user attention, probably should be X"

Copy link
Member Author

Choose a reason for hiding this comment

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

So MaybeIncorrect?

Copy link
Contributor

Choose a reason for hiding this comment

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

yea

Copy link
Contributor

@llogiq llogiq left a comment

Choose a reason for hiding this comment

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

Just one small nit.

format!("bytecount::count({}, {})",
snippet_with_applicability(cx, haystack.span, "..", &mut applicability),
snippet_with_applicability(cx, needle.span, "..", &mut applicability)),
Applicability::Unspecified,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that read applicability here?

@oli-obk oli-obk merged commit b2601be into rust-lang:master Nov 27, 2018
@flip1995 flip1995 deleted the sugg_appl branch November 27, 2018 18:52
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use suggestion applicabilities everywhere
4 participants