Skip to content

code suggestions for unused-mut, while-true, deprecated-attribute, and unused-parens lints #44942

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

Merged
merged 5 commits into from
Oct 2, 2017

Conversation

zackmdavis
Copy link
Member

`CodeSuggestion` doesn't live in the `diagnostic` module.
Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Small nitpicks, r=me once you deal with the first two.

"denote infinite loops with loop { ... }");
let msg = "denote infinite loops with `loop { ... }`";
let mut err = cx.struct_span_lint(WHILE_TRUE,
e.span, msg);
Copy link
Contributor

Choose a reason for hiding this comment

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

Fits in one line?

e.span, msg);
let condition_span = cx.tcx.sess.codemap().def_span(e.span);
err.span_suggestion_short(condition_span, "use `loop`",
"loop".to_owned());
Copy link
Contributor

Choose a reason for hiding this comment

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

When all arguments don't fit, I like to keep one line per argument (indented to the first one in the same line as the method).

let mut err = cx.struct_span_lint(UNUSED_PARENS,
value.span,
&span_msg);
// Remove exactly one pair of parentheses (rather than naïvely
Copy link
Contributor

Choose a reason for hiding this comment

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

@nikomatsakis do we care about unicode in rustc comments?

@zackmdavis
Copy link
Member Author

zackmdavis commented Sep 30, 2017

(force-pushed to address formatting review comments)

@zackmdavis
Copy link
Member Author

(updated again to fix compile-fail failures reported by Travis)

@estebank
Copy link
Contributor

estebank commented Oct 1, 2017

@bors r+

Thanks @zachreizner, great work!

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

📌 Commit a399757 has been approved by estebank

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

⌛ Testing commit a3997577226db861d103d27da91a77acf9fd4b3e with merge 7a521317b909286b51aa36af0dee6cbd7a5cc6c0...

@zackmdavis
Copy link
Member Author

Thanks @zachreizner, great work!

I agree; @zachreizner is the best! I especially like the one where he fixed the shift-right docs; it must have taken a very careful eye to catch that!

😉

@zachreizner
Copy link
Contributor

Not the first time I've been thanked in place of a a fellow Zach :-). I think my handle just comes up before the Zack Zach-s in the autocomplete.

@bors
Copy link
Collaborator

bors commented Oct 1, 2017

💔 Test failed - status-appveyor

@zackmdavis
Copy link
Member Author

Appveyor/Windows apparently doesn't like the --error-format json UI test. I think this is plausibly a bug in the test runner (rather than the JSON order being unstable or anything like that), since the only difference seems to be in the "file_name" value, which is supposed to be normalized (with "$DIR") and worked fine in the Travis/Unix-like tests, but for now I'll just rewrite it as a run-make test.

@estebank
Copy link
Contributor

estebank commented Oct 1, 2017

I believe we're really trying to move away from make tests. @zackmdavis, could you

  1. create a ticket to deal with the test output normalization failure on json output and
  2. revert your last change and add // ignore-windows to the test (this will make the test to be skipped under windows) with a comment linking to the previous ticket?

@zackmdavis
Copy link
Member Author

output normalization issue is #44968; updated tip of PR with ignore-windows is 979671ec

@zackmdavis
Copy link
Member Author

correction, PR is 8a14022

@estebank
Copy link
Contributor

estebank commented Oct 2, 2017

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 2, 2017

📌 Commit 8a14022 has been approved by estebank

@bors
Copy link
Collaborator

bors commented Oct 2, 2017

⌛ Testing commit 8a14022 with merge 9ae6ed7...

bors added a commit that referenced this pull request Oct 2, 2017
code suggestions for unused-mut, while-true, deprecated-attribute, and unused-parens lints

![lint_suggestions](https://user-images.githubusercontent.com/1076988/31044068-b2074de8-a57c-11e7-9319-6668508b6d1f.png)

r? @estebank
@bors
Copy link
Collaborator

bors commented Oct 2, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: estebank
Pushing 9ae6ed7 to master...

@bors bors merged commit 8a14022 into rust-lang:master Oct 2, 2017
bors added a commit that referenced this pull request Oct 18, 2017
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
bors added a commit that referenced this pull request Oct 18, 2017
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
bors added a commit that referenced this pull request Oct 19, 2017
code suggestions for non-shorthand field pattern, no-mangle lints

continuing in the spirit of #44942

![moar_lint_suggestions](https://user-images.githubusercontent.com/1076988/31485011-3b20cc80-aee7-11e7-993d-81267ab77732.png)

r? @estebank
@zackmdavis zackmdavis deleted the lint_suggestions branch January 13, 2018 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants