Skip to content

Commit ead6c4b

Browse files
ftxqxdftxqxd
committed
Add a lint for not using field pattern shorthands
Closes #17792.
1 parent 56d544f commit ead6c4b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+266
-203
lines changed

‎src/doc/guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3877,6 +3877,7 @@ match x {
38773877
If you have a struct, you can destructure it inside of a pattern:
38783878

38793879
```{rust}
3880+
# #![allow(non_shorthand_field_patterns)]
38803881
struct Point {
38813882
x: int,
38823883
y: int,
@@ -3892,6 +3893,7 @@ match origin {
38923893
If we only care about some of the values, we don't have to give them all names:
38933894

38943895
```{rust}
3896+
# #![allow(non_shorthand_field_patterns)]
38953897
struct Point {
38963898
x: int,
38973899
y: int,
@@ -3977,6 +3979,7 @@ You can also define methods that do not take a `self` parameter. Here's a
39773979
pattern that's very common in Rust code:
39783980

39793981
```{rust}
3982+
# #![allow(non_shorthand_field_patterns)]
39803983
struct Circle {
39813984
x: f64,
39823985
y: f64,

‎src/libcollections/treemap.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ impl<K: Ord, V> TreeMap<K, V> {
434434
/// assert_eq!(vec, vec![("a", 1), ("b", 2), ("c", 3)]);
435435
/// ```
436436
pub fn into_iter(self) -> MoveEntries<K, V> {
437-
let TreeMap { root: root, length: length } = self;
437+
let TreeMap { root, length } = self;
438438
let stk = match root {
439439
None => vec!(),
440440
Some(box tn) => vec!(tn)
@@ -898,11 +898,11 @@ impl<K, V> Iterator<(K, V)> for MoveEntries<K,V> {
898898
fn next(&mut self) -> Option<(K, V)> {
899899
while !self.stack.is_empty() {
900900
let TreeNode {
901-
key: key,
902-
value: value,
903-
left: left,
904-
right: right,
905-
level: level
901+
key,
902+
value,
903+
left,
904+
right,
905+
level,
906906
} = self.stack.pop().unwrap();
907907

908908
match left {

‎src/libgetopts/lib.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,10 @@ impl OptGroup {
235235
/// (Both short and long names correspond to different Opts).
236236
pub fn long_to_short(&self) -> Opt {
237237
let OptGroup {
238-
short_name: short_name,
239-
long_name: long_name,
240-
hasarg: hasarg,
241-
occur: occur,
238+
short_name,
239+
long_name,
240+
hasarg,
241+
occur,
242242
..
243243
} = (*self).clone();
244244

@@ -671,11 +671,11 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
671671
let desc_sep = format!("\n{}", " ".repeat(24));
672672

673673
let mut rows = opts.iter().map(|optref| {
674-
let OptGroup{short_name: short_name,
675-
long_name: long_name,
676-
hint: hint,
677-
desc: desc,
678-
hasarg: hasarg,
674+
let OptGroup{short_name,
675+
long_name,
676+
hint,
677+
desc,
678+
hasarg,
679679
..} = (*optref).clone();
680680

681681
let mut row = " ".repeat(4);

‎src/libgreen/sched.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@ impl Scheduler {
636636
mem::transmute(&**next_task.sched.as_mut().unwrap());
637637

638638
let current_task: &mut GreenTask = match sched.cleanup_job {
639-
Some(CleanupJob { task: ref mut task, .. }) => &mut **task,
639+
Some(CleanupJob { ref mut task, .. }) => &mut **task,
640640
None => rtabort!("no cleanup job")
641641
};
642642

@@ -953,7 +953,7 @@ impl CleanupJob {
953953
}
954954

955955
pub fn run(self, sched: &mut Scheduler) {
956-
let CleanupJob { task: task, f: f } = self;
956+
let CleanupJob { task, f } = self;
957957
f.to_fn()(sched, task)
958958
}
959959
}

‎src/librustc/lint/builtin.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1143,6 +1143,40 @@ impl LintPass for UnusedImportBraces {
11431143
}
11441144
}
11451145

1146+
declare_lint!(NON_SHORTHAND_FIELD_PATTERNS, Warn,
1147+
"using `Struct { x: x }` instead of `Struct { x }`")
1148+
1149+
pub struct NonShorthandFieldPatterns;
1150+
1151+
impl LintPass for NonShorthandFieldPatterns {
1152+
fn get_lints(&self) -> LintArray {
1153+
lint_array!(NON_SHORTHAND_FIELD_PATTERNS)
1154+
}
1155+
1156+
fn check_pat(&mut self, cx: &Context, pat: &ast::Pat) {
1157+
let def_map = cx.tcx.def_map.borrow();
1158+
match pat.node {
1159+
ast::PatStruct(_, ref v, _) => {
1160+
for fieldpat in v.iter()
1161+
.filter(|fieldpat| !fieldpat.node.is_shorthand)
1162+
.filter(|fieldpat| def_map.find(&fieldpat.node.pat.id)
1163+
== Some(&def::DefLocal(fieldpat.node.pat.id))) {
1164+
match fieldpat.node.pat.node {
1165+
ast::PatIdent(_, ident, None) if ident.node.as_str()
1166+
== fieldpat.node.ident.as_str() => {
1167+
cx.span_lint(NON_SHORTHAND_FIELD_PATTERNS, fieldpat.span,
1168+
format!("the `{}:` in this pattern is redundant and can \
1169+
be removed", ident.node.as_str()).as_slice())
1170+
},
1171+
_ => {},
1172+
}
1173+
}
1174+
},
1175+
_ => {}
1176+
}
1177+
}
1178+
}
1179+
11461180
declare_lint!(pub UNUSED_UNSAFE, Warn,
11471181
"unnecessary use of an `unsafe` block")
11481182

@@ -1523,12 +1557,12 @@ impl LintPass for Stability {
15231557
def_id
15241558
}
15251559
typeck::MethodTypeParam(typeck::MethodParam {
1526-
trait_ref: ref trait_ref,
1560+
ref trait_ref,
15271561
method_num: index,
15281562
..
15291563
}) |
15301564
typeck::MethodTraitObject(typeck::MethodObject {
1531-
trait_ref: ref trait_ref,
1565+
ref trait_ref,
15321566
method_num: index,
15331567
..
15341568
}) => {

‎src/librustc/lint/context.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ impl LintStore {
202202
NonUpperCaseGlobals,
203203
UnusedParens,
204204
UnusedImportBraces,
205+
NonShorthandFieldPatterns,
205206
UnusedUnsafe,
206207
UnsafeBlocks,
207208
UnusedMut,

‎src/librustc/middle/astencode.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,9 +1093,9 @@ impl<'a> rbml_writer_helpers for Encoder<'a> {
10931093
this.emit_enum_variant_arg(1, |this| idx.encode(this))
10941094
})
10951095
}
1096-
ty::UnsizeVtable(ty::TyTrait { def_id: def_id,
1096+
ty::UnsizeVtable(ty::TyTrait { def_id,
10971097
bounds: ref b,
1098-
substs: ref substs },
1098+
ref substs },
10991099
self_ty) => {
11001100
this.emit_enum_variant("UnsizeVtable", 2, 4, |this| {
11011101
this.emit_enum_variant_arg(

‎src/librustc/middle/cfg/construct.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
132132

133133
ast::PatStruct(_, ref subpats, _) => {
134134
let pats_exit =
135-
self.pats_all(subpats.iter().map(|f| &f.pat), pred);
135+
self.pats_all(subpats.iter().map(|f| &f.node.pat), pred);
136136
self.add_node(pat.id, [pats_exit])
137137
}
138138

‎src/librustc/middle/check_match.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -413,12 +413,16 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
413413
};
414414
if is_structure {
415415
let fields = ty::lookup_struct_fields(cx.tcx, vid);
416-
let field_pats: Vec<FieldPat> = fields.into_iter()
416+
let field_pats: Vec<Spanned<FieldPat>> = fields.into_iter()
417417
.zip(pats)
418418
.filter(|&(_, ref pat)| pat.node != PatWild(PatWildSingle))
419-
.map(|(field, pat)| FieldPat {
420-
ident: Ident::new(field.name),
421-
pat: pat
419+
.map(|(field, pat)| Spanned {
420+
span: DUMMY_SP,
421+
node: FieldPat {
422+
ident: Ident::new(field.name),
423+
pat: pat,
424+
is_shorthand: true,
425+
}
422426
}).collect();
423427
let has_more_fields = field_pats.len() < pats_len;
424428
PatStruct(def_to_path(cx.tcx, vid), field_pats, has_more_fields)
@@ -427,7 +431,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
427431
}
428432
}
429433

430-
ty::ty_rptr(_, ty::mt { ty: ty, .. }) => {
434+
ty::ty_rptr(_, ty::mt { ty, .. }) => {
431435
match ty::get(ty).sty {
432436
ty::ty_vec(_, Some(n)) => match ctor {
433437
&Single => {
@@ -495,7 +499,7 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: ty::t,
495499
ty::ty_nil =>
496500
vec!(ConstantValue(const_nil)),
497501

498-
ty::ty_rptr(_, ty::mt { ty: ty, .. }) => match ty::get(ty).sty {
502+
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty::get(ty).sty {
499503
ty::ty_vec(_, None) =>
500504
range_inclusive(0, max_slice_length).map(|length| Slice(length)).collect(),
501505
_ => vec!(Single)
@@ -692,7 +696,7 @@ pub fn constructor_arity(cx: &MatchCheckCtxt, ctor: &Constructor, ty: ty::t) ->
692696
match ty::get(ty).sty {
693697
ty::ty_tup(ref fs) => fs.len(),
694698
ty::ty_uniq(_) => 1u,
695-
ty::ty_rptr(_, ty::mt { ty: ty, .. }) => match ty::get(ty).sty {
699+
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty::get(ty).sty {
696700
ty::ty_vec(_, None) => match *ctor {
697701
Slice(length) => length,
698702
ConstantValue(_) => 0u,
@@ -740,7 +744,7 @@ fn range_covered_by_constructor(ctor: &Constructor,
740744
pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
741745
constructor: &Constructor, col: uint, arity: uint) -> Option<Vec<&'a Pat>> {
742746
let &Pat {
743-
id: pat_id, node: ref node, span: pat_span
747+
id: pat_id, ref node, span: pat_span
744748
} = raw_pat(r[col]);
745749
let head: Option<Vec<&Pat>> = match node {
746750

@@ -806,8 +810,8 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
806810
class_id.map(|variant_id| {
807811
let struct_fields = ty::lookup_struct_fields(cx.tcx, variant_id);
808812
let args = struct_fields.iter().map(|sf| {
809-
match pattern_fields.iter().find(|f| f.ident.name == sf.name) {
810-
Some(ref f) => &*f.pat,
813+
match pattern_fields.iter().find(|f| f.node.ident.name == sf.name) {
814+
Some(ref f) => &*f.node.pat,
811815
_ => DUMMY_WILD_PAT
812816
}
813817
}).collect();

‎src/librustc/middle/const_eval.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use syntax::parse::token::InternedString;
2525
use syntax::ptr::P;
2626
use syntax::visit::Visitor;
2727
use syntax::visit;
28-
use syntax::{ast, ast_map, ast_util};
28+
use syntax::{ast, ast_map, ast_util, codemap};
2929

3030
use std::rc::Rc;
3131
use std::collections::hashmap::Vacant;
@@ -115,7 +115,7 @@ fn lookup_variant_by_id<'a>(tcx: &'a ty::ctxt,
115115
match tcx.map.find(enum_def.node) {
116116
None => None,
117117
Some(ast_map::NodeItem(it)) => match it.node {
118-
ItemEnum(ast::EnumDef { variants: ref variants }, _) => {
118+
ItemEnum(ast::EnumDef { ref variants }, _) => {
119119
variant_expr(variants.as_slice(), variant_def.node)
120120
}
121121
_ => None
@@ -133,7 +133,7 @@ fn lookup_variant_by_id<'a>(tcx: &'a ty::ctxt,
133133
let expr_id = match csearch::maybe_get_item_ast(tcx, enum_def,
134134
|a, b, c, d| astencode::decode_inlined_item(a, b, c, d)) {
135135
csearch::found(&ast::IIItem(ref item)) => match item.node {
136-
ItemEnum(ast::EnumDef { variants: ref variants }, _) => {
136+
ItemEnum(ast::EnumDef { ref variants }, _) => {
137137
// NOTE this doesn't do the right thing, it compares inlined
138138
// NodeId's to the original variant_def's NodeId, but they
139139
// come from different crates, so they will likely never match.
@@ -336,9 +336,13 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P<Pat> {
336336
}
337337

338338
ExprStruct(ref path, ref fields, None) => {
339-
let field_pats = fields.iter().map(|field| FieldPat {
340-
ident: field.ident.node,
341-
pat: const_expr_to_pat(tcx, &*field.expr)
339+
let field_pats = fields.iter().map(|field| codemap::Spanned {
340+
span: codemap::DUMMY_SP,
341+
node: FieldPat {
342+
ident: field.ident.node,
343+
pat: const_expr_to_pat(tcx, &*field.expr),
344+
is_shorthand: true,
345+
},
342346
}).collect();
343347
PatStruct(path.clone(), field_pats, false)
344348
}

‎src/librustc/middle/dead.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,12 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
102102
}
103103
typeck::MethodStaticUnboxedClosure(_) => {}
104104
typeck::MethodTypeParam(typeck::MethodParam {
105-
trait_ref: ref trait_ref,
105+
ref trait_ref,
106106
method_num: index,
107107
..
108108
}) |
109109
typeck::MethodTraitObject(typeck::MethodObject {
110-
trait_ref: ref trait_ref,
110+
ref trait_ref,
111111
method_num: index,
112112
..
113113
}) => {
@@ -156,7 +156,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
156156
}
157157
}
158158

159-
fn handle_field_pattern_match(&mut self, lhs: &ast::Pat, pats: &[ast::FieldPat]) {
159+
fn handle_field_pattern_match(&mut self, lhs: &ast::Pat,
160+
pats: &[codemap::Spanned<ast::FieldPat>]) {
160161
let id = match (*self.tcx.def_map.borrow())[lhs.id] {
161162
def::DefVariant(_, id, _) => id,
162163
_ => {
@@ -174,7 +175,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
174175
let fields = ty::lookup_struct_fields(self.tcx, id);
175176
for pat in pats.iter() {
176177
let field_id = fields.iter()
177-
.find(|field| field.name == pat.ident.name).unwrap().id;
178+
.find(|field| field.name == pat.node.ident.name).unwrap().id;
178179
self.live_symbols.insert(field_id.node);
179180
}
180181
}

‎src/librustc/middle/expr_use_visitor.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ impl OverloadedCallType {
179179
MethodStaticUnboxedClosure(def_id) => {
180180
OverloadedCallType::from_unboxed_closure(tcx, def_id)
181181
}
182-
MethodTypeParam(MethodParam { trait_ref: ref trait_ref, .. }) |
183-
MethodTraitObject(MethodObject { trait_ref: ref trait_ref, .. }) => {
182+
MethodTypeParam(MethodParam { ref trait_ref, .. }) |
183+
MethodTraitObject(MethodObject { ref trait_ref, .. }) => {
184184
OverloadedCallType::from_trait_id(tcx, trait_ref.def_id)
185185
}
186186
}

‎src/librustc/middle/mem_categorization.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
433433

434434
ty::AdjustDerefRef(
435435
ty::AutoDerefRef {
436-
autoref: None, autoderefs: autoderefs}) => {
436+
autoref: None, autoderefs}) => {
437437
// Equivalent to *expr or something similar.
438438
self.cat_expr_autoderefd(expr, autoderefs)
439439
}
@@ -1222,9 +1222,9 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
12221222
ast::PatStruct(_, ref field_pats, _) => {
12231223
// {f1: p1, ..., fN: pN}
12241224
for fp in field_pats.iter() {
1225-
let field_ty = if_ok!(self.pat_ty(&*fp.pat)); // see (*2)
1226-
let cmt_field = self.cat_field(pat, cmt.clone(), fp.ident.name, field_ty);
1227-
if_ok!(self.cat_pattern(cmt_field, &*fp.pat, |x,y,z| op(x,y,z)));
1225+
let field_ty = if_ok!(self.pat_ty(&*fp.node.pat)); // see (*2)
1226+
let cmt_field = self.cat_field(pat, cmt.clone(), fp.node.ident.name, field_ty);
1227+
if_ok!(self.cat_pattern(cmt_field, &*fp.node.pat, |x,y,z| op(x,y,z)));
12281228
}
12291229
}
12301230

@@ -1524,7 +1524,7 @@ impl Repr for InteriorKind {
15241524

15251525
fn element_kind(t: ty::t) -> ElementKind {
15261526
match ty::get(t).sty {
1527-
ty::ty_rptr(_, ty::mt{ty:ty, ..}) |
1527+
ty::ty_rptr(_, ty::mt{ty, ..}) |
15281528
ty::ty_uniq(ty) => match ty::get(ty).sty {
15291529
ty::ty_vec(_, None) => VecElement,
15301530
_ => OtherElement

0 commit comments

Comments
 (0)