Skip to content

Commit

Permalink
Remove Expr postfix from ExprNamed, ExprIf, and ExprGenerator (
Browse files Browse the repository at this point in the history
…#10229)

The expression types in our AST are called `ExprYield`, `ExprAwait`,
`ExprStringLiteral` etc, except `ExprNamedExpr`, `ExprIfExpr` and
`ExprGenratorExpr`. This seems to align with [Python AST's
naming](https://docs.python.org/3/library/ast.html) but feels
inconsistent and excessive.

This PR removes the `Expr` postfix from `ExprNamedExpr`, `ExprIfExpr`,
and `ExprGeneratorExpr`.
  • Loading branch information
MichaReiser authored Mar 4, 2024
1 parent 8b749e1 commit 184241f
Show file tree
Hide file tree
Showing 64 changed files with 418 additions and 428 deletions.
10 changes: 5 additions & 5 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1327,8 +1327,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
}
Expr::IfExp(
if_exp @ ast::ExprIfExp {
Expr::If(
if_exp @ ast::ExprIf {
test,
body,
orelse,
Expand Down Expand Up @@ -1450,8 +1450,8 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
flake8_bugbear::rules::static_key_dict_comprehension(checker, dict_comp);
}
}
Expr::GeneratorExp(
generator @ ast::ExprGeneratorExp {
Expr::Generator(
generator @ ast::ExprGenerator {
generators,
elt: _,
range: _,
Expand Down Expand Up @@ -1517,7 +1517,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
ruff::rules::parenthesize_chained_logical_operators(checker, bool_op);
}
}
Expr::NamedExpr(..) => {
Expr::Named(..) => {
if checker.enabled(Rule::AssignmentInAssert) {
ruff::rules::assignment_in_assert(checker, expr);
}
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
generators,
range: _,
})
| Expr::GeneratorExp(ast::ExprGeneratorExp {
| Expr::Generator(ast::ExprGenerator {
elt,
generators,
range: _,
Expand Down Expand Up @@ -1048,7 +1048,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.visit.lambdas.push(self.semantic.snapshot());
self.analyze.lambdas.push(self.semantic.snapshot());
}
Expr::IfExp(ast::ExprIfExp {
Expr::If(ast::ExprIf {
test,
body,
orelse,
Expand Down Expand Up @@ -1372,7 +1372,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
self.semantic.flags |= SemanticModelFlags::F_STRING;
visitor::walk_expr(self, expr);
}
Expr::NamedExpr(ast::ExprNamedExpr {
Expr::Named(ast::ExprNamed {
target,
value,
range: _,
Expand All @@ -1388,7 +1388,7 @@ impl<'a> Visitor<'a> for Checker<'a> {
// Step 3: Clean-up
match expr {
Expr::Lambda(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::ListComp(_)
| Expr::DictComp(_)
| Expr::SetComp(_) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<'a> Visitor<'a> for NameFinder<'a> {
Expr::ListComp(ast::ExprListComp { generators, .. })
| Expr::DictComp(ast::ExprDictComp { generators, .. })
| Expr::SetComp(ast::ExprSetComp { generators, .. })
| Expr::GeneratorExp(ast::ExprGeneratorExp { generators, .. }) => {
| Expr::Generator(ast::ExprGenerator { generators, .. }) => {
for comp in generators {
self.visit_expr(&comp.iter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl<'a> Visitor<'a> for GroupNameFinder<'a> {
}

fn visit_expr(&mut self, expr: &'a Expr) {
if let Expr::NamedExpr(ast::ExprNamedExpr { target, .. }) = expr {
if let Expr::Named(ast::ExprNamed { target, .. }) = expr {
if self.name_matches(target) {
self.overridden = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub(crate) fn unnecessary_generator_dict(
else {
return;
};
let Expr::GeneratorExp(ast::ExprGeneratorExp { elt, .. }) = argument else {
let Expr::Generator(ast::ExprGenerator { elt, .. }) = argument else {
return;
};
let Expr::Tuple(ast::ExprTuple { elts, .. }) = elt.as_ref() else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub(crate) fn unnecessary_generator_list(checker: &mut Checker, call: &ast::Expr
if !checker.semantic().is_builtin("list") {
return;
}
if argument.is_generator_exp_expr() {
if argument.is_generator_expr() {
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorList, call.range());

// Convert `list(x for x in y)` to `[x for x in y]`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub(crate) fn unnecessary_generator_set(checker: &mut Checker, call: &ast::ExprC
if !checker.semantic().is_builtin("set") {
return;
}
if argument.is_generator_exp_expr() {
if argument.is_generator_expr() {
let mut diagnostic = Diagnostic::new(UnnecessaryGeneratorSet, call.range());

// Convert `set(x for x in y)` to `{x for x in y}`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ pub(crate) fn twisted_arms_in_ifexpr(
let node = body.clone();
let node1 = orelse.clone();
let node2 = orelse.clone();
let node3 = ast::ExprIfExp {
let node3 = ast::ExprIf {
test: Box::new(node2),
body: Box::new(node1),
orelse: Box::new(node),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ pub(crate) fn if_else_block_instead_of_if_exp(checker: &mut Checker, stmt_if: &a
}

fn ternary(target_var: &Expr, body_value: &Expr, test: &Expr, orelse_value: &Expr) -> Stmt {
let node = ast::ExprIfExp {
let node = ast::ExprIf {
test: Box::new(test.clone()),
body: Box::new(body_value.clone()),
orelse: Box::new(orelse_value.clone()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn merge_branches(
op: ast::BoolOp::Or,
..
}) | Expr::Lambda(_)
| Expr::NamedExpr(_)
| Expr::Named(_)
) {
Cow::Owned(format!("({})", locator.slice(following_branch.test)))
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ fn match_sibling_return<'a>(stmt: &'a Stmt, sibling: &'a Stmt) -> Option<Termina

/// Generate a return statement for an `any` or `all` builtin comprehension.
fn return_stmt(id: &str, test: &Expr, target: &Expr, iter: &Expr, generator: Generator) -> String {
let node = ast::ExprGeneratorExp {
let node = ast::ExprGenerator {
elt: Box::new(test.clone()),
generators: vec![Comprehension {
target: target.clone(),
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pandas_vet/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub(super) fn test_expression(expr: &Expr, semantic: &SemanticModel) -> Resoluti
| Expr::SetComp(_)
| Expr::ListComp(_)
| Expr::DictComp(_)
| Expr::GeneratorExp(_) => Resolution::IrrelevantExpression,
| Expr::Generator(_) => Resolution::IrrelevantExpression,
Expr::Name(name) => {
semantic
.resolve_name(name)
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_linter/src/rules/pyflakes/rules/strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ pub(crate) fn percent_format_expected_mapping(
| Expr::Set(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::GeneratorExp(_) => checker
| Expr::Generator(_) => checker
.diagnostics
.push(Diagnostic::new(PercentFormatExpectedMapping, location)),
_ => {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub(crate) fn literal_membership(checker: &mut Checker, compare: &ast::ExprCompa
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::Await(_)
| Expr::Yield(_)
| Expr::YieldFrom(_) => true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl Violation for NamedExprWithoutContext {

/// PLW0131
pub(crate) fn named_expr_without_context(checker: &mut Checker, value: &Expr) {
if let Expr::NamedExpr(ast::ExprNamedExpr { range, .. }) = value {
if let Expr::Named(ast::ExprNamed { range, .. }) = value {
checker
.diagnostics
.push(Diagnostic::new(NamedExprWithoutContext, *range));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub(crate) fn unnecessary_dict_index_lookup(checker: &mut Checker, stmt_for: &St

/// PLR1733
pub(crate) fn unnecessary_dict_index_lookup_comprehension(checker: &mut Checker, expr: &Expr) {
let (Expr::GeneratorExp(ast::ExprGeneratorExp {
let (Expr::Generator(ast::ExprGenerator {
elt, generators, ..
})
| Expr::DictComp(ast::ExprDictComp {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,8 @@ fn can_be_represented_without_parentheses(expr: &Expr) -> bool {
|| expr.is_literal_expr()
|| expr.is_call_expr()
|| expr.is_lambda_expr()
|| expr.is_if_exp_expr()
|| expr.is_generator_exp_expr()
|| expr.is_if_expr()
|| expr.is_generator_expr()
|| expr.is_subscript_expr()
|| expr.is_starred_expr()
|| expr.is_slice_expr()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &St

/// PLR1736
pub(crate) fn unnecessary_list_index_lookup_comprehension(checker: &mut Checker, expr: &Expr) {
let (Expr::GeneratorExp(ast::ExprGeneratorExp {
let (Expr::Generator(ast::ExprGenerator {
elt, generators, ..
})
| Expr::DictComp(ast::ExprDictComp {
Expand Down
12 changes: 6 additions & 6 deletions crates/ruff_linter/src/rules/pyupgrade/rules/f_strings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ fn parenthesize(expr: &Expr, text: &str, context: FormatContext) -> bool {
Expr::BinOp(_)
| Expr::UnaryOp(_)
| Expr::BoolOp(_)
| Expr::NamedExpr(_)
| Expr::Named(_)
| Expr::Compare(_)
| Expr::IfExp(_)
| Expr::If(_)
| Expr::Lambda(_)
| Expr::Await(_)
| Expr::Yield(_)
Expand All @@ -166,7 +166,7 @@ fn parenthesize(expr: &Expr, text: &str, context: FormatContext) -> bool {
// E.g., `{x, y}` should be parenthesized in `f"{(x, y)}"`.
(
_,
Expr::GeneratorExp(_)
Expr::Generator(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::SetComp(_)
Expand All @@ -175,7 +175,7 @@ fn parenthesize(expr: &Expr, text: &str, context: FormatContext) -> bool {
(_, Expr::Subscript(ast::ExprSubscript { value, .. })) => {
matches!(
value.as_ref(),
Expr::GeneratorExp(_)
Expr::Generator(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::SetComp(_)
Expand All @@ -185,7 +185,7 @@ fn parenthesize(expr: &Expr, text: &str, context: FormatContext) -> bool {
(_, Expr::Attribute(ast::ExprAttribute { value, .. })) => {
matches!(
value.as_ref(),
Expr::GeneratorExp(_)
Expr::Generator(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::SetComp(_)
Expand All @@ -195,7 +195,7 @@ fn parenthesize(expr: &Expr, text: &str, context: FormatContext) -> bool {
(_, Expr::Call(ast::ExprCall { func, .. })) => {
matches!(
func.as_ref(),
Expr::GeneratorExp(_)
Expr::Generator(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::SetComp(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,13 @@ fn is_allowed_value(expr: &Expr) -> bool {
Expr::BoolOp(_)
| Expr::BinOp(_)
| Expr::UnaryOp(_)
| Expr::IfExp(_)
| Expr::If(_)
| Expr::Dict(_)
| Expr::Set(_)
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::Compare(_)
| Expr::Call(_)
| Expr::FString(_)
Expand All @@ -191,7 +191,7 @@ fn is_allowed_value(expr: &Expr) -> bool {
| Expr::List(_) => true,
Expr::Tuple(tuple) => tuple.elts.iter().all(is_allowed_value),
// Maybe require parentheses.
Expr::NamedExpr(_) => false,
Expr::Named(_) => false,
// Invalid in binary expressions.
Expr::Await(_)
| Expr::Lambda(_)
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_linter/src/rules/refurb/rules/bit_count.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) {
| Expr::UnaryOp(_)
| Expr::BinOp(_)
| Expr::BoolOp(_)
| Expr::IfExp(_)
| Expr::NamedExpr(_)
| Expr::If(_)
| Expr::Named(_)
| Expr::Lambda(_)
| Expr::Slice(_)
| Expr::Yield(_)
Expand All @@ -146,7 +146,7 @@ pub(crate) fn bit_count(checker: &mut Checker, call: &ExprCall) {
| Expr::List(_)
| Expr::Compare(_)
| Expr::Tuple(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::IpyEscapeCommand(_) => true,

Expr::Call(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl Violation for IfExprMinMax {
}

/// FURB136
pub(crate) fn if_expr_min_max(checker: &mut Checker, if_exp: &ast::ExprIfExp) {
pub(crate) fn if_expr_min_max(checker: &mut Checker, if_exp: &ast::ExprIf) {
let Expr::Compare(ast::ExprCompare {
left,
ops,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,13 +163,13 @@ pub(crate) fn reimplemented_starmap(checker: &mut Checker, target: &StarmapCandi
/// An enum for a node that can be considered a candidate for replacement with `starmap`.
#[derive(Debug)]
pub(crate) enum StarmapCandidate<'a> {
Generator(&'a ast::ExprGeneratorExp),
Generator(&'a ast::ExprGenerator),
ListComp(&'a ast::ExprListComp),
SetComp(&'a ast::ExprSetComp),
}

impl<'a> From<&'a ast::ExprGeneratorExp> for StarmapCandidate<'a> {
fn from(generator: &'a ast::ExprGeneratorExp) -> Self {
impl<'a> From<&'a ast::ExprGenerator> for StarmapCandidate<'a> {
fn from(generator: &'a ast::ExprGenerator) -> Self {
Self::Generator(generator)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ fn is_non_callable_value(value: &Expr) -> bool {
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::FString(_))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,17 +279,17 @@ const fn is_valid_enclosing_node(node: AnyNodeRef) -> bool {
| AnyNodeRef::ElifElseClause(_) => true,

AnyNodeRef::ExprBoolOp(_)
| AnyNodeRef::ExprNamedExpr(_)
| AnyNodeRef::ExprNamed(_)
| AnyNodeRef::ExprBinOp(_)
| AnyNodeRef::ExprUnaryOp(_)
| AnyNodeRef::ExprLambda(_)
| AnyNodeRef::ExprIfExp(_)
| AnyNodeRef::ExprIf(_)
| AnyNodeRef::ExprDict(_)
| AnyNodeRef::ExprSet(_)
| AnyNodeRef::ExprListComp(_)
| AnyNodeRef::ExprSetComp(_)
| AnyNodeRef::ExprDictComp(_)
| AnyNodeRef::ExprGeneratorExp(_)
| AnyNodeRef::ExprGenerator(_)
| AnyNodeRef::ExprAwait(_)
| AnyNodeRef::ExprYield(_)
| AnyNodeRef::ExprYieldFrom(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,12 @@ fn is_constant_like(expr: &Expr) -> bool {
| Expr::ListComp(_)
| Expr::SetComp(_)
| Expr::DictComp(_)
| Expr::GeneratorExp(_)
| Expr::Generator(_)
| Expr::Await(_)
| Expr::Yield(_)
| Expr::YieldFrom(_)
| Expr::Call(_)
| Expr::NamedExpr(_)
| Expr::Named(_)
)
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ fn match_iteration_target(expr: &Expr, semantic: &SemanticModel) -> Option<Itera
}

match arg {
Expr::GeneratorExp(ast::ExprGeneratorExp {
Expr::Generator(ast::ExprGenerator {
elt, generators, ..
}) => match match_simple_comprehension(elt, generators) {
Some(range) => IterationTarget {
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_python_ast/src/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ where
}
}
}
Expr::NamedExpr(ast::ExprNamedExpr { value, .. }) => {
Expr::Named(ast::ExprNamed { value, .. }) => {
// Allow, e.g., `__all__ += (value := ["A", "B"])`.
return extract_elts(value, is_builtin);
}
Expand Down
Loading

0 comments on commit 184241f

Please sign in to comment.