diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_21.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_21.py new file mode 100644 index 0000000000000..b4c7f8033273f --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_21.py @@ -0,0 +1,4 @@ +"""Test for IPython-only builtins.""" + +x = 1 +display(x) diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_22.ipynb b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_22.ipynb new file mode 100644 index 0000000000000..7331df58d51db --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F821_22.ipynb @@ -0,0 +1,74 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": 1, + "id": "af8fee97-f9aa-47c2-b34c-b109a5f083d6", + "metadata": {}, + "outputs": [ + { + "data": { + "text/plain": [ + "1" + ] + }, + "execution_count": 1, + "metadata": {}, + "output_type": "execute_result" + } + ], + "source": [ + "1" + ] + }, + { + "cell_type": "code", + "execution_count": 2, + "id": "acd5eb1e-6991-42b8-806f-20d17d7e571f", + "metadata": {}, + "outputs": [ + { + "data": { + "text/plain": [ + "1" + ] + }, + "metadata": {}, + "output_type": "display_data" + } + ], + "source": [ + "display(1)" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "f8f3f599-030e-48b3-bcd3-31d97f725c68", + "metadata": {}, + "outputs": [], + "source": [] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3 (ipykernel)", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.11.5" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 2ccef25e7253b..20b9ea2756b43 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -54,7 +54,7 @@ use ruff_python_semantic::{ ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, Snapshot, StarImport, SubmoduleImport, }; -use ruff_python_stdlib::builtins::{BUILTINS, MAGIC_GLOBALS}; +use ruff_python_stdlib::builtins::{IPYTHON_BUILTINS, MAGIC_GLOBALS, PYTHON_BUILTINS}; use ruff_source_file::Locator; use crate::checkers::ast::deferred::Deferred; @@ -1592,9 +1592,16 @@ impl<'a> Checker<'a> { } fn bind_builtins(&mut self) { - for builtin in BUILTINS + for builtin in PYTHON_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)) { diff --git a/crates/ruff_linter/src/linter.rs b/crates/ruff_linter/src/linter.rs index 0f09b8b03623e..6b6b26f5048ca 100644 --- a/crates/ruff_linter/src/linter.rs +++ b/crates/ruff_linter/src/linter.rs @@ -639,7 +639,7 @@ mod tests { use crate::registry::Rule; use crate::source_kind::SourceKind; - use crate::test::{test_contents, test_notebook_path, TestedNotebook}; + use crate::test::{assert_notebook_path, test_contents, TestedNotebook}; use crate::{assert_messages, settings}; /// Construct a path to a Jupyter notebook in the `resources/test/fixtures/jupyter` directory. @@ -655,7 +655,7 @@ mod tests { messages, source_notebook, .. - } = test_notebook_path( + } = assert_notebook_path( &actual, expected, &settings::LinterSettings::for_rule(Rule::UnsortedImports), @@ -672,7 +672,7 @@ mod tests { messages, source_notebook, .. - } = test_notebook_path( + } = assert_notebook_path( &actual, expected, &settings::LinterSettings::for_rule(Rule::UnusedImport), @@ -689,7 +689,7 @@ mod tests { messages, source_notebook, .. - } = test_notebook_path( + } = assert_notebook_path( &actual, expected, &settings::LinterSettings::for_rule(Rule::UnusedVariable), @@ -706,7 +706,7 @@ mod tests { let TestedNotebook { linted_notebook: fixed_notebook, .. - } = test_notebook_path( + } = assert_notebook_path( actual_path, &expected_path, &settings::LinterSettings::for_rule(Rule::UnusedImport), diff --git a/crates/ruff_linter/src/rules/flake8_builtins/helpers.rs b/crates/ruff_linter/src/rules/flake8_builtins/helpers.rs index 483cecb5fa8f9..54f601b9c9a5b 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/helpers.rs @@ -1,5 +1,14 @@ -use ruff_python_stdlib::builtins::is_builtin; +use ruff_python_ast::PySourceType; +use ruff_python_stdlib::builtins::{is_ipython_builtin, is_python_builtin}; -pub(super) fn shadows_builtin(name: &str, ignorelist: &[String]) -> bool { - is_builtin(name) && ignorelist.iter().all(|ignore| ignore != name) +pub(super) fn shadows_builtin( + name: &str, + ignorelist: &[String], + source_type: PySourceType, +) -> bool { + if is_python_builtin(name) || source_type.is_ipynb() && is_ipython_builtin(name) { + ignorelist.iter().all(|ignore| ignore != name) + } else { + false + } } diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs index 9406879f2fff5..91790de0ea15b 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_argument_shadowing.rs @@ -67,6 +67,7 @@ pub(crate) fn builtin_argument_shadowing(checker: &mut Checker, parameter: &Para if shadows_builtin( parameter.name.as_str(), &checker.settings.flake8_builtins.builtins_ignorelist, + checker.source_type, ) { checker.diagnostics.push(Diagnostic::new( BuiltinArgumentShadowing { diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs index baf682a11011d..ffe8384186321 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_attribute_shadowing.rs @@ -74,7 +74,11 @@ pub(crate) fn builtin_attribute_shadowing( name: &str, range: TextRange, ) { - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { + if shadows_builtin( + name, + &checker.settings.flake8_builtins.builtins_ignorelist, + checker.source_type, + ) { // Ignore shadowing within `TypedDict` definitions, since these are only accessible through // subscripting and not through attribute access. if class_def @@ -102,7 +106,11 @@ pub(crate) fn builtin_method_shadowing( decorator_list: &[Decorator], range: TextRange, ) { - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { + if shadows_builtin( + name, + &checker.settings.flake8_builtins.builtins_ignorelist, + checker.source_type, + ) { // Ignore some standard-library methods. Ideally, we'd ignore all overridden methods, since // those should be flagged on the superclass, but that's more difficult. if is_standard_library_override(name, class_def, checker.semantic()) { diff --git a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs index 74aa7c2e25f0a..9815d8dffba4a 100644 --- a/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs +++ b/crates/ruff_linter/src/rules/flake8_builtins/rules/builtin_variable_shadowing.rs @@ -60,7 +60,11 @@ impl Violation for BuiltinVariableShadowing { /// A001 pub(crate) fn builtin_variable_shadowing(checker: &mut Checker, name: &str, range: TextRange) { - if shadows_builtin(name, &checker.settings.flake8_builtins.builtins_ignorelist) { + if shadows_builtin( + name, + &checker.settings.flake8_builtins.builtins_ignorelist, + checker.source_type, + ) { checker.diagnostics.push(Diagnostic::new( BuiltinVariableShadowing { name: name.to_string(), diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index cfd3f54749261..be2b9040d7f06 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -138,6 +138,8 @@ mod tests { #[test_case(Rule::UndefinedName, Path::new("F821_18.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_19.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_20.py"))] + #[test_case(Rule::UndefinedName, Path::new("F821_21.py"))] + #[test_case(Rule::UndefinedName, Path::new("F821_22.ipynb"))] #[test_case(Rule::UndefinedExport, Path::new("F822_0.py"))] #[test_case(Rule::UndefinedExport, Path::new("F822_1.py"))] #[test_case(Rule::UndefinedExport, Path::new("F822_2.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_21.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_21.py.snap new file mode 100644 index 0000000000000..40c911247ef23 --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_21.py.snap @@ -0,0 +1,11 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F821_21.py:4:1: F821 Undefined name `display` + | +3 | x = 1 +4 | display(x) + | ^^^^^^^ F821 + | + + diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_22.ipynb.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_22.ipynb.snap new file mode 100644 index 0000000000000..d0b409f39ee0b --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F821_F821_22.ipynb.snap @@ -0,0 +1,4 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- + diff --git a/crates/ruff_linter/src/test.rs b/crates/ruff_linter/src/test.rs index 27c94224d9d92..d193e49cc9b8f 100644 --- a/crates/ruff_linter/src/test.rs +++ b/crates/ruff_linter/src/test.rs @@ -38,12 +38,13 @@ pub(crate) fn test_resource_path(path: impl AsRef) -> std::path::PathBuf { Path::new("./resources/test/").join(path) } -/// Run [`check_path`] on a file in the `resources/test/fixtures` directory. +/// Run [`check_path`] on a Python file in the `resources/test/fixtures` directory. #[cfg(not(fuzzing))] pub(crate) fn test_path(path: impl AsRef, settings: &LinterSettings) -> Result> { let path = test_resource_path("fixtures").join(path); - let contents = std::fs::read_to_string(&path)?; - Ok(test_contents(&SourceKind::Python(contents), &path, settings).0) + let source_type = PySourceType::from(&path); + let source_kind = SourceKind::from_path(path.as_ref(), source_type)?.expect("valid source"); + Ok(test_contents(&source_kind, &path, settings).0) } #[cfg(not(fuzzing))] @@ -54,7 +55,7 @@ pub(crate) struct TestedNotebook { } #[cfg(not(fuzzing))] -pub(crate) fn test_notebook_path( +pub(crate) fn assert_notebook_path( path: impl AsRef, expected: impl AsRef, settings: &LinterSettings, diff --git a/crates/ruff_python_stdlib/src/builtins.rs b/crates/ruff_python_stdlib/src/builtins.rs index 56412a19f6f87..39c0c9770d0a4 100644 --- a/crates/ruff_python_stdlib/src/builtins.rs +++ b/crates/ruff_python_stdlib/src/builtins.rs @@ -1,7 +1,7 @@ /// A list of all Python builtins. /// -/// Intended to be kept in sync with [`is_builtin`]. -pub const BUILTINS: &[&str] = &[ +/// Intended to be kept in sync with [`is_python_builtin`]. +pub const PYTHON_BUILTINS: &[&str] = &[ "ArithmeticError", "AssertionError", "AttributeError", @@ -161,6 +161,11 @@ pub const BUILTINS: &[&str] = &[ "zip", ]; +/// A list of all builtins that are available in IPython. +/// +/// Intended to be kept in sync with [`is_ipython_builtin`]. +pub const IPYTHON_BUILTINS: &[&str] = &["display"]; + /// Globally defined names which are not attributes of the builtins module, or /// are only present on some platforms. pub const MAGIC_GLOBALS: &[&str] = &[ @@ -173,9 +178,9 @@ pub const MAGIC_GLOBALS: &[&str] = &[ /// Returns `true` if the given name is that of a Python builtin. /// -/// Intended to be kept in sync with [`BUILTINS`]. -pub fn is_builtin(name: &str) -> bool { - // Constructed by converting the `BUILTINS` slice to a `match` expression. +/// Intended to be kept in sync with [`PYTHON_BUILTINS`]. +pub fn is_python_builtin(name: &str) -> bool { + // Constructed by converting the `PYTHON_BUILTINS` slice to a `match` expression. matches!( name, "ArithmeticError" @@ -345,3 +350,11 @@ pub fn is_iterator(name: &str) -> bool { "enumerate" | "filter" | "map" | "reversed" | "zip" | "iter" ) } + +/// 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 { + // Constructed by converting the `IPYTHON_BUILTINS` slice to a `match` expression. + matches!(name, "display") +}