Skip to content

Commit

Permalink
Suggest Semicolon in Incorrect Repeat Expressions
Browse files Browse the repository at this point in the history
  • Loading branch information
veera-sivarajan committed Jul 23, 2024
1 parent 49e720d commit 14c4da1
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 11 deletions.
14 changes: 13 additions & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::intravisit::FnKind;
use crate::LangItem;
use rustc_ast as ast;
use rustc_ast::util::parser::ExprPrecedence;
use rustc_ast::{Attribute, FloatTy, IntTy, Label, LitKind, TraitObjectSyntax, UintTy};
use rustc_ast::{Attribute, FloatTy, IntTy, Label, LitIntType, LitKind, TraitObjectSyntax, UintTy};
pub use rustc_ast::{BinOp, BinOpKind, BindingMode, BorrowKind, ByRef, CaptureBy};
pub use rustc_ast::{ImplPolarity, IsAuto, Movability, Mutability, UnOp};
use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
Expand Down Expand Up @@ -1795,6 +1795,18 @@ impl Expr<'_> {
}
}

/// Check if expression is an integer literal that can be used
/// where `usize` is expected.
pub fn is_size_lit(&self) -> bool {
matches!(
self.kind,
ExprKind::Lit(Lit {
node: LitKind::Int(_, LitIntType::Unsuffixed | LitIntType::Unsigned(UintTy::Usize)),
..
})
)
}

/// If `Self.kind` is `ExprKind::DropTemps(expr)`, drill down until we get a non-`DropTemps`
/// `Expr`. This is used in suggestions to ignore this `ExprKind` as it is semantically
/// silent, only signaling the ownership system. By doing this, suggestions that check the
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ hir_typeck_remove_semi_for_coerce_ret = the `match` arms can conform to this ret
hir_typeck_remove_semi_for_coerce_semi = the `match` is a statement because of this semicolon, consider removing it
hir_typeck_remove_semi_for_coerce_suggestion = remove this semicolon
hir_typeck_replace_comma_with_semicolon = replace comma with semicolon to create {$descr}
hir_typeck_return_stmt_outside_of_fn_body =
{$statement_kind} statement outside of function body
.encl_body_label = the {$statement_kind} is part of this body...
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if expr_ty == expected {
return;
}

self.annotate_alternative_method_deref(err, expr, error);
self.explain_self_literal(err, expr, expected, expr_ty);

Expand All @@ -40,6 +39,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
|| self.suggest_missing_unwrap_expect(err, expr, expected, expr_ty)
|| self.suggest_remove_last_method_call(err, expr, expected)
|| self.suggest_associated_const(err, expr, expected)
|| self.suggest_semicolon_in_repeat_expr(err, expr, expr_ty)
|| self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr)
|| self.suggest_option_to_bool(err, expr, expr_ty, expected)
|| self.suggest_compatible_variants(err, expr, expected, expr_ty)
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_hir_typeck/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,3 +724,16 @@ pub(crate) struct PassToVariadicFunction<'tcx, 'a> {
#[note(hir_typeck_teach_help)]
pub(crate) teach: Option<()>,
}

