Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Be more precise when suggesting removal of parens on unit ctor #99350

Merged
merged 2 commits into from
Jul 18, 2022
Merged

Be more precise when suggesting removal of parens on unit ctor #99350

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04c590b
Be more precise when suggesting removal of parens on unit adt ctor
compiler-errors Jul 16, 2022
7a45a60
use rustc_hir_pretty::qpath_to_string to avoid span_to_snippet when r…
compiler-errors Jul 17, 2022
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
36 changes: 21 additions & 15 deletions compiler/rustc_typeck/src/check/callee.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::type_error_struct;

use rustc_errors::{struct_span_err, Applicability, Diagnostic};
use rustc_hir as hir;
use rustc_hir::def::{Namespace, Res};
use rustc_hir::def::{self, Namespace, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_infer::{
infer,
Expand Down Expand Up @@ -390,17 +390,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(fn_sig, Some(def_id))
}
ty::FnPtr(sig) => (sig, None),
ref t => {
_ => {
let mut unit_variant = None;
let mut removal_span = call_expr.span;
if let ty::Adt(adt_def, ..) = t
&& adt_def.is_enum()
&& let hir::ExprKind::Call(expr, _) = call_expr.kind
if let hir::ExprKind::Path(qpath) = &callee_expr.kind
&& let Res::Def(def::DefKind::Ctor(kind, def::CtorKind::Const), _)
= self.typeck_results.borrow().qpath_res(qpath, callee_expr.hir_id)
// Only suggest removing parens if there are no arguments
&& arg_exprs.is_empty()
&& let Ok(path) = self.tcx.sess.source_map().span_to_snippet(callee_expr.span)
Copy link
Member Author

Choose a reason for hiding this comment

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

I would like to use tcx.def_path_str instead of span_to_snippet, but that does not preserve aliases, and causes regressions in src/test/ui/type-alias-enum-variants/incorrect-variant-form-through-alias-caught.rs by changing Alias::Unit to Enum::Unit 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

I could possibly find a way to format qpath into a string by joining all of its segments, but that's a bit overkill and this PR does not represent a regression in the status quo.

Copy link
Member

@fee1-dead fee1-dead Jul 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It wouldn't work well with improperly formatted code, right? If the path is a macro expansion what would happen..? To me I don't think it should preserve the alias path.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, you're right. If we had like

path::
split::
up::
over_many_lines

then we shouldn't use that in diagnostics.

Copy link
Member Author

@compiler-errors compiler-errors Jul 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I just used rustc_hir_pretty which does hir -> string for us. Avoids weirdness like:

enum Enum { Braced {} }
type Alias = Enum;

fn main() {
    Alias::
    Braced;
}
error[[E0533]](https://doc.rust-lang.org/stable/error-index.html#E0533): expected unit struct, unit variant or constant, found struct variant `Alias::
                  Braced`
 --> src/main.rs:5:5
  |
5 | /     Alias::
6 | |     Braced;
  | |__________^

Copy link
Member

Choose a reason for hiding this comment

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

Nice, please add that as a test.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

{
removal_span =
expr.span.shrink_to_hi().to(call_expr.span.shrink_to_hi());
let descr = match kind {
def::CtorOf::Struct => "struct",
def::CtorOf::Variant => "enum variant",
};
let removal_span =
callee_expr.span.shrink_to_hi().to(call_expr.span.shrink_to_hi());
unit_variant =
self.tcx.sess.source_map().span_to_snippet(expr.span).ok();
Some((removal_span, descr, path));
}

let callee_ty = self.resolve_vars_if_possible(callee_ty);
Expand All @@ -410,8 +416,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
callee_ty,
E0618,
"expected function, found {}",
match unit_variant {
Some(ref path) => format!("enum variant `{path}`"),
match &unit_variant {
Some((_, kind, path)) => format!("{kind} `{path}`"),
None => format!("`{callee_ty}`"),
}
);
Expand All @@ -423,11 +429,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
callee_expr.span,
);

if let Some(ref path) = unit_variant {
if let Some((removal_span, kind, path)) = &unit_variant {
err.span_suggestion_verbose(
removal_span,
*removal_span,
&format!(
"`{path}` is a unit variant, you need to write it without the parentheses",
"`{path}` is a unit {kind}, and does not take parentheses to be constructed",
),
"",
Applicability::MachineApplicable,
Expand Down Expand Up @@ -470,7 +476,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(span) = self.tcx.hir().res_span(def) {
let callee_ty = callee_ty.to_string();
let label = match (unit_variant, inner_callee_path) {
(Some(path), _) => Some(format!("`{path}` defined here")),
(Some((_, kind, path)), _) => Some(format!("{kind} `{path}` defined here")),
(_, Some(hir::QPath::Resolved(_, path))) => self
.tcx
.sess
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/empty/empty-struct-unit-expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ enum E {
}

fn main() {
let e2 = Empty2(); //~ ERROR expected function, found `Empty2`
let e2 = Empty2(); //~ ERROR expected function, found struct `Empty2`
let e4 = E::Empty4();
//~^ ERROR expected function, found enum variant `E::Empty4` [E0618]
let xe2 = XEmpty2(); //~ ERROR expected function, found `empty_struct::XEmpty2`
let xe2 = XEmpty2(); //~ ERROR expected function, found struct `XEmpty2`
let xe4 = XE::XEmpty4();
//~^ ERROR expected function, found enum variant `XE::XEmpty4` [E0618]
}
24 changes: 18 additions & 6 deletions src/test/ui/empty/empty-struct-unit-expr.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,38 +1,50 @@
error[E0618]: expected function, found `Empty2`
error[E0618]: expected function, found struct `Empty2`
--> $DIR/empty-struct-unit-expr.rs:15:14
|
LL | struct Empty2;
| ------------- `Empty2` defined here
| ------------- struct `Empty2` defined here
...
LL | let e2 = Empty2();
| ^^^^^^--
| |
| call expression requires function
|
help: `Empty2` is a unit struct, and does not take parentheses to be constructed
|
LL - let e2 = Empty2();
LL + let e2 = Empty2;
|

error[E0618]: expected function, found enum variant `E::Empty4`
--> $DIR/empty-struct-unit-expr.rs:16:14
|
LL | Empty4
| ------ `E::Empty4` defined here
| ------ enum variant `E::Empty4` defined here
...
LL | let e4 = E::Empty4();
| ^^^^^^^^^--
| |
| call expression requires function
|
help: `E::Empty4` is a unit variant, you need to write it without the parentheses
help: `E::Empty4` is a unit enum variant, and does not take parentheses to be constructed
|
LL - let e4 = E::Empty4();
LL + let e4 = E::Empty4;
|

error[E0618]: expected function, found `empty_struct::XEmpty2`
error[E0618]: expected function, found struct `XEmpty2`
--> $DIR/empty-struct-unit-expr.rs:18:15
|
LL | let xe2 = XEmpty2();
| ^^^^^^^--
| |
| call expression requires function
|
help: `XEmpty2` is a unit struct, and does not take parentheses to be constructed
|
LL - let xe2 = XEmpty2();
LL + let xe2 = XEmpty2;
|

error[E0618]: expected function, found enum variant `XE::XEmpty4`
--> $DIR/empty-struct-unit-expr.rs:19:15
Expand All @@ -42,7 +54,7 @@ LL | let xe4 = XE::XEmpty4();
| |
| call expression requires function
|
help: `XE::XEmpty4` is a unit variant, you need to write it without the parentheses
help: `XE::XEmpty4` is a unit enum variant, and does not take parentheses to be constructed
|
LL - let xe4 = XE::XEmpty4();
LL + let xe4 = XE::XEmpty4;
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0618.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ error[E0618]: expected function, found enum variant `X::Entry`
--> $DIR/E0618.rs:6:5
|
LL | Entry,
| ----- `X::Entry` defined here
| ----- enum variant `X::Entry` defined here
...
LL | X::Entry();
| ^^^^^^^^--
| |
| call expression requires function
|
help: `X::Entry` is a unit variant, you need to write it without the parentheses
help: `X::Entry` is a unit enum variant, and does not take parentheses to be constructed
|
LL - X::Entry();
LL + X::Entry;
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-20714.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
struct G;

fn main() {
let g = G(); //~ ERROR: expected function, found `G`
let g = G(); //~ ERROR: expected function, found struct `G`
}
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-20714.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
error[E0618]: expected function, found `G`
error[E0618]: expected function, found struct `G`
--> $DIR/issue-20714.rs:4:13
|
LL | struct G;
| -------- `G` defined here
| -------- struct `G` defined here
...
LL | let g = G();
| ^--
| |
| call expression requires function
|
help: `G` is a unit struct, and does not take parentheses to be constructed
|
LL - let g = G();
LL + let g = G;
|

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/issues/issue-21701.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ struct Bar;

pub fn some_func() {
let f = Bar();
//~^ ERROR: expected function, found `Bar`
//~^ ERROR: expected function, found struct `Bar`
}

fn main() {
Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/issues/issue-21701.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,22 @@ LL | let y = t();
| |
| call expression requires function

error[E0618]: expected function, found `Bar`
error[E0618]: expected function, found struct `Bar`
--> $DIR/issue-21701.rs:9:13
|
LL | struct Bar;
| ---------- `Bar` defined here
| ---------- struct `Bar` defined here
...
LL | let f = Bar();
| ^^^--
| |
| call expression requires function
|
help: `Bar` is a unit struct, and does not take parentheses to be constructed
|
LL - let f = Bar();
LL + let f = Bar;
|

error: aborting due to 2 previous errors

Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/resolve/privacy-enum-ctor.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,14 @@ error[E0618]: expected function, found enum variant `Z::Unit`
--> $DIR/privacy-enum-ctor.rs:31:17
|
LL | Unit,
| ---- `Z::Unit` defined here
| ---- enum variant `Z::Unit` defined here
...
LL | let _ = Z::Unit();
| ^^^^^^^--
| |
| call expression requires function
|
help: `Z::Unit` is a unit variant, you need to write it without the parentheses
help: `Z::Unit` is a unit enum variant, and does not take parentheses to be constructed
|
LL - let _ = Z::Unit();
LL + let _ = Z::Unit;
Expand Down Expand Up @@ -371,14 +371,14 @@ error[E0618]: expected function, found enum variant `m::E::Unit`
--> $DIR/privacy-enum-ctor.rs:47:16
|
LL | Unit,
| ---- `m::E::Unit` defined here
| ---- enum variant `m::E::Unit` defined here
...
LL | let _: E = m::E::Unit();
| ^^^^^^^^^^--
| |
| call expression requires function
|
help: `m::E::Unit` is a unit variant, you need to write it without the parentheses
help: `m::E::Unit` is a unit enum variant, and does not take parentheses to be constructed
|
LL - let _: E = m::E::Unit();
LL + let _: E = m::E::Unit;
Expand Down Expand Up @@ -406,14 +406,14 @@ error[E0618]: expected function, found enum variant `E::Unit`
--> $DIR/privacy-enum-ctor.rs:55:16
|
LL | Unit,
| ---- `E::Unit` defined here
| ---- enum variant `E::Unit` defined here
...
LL | let _: E = E::Unit();
| ^^^^^^^--
| |
| call expression requires function
|
help: `E::Unit` is a unit variant, you need to write it without the parentheses
help: `E::Unit` is a unit enum variant, and does not take parentheses to be constructed
|
LL - let _: E = E::Unit();
LL + let _: E = E::Unit;
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/suggestions/issue-99240.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
fn fmt(it: &(std::cell::Cell<Option<impl FnOnce()>>,)) {
(it.0.take())()
//~^ ERROR expected function
}

fn main() {}
11 changes: 11 additions & 0 deletions src/test/ui/suggestions/issue-99240.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0618]: expected function, found `Option<impl FnOnce()>`
--> $DIR/issue-99240.rs:2:5
|
LL | (it.0.take())()
| ^^^^^^^^^^^^^--
| |
| call expression requires function

error: aborting due to previous error

For more information about this error, try `rustc --explain E0618`.
4 changes: 2 additions & 2 deletions src/test/ui/type-alias-enum-variants/incorrect-variant-form-through-alias-caught.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ error[E0618]: expected function, found enum variant `Alias::Unit`
--> $DIR/incorrect-variant-form-through-alias-caught.rs:15:5
|
LL | enum Enum { Braced {}, Unit, Tuple() }
| ---- `Alias::Unit` defined here
| ---- enum variant `Alias::Unit` defined here
...
LL | Alias::Unit();
| ^^^^^^^^^^^--
| |
| call expression requires function
|
help: `Alias::Unit` is a unit variant, you need to write it without the parentheses
help: `Alias::Unit` is a unit enum variant, and does not take parentheses to be constructed
|
LL - Alias::Unit();
LL + Alias::Unit;
Expand Down