Silence some invalid type params in field errors

estebank · estebank · commit ce25b7cc06c4 · 2020-02-08T20:43:22.000-08:00
Reduce the verbosity of forgetting parentheses in methods with type
parameters by keeping that AST information until HIR type-checking.
diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs
@@ -129,7 +129,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 self.lower_expr(el),
                 self.lower_expr(er),
             ),
-            ExprKind::Field(ref el, ident) => hir::ExprKind::Field(self.lower_expr(el), ident),
+            ExprKind::Field(ref el, ident, args) => {
+                hir::ExprKind::Field(self.lower_expr(el), ident, args)
+            }
             ExprKind::Index(ref el, ref er) => {
                 hir::ExprKind::Index(self.lower_expr(el), self.lower_expr(er))
             }
diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs
@@ -2025,7 +2025,7 @@ impl<'a> State<'a> {
                 self.word_space("=");
                 self.print_expr_maybe_paren(rhs, prec);
             }
-            ast::ExprKind::Field(ref expr, ident) => {
+            ast::ExprKind::Field(ref expr, ident, _) => {
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
                 self.s.word(".");
                 self.print_ident(ident);
diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs
@@ -1396,7 +1396,7 @@ impl Expr<'_> {
 
             ExprKind::Unary(UnOp::UnDeref, _) => true,
 
-            ExprKind::Field(ref base, _) | ExprKind::Index(ref base, _) => {
+            ExprKind::Field(ref base, ..) | ExprKind::Index(ref base, _) => {
                 allow_projections_from(base) || base.is_place_expr(allow_projections_from)
             }
 
@@ -1588,7 +1588,8 @@ pub enum ExprKind<'hir> {
     /// E.g., `a += 1`.
     AssignOp(BinOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct or tuple field.
-    Field(&'hir Expr<'hir>, Ident),
+    /// The optional `Span` points at an *invalid* type parameter in the field expression.
+    Field(&'hir Expr<'hir>, Ident, Option<Span>),
     /// An indexing operation (`foo[2]`).
     Index(&'hir Expr<'hir>, &'hir Expr<'hir>),
 
diff --git a/src/librustc_hir/intravisit.rs b/src/librustc_hir/intravisit.rs
@@ -1110,7 +1110,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
             visitor.visit_expr(right_expression);
             visitor.visit_expr(left_expression);
         }
-        ExprKind::Field(ref subexpression, ident) => {
+        ExprKind::Field(ref subexpression, ident, _) => {
             visitor.visit_expr(subexpression);
             visitor.visit_ident(ident);
         }
diff --git a/src/librustc_hir/print.rs b/src/librustc_hir/print.rs
@@ -1344,7 +1344,7 @@ impl<'a> State<'a> {
                 self.word_space("=");
                 self.print_expr_maybe_paren(&rhs, prec);
             }
-            hir::ExprKind::Field(ref expr, ident) => {
+            hir::ExprKind::Field(ref expr, ident, _) => {
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
                 self.s.word(".");
                 self.print_ident(ident);
@@ -2315,7 +2315,7 @@ fn contains_exterior_struct_lit(value: &hir::Expr<'_>) -> bool {
         hir::ExprKind::Unary(_, ref x)
         | hir::ExprKind::Cast(ref x, _)
         | hir::ExprKind::Type(ref x, _)
-        | hir::ExprKind::Field(ref x, _)
+        | hir::ExprKind::Field(ref x, ..)
         | hir::ExprKind::Index(ref x, _) => {
             // `&X { y: 1 }, X { y: 1 }.y`
             contains_exterior_struct_lit(&x)
diff --git a/src/librustc_parse/parser/expr.rs b/src/librustc_parse/parser/expr.rs
@@ -786,7 +786,7 @@ impl<'a> Parser<'a> {
     ) -> P<Expr> {
         let span = self.token.span;
         self.bump();
-        let field = ExprKind::Field(base, Ident::new(field, span));
+        let field = ExprKind::Field(base, Ident::new(field, span), None);
         self.expect_no_suffix(span, "a tuple index", suffix);
         self.mk_expr(lo.to(span), field, AttrVec::new())
     }
@@ -825,16 +825,12 @@ impl<'a> Parser<'a> {
             Ok(self.mk_expr(span, ExprKind::MethodCall(segment, args), AttrVec::new()))
         } else {
             // Field access `expr.f`
-            if let Some(args) = segment.args {
-                self.struct_span_err(
-                    args.span(),
-                    "field expressions may not have generic arguments",
-                )
-                .emit();
-            }
-
             let span = lo.to(self.prev_span);
-            Ok(self.mk_expr(span, ExprKind::Field(self_arg, segment.ident), AttrVec::new()))
+            Ok(self.mk_expr(
+                span,
+                ExprKind::Field(self_arg, segment.ident, segment.args.map(|args| args.span())),
+                AttrVec::new(),
+            ))
         }
     }
 
diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs
@@ -984,7 +984,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
                 self.access_path(expr.hir_id, path, succ, ACC_READ | ACC_USE)
             }
 
-            hir::ExprKind::Field(ref e, _) => self.propagate_through_expr(&e, succ),
+            hir::ExprKind::Field(ref e, ..) => self.propagate_through_expr(&e, succ),
 
             hir::ExprKind::Closure(..) => {
                 debug!(
@@ -1254,7 +1254,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
 
         match expr.kind {
             hir::ExprKind::Path(_) => succ,
-            hir::ExprKind::Field(ref e, _) => self.propagate_through_expr(&e, succ),
+            hir::ExprKind::Field(ref e, ..) => self.propagate_through_expr(&e, succ),
             _ => self.propagate_through_expr(expr, succ),
         }
     }
diff --git a/src/librustc_passes/region.rs b/src/librustc_passes/region.rs
@@ -653,7 +653,7 @@ fn resolve_local<'tcx>(
             match expr.kind {
                 hir::ExprKind::AddrOf(_, _, ref subexpr)
                 | hir::ExprKind::Unary(hir::UnOp::UnDeref, ref subexpr)
-                | hir::ExprKind::Field(ref subexpr, _)
+                | hir::ExprKind::Field(ref subexpr, ..)
                 | hir::ExprKind::Index(ref subexpr, _) => {
                     expr = &subexpr;
                 }
diff --git a/src/librustc_resolve/late.rs b/src/librustc_resolve/late.rs
@@ -2004,7 +2004,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             ExprKind::Block(ref block, label) => self.resolve_labeled_block(label, block.id, block),
 
             // Equivalent to `visit::walk_expr` + passing some context to children.
-            ExprKind::Field(ref subexpression, _) => {
+            ExprKind::Field(ref subexpression, ..) => {
                 self.resolve_expr(subexpression, Some(expr));
             }
             ExprKind::MethodCall(ref segment, ref arguments) => {
@@ -2055,7 +2055,7 @@ impl<'a, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
     fn record_candidate_traits_for_expr_if_necessary(&mut self, expr: &'ast Expr) {
         match expr.kind {
-            ExprKind::Field(_, ident) => {
+            ExprKind::Field(_, ident, _) => {
                 // FIXME(#6890): Even though you can't treat a method like a
                 // field, we need to add any trait methods we find that match
                 // the field name so that we can do some nice error reporting
diff --git a/src/librustc_resolve/late/diagnostics.rs b/src/librustc_resolve/late/diagnostics.rs
@@ -415,7 +415,7 @@ impl<'a> LateResolutionVisitor<'a, '_, '_> {
         let is_expected = &|res| source.is_expected(res);
 
         let path_sep = |err: &mut DiagnosticBuilder<'_>, expr: &Expr| match expr.kind {
-            ExprKind::Field(_, ident) => {
+            ExprKind::Field(_, ident, _) => {
                 err.span_suggestion(
                     expr.span,
                     "use the path separator to refer to an item",
diff --git a/src/librustc_save_analysis/dump_visitor.rs b/src/librustc_save_analysis/dump_visitor.rs
@@ -1431,7 +1431,7 @@ impl<'l, 'tcx> Visitor<'l> for DumpVisitor<'l, 'tcx> {
                 self.process_struct_lit(ex, path, fields, adt.variant_of_res(res), base)
             }
             ast::ExprKind::MethodCall(ref seg, ref args) => self.process_method_call(ex, seg, args),
-            ast::ExprKind::Field(ref sub_ex, _) => {
+            ast::ExprKind::Field(ref sub_ex, ..) => {
                 self.visit_expr(&sub_ex);
 
                 if let Some(field_data) = self.save_ctxt.get_expr_data(ex) {
diff --git a/src/librustc_save_analysis/lib.rs b/src/librustc_save_analysis/lib.rs
@@ -514,7 +514,7 @@ impl<'l, 'tcx> SaveContext<'l, 'tcx> {
             return None;
         }
         match expr.kind {
-            ast::ExprKind::Field(ref sub_ex, ident) => {
+            ast::ExprKind::Field(ref sub_ex, ident, _) => {
                 let sub_ex_hir_id = self.tcx.hir().node_to_hir_id(sub_ex.id);
                 let hir_node = match self.tcx.hir().find(sub_ex_hir_id) {
                     Some(Node::Expr(expr)) => expr,
diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs
@@ -277,7 +277,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             ExprKind::Struct(ref qpath, fields, ref base_expr) => {
                 self.check_expr_struct(expr, expected, qpath, fields, base_expr)
             }
-            ExprKind::Field(ref base, field) => self.check_field(expr, needs, &base, field),
+            ExprKind::Field(ref base, field, _) => self.check_field(expr, needs, &base, field),
             ExprKind::Index(ref base, ref idx) => self.check_expr_index(base, idx, needs, expr),
             ExprKind::Yield(ref value, ref src) => self.check_expr_yield(value, expr, src),
             hir::ExprKind::Err => tcx.types.err,
@@ -1440,6 +1440,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         display
     }
 
+    fn complain_about_invalid_type_params(&self, args: Option<Span>) {
+        if let Some(args) = args {
+            self.tcx
+                .sess
+                .struct_span_err(args, "field expressions may not have generic arguments")
+                .emit();
+        }
+    }
+
     // Check field access expressions
     fn check_field(
         &self,
@@ -1452,6 +1461,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         let expr_t = self.structurally_resolved_type(base.span, expr_t);
         let mut private_candidate = None;
         let mut autoderef = self.autoderef(expr.span, expr_t);
+        let invalid_args = match &expr.kind {
+            ExprKind::Field(_, _, Some(args)) => Some(*args),
+            _ => None,
+        };
         while let Some((base_t, _)) = autoderef.next() {
             match base_t.kind {
                 ty::Adt(base_def, substs) if !base_def.is_enum() => {
@@ -1471,6 +1484,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                             autoderef.finalize(self);
 
                             self.tcx.check_stability(field.did, Some(expr.hir_id), expr.span);
+                            self.complain_about_invalid_type_params(invalid_args);
                             return field_ty;
                         }
                         private_candidate = Some((base_def.did, field_ty));
@@ -1486,6 +1500,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 autoderef.finalize(self);
 
                                 self.write_field_index(expr.hir_id, index);
+                                self.complain_about_invalid_type_params(invalid_args);
                                 return field_ty.expect_ty();
                             }
                         }
@@ -1501,8 +1516,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             return field_ty;
         }
 
+        let method_exists = self.method_exists(field, expr_t, expr.hir_id, true);
+        if let (false, ExprKind::Field(_, _, Some(_))) = (method_exists, &expr.kind) {
+            // Only complain about incorrect field expressions with type parameters like
+            // `foo.bar::<baz>` when `bar` isn't a valid method.
+            self.complain_about_invalid_type_params(invalid_args);
+        }
         if field.name == kw::Invalid {
-        } else if self.method_exists(field, expr_t, expr.hir_id, true) {
+        } else if method_exists {
             self.ban_take_value_of_method(expr, expr_t, field);
         } else if !expr_t.is_primitive_ty() {
             self.ban_nonexisting_field(field, base, expr, expr_t);
diff --git a/src/librustc_typeck/check/method/confirm.rs b/src/librustc_typeck/check/method/confirm.rs
@@ -423,7 +423,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
 
         loop {
             match exprs.last().unwrap().kind {
-                hir::ExprKind::Field(ref expr, _)
+                hir::ExprKind::Field(ref expr, ..)
                 | hir::ExprKind::Index(ref expr, _)
                 | hir::ExprKind::Unary(hir::UnOp::UnDeref, ref expr) => exprs.push(&expr),
                 _ => break,
diff --git a/src/librustc_typeck/expr_use_visitor.rs b/src/librustc_typeck/expr_use_visitor.rs
@@ -168,7 +168,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
                 self.select_from_expr(base);
             }
 
-            hir::ExprKind::Field(ref base, _) => {
+            hir::ExprKind::Field(ref base, ..) => {
                 // base.f
                 self.select_from_expr(base);
             }
diff --git a/src/librustc_typeck/mem_categorization.rs b/src/librustc_typeck/mem_categorization.rs
@@ -354,7 +354,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> {
                 }
             }
 
-            hir::ExprKind::Field(ref base, _) => {
+            hir::ExprKind::Field(ref base, ..) => {
                 let base = self.cat_expr(&base)?;
                 debug!("cat_expr(cat_field): id={} expr={:?} base={:?}", expr.hir_id, expr, base);
                 Ok(self.cat_projection(expr, base, expr_ty))
diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs
@@ -1224,7 +1224,8 @@ pub enum ExprKind {
     /// E.g., `a += 1`.
     AssignOp(BinOp, P<Expr>, P<Expr>),
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field.
-    Field(P<Expr>, Ident),
+    /// The optional `Span` points at an *invalid* type parameter in the field expression.
+    Field(P<Expr>, Ident, Option<Span>),
     /// An indexing operation (e.g., `foo[2]`).
     Index(P<Expr>, P<Expr>),
     /// A range (e.g., `1..2`, `1..`, `..2`, `1..=2`, `..=2`).
diff --git a/src/libsyntax/mut_visit.rs b/src/libsyntax/mut_visit.rs
@@ -1187,7 +1187,7 @@ pub fn noop_visit_expr<T: MutVisitor>(Expr { kind, id, span, attrs }: &mut Expr,
             vis.visit_expr(el);
             vis.visit_expr(er);
         }
-        ExprKind::Field(el, ident) => {
+        ExprKind::Field(el, ident, _) => {
             vis.visit_expr(el);
             vis.visit_ident(ident);
         }
diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs
@@ -388,7 +388,7 @@ pub fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
         | ast::ExprKind::Unary(_, ref x)
         | ast::ExprKind::Cast(ref x, _)
         | ast::ExprKind::Type(ref x, _)
-        | ast::ExprKind::Field(ref x, _)
+        | ast::ExprKind::Field(ref x, ..)
         | ast::ExprKind::Index(ref x, _) => {
             // &X { y: 1 }, X { y: 1 }.y
             contains_exterior_struct_lit(&x)
diff --git a/src/libsyntax/visit.rs b/src/libsyntax/visit.rs
@@ -773,7 +773,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
             visitor.visit_expr(left_expression);
             visitor.visit_expr(right_expression);
         }
-        ExprKind::Field(ref subexpression, ident) => {
+        ExprKind::Field(ref subexpression, ident, _) => {
             visitor.visit_expr(subexpression);
             visitor.visit_ident(ident);
         }
diff --git a/src/test/ui-fulldeps/pprust-expr-roundtrip.rs b/src/test/ui-fulldeps/pprust-expr-roundtrip.rs
@@ -132,7 +132,7 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P<Expr>)) {
                 iter_exprs(depth - 1, &mut |e| g(ExprKind::Assign(make_x(), e, DUMMY_SP)));
             },
             13 => {
-                iter_exprs(depth - 1, &mut |e| g(ExprKind::Field(e, Ident::from_str("f"))));
+                iter_exprs(depth - 1, &mut |e| g(ExprKind::Field(e, Ident::from_str("f"), None)));
             },
             14 => {
                 iter_exprs(depth - 1, &mut |e| g(ExprKind::Range(
diff --git a/src/test/ui/suggestions/method-missing-parentheses.rs b/src/test/ui/suggestions/method-missing-parentheses.rs
@@ -1,5 +1,4 @@
 fn main() {
     let _ = vec![].into_iter().collect::<usize>;
     //~^ ERROR attempted to take value of method `collect` on type `std::vec::IntoIter<_>`
-    //~| ERROR field expressions may not have generic arguments
 }
diff --git a/src/test/ui/suggestions/method-missing-parentheses.stderr b/src/test/ui/suggestions/method-missing-parentheses.stderr
@@ -1,9 +1,3 @@
-error: field expressions may not have generic arguments
-  --> $DIR/method-missing-parentheses.rs:2:41
-   |
-LL |     let _ = vec![].into_iter().collect::<usize>;
-   |                                         ^^^^^^^
-
 error[E0615]: attempted to take value of method `collect` on type `std::vec::IntoIter<_>`
   --> $DIR/method-missing-parentheses.rs:2:32
    |
@@ -12,6 +6,6 @@ LL |     let _ = vec![].into_iter().collect::<usize>;
    |                                |
    |                                help: use parentheses to call the method: `collect::<usize>()`
 
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 
 For more information about this error, try `rustc --explain E0615`.

Original file line number	Diff line number	Diff line change
`@@ -2025,7 +2025,7 @@ impl<'a> State<'a> {`
`2025`	`2025`	`self.word_space("=");`
`2026`	`2026`	`self.print_expr_maybe_paren(rhs, prec);`
`2027`	`2027`	`}`
`2028`		`- ast::ExprKind::Field(ref expr, ident) => {`
	`2028`	`+ ast::ExprKind::Field(ref expr, ident, _) => {`
`2029`	`2029`	`self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);`
`2030`	`2030`	`self.s.word(".");`
`2031`	`2031`	`self.print_ident(ident);`
Original file line number	Diff line number	Diff line change
`@@ -1110,7 +1110,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)`
`1110`	`1110`	`visitor.visit_expr(right_expression);`
`1111`	`1111`	`visitor.visit_expr(left_expression);`
`1112`	`1112`	`}`
`1113`		`- ExprKind::Field(ref subexpression, ident) => {`
	`1113`	`+ ExprKind::Field(ref subexpression, ident, _) => {`
`1114`	`1114`	`visitor.visit_expr(subexpression);`
`1115`	`1115`	`visitor.visit_ident(ident);`
`1116`	`1116`	`}`
Original file line number	Diff line number	Diff line change
`@@ -984,7 +984,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {`
`984`	`984`	`self.access_path(expr.hir_id, path, succ, ACC_READ \| ACC_USE)`
`985`	`985`	`}`
`986`	`986`
`987`		`- hir::ExprKind::Field(ref e, _) => self.propagate_through_expr(&e, succ),`
	`987`	`+ hir::ExprKind::Field(ref e, ..) => self.propagate_through_expr(&e, succ),`
`988`	`988`
`989`	`989`	`hir::ExprKind::Closure(..) => {`
`990`	`990`	`debug!(`
`@@ -1254,7 +1254,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {`
`1254`	`1254`
`1255`	`1255`	`match expr.kind {`
`1256`	`1256`	`hir::ExprKind::Path(_) => succ,`
`1257`		`- hir::ExprKind::Field(ref e, _) => self.propagate_through_expr(&e, succ),`
	`1257`	`+ hir::ExprKind::Field(ref e, ..) => self.propagate_through_expr(&e, succ),`
`1258`	`1258`	`_ => self.propagate_through_expr(expr, succ),`
`1259`	`1259`	`}`
`1260`	`1260`	`}`
Original file line number	Diff line number	Diff line change
`@@ -653,7 +653,7 @@ fn resolve_local<'tcx>(`
`653`	`653`	`match expr.kind {`
`654`	`654`	`hir::ExprKind::AddrOf(_, _, ref subexpr)`
`655`	`655`	`\| hir::ExprKind::Unary(hir::UnOp::UnDeref, ref subexpr)`
`656`		`- \| hir::ExprKind::Field(ref subexpr, _)`
	`656`	`+ \| hir::ExprKind::Field(ref subexpr, ..)`
`657`	`657`	`\| hir::ExprKind::Index(ref subexpr, _) => {`
`658`	`658`	`expr = &subexpr;`
`659`	`659`	`}`