Skip to content

Commit

Permalink
Change more usages of SemanticModel::is_builtin to use `resolve_bui…
Browse files Browse the repository at this point in the history
…ltin_symbol` or `match_builtin_expr`
  • Loading branch information
AlexWaygood committed Apr 16, 2024
1 parent 2882604 commit 3c5bf04
Show file tree
Hide file tree
Showing 23 changed files with 195 additions and 256 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,6 @@ pub(crate) fn blind_except(
let Some(type_) = type_ else {
return;
};
let Expr::Name(ast::ExprName { id, .. }) = &type_ else {
return;
};

if !matches!(id.as_str(), "BaseException" | "Exception") {
return;
}

if !checker.semantic().is_builtin(id) {
return;
}

// If the exception is re-raised, don't flag an error.
if body.iter().any(|stmt| {
Expand All @@ -110,6 +99,14 @@ pub(crate) fn blind_except(
return;
}

let semantic = checker.semantic();
let Some(builtin_exception_type) = semantic.resolve_builtin_symbol(type_) else {
return;
};
if !matches!(builtin_exception_type, "BaseException" | "Exception") {
return;
}

// If the exception is logged, don't flag an error.
if body.iter().any(|stmt| {
if let Stmt::Expr(ast::StmtExpr { value, range: _ }) = stmt {
Expand All @@ -121,7 +118,7 @@ pub(crate) fn blind_except(
Expr::Attribute(ast::ExprAttribute { attr, .. }) => {
if logging::is_logger_candidate(
func,
checker.semantic(),
semantic,
&checker.settings.logger_objects,
) {
match attr.as_str() {
Expand All @@ -138,9 +135,8 @@ pub(crate) fn blind_except(
}
}
Expr::Name(ast::ExprName { .. }) => {
if checker
.semantic()
.resolve_qualified_name(func.as_ref())
if semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| match qualified_name.segments() {
["logging", "exception"] => true,
["logging", "error"] => {
Expand Down Expand Up @@ -170,7 +166,7 @@ pub(crate) fn blind_except(

checker.diagnostics.push(Diagnostic::new(
BlindExcept {
name: id.to_string(),
name: builtin_exception_type.to_string(),
},
type_.range(),
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,40 +60,35 @@ impl AlwaysFixableViolation for UnnecessaryCallAroundSorted {
pub(crate) fn unnecessary_call_around_sorted(
checker: &mut Checker,
expr: &Expr,
func: &Expr,
outer_func: &Expr,
args: &[Expr],
) {
let Some(outer) = func.as_name_expr() else {
let Some(Expr::Call(ast::ExprCall {
func: inner_func, ..
})) = args.first()
else {
return;
};
if !matches!(outer.id.as_str(), "list" | "reversed") {
return;
}
let Some(arg) = args.first() else {
return;
};
let Expr::Call(ast::ExprCall { func, .. }) = arg else {
return;
};
let Some(inner) = func.as_name_expr() else {
let semantic = checker.semantic();
let Some(outer_func_name) = semantic.resolve_builtin_symbol(outer_func) else {
return;
};
if inner.id != "sorted" {
if !matches!(outer_func_name, "list" | "reversed") {
return;
}
if !checker.semantic().is_builtin(&inner.id) || !checker.semantic().is_builtin(&outer.id) {
if !semantic.match_builtin_expr(inner_func, "sorted") {
return;
}
let mut diagnostic = Diagnostic::new(
UnnecessaryCallAroundSorted {
func: outer.id.to_string(),
func: outer_func_name.to_string(),
},
expr.range(),
);
diagnostic.try_set_fix(|| {
Ok(Fix::applicable_edit(
fixes::fix_unnecessary_call_around_sorted(expr, checker.locator(), checker.stylist())?,
if outer.id == "reversed" {
if outer_func_name == "reversed" {
Applicability::Unsafe
} else {
Applicability::Safe
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@ pub(crate) fn unnecessary_collection_call(
if !call.arguments.args.is_empty() {
return;
}
let Some(func) = call.func.as_name_expr() else {
let Some(builtin) = checker.semantic().resolve_builtin_symbol(&call.func) else {
return;
};
let collection = match func.id.as_str() {
let collection = match builtin {
"dict"
if call.arguments.keywords.is_empty()
|| (!settings.allow_dict_calls_with_keyword_arguments
Expand All @@ -87,13 +87,10 @@ pub(crate) fn unnecessary_collection_call(
}
_ => return,
};
if !checker.semantic().is_builtin(func.id.as_str()) {
return;
}

let mut diagnostic = Diagnostic::new(
UnnecessaryCollectionCall {
obj_type: func.id.to_string(),
obj_type: builtin.to_string(),
},
call.range(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,6 @@ pub(crate) fn unnecessary_comprehension_in_call(
if !keywords.is_empty() {
return;
}

let Expr::Name(ast::ExprName { id, .. }) = func else {
return;
};
if !(matches!(id.as_str(), "any" | "all")
|| (checker.settings.preview.is_enabled() && matches!(id.as_str(), "sum" | "min" | "max")))
{
return;
}
let [arg] = args else {
return;
};
Expand All @@ -110,7 +101,13 @@ pub(crate) fn unnecessary_comprehension_in_call(
if contains_await(elt) {
return;
}
if !checker.semantic().is_builtin(id) {
let Some(builtin_function) = checker.semantic().resolve_builtin_symbol(func) else {
return;
};
if !(matches!(builtin_function, "any" | "all")
|| (checker.settings.preview.is_enabled()
&& matches!(builtin_function, "sum" | "min" | "max")))
{
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,15 @@ impl AlwaysFixableViolation for UnnecessaryDoubleCastOrProcess {
pub(crate) fn unnecessary_double_cast_or_process(
checker: &mut Checker,
expr: &Expr,
func: &Expr,
outer_func: &Expr,
args: &[Expr],
outer_kw: &[Keyword],
) {
let Some(outer) = func.as_name_expr() else {
return;
};
if !matches!(
outer.id.as_str(),
"list" | "tuple" | "set" | "reversed" | "sorted"
) {
return;
}
let Some(arg) = args.first() else {
return;
};
let Expr::Call(ast::ExprCall {
func,
func: inner_func,
arguments: Arguments {
keywords: inner_kw, ..
},
Expand All @@ -96,16 +87,23 @@ pub(crate) fn unnecessary_double_cast_or_process(
else {
return;
};
let Some(inner) = func.as_name_expr() else {
let semantic = checker.semantic();
let Some(outer_func_name) = semantic.resolve_builtin_symbol(outer_func) else {
return;
};
if !checker.semantic().is_builtin(&inner.id) || !checker.semantic().is_builtin(&outer.id) {
if !matches!(
outer_func_name,
"list" | "tuple" | "set" | "reversed" | "sorted"
) {
return;
}
let Some(inner_func_name) = semantic.resolve_builtin_symbol(inner_func) else {
return;
};

// Avoid collapsing nested `sorted` calls with non-identical keyword arguments
// (i.e., `key`, `reverse`).
if inner.id == "sorted" && outer.id == "sorted" {
if inner_func_name == "sorted" && outer_func_name == "sorted" {
if inner_kw.len() != outer_kw.len() {
return;
}
Expand All @@ -122,15 +120,15 @@ pub(crate) fn unnecessary_double_cast_or_process(
// Ex) `list(tuple(...))`
// Ex) `set(set(...))`
if matches!(
(outer.id.as_str(), inner.id.as_str()),
(outer_func_name, inner_func_name),
("set" | "sorted", "list" | "tuple" | "reversed" | "sorted")
| ("set", "set")
| ("list" | "tuple", "list" | "tuple")
) {
let mut diagnostic = Diagnostic::new(
UnnecessaryDoubleCastOrProcess {
inner: inner.id.to_string(),
outer: outer.id.to_string(),
inner: inner_func_name.to_string(),
outer: outer_func_name.to_string(),
},
expr.range(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,18 @@ pub(crate) fn unnecessary_map(
func: &Expr,
args: &[Expr],
) {
let Some(func) = func.as_name_expr() else {
let Some(builtin_name) = checker.semantic().resolve_builtin_symbol(func) else {
return;
};

let object_type = match func.id.as_str() {
let object_type = match builtin_name {
"map" => ObjectType::Generator,
"list" => ObjectType::List,
"set" => ObjectType::Set,
"dict" => ObjectType::Dict,
_ => return,
};

if !checker.semantic().is_builtin(&func.id) {
return;
}

match object_type {
ObjectType::Generator => {
// Exclude the parent if already matched by other arms.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,6 @@ pub(crate) fn unnecessary_subscript_reversal(checker: &mut Checker, call: &ast::
let Some(first_arg) = call.arguments.args.first() else {
return;
};
let Some(func) = call.func.as_name_expr() else {
return;
};
if !matches!(func.id.as_str(), "reversed" | "set" | "sorted") {
return;
}
if !checker.semantic().is_builtin(&func.id) {
return;
}
let Expr::Subscript(ast::ExprSubscript { slice, .. }) = first_arg else {
return;
};
Expand Down Expand Up @@ -89,9 +80,15 @@ pub(crate) fn unnecessary_subscript_reversal(checker: &mut Checker, call: &ast::
if *val != 1 {
return;
};
let Some(function_name) = checker.semantic().resolve_builtin_symbol(&call.func) else {
return;
};
if !matches!(function_name, "reversed" | "set" | "sorted") {
return;
}
checker.diagnostics.push(Diagnostic::new(
UnnecessarySubscriptReversal {
func: func.id.to_string(),
func: function_name.to_string(),
},
call.range(),
));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,24 +65,26 @@ pub(crate) fn reimplemented_container_builtin(checker: &mut Checker, expr: &Expr
range: _,
} = expr;

if parameters.is_none() {
let container = match body.as_ref() {
Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Some(Container::List),
Expr::Dict(ast::ExprDict { values, .. }) if values.is_empty() => Some(Container::Dict),
_ => None,
};
if let Some(container) = container {
let mut diagnostic =
Diagnostic::new(ReimplementedContainerBuiltin { container }, expr.range());
if checker.semantic().is_builtin(container.as_str()) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
container.to_string(),
expr.range(),
)));
}
checker.diagnostics.push(diagnostic);
}
if parameters.is_some() {
return;
}

let container = match &**body {
Expr::List(ast::ExprList { elts, .. }) if elts.is_empty() => Container::List,
Expr::Dict(ast::ExprDict { values, .. }) if values.is_empty() => Container::Dict,
_ => return,
};
let mut diagnostic = Diagnostic::new(ReimplementedContainerBuiltin { container }, expr.range());
diagnostic.try_set_fix(|| {
let (import_edit, binding) = checker.importer().get_or_import_builtin_symbol(
container.as_str(),
expr.start(),
checker.semantic(),
)?;
let binding_edit = Edit::range_replacement(binding, expr.range());
Ok(Fix::safe_edits(binding_edit, import_edit))
});
checker.diagnostics.push(diagnostic);
}

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand All @@ -92,7 +94,7 @@ enum Container {
}

impl Container {
fn as_str(self) -> &'static str {
const fn as_str(self) -> &'static str {
match self {
Container::List => "list",
Container::Dict => "dict",
Expand All @@ -102,9 +104,6 @@ impl Container {

impl std::fmt::Display for Container {
fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Container::List => fmt.write_str("list"),
Container::Dict => fmt.write_str("dict"),
}
fmt.write_str(self.as_str())
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,7 @@ impl fmt::Display for ExprType {
/// Return the [`ExprType`] of an [`Expr]` if it is a builtin type (e.g. `int`, `bool`, `float`,
/// `str`, `bytes`, or `complex`).
fn match_builtin_type(expr: &Expr, semantic: &SemanticModel) -> Option<ExprType> {
let name = expr.as_name_expr()?;
let result = match name.id.as_str() {
let result = match semantic.resolve_builtin_symbol(expr)? {
"int" => ExprType::Int,
"bool" => ExprType::Bool,
"str" => ExprType::Str,
Expand All @@ -139,9 +138,6 @@ fn match_builtin_type(expr: &Expr, semantic: &SemanticModel) -> Option<ExprType>
"complex" => ExprType::Complex,
_ => return None,
};
if !semantic.is_builtin(name.id.as_str()) {
return None;
}
Some(result)
}

Expand Down
Loading

0 comments on commit 3c5bf04

Please sign in to comment.