Skip to content

Add a lint for not using field pattern shorthands #17813

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 1 commit into from
Oct 24, 2014
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
3 changes: 3 additions & 0 deletions src/doc/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -3877,6 +3877,7 @@ match x {
If you have a struct, you can destructure it inside of a pattern:

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

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

```{rust}
# #![allow(non_shorthand_field_patterns)]
struct Circle {
x: f64,
y: f64,
Expand Down
12 changes: 6 additions & 6 deletions src/libcollections/treemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl<K: Ord, V> TreeMap<K, V> {
/// assert_eq!(vec, vec![("a", 1), ("b", 2), ("c", 3)]);
/// ```
pub fn into_iter(self) -> MoveEntries<K, V> {
let TreeMap { root: root, length: length } = self;
let TreeMap { root, length } = self;
let stk = match root {
None => vec!(),
Some(box tn) => vec!(tn)
Expand Down Expand Up @@ -898,11 +898,11 @@ impl<K, V> Iterator<(K, V)> for MoveEntries<K,V> {
fn next(&mut self) -> Option<(K, V)> {
while !self.stack.is_empty() {
let TreeNode {
key: key,
value: value,
left: left,
right: right,
level: level
key,
value,
left,
right,
level,
} = self.stack.pop().unwrap();

match left {
Expand Down
18 changes: 9 additions & 9 deletions src/libgetopts/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,10 @@ impl OptGroup {
/// (Both short and long names correspond to different Opts).
pub fn long_to_short(&self) -> Opt {
let OptGroup {
short_name: short_name,
long_name: long_name,
hasarg: hasarg,
occur: occur,
short_name,
long_name,
hasarg,
occur,
..
} = (*self).clone();

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

let mut rows = opts.iter().map(|optref| {
let OptGroup{short_name: short_name,
long_name: long_name,
hint: hint,
desc: desc,
hasarg: hasarg,
let OptGroup{short_name,
long_name,
hint,
desc,
hasarg,
..} = (*optref).clone();

let mut row = " ".repeat(4);
Expand Down
4 changes: 2 additions & 2 deletions src/libgreen/sched.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ impl Scheduler {
mem::transmute(&**next_task.sched.as_mut().unwrap());

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

Expand Down Expand Up @@ -953,7 +953,7 @@ impl CleanupJob {
}

pub fn run(self, sched: &mut Scheduler) {
let CleanupJob { task: task, f: f } = self;
let CleanupJob { task, f } = self;
f.to_fn()(sched, task)
}
}
Expand Down
38 changes: 36 additions & 2 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,40 @@ impl LintPass for UnusedImportBraces {
}
}

declare_lint!(NON_SHORTHAND_FIELD_PATTERNS, Warn,
"using `Struct { x: x }` instead of `Struct { x }`")

pub struct NonShorthandFieldPatterns;

impl LintPass for NonShorthandFieldPatterns {
fn get_lints(&self) -> LintArray {
lint_array!(NON_SHORTHAND_FIELD_PATTERNS)
}

fn check_pat(&mut self, cx: &Context, pat: &ast::Pat) {
let def_map = cx.tcx.def_map.borrow();
match pat.node {
ast::PatStruct(_, ref v, _) => {
for fieldpat in v.iter()
.filter(|fieldpat| !fieldpat.node.is_shorthand)
.filter(|fieldpat| def_map.find(&fieldpat.node.pat.id)
== Some(&def::DefLocal(fieldpat.node.pat.id))) {
match fieldpat.node.pat.node {
ast::PatIdent(_, ident, None) if ident.node.as_str()
== fieldpat.node.ident.as_str() => {
cx.span_lint(NON_SHORTHAND_FIELD_PATTERNS, fieldpat.span,
format!("the `{}:` in this pattern is redundant and can \
be removed", ident.node.as_str()).as_slice())
},
_ => {},
}
}
},
_ => {}
}
}
}

declare_lint!(pub UNUSED_UNSAFE, Warn,
"unnecessary use of an `unsafe` block")

Expand Down Expand Up @@ -1523,12 +1557,12 @@ impl LintPass for Stability {
def_id
}
typeck::MethodTypeParam(typeck::MethodParam {
trait_ref: ref trait_ref,
ref trait_ref,
method_num: index,
..
}) |
typeck::MethodTraitObject(typeck::MethodObject {
trait_ref: ref trait_ref,
ref trait_ref,
method_num: index,
..
}) => {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ impl LintStore {
NonUpperCaseGlobals,
UnusedParens,
UnusedImportBraces,
NonShorthandFieldPatterns,
UnusedUnsafe,
UnsafeBlocks,
UnusedMut,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/astencode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,9 +1093,9 @@ impl<'a> rbml_writer_helpers for Encoder<'a> {
this.emit_enum_variant_arg(1, |this| idx.encode(this))
})
}
ty::UnsizeVtable(ty::TyTrait { def_id: def_id,
ty::UnsizeVtable(ty::TyTrait { def_id,
bounds: ref b,
substs: ref substs },
ref substs },
self_ty) => {
this.emit_enum_variant("UnsizeVtable", 2, 4, |this| {
this.emit_enum_variant_arg(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {

ast::PatStruct(_, ref subpats, _) => {
let pats_exit =
self.pats_all(subpats.iter().map(|f| &f.pat), pred);
self.pats_all(subpats.iter().map(|f| &f.node.pat), pred);
self.add_node(pat.id, [pats_exit])
}

Expand Down
24 changes: 14 additions & 10 deletions src/librustc/middle/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,16 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
};
if is_structure {
let fields = ty::lookup_struct_fields(cx.tcx, vid);
let field_pats: Vec<FieldPat> = fields.into_iter()
let field_pats: Vec<Spanned<FieldPat>> = fields.into_iter()
.zip(pats)
.filter(|&(_, ref pat)| pat.node != PatWild(PatWildSingle))
.map(|(field, pat)| FieldPat {
ident: Ident::new(field.name),
pat: pat
.map(|(field, pat)| Spanned {
span: DUMMY_SP,
node: FieldPat {
ident: Ident::new(field.name),
pat: pat,
is_shorthand: true,
}
}).collect();
let has_more_fields = field_pats.len() < pats_len;
PatStruct(def_to_path(cx.tcx, vid), field_pats, has_more_fields)
Expand All @@ -427,7 +431,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
}
}

ty::ty_rptr(_, ty::mt { ty: ty, .. }) => {
ty::ty_rptr(_, ty::mt { ty, .. }) => {
match ty::get(ty).sty {
ty::ty_vec(_, Some(n)) => match ctor {
&Single => {
Expand Down Expand Up @@ -495,7 +499,7 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: ty::t,
ty::ty_nil =>
vec!(ConstantValue(const_nil)),

ty::ty_rptr(_, ty::mt { ty: ty, .. }) => match ty::get(ty).sty {
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty::get(ty).sty {
ty::ty_vec(_, None) =>
range_inclusive(0, max_slice_length).map(|length| Slice(length)).collect(),
_ => vec!(Single)
Expand Down Expand Up @@ -692,7 +696,7 @@ pub fn constructor_arity(cx: &MatchCheckCtxt, ctor: &Constructor, ty: ty::t) ->
match ty::get(ty).sty {
ty::ty_tup(ref fs) => fs.len(),
ty::ty_uniq(_) => 1u,
ty::ty_rptr(_, ty::mt { ty: ty, .. }) => match ty::get(ty).sty {
ty::ty_rptr(_, ty::mt { ty, .. }) => match ty::get(ty).sty {
ty::ty_vec(_, None) => match *ctor {
Slice(length) => length,
ConstantValue(_) => 0u,
Expand Down Expand Up @@ -740,7 +744,7 @@ fn range_covered_by_constructor(ctor: &Constructor,
pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
constructor: &Constructor, col: uint, arity: uint) -> Option<Vec<&'a Pat>> {
let &Pat {
id: pat_id, node: ref node, span: pat_span
id: pat_id, ref node, span: pat_span
} = raw_pat(r[col]);
let head: Option<Vec<&Pat>> = match node {

Expand Down Expand Up @@ -806,8 +810,8 @@ pub fn specialize<'a>(cx: &MatchCheckCtxt, r: &[&'a Pat],
class_id.map(|variant_id| {
let struct_fields = ty::lookup_struct_fields(cx.tcx, variant_id);
let args = struct_fields.iter().map(|sf| {
match pattern_fields.iter().find(|f| f.ident.name == sf.name) {
Some(ref f) => &*f.pat,
match pattern_fields.iter().find(|f| f.node.ident.name == sf.name) {
Some(ref f) => &*f.node.pat,
_ => DUMMY_WILD_PAT
}
}).collect();
Expand Down
16 changes: 10 additions & 6 deletions src/librustc/middle/const_eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use syntax::parse::token::InternedString;
use syntax::ptr::P;
use syntax::visit::Visitor;
use syntax::visit;
use syntax::{ast, ast_map, ast_util};
use syntax::{ast, ast_map, ast_util, codemap};

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

ExprStruct(ref path, ref fields, None) => {
let field_pats = fields.iter().map(|field| FieldPat {
ident: field.ident.node,
pat: const_expr_to_pat(tcx, &*field.expr)
let field_pats = fields.iter().map(|field| codemap::Spanned {
span: codemap::DUMMY_SP,
node: FieldPat {
ident: field.ident.node,
pat: const_expr_to_pat(tcx, &*field.expr),
is_shorthand: true,
},
}).collect();
PatStruct(path.clone(), field_pats, false)
}
Expand Down
9 changes: 5 additions & 4 deletions src/librustc/middle/dead.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,12 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
}
typeck::MethodStaticUnboxedClosure(_) => {}
typeck::MethodTypeParam(typeck::MethodParam {
trait_ref: ref trait_ref,
ref trait_ref,
method_num: index,
..
}) |
typeck::MethodTraitObject(typeck::MethodObject {
trait_ref: ref trait_ref,
ref trait_ref,
method_num: index,
..
}) => {
Expand Down Expand Up @@ -156,7 +156,8 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
}
}

fn handle_field_pattern_match(&mut self, lhs: &ast::Pat, pats: &[ast::FieldPat]) {
fn handle_field_pattern_match(&mut self, lhs: &ast::Pat,
pats: &[codemap::Spanned<ast::FieldPat>]) {
let id = match (*self.tcx.def_map.borrow())[lhs.id] {
def::DefVariant(_, id, _) => id,
_ => {
Expand All @@ -174,7 +175,7 @@ impl<'a, 'tcx> MarkSymbolVisitor<'a, 'tcx> {
let fields = ty::lookup_struct_fields(self.tcx, id);
for pat in pats.iter() {
let field_id = fields.iter()
.find(|field| field.name == pat.ident.name).unwrap().id;
.find(|field| field.name == pat.node.ident.name).unwrap().id;
self.live_symbols.insert(field_id.node);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ impl OverloadedCallType {
MethodStaticUnboxedClosure(def_id) => {
OverloadedCallType::from_unboxed_closure(tcx, def_id)
}
MethodTypeParam(MethodParam { trait_ref: ref trait_ref, .. }) |
MethodTraitObject(MethodObject { trait_ref: ref trait_ref, .. }) => {
MethodTypeParam(MethodParam { ref trait_ref, .. }) |
MethodTraitObject(MethodObject { ref trait_ref, .. }) => {
OverloadedCallType::from_trait_id(tcx, trait_ref.def_id)
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {

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

Expand Down Expand Up @@ -1524,7 +1524,7 @@ impl Repr for InteriorKind {

fn element_kind(t: ty::t) -> ElementKind {
match ty::get(t).sty {
ty::ty_rptr(_, ty::mt{ty:ty, ..}) |
ty::ty_rptr(_, ty::mt{ty, ..}) |
ty::ty_uniq(ty) => match ty::get(ty).sty {
ty::ty_vec(_, None) => VecElement,
_ => OtherElement
Expand Down
Loading