Improve detection of whether a symbol refers to a builtin exception (#13215)

This commit is contained in:
Alex Waygood 2024-09-03 11:33:03 +01:00 committed by GitHub
parent 9d517061f2
commit 387af831f9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 150 additions and 95 deletions

View file

@ -119,3 +119,10 @@ def func():
def func():
with suppress(AttributeError):
raise AttributeError("This is an exception") # OK
import builtins
builtins.TypeError("still an exception even though it's an Attribute")
PythonFinalizationError("Added in Python 3.13")

View file

@ -57,7 +57,7 @@ use ruff_python_semantic::{
ModuleKind, ModuleSource, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags,
StarImport, SubmoduleImport,
};
use ruff_python_stdlib::builtins::{python_builtins, IPYTHON_BUILTINS, MAGIC_GLOBALS};
use ruff_python_stdlib::builtins::{python_builtins, MAGIC_GLOBALS};
use ruff_python_trivia::CommentRanges;
use ruff_source_file::{Locator, OneIndexed, SourceRow};
use ruff_text_size::{Ranged, TextRange, TextSize};
@ -1951,16 +1951,13 @@ impl<'a> Checker<'a> {
}
fn bind_builtins(&mut self) {
for builtin in python_builtins(self.settings.target_version.minor())
let standard_builtins = python_builtins(
self.settings.target_version.minor(),
self.source_type.is_ipynb(),
);
for builtin in standard_builtins
.iter()
.chain(MAGIC_GLOBALS.iter())
.chain(
self.source_type
.is_ipynb()
.then_some(IPYTHON_BUILTINS)
.into_iter()
.flatten(),
)
.copied()
.chain(self.settings.builtins.iter().map(String::as_str))
{

View file

@ -1,6 +1,6 @@
use crate::settings::types::PythonVersion;
use ruff_python_ast::PySourceType;
use ruff_python_stdlib::builtins::{is_ipython_builtin, is_python_builtin};
use ruff_python_stdlib::builtins::is_python_builtin;
pub(super) fn shadows_builtin(
name: &str,
@ -8,9 +8,7 @@ pub(super) fn shadows_builtin(
ignorelist: &[String],
python_version: PythonVersion,
) -> bool {
if is_python_builtin(name, python_version.minor())
|| source_type.is_ipynb() && is_ipython_builtin(name)
{
if is_python_builtin(name, python_version.minor(), source_type.is_ipynb()) {
ignorelist.iter().all(|ignore| ignore != name)
} else {
false

View file

@ -6,6 +6,7 @@ use ruff_python_stdlib::builtins;
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
use crate::settings::types::PythonVersion;
/// ## What it does
/// Checks for an exception that is not raised.
@ -54,7 +55,7 @@ pub(crate) fn useless_exception_statement(checker: &mut Checker, expr: &ast::Stm
return;
};
if is_builtin_exception(func, checker.semantic()) {
if is_builtin_exception(func, checker.semantic(), checker.settings.target_version) {
let mut diagnostic = Diagnostic::new(UselessExceptionStatement, expr.range());
diagnostic.set_fix(Fix::unsafe_edit(Edit::insertion(
"raise ".to_string(),
@ -65,8 +66,15 @@ pub(crate) fn useless_exception_statement(checker: &mut Checker, expr: &ast::Stm
}
/// Returns `true` if the given expression is a builtin exception.
fn is_builtin_exception(expr: &Expr, semantic: &SemanticModel) -> bool {
fn is_builtin_exception(
expr: &Expr,
semantic: &SemanticModel,
target_version: PythonVersion,
) -> bool {
semantic
.resolve_qualified_name(expr)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["", name] if builtins::is_exception(name)))
.is_some_and(|qualified_name| {
matches!(qualified_name.segments(), ["" | "builtins", name]
if builtins::is_exception(name, target_version.minor()))
})
}

View file

@ -213,3 +213,39 @@ useless_exception_statement.py:71:5: PLW0133 [*] Missing `raise` statement on ex
72 72 |
73 73 |
74 74 | # Non-violation test cases: PLW0133
useless_exception_statement.py:126:1: PLW0133 [*] Missing `raise` statement on exception
|
124 | import builtins
125 |
126 | builtins.TypeError("still an exception even though it's an Attribute")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
127 |
128 | PythonFinalizationError("Added in Python 3.13")
|
= help: Add `raise` keyword
Unsafe fix
123 123 |
124 124 | import builtins
125 125 |
126 |-builtins.TypeError("still an exception even though it's an Attribute")
126 |+raise builtins.TypeError("still an exception even though it's an Attribute")
127 127 |
128 128 | PythonFinalizationError("Added in Python 3.13")
useless_exception_statement.py:128:1: PLW0133 [*] Missing `raise` statement on exception
|
126 | builtins.TypeError("still an exception even though it's an Attribute")
127 |
128 | PythonFinalizationError("Added in Python 3.13")
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW0133
|
= help: Add `raise` keyword
Unsafe fix
125 125 |
126 126 | builtins.TypeError("still an exception even though it's an Attribute")
127 127 |
128 |-PythonFinalizationError("Added in Python 3.13")
128 |+raise PythonFinalizationError("Added in Python 3.13")

View file

@ -11,7 +11,7 @@
/// ```
///
/// Intended to be kept in sync with [`is_ipython_builtin`].
pub const IPYTHON_BUILTINS: &[&str] = &["__IPYTHON__", "display", "get_ipython"];
const IPYTHON_BUILTINS: &[&str] = &["__IPYTHON__", "display", "get_ipython"];
/// Globally defined names which are not attributes of the builtins module, or
/// are only present on some platforms.
@ -26,7 +26,7 @@ pub const MAGIC_GLOBALS: &[&str] = &[
/// Return the list of builtins for the given Python minor version.
///
/// Intended to be kept in sync with [`is_python_builtin`].
pub fn python_builtins(minor: u8) -> Vec<&'static str> {
pub fn python_builtins(minor_version: u8, is_notebook: bool) -> Vec<&'static str> {
let mut builtins = vec![
"ArithmeticError",
"AssertionError",
@ -182,16 +182,20 @@ pub fn python_builtins(minor: u8) -> Vec<&'static str> {
"zip",
];
if minor >= 10 {
builtins.extend(vec!["EncodingWarning", "aiter", "anext"]);
if minor_version >= 10 {
builtins.extend(&["EncodingWarning", "aiter", "anext"]);
}
if minor >= 11 {
builtins.extend(vec!["BaseExceptionGroup", "ExceptionGroup"]);
if minor_version >= 11 {
builtins.extend(&["BaseExceptionGroup", "ExceptionGroup"]);
}
if minor >= 13 {
builtins.extend(vec!["PythonFinalizationError"]);
if minor_version >= 13 {
builtins.push("PythonFinalizationError");
}
if is_notebook {
builtins.extend(IPYTHON_BUILTINS);
}
builtins
@ -200,7 +204,10 @@ pub fn python_builtins(minor: u8) -> Vec<&'static str> {
/// Returns `true` if the given name is that of a Python builtin.
///
/// Intended to be kept in sync with [`python_builtins`].
pub fn is_python_builtin(name: &str, minor_version: u8) -> bool {
pub fn is_python_builtin(name: &str, minor_version: u8, is_notebook: bool) -> bool {
if is_notebook && is_ipython_builtin(name) {
return true;
}
matches!(
(minor_version, name),
(
@ -374,7 +381,7 @@ pub fn is_iterator(name: &str) -> bool {
/// Returns `true` if the given name is that of an IPython builtin.
///
/// Intended to be kept in sync with [`IPYTHON_BUILTINS`].
pub fn is_ipython_builtin(name: &str) -> bool {
fn is_ipython_builtin(name: &str) -> bool {
// Constructed by converting the `IPYTHON_BUILTINS` slice to a `match` expression.
matches!(name, "__IPYTHON__" | "display" | "get_ipython")
}
@ -382,11 +389,12 @@ pub fn is_ipython_builtin(name: &str) -> bool {
/// Returns `true` if the given name is that of a builtin exception.
///
/// See: <https://docs.python.org/3/library/exceptions.html#exception-hierarchy>
pub fn is_exception(name: &str) -> bool {
pub fn is_exception(name: &str, minor_version: u8) -> bool {
matches!(
name,
(minor_version, name),
(
_,
"BaseException"
| "BaseExceptionGroup"
| "GeneratorExit"
| "KeyboardInterrupt"
| "SystemExit"
@ -399,7 +407,6 @@ pub fn is_exception(name: &str) -> bool {
| "AttributeError"
| "BufferError"
| "EOFError"
| "ExceptionGroup"
| "ImportError"
| "ModuleNotFoundError"
| "LookupError"
@ -443,7 +450,6 @@ pub fn is_exception(name: &str) -> bool {
| "Warning"
| "BytesWarning"
| "DeprecationWarning"
| "EncodingWarning"
| "FutureWarning"
| "ImportWarning"
| "PendingDeprecationWarning"
@ -452,5 +458,8 @@ pub fn is_exception(name: &str) -> bool {
| "SyntaxWarning"
| "UnicodeWarning"
| "UserWarning"
) | (10..=13, "EncodingWarning")
| (11..=13, "BaseExceptionGroup" | "ExceptionGroup")
| (13, "PythonFinalizationError")
)
}