Fix PTH123 false positive when open is passed a file descriptor (#13616)

Closes https://github.com/astral-sh/ruff/issues/12871

Includes some minor semantic type inference extensions changes to help
with reliably detecting integers
This commit is contained in:
Zanie Blue 2024-10-04 08:48:47 -05:00 committed by GitHub
parent 975be9c1c6
commit d726f09cf0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 59 additions and 10 deletions

View file

@ -46,3 +46,12 @@ open(p, opener=opener)
open(p, mode='r', buffering=-1, encoding=None, errors=None, newline=None, closefd=True, opener=None)
open(p, 'r', - 1, None, None, None, True, None)
open(p, 'r', - 1, None, None, None, False, opener)
# Cannot be upgraded `pathlib.Open` does not support fds
# See https://github.com/astral-sh/ruff/issues/12871
open(1)
open(1, "w")
x = 2
open(x)
def foo(y: int):
open(y)

View file

@ -1,5 +1,7 @@
use ruff_diagnostics::{Diagnostic, DiagnosticKind};
use ruff_python_ast::{Expr, ExprBooleanLiteral, ExprCall};
use ruff_python_ast::{self as ast, Expr, ExprBooleanLiteral, ExprCall};
use ruff_python_semantic::analyze::typing;
use ruff_python_semantic::SemanticModel;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
@ -124,6 +126,10 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) {
.arguments
.find_argument("opener", 7)
.is_some_and(|expr| !expr.is_none_literal_expr())
|| call
.arguments
.find_positional(0)
.is_some_and(|expr| is_file_descriptor(expr, checker.semantic()))
{
return None;
}
@ -159,3 +165,26 @@ pub(crate) fn replaceable_by_pathlib(checker: &mut Checker, call: &ExprCall) {
}
}
}
/// Returns `true` if the given expression looks like a file descriptor, i.e., if it is an integer.
fn is_file_descriptor(expr: &Expr, semantic: &SemanticModel) -> bool {
if matches!(
expr,
Expr::NumberLiteral(ast::ExprNumberLiteral {
value: ast::Number::Int(_),
..
})
) {
return true;
};
let Some(name) = expr.as_name_expr() else {
return false;
};
let Some(binding) = semantic.only_binding(name).map(|id| semantic.binding(id)) else {
return false;
};
typing::is_int(binding, semantic)
}

View file

@ -317,5 +317,3 @@ full_name.py:47:1: PTH123 `open()` should be replaced by `Path.open()`
| ^^^^ PTH123
48 | open(p, 'r', - 1, None, None, None, False, opener)
|

View file

@ -11,7 +11,7 @@ use ruff_python_stdlib::typing::{
};
use ruff_text_size::Ranged;
use crate::analyze::type_inference::{PythonType, ResolvedPythonType};
use crate::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType};
use crate::model::SemanticModel;
use crate::{Binding, BindingKind, Modules};
@ -576,7 +576,7 @@ trait BuiltinTypeChecker {
/// Builtin type name.
const BUILTIN_TYPE_NAME: &'static str;
/// Type name as found in the `Typing` module.
const TYPING_NAME: &'static str;
const TYPING_NAME: Option<&'static str>;
/// [`PythonType`] associated with the intended type.
const EXPR_TYPE: PythonType;
@ -584,7 +584,7 @@ trait BuiltinTypeChecker {
fn match_annotation(annotation: &Expr, semantic: &SemanticModel) -> bool {
let value = map_subscript(annotation);
semantic.match_builtin_expr(value, Self::BUILTIN_TYPE_NAME)
|| semantic.match_typing_expr(value, Self::TYPING_NAME)
|| Self::TYPING_NAME.is_some_and(|name| semantic.match_typing_expr(value, name))
}
/// Check initializer expression to match the intended type.
@ -624,7 +624,7 @@ struct ListChecker;
impl BuiltinTypeChecker for ListChecker {
const BUILTIN_TYPE_NAME: &'static str = "list";
const TYPING_NAME: &'static str = "List";
const TYPING_NAME: Option<&'static str> = Some("List");
const EXPR_TYPE: PythonType = PythonType::List;
}
@ -632,7 +632,7 @@ struct DictChecker;
impl BuiltinTypeChecker for DictChecker {
const BUILTIN_TYPE_NAME: &'static str = "dict";
const TYPING_NAME: &'static str = "Dict";
const TYPING_NAME: Option<&'static str> = Some("Dict");
const EXPR_TYPE: PythonType = PythonType::Dict;
}
@ -640,7 +640,7 @@ struct SetChecker;
impl BuiltinTypeChecker for SetChecker {
const BUILTIN_TYPE_NAME: &'static str = "set";
const TYPING_NAME: &'static str = "Set";
const TYPING_NAME: Option<&'static str> = Some("Set");
const EXPR_TYPE: PythonType = PythonType::Set;
}
@ -648,10 +648,18 @@ struct TupleChecker;
impl BuiltinTypeChecker for TupleChecker {
const BUILTIN_TYPE_NAME: &'static str = "tuple";
const TYPING_NAME: &'static str = "Tuple";
const TYPING_NAME: Option<&'static str> = Some("Tuple");
const EXPR_TYPE: PythonType = PythonType::Tuple;
}
struct IntChecker;
impl BuiltinTypeChecker for IntChecker {
const BUILTIN_TYPE_NAME: &'static str = "int";
const TYPING_NAME: Option<&'static str> = None;
const EXPR_TYPE: PythonType = PythonType::Number(NumberLike::Integer);
}
pub struct IoBaseChecker;
impl TypeChecker for IoBaseChecker {
@ -761,6 +769,11 @@ pub fn is_dict(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<DictChecker>(binding, semantic)
}
/// Test whether the given binding can be considered an integer.
pub fn is_int(binding: &Binding, semantic: &SemanticModel) -> bool {
check_type::<IntChecker>(binding, semantic)
}
/// Test whether the given binding can be considered a set.
///
/// For this, we check what value might be associated with it through it's initialization and