Skip to content

Commit

Permalink
Remove one usage of LibCST
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 5, 2024
1 parent ad01216 commit b45724a
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 64 deletions.
4 changes: 1 addition & 3 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
);
}
if checker.enabled(Rule::UnnecessaryListComprehensionSet) {
flake8_comprehensions::rules::unnecessary_list_comprehension_set(
checker, expr, func, args, keywords,
);
flake8_comprehensions::rules::unnecessary_list_comprehension_set(checker, call);
}
if checker.enabled(Rule::UnnecessaryListComprehensionDict) {
flake8_comprehensions::rules::unnecessary_list_comprehension_dict(
Expand Down
87 changes: 45 additions & 42 deletions crates/ruff_linter/src/rules/flake8_comprehensions/fixes.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::iter;

use anyhow::{bail, Result};
use itertools::Itertools;
use libcst_native::{
Expand All @@ -7,7 +9,6 @@ use libcst_native::{
RightCurlyBrace, RightParen, RightSquareBracket, Set, SetComp, SimpleString, SimpleWhitespace,
TrailingWhitespace, Tuple,
};
use std::iter;

use ruff_diagnostics::{Edit, Fix};
use ruff_python_ast::Expr;
Expand Down Expand Up @@ -151,46 +152,6 @@ pub(crate) fn fix_unnecessary_generator_dict(expr: &Expr, checker: &Checker) ->
))
}

/// (C403) Convert `set([x for x in y])` to `{x for x in y}`.
pub(crate) fn fix_unnecessary_list_comprehension_set(
expr: &Expr,
checker: &Checker,
) -> Result<Edit> {
let locator = checker.locator();
let stylist = checker.stylist();
// Expr(Call(ListComp)))) ->
// Expr(SetComp)))
let module_text = locator.slice(expr);
let mut tree = match_expression(module_text)?;
let call = match_call_mut(&mut tree)?;
let arg = match_arg(call)?;

let list_comp = match_list_comp(&arg.value)?;

tree = Expression::SetComp(Box::new(SetComp {
elt: list_comp.elt.clone(),
for_in: list_comp.for_in.clone(),
lbrace: LeftCurlyBrace {
whitespace_after: call.whitespace_before_args.clone(),
},
rbrace: RightCurlyBrace {
whitespace_before: arg.whitespace_after_arg.clone(),
},
lpar: list_comp.lpar.clone(),
rpar: list_comp.rpar.clone(),
}));

Ok(Edit::range_replacement(
pad_expression(
tree.codegen_stylist(stylist),
expr.range(),
checker.locator(),
checker.semantic(),
),
expr.range(),
))
}

/// (C404) Convert `dict([(i, i) for i in range(3)])` to `{i: i for i in
/// range(3)}`.
pub(crate) fn fix_unnecessary_list_comprehension_dict(
Expand Down Expand Up @@ -544,7 +505,7 @@ pub(crate) fn fix_unnecessary_collection_call(expr: &Expr, checker: &Checker) ->
/// However, this is a syntax error under the f-string grammar. As such,
/// this method will pad the start and end of an expression as needed to
/// avoid producing invalid syntax.
fn pad_expression(
pub(crate) fn pad_expression(
content: String,
range: TextRange,
locator: &Locator,
Expand Down Expand Up @@ -575,6 +536,48 @@ fn pad_expression(
}
}

/// Like [`pad_expression`], but only pads the start of the expression.
pub(crate) fn pad_start(
content: &str,
range: TextRange,
locator: &Locator,
semantic: &SemanticModel,
) -> String {
if !semantic.in_f_string() {
return content.into();
}

// If the expression is immediately preceded by an opening brace, then
// we need to add a space before the expression.
let prefix = locator.up_to(range.start());
if matches!(prefix.chars().next_back(), Some('{')) {
format!(" {content}")
} else {
content.into()
}
}

/// Like [`pad_expression`], but only pads the end of the expression.
pub(crate) fn pad_end(
content: &str,
range: TextRange,
locator: &Locator,
semantic: &SemanticModel,
) -> String {
if !semantic.in_f_string() {
return content.into();
}

// If the expression is immediately preceded by an opening brace, then
// we need to add a space before the expression.
let suffix = locator.after(range.end());
if matches!(suffix.chars().next(), Some('}')) {
format!("{content} ")
} else {
content.into()
}
}

/// (C409) Convert `tuple([1, 2])` to `tuple(1, 2)`
pub(crate) fn fix_unnecessary_literal_within_tuple_call(
expr: &Expr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ pub(crate) fn unnecessary_collection_call(
{
// `dict()` or `dict(a=1)` (as opposed to `dict(**a)`)
}
"list" | "tuple" => {
"list" | "tuple" if keywords.is_empty() => {
// `list()` or `tuple()`
}
_ => return,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use ruff_python_ast::{Expr, Keyword};

use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_text_size::Ranged;
use ruff_python_ast as ast;
use ruff_text_size::{Ranged, TextSize};

use crate::checkers::ast::Checker;

use crate::rules::flake8_comprehensions::fixes;
use crate::rules::flake8_comprehensions::fixes::{pad_end, pad_start};

use super::helpers;

Expand Down Expand Up @@ -45,25 +43,46 @@ impl AlwaysFixableViolation for UnnecessaryListComprehensionSet {
}

/// C403 (`set([...])`)
pub(crate) fn unnecessary_list_comprehension_set(
checker: &mut Checker,
expr: &Expr,
func: &Expr,
args: &[Expr],
keywords: &[Keyword],
) {
let Some(argument) =
helpers::exactly_one_argument_with_matching_function("set", func, args, keywords)
else {
pub(crate) fn unnecessary_list_comprehension_set(checker: &mut Checker, call: &ast::ExprCall) {
let Some(argument) = helpers::exactly_one_argument_with_matching_function(
"set",
&call.func,
&call.arguments.args,
&call.arguments.keywords,
) else {
return;
};
if !checker.semantic().is_builtin("set") {
return;
}
if argument.is_list_comp_expr() {
let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, expr.range());
let mut diagnostic = Diagnostic::new(UnnecessaryListComprehensionSet, call.range());
diagnostic.try_set_fix(|| {
fixes::fix_unnecessary_list_comprehension_set(expr, checker).map(Fix::unsafe_edit)
// Replace `set(` with `{`.
let call_start = Edit::replacement(
pad_start("{", call.range(), checker.locator(), checker.semantic()),
call.start(),
call.arguments.start() + TextSize::from(1),
);

// Replace `)` with `}`.
let call_end = Edit::replacement(
pad_end("}", call.range(), checker.locator(), checker.semantic()),
call.arguments.end() - TextSize::from(1),
call.end(),
);

// Delete the open bracket (`[`).
let argument_start =
Edit::deletion(argument.start(), argument.start() + TextSize::from(1));

// Delete the close bracket (`]`).
let argument_end = Edit::deletion(argument.end() - TextSize::from(1), argument.end());

Ok(Fix::unsafe_edits(
call_start,
[argument_start, argument_end, call_end],
))
});
checker.diagnostics.push(diagnostic);
}
Expand Down

0 comments on commit b45724a

Please sign in to comment.