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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc_errors/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ impl Diagnostic {
/// Prints out a message with a suggested edit of the code. If the suggestion is presented
/// inline it will only show the text message and not the text.
///
/// See `diagnostic::CodeSuggestion` for more information.
/// See `CodeSuggestion` for more information.
pub fn span_suggestion_short(&mut self, sp: Span, msg: &str, suggestion: String) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitution_parts: vec![Substitution {
Expand All @@ -235,7 +235,7 @@ impl Diagnostic {
/// * may look like "to do xyz, use" or "to do xyz, use abc"
/// * may contain a name of a function, variable or type, but not whole expressions
///
/// See `diagnostic::CodeSuggestion` for more information.
/// See `CodeSuggestion` for more information.
pub fn span_suggestion(&mut self, sp: Span, msg: &str, suggestion: String) -> &mut Self {
self.suggestions.push(CodeSuggestion {
substitution_parts: vec![Substitution {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl Emitter for EmitterWriter {
sugg.substitution_parts[0].substitutions[0].find('\n').is_none() {
let substitution = &sugg.substitution_parts[0].substitutions[0];
let msg = if substitution.len() == 0 || !sugg.show_code_when_inline {
// This substitution is only removal or we explicitely don't want to show the
// This substitution is only removal or we explicitly don't want to show the
// code inline, don't show it
format!("help: {}", sugg.msg)
} else {
Expand Down
19 changes: 12 additions & 7 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,13 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for WhileTrue {
if let hir::ExprLit(ref lit) = cond.node {
if let ast::LitKind::Bool(true) = lit.node {
if lit.span.ctxt() == SyntaxContext::empty() {
cx.span_lint(WHILE_TRUE,
e.span,
"denote infinite loops with loop { ... }");
let msg = "denote infinite loops with `loop { ... }`";
let mut err = cx.struct_span_lint(WHILE_TRUE, 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());
err.emit();
}
}
}
Expand Down Expand Up @@ -650,10 +654,11 @@ impl EarlyLintPass for DeprecatedAttr {
ref name,
ref reason,
_) = g {
cx.span_lint(DEPRECATED,
attr.span,
&format!("use of deprecated attribute `{}`: {}. See {}",
name, reason, link));
let msg = format!("use of deprecated attribute `{}`: {}. See {}",
name, reason, link);
let mut err = cx.struct_span_lint(DEPRECATED, attr.span, &msg);
err.span_suggestion_short(attr.span, "remove this attribute", "".to_owned());
err.emit();
}
return;
}
Expand Down
48 changes: 42 additions & 6 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ use syntax::attr;
use syntax::feature_gate::{BUILTIN_ATTRIBUTES, AttributeType};
use syntax::symbol::keywords;
use syntax::ptr::P;
use syntax::print::pprust;
use syntax::util::parser;
use syntax_pos::Span;

Expand Down Expand Up @@ -70,9 +71,13 @@ impl UnusedMut {
let used_mutables = cx.tcx.used_mut_nodes.borrow();
for (_, v) in &mutables {
if !v.iter().any(|e| used_mutables.contains(e)) {
cx.span_lint(UNUSED_MUT,
cx.tcx.hir.span(v[0]),
"variable does not need to be mutable");
let binding_span = cx.tcx.hir.span(v[0]);
let mut_span = cx.tcx.sess.codemap().span_until_char(binding_span, ' ');
let mut err = cx.struct_span_lint(UNUSED_MUT,
binding_span,
"variable does not need to be mutable");
err.span_suggestion_short(mut_span, "remove this `mut`", "".to_owned());
err.emit();
}
}
}
Expand Down Expand Up @@ -325,9 +330,40 @@ impl UnusedParens {
let necessary = struct_lit_needs_parens &&
parser::contains_exterior_struct_lit(&inner);
if !necessary {
cx.span_lint(UNUSED_PARENS,
value.span,
&format!("unnecessary parentheses around {}", msg))
let span_msg = format!("unnecessary parentheses around {}", msg);
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?

// stripping all paren characters)
let mut ate_left_paren = false;
let mut ate_right_paren = false;
let parens_removed = pprust::expr_to_string(value)
.trim_matches(|c| {
match c {
'(' => {
if ate_left_paren {
false
} else {
ate_left_paren = true;
true
}
},
')' => {
if ate_right_paren {
false
} else {
ate_right_paren = true;
true
}
},
_ => false,
}
}).to_owned();
err.span_suggestion_short(value.span,
"remove these parentheses",
parens_removed);
err.emit();
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,8 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG
("no_debug", Whitelisted, Gated(
Stability::Deprecated("https://github.com/rust-lang/rust/issues/29721"),
"no_debug",
"the `#[no_debug]` attribute is an experimental feature",
"the `#[no_debug]` attribute was an experimental feature that has been \
deprecated due to lack of demand",
cfg_fn!(no_debug))),
("omit_gdb_pretty_printer_section", Whitelisted, Gated(Stability::Unstable,
"omit_gdb_pretty_printer_section",
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/feature-gate-no-debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

#![allow(deprecated)]

#[no_debug] //~ ERROR the `#[no_debug]` attribute is
#[no_debug] //~ ERROR the `#[no_debug]` attribute was
fn main() {}
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-1962.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
// compile-flags: -D while-true
fn main() {
let mut i = 0;
while true { //~ ERROR denote infinite loops with loop
while true { //~ ERROR denote infinite loops with `loop
i += 1;
if i == 5 { break; }
}
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/lint/suggestions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
#![feature(no_debug)]

#[no_debug] // should suggest removal of deprecated attribute
fn main() {
while true { // should suggest `loop`
let mut a = (1); // should suggest no `mut`, no parens
println!("{}", a);
}
}
45 changes: 45 additions & 0 deletions src/test/ui/lint/suggestions.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
warning: unnecessary parentheses around assigned value
--> $DIR/suggestions.rs:17:21
|
17 | let mut a = (1); // should suggest no `mut`, no parens
| ^^^ help: remove these parentheses
|
= note: #[warn(unused_parens)] on by default

warning: use of deprecated attribute `no_debug`: the `#[no_debug]` attribute was an experimental feature that has been deprecated due to lack of demand. See https://github.com/rust-lang/rust/issues/29721
--> $DIR/suggestions.rs:14:1
|
14 | #[no_debug] // should suggest removal of deprecated attribute
| ^^^^^^^^^^^ help: remove this attribute
|
= note: #[warn(deprecated)] on by default

warning: denote infinite loops with `loop { ... }`
--> $DIR/suggestions.rs:16:5
|
16 | while true { // should suggest `loop`
| ^---------
| |
| _____help: use `loop`
| |
17 | | let mut a = (1); // should suggest no `mut`, no parens
18 | | println!("{}", a);
19 | | }
| |_____^
|
= note: #[warn(while_true)] on by default

warning: variable does not need to be mutable
--> $DIR/suggestions.rs:17:13
|
17 | let mut a = (1); // should suggest no `mut`, no parens
| ---^^
| |
| help: remove this `mut`
|
note: lint level defined here
--> $DIR/suggestions.rs:11:9
|
11 | #![warn(unused_mut)] // UI tests pass `-A unused`—see Issue #43896
| ^^^^^^^^^^

25 changes: 25 additions & 0 deletions src/test/ui/lint/unused_parens_json_suggestion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: --error-format json

// ignore-windows (see Issue #44968)

// The output for humans should just highlight the whole span without showing
// the suggested replacement, but we also want to test that suggested
// replacement only removes one set of parentheses, rather than naïvely
// stripping away any starting or ending parenthesis characters—hence this
// test of the JSON error format.

fn main() {
// We want to suggest the properly-balanced expression `1 / (2 + 3)`, not
// the malformed `1 / (2 + 3`
let _a = (1 / (2 + 3));
}
1 change: 1 addition & 0 deletions src/test/ui/lint/unused_parens_json_suggestion.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"message":"unnecessary parentheses around assigned value","code":null,"level":"warning","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":1014,"byte_end":1027,"line_start":24,"line_end":24,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":" let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[{"message":"#[warn(unused_parens)] on by default","code":null,"level":"note","spans":[],"children":[],"rendered":null},{"message":"remove these parentheses","code":null,"level":"help","spans":[{"file_name":"$DIR/unused_parens_json_suggestion.rs","byte_start":1014,"byte_end":1027,"line_start":24,"line_end":24,"column_start":14,"column_end":27,"is_primary":true,"text":[{"text":" let _a = (1 / (2 + 3));","highlight_start":14,"highlight_end":27}],"label":null,"suggested_replacement":"1 / (2 + 3)","expansion":null}],"children":[],"rendered":" let _a = 1 / (2 + 3);"}],"rendered":null}
2 changes: 1 addition & 1 deletion src/test/ui/path-lookahead.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ warning: unnecessary parentheses around `return` value
--> $DIR/path-lookahead.rs:18:10
|
18 | return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove these parentheses
|
= note: #[warn(unused_parens)] on by default

Expand Down