#[derive(Subdiagnostic)]
#[suggestion(
hir_typeck_replace_comma_with_semicolon,
applicability = "machine-applicable",
style = "verbose",
code = "; "
)]
pub struct ReplaceCommaWithSemicolon {
#[primary_span]
pub comma_span: Span,
pub descr: &'static str,
}
82 changes: 74 additions & 8 deletions compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1298,14 +1298,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let span = expr.span.shrink_to_hi();
let subdiag = if self.type_is_copy_modulo_regions(self.param_env, ty) {
errors::OptionResultRefMismatch::Copied { span, def_path }
} else if let Some(clone_did) = self.tcx.lang_items().clone_trait()
&& rustc_trait_selection::traits::type_known_to_meet_bound_modulo_regions(
self,
self.param_env,
ty,
clone_did,
)
{
} else if self.type_is_clone_modulo_regions(self.param_env, ty) {
errors::OptionResultRefMismatch::Cloned { span, def_path }
} else {
return false;
Expand Down Expand Up @@ -2158,6 +2151,79 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

/// Suggest replacing comma with semicolon in incorrect repeat expressions
/// like `["_", 10]` or `vec![String::new(), 10]`.
pub(crate) fn suggest_semicolon_in_repeat_expr(
&self,
err: &mut Diag<'_>,
expr: &hir::Expr<'_>,
expr_ty: Ty<'tcx>,
) -> bool {
// Check if `expr` is contained in array of two elements
if let hir::Node::Expr(array_expr) = self.tcx.parent_hir_node(expr.hir_id)
&& let hir::ExprKind::Array(elements) = array_expr.kind
&& let [first, second] = &elements[..]
&& second.hir_id == expr.hir_id
{
// Span between the two elements of the array
let comma_span = first.span.between(second.span);

// Check if `expr` is a constant value of type `usize`.
// This can only detect const variable declarations and
// calls to const functions.

// Checking this here instead of rustc_hir::hir because
// this check needs access to `self.tcx` but rustc_hir
// has no access to `TyCtxt`.
let expr_is_const_usize = expr_ty.is_usize()
&& match expr.kind {
ExprKind::Path(QPath::Resolved(
None,
Path { res: Res::Def(DefKind::Const, _), .. },
)) => true,
ExprKind::Call(
Expr {
kind:
ExprKind::Path(QPath::Resolved(
None,
Path { res: Res::Def(DefKind::Fn, fn_def_id), .. },
)),
..
},
_,
) => self.tcx.is_const_fn(*fn_def_id),
_ => false,
};

// `array_expr` is from a macro `vec!["a", 10]` if
// 1. array expression's span is imported from a macro
// 2. first element of array implements `Clone` trait
// 3. second element is an integer literal or is an expression of `usize` like type
if self.tcx.sess.source_map().is_imported(array_expr.span)
&& self.type_is_clone_modulo_regions(self.param_env, self.check_expr(first))
&& (second.is_size_lit() || expr_ty.is_usize_like())
{
err.subdiagnostic(errors::ReplaceCommaWithSemicolon {
comma_span,
descr: "a vector",
});
return true;
} else if self.type_is_copy_modulo_regions(self.param_env, self.check_expr(first))
&& (second.is_size_lit() || expr_is_const_usize)
{
// `array_expr` is from an array `["a", 10]` if
// 1. first element of array implements `Copy` trait
// 2. second element is an integer literal or is a const value of type `usize`
err.subdiagnostic(errors::ReplaceCommaWithSemicolon {
comma_span,
descr: "an array",
});
return true;
}
}
false
}

/// If the expected type is an enum (Issue #55250) with any variants whose
/// sole field is of the found type, suggest such variants. (Issue #42764)
pub(crate) fn suggest_compatible_variants(
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::ty::{
TypeVisitable, TypeVisitor,
};
use crate::ty::{GenericArg, GenericArgs, GenericArgsRef};
use crate::ty::{List, ParamEnv};
use crate::ty::{List, ParamEnv, UintTy};
use hir::def::{CtorKind, DefKind};
use rustc_data_structures::captures::Captures;
use rustc_errors::{ErrorGuaranteed, MultiSpan};
Expand Down Expand Up @@ -990,6 +990,18 @@ impl<'tcx> Ty<'tcx> {
}
}

/// Check if type is an `usize`.
#[inline]
pub fn is_usize(self) -> bool {
matches!(self.kind(), Uint(UintTy::Usize))
}

/// Check if type is an `usize` or an integral type variable.
#[inline]
pub fn is_usize_like(self) -> bool {
matches!(self.kind(), Uint(UintTy::Usize) | Infer(IntVar(_)))
}

#[inline]
pub fn is_never(self) -> bool {
matches!(self.kind(), Never)
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_trait_selection/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ impl<'tcx> InferCtxt<'tcx> {
})
}

/// Check if `ty` implements the `Copy` trait.
fn type_is_copy_modulo_regions(&self, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
let ty = self.resolve_vars_if_possible(ty);

Expand All @@ -44,6 +45,14 @@ impl<'tcx> InferCtxt<'tcx> {
traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, copy_def_id)
}

/// Check if `ty` implements the `Clone` trait.
fn type_is_clone_modulo_regions(&self, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
let ty = self.resolve_vars_if_possible(ty);
let clone_def_id = self.tcx.require_lang_item(LangItem::Clone, None);
traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, clone_def_id)
}

/// Check if `ty` implements the `Sized` trait.
fn type_is_sized_modulo_regions(&self, param_env: ty::ParamEnv<'tcx>, ty: Ty<'tcx>) -> bool {
let lang_item = self.tcx.require_lang_item(LangItem::Sized, None);
traits::type_known_to_meet_bound_modulo_regions(self, param_env, ty, lang_item)
Expand Down

0 comments on commit 14c4da1

Please sign in to comment.