Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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")
15 changes: 6 additions & 9 deletions crates/ruff_linter/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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))
{
Expand Down
6 changes: 2 additions & 4 deletions crates/ruff_linter/src/rules/flake8_builtins/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
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,
source_type: PySourceType,
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(),
Expand All @@ -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()))
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -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")
167 changes: 88 additions & 79 deletions crates/ruff_python_stdlib/src/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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),
(
Expand Down Expand Up @@ -374,83 +381,85 @@ 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")
}

/// 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,
"BaseException"
| "BaseExceptionGroup"
| "GeneratorExit"
| "KeyboardInterrupt"
| "SystemExit"
| "Exception"
| "ArithmeticError"
| "FloatingPointError"
| "OverflowError"
| "ZeroDivisionError"
| "AssertionError"
| "AttributeError"
| "BufferError"
| "EOFError"
| "ExceptionGroup"
| "ImportError"
| "ModuleNotFoundError"
| "LookupError"
| "IndexError"
| "KeyError"
| "MemoryError"
| "NameError"
| "UnboundLocalError"
| "OSError"
| "BlockingIOError"
| "ChildProcessError"
| "ConnectionError"
| "BrokenPipeError"
| "ConnectionAbortedError"
| "ConnectionRefusedError"
| "ConnectionResetError"
| "FileExistsError"
| "FileNotFoundError"
| "InterruptedError"
| "IsADirectoryError"
| "NotADirectoryError"
| "PermissionError"
| "ProcessLookupError"
| "TimeoutError"
| "ReferenceError"
| "RuntimeError"
| "NotImplementedError"
| "RecursionError"
| "StopAsyncIteration"
| "StopIteration"
| "SyntaxError"
| "IndentationError"
| "TabError"
| "SystemError"
| "TypeError"
| "ValueError"
| "UnicodeError"
| "UnicodeDecodeError"
| "UnicodeEncodeError"
| "UnicodeTranslateError"
| "Warning"
| "BytesWarning"
| "DeprecationWarning"
| "EncodingWarning"
| "FutureWarning"
| "ImportWarning"
| "PendingDeprecationWarning"
| "ResourceWarning"
| "RuntimeWarning"
| "SyntaxWarning"
| "UnicodeWarning"
| "UserWarning"
(minor_version, name),
(
_,
"BaseException"
| "GeneratorExit"
| "KeyboardInterrupt"
| "SystemExit"
| "Exception"
| "ArithmeticError"
| "FloatingPointError"
| "OverflowError"
| "ZeroDivisionError"
| "AssertionError"
| "AttributeError"
| "BufferError"
| "EOFError"
| "ImportError"
| "ModuleNotFoundError"
| "LookupError"
| "IndexError"
| "KeyError"
| "MemoryError"
| "NameError"
| "UnboundLocalError"
| "OSError"
| "BlockingIOError"
| "ChildProcessError"
| "ConnectionError"
| "BrokenPipeError"
| "ConnectionAbortedError"
| "ConnectionRefusedError"
| "ConnectionResetError"
| "FileExistsError"
| "FileNotFoundError"
| "InterruptedError"
| "IsADirectoryError"
| "NotADirectoryError"
| "PermissionError"
| "ProcessLookupError"
| "TimeoutError"
| "ReferenceError"
| "RuntimeError"
| "NotImplementedError"
| "RecursionError"
| "StopAsyncIteration"
| "StopIteration"
| "SyntaxError"
| "IndentationError"
| "TabError"
| "SystemError"
| "TypeError"
| "ValueError"
| "UnicodeError"
| "UnicodeDecodeError"
| "UnicodeEncodeError"
| "UnicodeTranslateError"
| "Warning"
| "BytesWarning"
| "DeprecationWarning"
| "FutureWarning"
| "ImportWarning"
| "PendingDeprecationWarning"
| "ResourceWarning"
| "RuntimeWarning"
| "SyntaxWarning"
| "UnicodeWarning"
| "UserWarning"
) | (10..=13, "EncodingWarning")
| (11..=13, "BaseExceptionGroup" | "ExceptionGroup")
| (13, "PythonFinalizationError")
)
}
Loading