From c269c1a7063bb27e75dc81c2a598eab13aa8b754 Mon Sep 17 00:00:00 2001 From: boolean <61526956+boolean-light@users.noreply.github.com> Date: Wed, 13 Mar 2024 23:13:45 +0900 Subject: [PATCH 1/4] [`pylint`] Implement `invalid-bool-return-type` (`E304`) (#10377) ## Summary Implement `E304` in the issue #970. Throws an error when the returning value of `__bool__` method is not boolean. Reference: https://pylint.readthedocs.io/en/stable/user_guide/messages/error/invalid-bool-returned.html ## Test Plan Add test cases and run `cargo test` --- .../pylint/invalid_return_type_bool.py | 37 +++++++++ .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/pylint/mod.rs | 1 + .../rules/pylint/rules/invalid_bool_return.rs | 78 +++++++++++++++++++ .../ruff_linter/src/rules/pylint/rules/mod.rs | 2 + ...__PLE0304_invalid_return_type_bool.py.snap | 20 +++++ ruff.schema.json | 1 + 8 files changed, 143 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py create mode 100644 crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py new file mode 100644 index 00000000000000..31ab90b738a2e1 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pylint/invalid_return_type_bool.py @@ -0,0 +1,37 @@ +# These testcases should raise errors + +class Float: + def __bool__(self): + return 3.05 # [invalid-bool-return] + +class Int: + def __bool__(self): + return 0 # [invalid-bool-return] + + +class Str: + def __bool__(self): + x = "ruff" + return x # [invalid-bool-return] + +# TODO: Once Ruff has better type checking +def return_int(): + return 3 + +class ComplexReturn: + def __bool__(self): + return return_int() # [invalid-bool-return] + + + +# These testcases should NOT raise errors + +class Bool: + def __bool__(self): + return True + + +class Bool2: + def __bool__(self): + x = True + return x \ No newline at end of file diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index ea1335bbc0cd15..191456bf0766b3 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -91,6 +91,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { checker.diagnostics.push(diagnostic); } } + if checker.enabled(Rule::InvalidBoolReturnType) { + pylint::rules::invalid_bool_return(checker, name, body); + } if checker.enabled(Rule::InvalidStrReturnType) { pylint::rules::invalid_str_return(checker, name, body); } diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index ef05dcb053128c..c471c890a0f1a7 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -240,6 +240,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Pylint, "E0237") => (RuleGroup::Stable, rules::pylint::rules::NonSlotAssignment), (Pylint, "E0241") => (RuleGroup::Stable, rules::pylint::rules::DuplicateBases), (Pylint, "E0302") => (RuleGroup::Stable, rules::pylint::rules::UnexpectedSpecialMethodSignature), + (Pylint, "E0304") => (RuleGroup::Preview, rules::pylint::rules::InvalidBoolReturnType), (Pylint, "E0307") => (RuleGroup::Stable, rules::pylint::rules::InvalidStrReturnType), (Pylint, "E0604") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllObject), (Pylint, "E0605") => (RuleGroup::Stable, rules::pylint::rules::InvalidAllFormat), diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index e03e9303b3f260..0cbd284ddbc8f0 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -71,6 +71,7 @@ mod tests { #[test_case(Rule::ImportSelf, Path::new("import_self/module.py"))] #[test_case(Rule::InvalidAllFormat, Path::new("invalid_all_format.py"))] #[test_case(Rule::InvalidAllObject, Path::new("invalid_all_object.py"))] + #[test_case(Rule::InvalidBoolReturnType, Path::new("invalid_return_type_bool.py"))] #[test_case(Rule::InvalidStrReturnType, Path::new("invalid_return_type_str.py"))] #[test_case(Rule::DuplicateBases, Path::new("duplicate_bases.py"))] #[test_case(Rule::InvalidCharacterBackspace, Path::new("invalid_characters.py"))] diff --git a/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs b/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs new file mode 100644 index 00000000000000..d677b86e51832f --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/rules/invalid_bool_return.rs @@ -0,0 +1,78 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::helpers::ReturnStatementVisitor; +use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::Stmt; +use ruff_python_semantic::analyze::type_inference::{NumberLike, PythonType, ResolvedPythonType}; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for `__bool__` implementations that return a type other than `bool`. +/// +/// ## Why is this bad? +/// The `__bool__` method should return a `bool` object. Returning a different +/// type may cause unexpected behavior. +/// +/// ## Example +/// ```python +/// class Foo: +/// def __bool__(self): +/// return 2 +/// ``` +/// +/// Use instead: +/// ```python +/// class Foo: +/// def __bool__(self): +/// return True +/// ``` +/// +/// ## References +/// - [Python documentation: The `__bool__` method](https://docs.python.org/3/reference/datamodel.html#object.__bool__) +#[violation] +pub struct InvalidBoolReturnType; + +impl Violation for InvalidBoolReturnType { + #[derive_message_formats] + fn message(&self) -> String { + format!("`__bool__` does not return `bool`") + } +} + +/// E0307 +pub(crate) fn invalid_bool_return(checker: &mut Checker, name: &str, body: &[Stmt]) { + if name != "__bool__" { + return; + } + + if !checker.semantic().current_scope().kind.is_class() { + return; + } + + let returns = { + let mut visitor = ReturnStatementVisitor::default(); + visitor.visit_body(body); + visitor.returns + }; + + for stmt in returns { + if let Some(value) = stmt.value.as_deref() { + if !matches!( + ResolvedPythonType::from(value), + ResolvedPythonType::Unknown + | ResolvedPythonType::Atom(PythonType::Number(NumberLike::Bool)) + ) { + checker + .diagnostics + .push(Diagnostic::new(InvalidBoolReturnType, value.range())); + } + } else { + // Disallow implicit `None`. + checker + .diagnostics + .push(Diagnostic::new(InvalidBoolReturnType, stmt.range())); + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/mod.rs b/crates/ruff_linter/src/rules/pylint/rules/mod.rs index 34289564eed02a..8f6d40554d2ad8 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/mod.rs @@ -25,6 +25,7 @@ pub(crate) use import_private_name::*; pub(crate) use import_self::*; pub(crate) use invalid_all_format::*; pub(crate) use invalid_all_object::*; +pub(crate) use invalid_bool_return::*; pub(crate) use invalid_envvar_default::*; pub(crate) use invalid_envvar_value::*; pub(crate) use invalid_str_return::*; @@ -113,6 +114,7 @@ mod import_private_name; mod import_self; mod invalid_all_format; mod invalid_all_object; +mod invalid_bool_return; mod invalid_envvar_default; mod invalid_envvar_value; mod invalid_str_return; diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap new file mode 100644 index 00000000000000..b28107c0851d8a --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE0304_invalid_return_type_bool.py.snap @@ -0,0 +1,20 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +invalid_return_type_bool.py:5:16: PLE0304 `__bool__` does not return `bool` + | +3 | class Float: +4 | def __bool__(self): +5 | return 3.05 # [invalid-bool-return] + | ^^^^ PLE0304 +6 | +7 | class Int: + | + +invalid_return_type_bool.py:9:16: PLE0304 `__bool__` does not return `bool` + | +7 | class Int: +8 | def __bool__(self): +9 | return 0 # [invalid-bool-return] + | ^ PLE0304 + | diff --git a/ruff.schema.json b/ruff.schema.json index 5d44debca2f2fa..d0f0f9a6394e06 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3219,6 +3219,7 @@ "PLE03", "PLE030", "PLE0302", + "PLE0304", "PLE0307", "PLE06", "PLE060", From 2bf1882398273054202d35a1205811d46c24f6a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ho=C3=ABl=20Bagard?= <34478245+hoel-bagard@users.noreply.github.com> Date: Thu, 14 Mar 2024 00:10:48 +0900 Subject: [PATCH 2/4] `docs`: remove `.` from check and format commands (#10217) ## Summary This PR modifies the documentation to use `ruff check` instead of `ruff check .`, and `ruff format` instead of `ruff format .`, as discussed [here](https://github.com/astral-sh/ruff/pull/10168#discussion_r1509976904) --------- Co-authored-by: Micha Reiser Co-authored-by: Zanie Blue --- README.md | 4 ++-- docs/formatter.md | 9 +++++---- docs/installation.md | 8 ++++---- docs/linter.md | 14 ++++++++------ docs/tutorial.md | 20 +++++++++++++------- 5 files changed, 32 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index ba2f07901d0ff0..7bf488eae9eef1 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ and with [a variety of other package managers](https://docs.astral.sh/ruff/insta To run Ruff as a linter, try any of the following: ```shell -ruff check . # Lint all files in the current directory (and any subdirectories). +ruff check # Lint all files in the current directory (and any subdirectories). ruff check path/to/code/ # Lint all files in `/path/to/code` (and any subdirectories). ruff check path/to/code/*.py # Lint all `.py` files in `/path/to/code`. ruff check path/to/code/to/file.py # Lint `file.py`. @@ -139,7 +139,7 @@ ruff check @arguments.txt # Lint using an input file, treating its con Or, to run Ruff as a formatter: ```shell -ruff format . # Format all files in the current directory (and any subdirectories). +ruff format # Format all files in the current directory (and any subdirectories). ruff format path/to/code/ # Format all files in `/path/to/code` (and any subdirectories). ruff format path/to/code/*.py # Format all `.py` files in `/path/to/code`. ruff format path/to/code/to/file.py # Format `file.py`. diff --git a/docs/formatter.md b/docs/formatter.md index df51389979ea11..04d04b14eaf3f1 100644 --- a/docs/formatter.md +++ b/docs/formatter.md @@ -11,8 +11,9 @@ The Ruff formatter is available as of Ruff [v0.1.2](https://astral.sh/blog/the-r directories, and formats all discovered Python files: ```shell -ruff format . # Format all files in the current directory. -ruff format /path/to/file.py # Format a single file. +ruff format # Format all files in the current directory. +ruff format path/to/code/ # Lint all files in `path/to/code` (and any subdirectories). +ruff format path/to/file.py # Format a single file. ``` Similar to Black, running `ruff format /path/to/file.py` will format the given file or directory @@ -422,8 +423,8 @@ Currently, the Ruff formatter does not sort imports. In order to both sort impor call the Ruff linter and then the formatter: ```shell -ruff check --select I --fix . -ruff format . +ruff check --select I --fix +ruff format ``` A unified command for both linting and formatting is [planned](https://github.com/astral-sh/ruff/issues/8232). diff --git a/docs/installation.md b/docs/installation.md index 03e3fe345edca7..06486f98179590 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -9,8 +9,8 @@ pip install ruff Once installed, you can run Ruff from the command line: ```shell -ruff check . # Lint all files in the current directory. -ruff format . # Format all files in the current directory. +ruff check # Lint all files in the current directory. +ruff format # Format all files in the current directory. ``` For **macOS Homebrew** and **Linuxbrew** users, Ruff is also available as [`ruff`](https://formulae.brew.sh/formula/ruff) @@ -58,8 +58,8 @@ On **Docker**, it is published as `ghcr.io/astral-sh/ruff`, tagged for each rele the latest release. ```shell -docker run -v .:/io --rm ghcr.io/astral-sh/ruff check . -docker run -v .:/io --rm ghcr.io/astral-sh/ruff:0.3.0 check . +docker run -v .:/io --rm ghcr.io/astral-sh/ruff check +docker run -v .:/io --rm ghcr.io/astral-sh/ruff:0.3.0 check ``` [![Packaging status](https://repology.org/badge/vertical-allrepos/ruff-python-linter.svg?exclude_unsupported=1)](https://repology.org/project/ruff-python-linter/versions) diff --git a/docs/linter.md b/docs/linter.md index a0119c2e018b92..3f5212eb650d90 100644 --- a/docs/linter.md +++ b/docs/linter.md @@ -11,15 +11,17 @@ and more. directories, and lints all discovered Python files, optionally fixing any fixable errors: ```shell -ruff check . # Lint all files in the current directory. -ruff check . --fix # Lint all files in the current directory, and fix any fixable errors. -ruff check . --watch # Lint all files in the current directory, and re-lint on change. +ruff check # Lint all files in the current directory. +ruff check --fix # Lint all files in the current directory, and fix any fixable errors. +ruff check --watch # Lint all files in the current directory, and re-lint on change. +ruff check path/to/code/ # Lint all files in `path/to/code` (and any subdirectories). ``` For the full list of supported options, run `ruff check --help`. !!! note As of Ruff v0.1.7 the `ruff check` command uses the current working directory (`.`) as the default path to check. + On older versions, you must provide this manually e.g. `ruff check .`. See [the file discovery documentation](configuration.md#python-file-discovery) for details. ## Rule selection @@ -150,7 +152,7 @@ imports, reformat docstrings, rewrite type annotations to use newer Python synta To enable fixes, pass the `--fix` flag to `ruff check`: ```shell -ruff check . --fix +ruff check --fix ``` By default, Ruff will fix all violations for which safe fixes are available; to determine @@ -197,10 +199,10 @@ Ruff only enables safe fixes by default. Unsafe fixes can be enabled by settings ```shell # Show unsafe fixes -ruff check . --unsafe-fixes +ruff check --unsafe-fixes # Apply unsafe fixes -ruff check . --fix --unsafe-fixes +ruff check --fix --unsafe-fixes ``` By default, Ruff will display a hint when unsafe fixes are available but not enabled. The suggestion can be silenced diff --git a/docs/tutorial.md b/docs/tutorial.md index 4e5f242daf4fda..cb7139d473b325 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -38,7 +38,7 @@ def sum_even_numbers(numbers: Iterable[int]) -> int: We can run the Ruff linter over our project via `ruff check`: ```shell -❯ ruff check . +❯ ruff check numbers/numbers.py:3:8: F401 [*] `os` imported but unused Found 1 error. [*] 1 fixable with the `--fix` option. @@ -48,7 +48,7 @@ Ruff identified an unused import, which is a common error in Python code. Ruff c "fixable" error, so we can resolve the issue automatically by running `ruff check --fix`: ```shell -❯ ruff check --fix . +❯ ruff check --fix Found 1 error (1 fixed, 0 remaining). ``` @@ -71,10 +71,16 @@ def sum_even_numbers(numbers: Iterable[int]) -> int: ) ``` +Note Ruff runs in the current directory by default, but you can pass specific paths to check: + +```shell +❯ ruff check numbers/numbers.py +``` + Now that our project is passing `ruff check`, we can run the Ruff formatter via `ruff format`: ```shell -❯ ruff format . +❯ ruff format 1 file reformatted ``` @@ -135,7 +141,7 @@ To configure Ruff, let's create a configuration file in our project's root direc Running Ruff again, we see that it now enforces a maximum line width, with a limit of 79: ```shell -❯ ruff check . +❯ ruff check numbers/numbers.py:5:80: E501 Line too long (90 > 79) Found 1 error. ``` @@ -217,7 +223,7 @@ If we run Ruff again, we'll see that it now enforces the pyupgrade rules. In par the use of the deprecated `typing.Iterable` instead of `collections.abc.Iterable`: ```shell -❯ ruff check . +❯ ruff check numbers/numbers.py:1:1: UP035 [*] Import from `collections.abc` instead: `Iterable` Found 1 error. [*] 1 fixable with the `--fix` option. @@ -260,7 +266,7 @@ all functions have docstrings: If we run Ruff again, we'll see that it now enforces the pydocstyle rules: ```shell -❯ ruff check . +❯ ruff check numbers/__init__.py:1:1: D104 Missing docstring in public package numbers/numbers.py:1:1: UP035 [*] Import from `collections.abc` instead: `Iterable` numbers/numbers.py:1:1: D100 Missing docstring in public module @@ -285,7 +291,7 @@ def sum_even_numbers(numbers: Iterable[int]) -> int: Running `ruff check` again, we'll see that it no longer flags the `Iterable` import: ```shell -❯ ruff check . +❯ ruff check numbers/__init__.py:1:1: D104 Missing docstring in public package numbers/numbers.py:1:1: D100 Missing docstring in public module Found 3 errors. From d59433b12edf0c6a0b8fe075461a6bcd46fa2502 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 13 Mar 2024 08:44:28 -0700 Subject: [PATCH 3/4] Avoid removing shadowed imports that point to different symbols (#10387) This ensures that we don't have incorrect, automated fixes for shadowed names that actually point to different imports. See: https://github.com/astral-sh/ruff/issues/10384. --- .../test/fixtures/pyflakes/F811_28.py | 6 +++ .../checkers/ast/analyze/deferred_scopes.rs | 40 +++++++++++-------- crates/ruff_linter/src/rules/pyflakes/mod.rs | 1 + ...ules__pyflakes__tests__F811_F811_1.py.snap | 8 +--- ...les__pyflakes__tests__F811_F811_12.py.snap | 12 +----- ...ules__pyflakes__tests__F811_F811_2.py.snap | 8 +--- ...les__pyflakes__tests__F811_F811_23.py.snap | 10 +---- ...les__pyflakes__tests__F811_F811_28.py.snap | 12 ++++++ 8 files changed, 46 insertions(+), 51 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/pyflakes/F811_28.py create mode 100644 crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_28.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_28.py b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_28.py new file mode 100644 index 00000000000000..4fc238e98022a3 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/pyflakes/F811_28.py @@ -0,0 +1,6 @@ +"""Regression test for: https://github.com/astral-sh/ruff/issues/10384""" + +import datetime +from datetime import datetime + +datetime(1, 2, 3) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs index e1d662922384b9..c35cf53a77a3a1 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/deferred_scopes.rs @@ -259,23 +259,29 @@ pub(crate) fn deferred_scopes(checker: &mut Checker) { diagnostic.set_parent(range.start()); } - if let Some(import) = binding.as_any_import() { - if let Some(source) = binding.source { - diagnostic.try_set_fix(|| { - let statement = checker.semantic().statement(source); - let parent = checker.semantic().parent_statement(source); - let edit = fix::edits::remove_unused_imports( - std::iter::once(import.member_name().as_ref()), - statement, - parent, - checker.locator(), - checker.stylist(), - checker.indexer(), - )?; - Ok(Fix::safe_edit(edit).isolate(Checker::isolation( - checker.semantic().parent_statement_id(source), - ))) - }); + // Remove the import if the binding and the shadowed binding are both imports, + // and both point to the same qualified name. + if let Some(shadowed_import) = shadowed.as_any_import() { + if let Some(import) = binding.as_any_import() { + if shadowed_import.qualified_name() == import.qualified_name() { + if let Some(source) = binding.source { + diagnostic.try_set_fix(|| { + let statement = checker.semantic().statement(source); + let parent = checker.semantic().parent_statement(source); + let edit = fix::edits::remove_unused_imports( + std::iter::once(import.member_name().as_ref()), + statement, + parent, + checker.locator(), + checker.stylist(), + checker.indexer(), + )?; + Ok(Fix::safe_edit(edit).isolate(Checker::isolation( + checker.semantic().parent_statement_id(source), + ))) + }); + } + } } } diff --git a/crates/ruff_linter/src/rules/pyflakes/mod.rs b/crates/ruff_linter/src/rules/pyflakes/mod.rs index 64840631d8bbc6..e913278245c499 100644 --- a/crates/ruff_linter/src/rules/pyflakes/mod.rs +++ b/crates/ruff_linter/src/rules/pyflakes/mod.rs @@ -124,6 +124,7 @@ mod tests { #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_25.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_26.py"))] #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_27.py"))] + #[test_case(Rule::RedefinedWhileUnused, Path::new("F811_28.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_0.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_1.py"))] #[test_case(Rule::UndefinedName, Path::new("F821_2.py"))] diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_1.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_1.py.snap index b3d2ebd4536678..19db1b0eddce71 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_1.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_1.py.snap @@ -1,15 +1,9 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F811_1.py:1:25: F811 [*] Redefinition of unused `FU` from line 1 +F811_1.py:1:25: F811 Redefinition of unused `FU` from line 1 | 1 | import fu as FU, bar as FU | ^^ F811 | = help: Remove definition: `FU` - -ℹ Safe fix -1 |-import fu as FU, bar as FU - 1 |+import fu as FU - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_12.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_12.py.snap index 1411fddf047c7c..0d490baf578079 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_12.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_12.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F811_12.py:6:20: F811 [*] Redefinition of unused `mixer` from line 2 +F811_12.py:6:20: F811 Redefinition of unused `mixer` from line 2 | 4 | pass 5 | else: @@ -10,13 +10,3 @@ F811_12.py:6:20: F811 [*] Redefinition of unused `mixer` from line 2 7 | mixer(123) | = help: Remove definition: `mixer` - -ℹ Safe fix -3 3 | except ImportError: -4 4 | pass -5 5 | else: -6 |- from bb import mixer - 6 |+ pass -7 7 | mixer(123) - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_2.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_2.py.snap index 9e87d29c8d81ef..ce01b9cd148c83 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_2.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_2.py.snap @@ -1,15 +1,9 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F811_2.py:1:34: F811 [*] Redefinition of unused `FU` from line 1 +F811_2.py:1:34: F811 Redefinition of unused `FU` from line 1 | 1 | from moo import fu as FU, bar as FU | ^^ F811 | = help: Remove definition: `FU` - -ℹ Safe fix -1 |-from moo import fu as FU, bar as FU - 1 |+from moo import fu as FU - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_23.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_23.py.snap index d03de5d32f98b6..4d77d04b236e2f 100644 --- a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_23.py.snap +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_23.py.snap @@ -1,18 +1,10 @@ --- source: crates/ruff_linter/src/rules/pyflakes/mod.rs --- -F811_23.py:4:15: F811 [*] Redefinition of unused `foo` from line 3 +F811_23.py:4:15: F811 Redefinition of unused `foo` from line 3 | 3 | import foo as foo 4 | import bar as foo | ^^^ F811 | = help: Remove definition: `foo` - -ℹ Safe fix -1 1 | """Test that shadowing an explicit re-export produces a warning.""" -2 2 | -3 3 | import foo as foo -4 |-import bar as foo - - diff --git a/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_28.py.snap b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_28.py.snap new file mode 100644 index 00000000000000..51c91bc474062f --- /dev/null +++ b/crates/ruff_linter/src/rules/pyflakes/snapshots/ruff_linter__rules__pyflakes__tests__F811_F811_28.py.snap @@ -0,0 +1,12 @@ +--- +source: crates/ruff_linter/src/rules/pyflakes/mod.rs +--- +F811_28.py:4:22: F811 Redefinition of unused `datetime` from line 3 + | +3 | import datetime +4 | from datetime import datetime + | ^^^^^^^^ F811 +5 | +6 | datetime(1, 2, 3) + | + = help: Remove definition: `datetime` From c2e15f38ee37f82d586a6dc1dd7c5ed1075499ee Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 13 Mar 2024 17:19:17 +0000 Subject: [PATCH 4/4] Unify enums used for internal representation of quoting style (#10383) --- Cargo.lock | 1 + crates/ruff_linter/src/checkers/ast/mod.rs | 19 ++--- .../src/rules/flake8_quotes/settings.rs | 8 +-- .../rules/pydocstyle/rules/triple_quotes.rs | 2 +- crates/ruff_python_ast/src/nodes.rs | 29 +++++--- crates/ruff_python_ast/src/str.rs | 40 +++++++++-- crates/ruff_python_codegen/src/generator.rs | 21 +++--- crates/ruff_python_codegen/src/lib.rs | 2 +- crates/ruff_python_codegen/src/stylist.rs | 58 ++-------------- crates/ruff_python_formatter/src/context.rs | 8 +-- .../src/string/docstring.rs | 11 +-- .../ruff_python_formatter/src/string/mod.rs | 69 +++++-------------- .../src/string/normalize.rs | 37 +++++----- crates/ruff_python_literal/Cargo.toml | 2 + crates/ruff_python_literal/src/escape.rs | 43 ++---------- .../src/string_token_flags.rs | 16 ++--- 16 files changed, 143 insertions(+), 223 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 99cc55165ad97a..b8345646d05ace 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2345,6 +2345,7 @@ dependencies = [ "itertools 0.12.1", "lexical-parse-float", "rand", + "ruff_python_ast", "unic-ucd-category", ] diff --git a/crates/ruff_linter/src/checkers/ast/mod.rs b/crates/ruff_linter/src/checkers/ast/mod.rs index 11c8b47aa59e67..cda951ecba299f 100644 --- a/crates/ruff_linter/src/checkers/ast/mod.rs +++ b/crates/ruff_linter/src/checkers/ast/mod.rs @@ -44,10 +44,10 @@ use ruff_python_ast::helpers::{ }; use ruff_python_ast::identifier::Identifier; use ruff_python_ast::name::QualifiedName; -use ruff_python_ast::str::trailing_quote; +use ruff_python_ast::str::Quote; use ruff_python_ast::visitor::{walk_except_handler, walk_f_string_element, walk_pattern, Visitor}; use ruff_python_ast::{helpers, str, visitor, PySourceType}; -use ruff_python_codegen::{Generator, Quote, Stylist}; +use ruff_python_codegen::{Generator, Stylist}; use ruff_python_index::Indexer; use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind}; use ruff_python_semantic::analyze::{imports, typing, visibility}; @@ -228,16 +228,11 @@ impl<'a> Checker<'a> { } // Find the quote character used to start the containing f-string. - let expr = self.semantic.current_expression()?; - let string_range = self.indexer.fstring_ranges().innermost(expr.start())?; - let trailing_quote = trailing_quote(self.locator.slice(string_range))?; - - // Invert the quote character, if it's a single quote. - match trailing_quote { - "'" => Some(Quote::Double), - "\"" => Some(Quote::Single), - _ => None, - } + let ast::ExprFString { value, .. } = self + .semantic + .current_expressions() + .find_map(|expr| expr.as_f_string_expr())?; + Some(value.iter().next()?.quote_style().opposite()) } /// Returns the [`SourceRow`] for the given offset. diff --git a/crates/ruff_linter/src/rules/flake8_quotes/settings.rs b/crates/ruff_linter/src/rules/flake8_quotes/settings.rs index 31160302f033bb..5e0c93beadab04 100644 --- a/crates/ruff_linter/src/rules/flake8_quotes/settings.rs +++ b/crates/ruff_linter/src/rules/flake8_quotes/settings.rs @@ -22,11 +22,11 @@ impl Default for Quote { } } -impl From for Quote { - fn from(value: ruff_python_ast::str::QuoteStyle) -> Self { +impl From for Quote { + fn from(value: ruff_python_ast::str::Quote) -> Self { match value { - ruff_python_ast::str::QuoteStyle::Double => Self::Double, - ruff_python_ast::str::QuoteStyle::Single => Self::Single, + ruff_python_ast::str::Quote::Double => Self::Double, + ruff_python_ast::str::Quote::Single => Self::Single, } } } diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/triple_quotes.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/triple_quotes.rs index 5e28786cf6521d..5109c8b86303f4 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/triple_quotes.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/triple_quotes.rs @@ -1,6 +1,6 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_codegen::Quote; +use ruff_python_ast::str::Quote; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; diff --git a/crates/ruff_python_ast/src/nodes.rs b/crates/ruff_python_ast/src/nodes.rs index 89133331f1719e..4644164bb5a522 100644 --- a/crates/ruff_python_ast/src/nodes.rs +++ b/crates/ruff_python_ast/src/nodes.rs @@ -11,7 +11,7 @@ use itertools::Itertools; use ruff_text_size::{Ranged, TextRange, TextSize}; -use crate::{int, str::QuoteStyle, LiteralExpressionRef}; +use crate::{int, str::Quote, LiteralExpressionRef}; /// See also [mod](https://docs.python.org/3/library/ast.html#ast.mod) #[derive(Clone, Debug, PartialEq, is_macro::Is)] @@ -1159,6 +1159,15 @@ pub enum FStringPart { FString(FString), } +impl FStringPart { + pub fn quote_style(&self) -> Quote { + match self { + Self::Literal(string_literal) => string_literal.flags.quote_style(), + Self::FString(f_string) => f_string.flags.quote_style(), + } + } +} + impl Ranged for FStringPart { fn range(&self) -> TextRange { match self { @@ -1221,11 +1230,11 @@ impl FStringFlags { } /// Does the f-string use single or double quotes in its opener and closer? - pub const fn quote_style(self) -> QuoteStyle { + pub const fn quote_style(self) -> Quote { if self.0.contains(FStringFlagsInner::DOUBLE) { - QuoteStyle::Double + Quote::Double } else { - QuoteStyle::Single + Quote::Single } } } @@ -1535,11 +1544,11 @@ impl StringLiteralFlags { } /// Does the string use single or double quotes in its opener and closer? - pub const fn quote_style(self) -> QuoteStyle { + pub const fn quote_style(self) -> Quote { if self.0.contains(StringLiteralFlagsInner::DOUBLE) { - QuoteStyle::Double + Quote::Double } else { - QuoteStyle::Single + Quote::Single } } @@ -1864,11 +1873,11 @@ impl BytesLiteralFlags { } /// Does the bytestring use single or double quotes in its opener and closer? - pub const fn quote_style(self) -> QuoteStyle { + pub const fn quote_style(self) -> Quote { if self.0.contains(BytesLiteralFlagsInner::DOUBLE) { - QuoteStyle::Double + Quote::Double } else { - QuoteStyle::Single + Quote::Single } } } diff --git a/crates/ruff_python_ast/src/str.rs b/crates/ruff_python_ast/src/str.rs index 7f1c6777c40a46..220df07857f703 100644 --- a/crates/ruff_python_ast/src/str.rs +++ b/crates/ruff_python_ast/src/str.rs @@ -1,18 +1,23 @@ +use std::fmt; + use aho_corasick::{AhoCorasick, AhoCorasickKind, Anchored, Input, MatchKind, StartKind}; use once_cell::sync::Lazy; use ruff_text_size::{TextLen, TextRange}; +/// Enumeration of the two kinds of quotes that can be used +/// for Python string/f-string/bytestring literals #[derive(Debug, Default, Copy, Clone, Hash, PartialEq, Eq, is_macro::Is)] -pub enum QuoteStyle { - /// E.g. ' +pub enum Quote { + /// E.g. `'` Single, - /// E.g. " + /// E.g. `"` #[default] Double, } -impl QuoteStyle { +impl Quote { + #[inline] pub const fn as_char(self) -> char { match self { Self::Single => '\'', @@ -21,12 +26,39 @@ impl QuoteStyle { } #[must_use] + #[inline] pub const fn opposite(self) -> Self { match self { Self::Single => Self::Double, Self::Double => Self::Single, } } + + #[inline] + pub const fn as_byte(self) -> u8 { + match self { + Self::Single => b'\'', + Self::Double => b'"', + } + } +} + +impl fmt::Display for Quote { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_char()) + } +} + +impl TryFrom for Quote { + type Error = (); + + fn try_from(value: char) -> Result { + match value { + '\'' => Ok(Quote::Single), + '"' => Ok(Quote::Double), + _ => Err(()), + } + } } /// Includes all permutations of `r`, `u`, `f`, and `fr` (`ur` is invalid, as is `uf`). This diff --git a/crates/ruff_python_codegen/src/generator.rs b/crates/ruff_python_codegen/src/generator.rs index 64d27ee55037e1..01f7449a3aa876 100644 --- a/crates/ruff_python_codegen/src/generator.rs +++ b/crates/ruff_python_codegen/src/generator.rs @@ -2,6 +2,7 @@ use std::ops::Deref; +use ruff_python_ast::str::Quote; use ruff_python_ast::{ self as ast, Alias, ArgOrKeyword, BoolOp, CmpOp, Comprehension, ConversionFlag, DebugText, ExceptHandler, Expr, Identifier, MatchCase, Operator, Parameter, Parameters, Pattern, @@ -12,7 +13,7 @@ use ruff_python_ast::{ParameterWithDefault, TypeParams}; use ruff_python_literal::escape::{AsciiEscape, Escape, UnicodeEscape}; use ruff_source_file::LineEnding; -use super::stylist::{Indentation, Quote, Stylist}; +use super::stylist::{Indentation, Stylist}; mod precedence { pub(crate) const NAMED_EXPR: u8 = 1; @@ -150,7 +151,7 @@ impl<'a> Generator<'a> { } fn p_bytes_repr(&mut self, s: &[u8]) { - let escape = AsciiEscape::with_preferred_quote(s, self.quote.into()); + let escape = AsciiEscape::with_preferred_quote(s, self.quote); if let Some(len) = escape.layout().len { self.buffer.reserve(len); } @@ -158,7 +159,7 @@ impl<'a> Generator<'a> { } fn p_str_repr(&mut self, s: &str) { - let escape = UnicodeEscape::with_preferred_quote(s, self.quote.into()); + let escape = UnicodeEscape::with_preferred_quote(s, self.quote); if let Some(len) = escape.layout().len { self.buffer.reserve(len); } @@ -1373,14 +1374,8 @@ impl<'a> Generator<'a> { self.unparse_f_string_body(values); } else { self.p("f"); - let mut generator = Generator::new( - self.indent, - match self.quote { - Quote::Single => Quote::Double, - Quote::Double => Quote::Single, - }, - self.line_ending, - ); + let mut generator = + Generator::new(self.indent, self.quote.opposite(), self.line_ending); generator.unparse_f_string_body(values); let body = &generator.buffer; self.p_str_repr(body); @@ -1406,11 +1401,11 @@ impl<'a> Generator<'a> { #[cfg(test)] mod tests { - use ruff_python_ast::{Mod, ModModule}; + use ruff_python_ast::{str::Quote, Mod, ModModule}; use ruff_python_parser::{self, parse_suite, Mode}; use ruff_source_file::LineEnding; - use crate::stylist::{Indentation, Quote}; + use crate::stylist::Indentation; use super::Generator; diff --git a/crates/ruff_python_codegen/src/lib.rs b/crates/ruff_python_codegen/src/lib.rs index de55f0435eb826..baa71ea1278fb4 100644 --- a/crates/ruff_python_codegen/src/lib.rs +++ b/crates/ruff_python_codegen/src/lib.rs @@ -4,7 +4,7 @@ mod stylist; pub use generator::Generator; use ruff_python_parser::{lexer, parse_suite, Mode, ParseError}; use ruff_source_file::Locator; -pub use stylist::{Quote, Stylist}; +pub use stylist::Stylist; /// Run round-trip source code generation on a given Python code. pub fn round_trip(code: &str) -> Result { diff --git a/crates/ruff_python_codegen/src/stylist.rs b/crates/ruff_python_codegen/src/stylist.rs index b6b3b1a64fdd26..ffaed80f543f21 100644 --- a/crates/ruff_python_codegen/src/stylist.rs +++ b/crates/ruff_python_codegen/src/stylist.rs @@ -1,15 +1,13 @@ //! Detect code style from Python source code. -use std::fmt; use std::ops::Deref; use once_cell::unsync::OnceCell; -use ruff_python_literal::escape::Quote as StrQuote; + +use ruff_python_ast::str::Quote; use ruff_python_parser::lexer::LexResult; use ruff_python_parser::Tok; -use ruff_source_file::{find_newline, LineEnding}; - -use ruff_source_file::Locator; +use ruff_source_file::{find_newline, LineEnding, Locator}; #[derive(Debug, Clone)] pub struct Stylist<'a> { @@ -52,10 +50,8 @@ impl<'a> Stylist<'a> { fn detect_quote(tokens: &[LexResult]) -> Quote { for (token, _) in tokens.iter().flatten() { match token { - Tok::String { kind, .. } if !kind.is_triple_quoted() => { - return kind.quote_style().into() - } - Tok::FStringStart(kind) => return kind.quote_style().into(), + Tok::String { kind, .. } if !kind.is_triple_quoted() => return kind.quote_style(), + Tok::FStringStart(kind) => return kind.quote_style(), _ => continue, } } @@ -94,50 +90,6 @@ fn detect_indention(tokens: &[LexResult], locator: &Locator) -> Indentation { } } -/// The quotation style used in Python source code. -#[derive(Debug, Default, PartialEq, Eq, Copy, Clone)] -pub enum Quote { - Single, - #[default] - Double, -} - -impl From for Quote { - fn from(value: ruff_python_ast::str::QuoteStyle) -> Self { - match value { - ruff_python_ast::str::QuoteStyle::Double => Self::Double, - ruff_python_ast::str::QuoteStyle::Single => Self::Single, - } - } -} - -impl From for char { - fn from(val: Quote) -> Self { - match val { - Quote::Single => '\'', - Quote::Double => '"', - } - } -} - -impl From for StrQuote { - fn from(val: Quote) -> Self { - match val { - Quote::Single => StrQuote::Single, - Quote::Double => StrQuote::Double, - } - } -} - -impl fmt::Display for Quote { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Quote::Single => write!(f, "\'"), - Quote::Double => write!(f, "\""), - } - } -} - /// The indentation style used in Python source code. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Indentation(String); diff --git a/crates/ruff_python_formatter/src/context.rs b/crates/ruff_python_formatter/src/context.rs index 2bbab9fbb02614..ebfdb782ff5d8f 100644 --- a/crates/ruff_python_formatter/src/context.rs +++ b/crates/ruff_python_formatter/src/context.rs @@ -1,8 +1,8 @@ use crate::comments::Comments; use crate::other::f_string::FStringContext; -use crate::string::QuoteChar; use crate::PyFormatOptions; use ruff_formatter::{Buffer, FormatContext, GroupId, IndentWidth, SourceCode}; +use ruff_python_ast::str::Quote; use ruff_source_file::Locator; use std::fmt::{Debug, Formatter}; use std::ops::{Deref, DerefMut}; @@ -22,7 +22,7 @@ pub struct PyFormatContext<'a> { /// works. For example, multi-line strings will always be written with a /// quote style that is inverted from the one here in order to ensure that /// the formatted Python code will be valid. - docstring: Option, + docstring: Option, /// The state of the formatter with respect to f-strings. f_string_state: FStringState, } @@ -74,7 +74,7 @@ impl<'a> PyFormatContext<'a> { /// /// The quote character returned corresponds to the quoting used for the /// docstring containing the code snippet currently being formatted. - pub(crate) fn docstring(&self) -> Option { + pub(crate) fn docstring(&self) -> Option { self.docstring } @@ -83,7 +83,7 @@ impl<'a> PyFormatContext<'a> { /// /// The quote character given should correspond to the quote character used /// for the docstring containing the code snippets. - pub(crate) fn in_docstring(self, quote: QuoteChar) -> PyFormatContext<'a> { + pub(crate) fn in_docstring(self, quote: Quote) -> PyFormatContext<'a> { PyFormatContext { docstring: Some(quote), ..self diff --git a/crates/ruff_python_formatter/src/string/docstring.rs b/crates/ruff_python_formatter/src/string/docstring.rs index a6b4539024ed28..2e0a0b0aa1d810 100644 --- a/crates/ruff_python_formatter/src/string/docstring.rs +++ b/crates/ruff_python_formatter/src/string/docstring.rs @@ -8,6 +8,7 @@ use std::{borrow::Cow, collections::VecDeque}; use itertools::Itertools; use ruff_formatter::printer::SourceMapGeneration; +use ruff_python_ast::str::Quote; use ruff_python_parser::ParseError; use {once_cell::sync::Lazy, regex::Regex}; use { @@ -19,7 +20,7 @@ use { use crate::{prelude::*, DocstringCodeLineWidth, FormatModuleError}; -use super::{NormalizedString, QuoteChar}; +use super::NormalizedString; /// Format a docstring by trimming whitespace and adjusting the indentation. /// @@ -253,7 +254,7 @@ struct DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { already_normalized: bool, /// The quote character used by the docstring being printed. - quote_char: QuoteChar, + quote_char: Quote, /// The current code example detected in the docstring. code_example: CodeExample<'src>, @@ -550,8 +551,8 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { // remove this check. See the `doctest_invalid_skipped` tests in // `docstring_code_examples.py` for when this check is relevant. let wrapped = match self.quote_char { - QuoteChar::Single => std::format!("'''{}'''", printed.as_code()), - QuoteChar::Double => { + Quote::Single => std::format!("'''{}'''", printed.as_code()), + Quote::Double => { std::format!(r#""""{}""""#, printed.as_code()) } }; @@ -1542,7 +1543,7 @@ enum CodeExampleAddAction<'src> { /// inside of a docstring. fn docstring_format_source( options: crate::PyFormatOptions, - docstring_quote_style: QuoteChar, + docstring_quote_style: Quote, source: &str, ) -> Result { use ruff_python_parser::AsMode; diff --git a/crates/ruff_python_formatter/src/string/mod.rs b/crates/ruff_python_formatter/src/string/mod.rs index 1980e1a3923f29..eb5b834f4304a8 100644 --- a/crates/ruff_python_formatter/src/string/mod.rs +++ b/crates/ruff_python_formatter/src/string/mod.rs @@ -3,6 +3,7 @@ use bitflags::bitflags; pub(crate) use any::AnyString; pub(crate) use normalize::{normalize_string, NormalizedString, StringNormalizer}; use ruff_formatter::format_args; +use ruff_python_ast::str::Quote; use ruff_source_file::Locator; use ruff_text_size::{TextLen, TextRange, TextSize}; @@ -187,7 +188,7 @@ impl Format> for StringPrefix { #[derive(Copy, Clone, Debug)] pub(crate) struct StringQuotes { triple: bool, - quote_char: QuoteChar, + quote_char: Quote, } impl StringQuotes { @@ -195,7 +196,7 @@ impl StringQuotes { let mut chars = input.chars(); let quote_char = chars.next()?; - let quote = QuoteChar::try_from(quote_char).ok()?; + let quote = Quote::try_from(quote_char).ok()?; let triple = chars.next() == Some(quote_char) && chars.next() == Some(quote_char); @@ -221,69 +222,33 @@ impl StringQuotes { impl Format> for StringQuotes { fn fmt(&self, f: &mut PyFormatter) -> FormatResult<()> { let quotes = match (self.quote_char, self.triple) { - (QuoteChar::Single, false) => "'", - (QuoteChar::Single, true) => "'''", - (QuoteChar::Double, false) => "\"", - (QuoteChar::Double, true) => "\"\"\"", + (Quote::Single, false) => "'", + (Quote::Single, true) => "'''", + (Quote::Double, false) => "\"", + (Quote::Double, true) => "\"\"\"", }; token(quotes).fmt(f) } } -/// The quotation character used to quote a string, byte, or fstring literal. -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum QuoteChar { - /// A single quote: `'` - Single, - - /// A double quote: '"' - Double, -} - -impl QuoteChar { - pub const fn as_char(self) -> char { - match self { - QuoteChar::Single => '\'', - QuoteChar::Double => '"', - } - } - - #[must_use] - pub const fn invert(self) -> QuoteChar { - match self { - QuoteChar::Single => QuoteChar::Double, - QuoteChar::Double => QuoteChar::Single, - } - } +impl TryFrom for Quote { + type Error = (); - #[must_use] - pub const fn from_style(style: QuoteStyle) -> Option { + fn try_from(style: QuoteStyle) -> Result { match style { - QuoteStyle::Single => Some(QuoteChar::Single), - QuoteStyle::Double => Some(QuoteChar::Double), - QuoteStyle::Preserve => None, - } - } -} - -impl From for QuoteStyle { - fn from(value: QuoteChar) -> Self { - match value { - QuoteChar::Single => QuoteStyle::Single, - QuoteChar::Double => QuoteStyle::Double, + QuoteStyle::Single => Ok(Quote::Single), + QuoteStyle::Double => Ok(Quote::Double), + QuoteStyle::Preserve => Err(()), } } } -impl TryFrom for QuoteChar { - type Error = (); - - fn try_from(value: char) -> Result { +impl From for QuoteStyle { + fn from(value: Quote) -> Self { match value { - '\'' => Ok(QuoteChar::Single), - '"' => Ok(QuoteChar::Double), - _ => Err(()), + Quote::Single => QuoteStyle::Single, + Quote::Double => QuoteStyle::Double, } } } diff --git a/crates/ruff_python_formatter/src/string/normalize.rs b/crates/ruff_python_formatter/src/string/normalize.rs index 18030528020a1d..7af07597c01ef3 100644 --- a/crates/ruff_python_formatter/src/string/normalize.rs +++ b/crates/ruff_python_formatter/src/string/normalize.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::iter::FusedIterator; use ruff_formatter::FormatContext; +use ruff_python_ast::str::Quote; use ruff_source_file::Locator; use ruff_text_size::{Ranged, TextRange}; @@ -9,13 +10,13 @@ use crate::context::FStringState; use crate::options::PythonVersion; use crate::prelude::*; use crate::preview::is_f_string_formatting_enabled; -use crate::string::{QuoteChar, Quoting, StringPart, StringPrefix, StringQuotes}; +use crate::string::{Quoting, StringPart, StringPrefix, StringQuotes}; use crate::QuoteStyle; pub(crate) struct StringNormalizer { quoting: Quoting, preferred_quote_style: QuoteStyle, - parent_docstring_quote_char: Option, + parent_docstring_quote_char: Option, f_string_state: FStringState, target_version: PythonVersion, format_fstring: bool, @@ -130,7 +131,7 @@ impl StringNormalizer { // style from what the parent ultimately decided upon works, even // if it doesn't have perfect alignment with PEP8. if let Some(quote) = self.parent_docstring_quote_char { - QuoteStyle::from(quote.invert()) + QuoteStyle::from(quote.opposite()) } else if self.preferred_quote_style.is_preserve() { QuoteStyle::Preserve } else { @@ -140,7 +141,7 @@ impl StringNormalizer { self.preferred_quote_style }; - if let Some(preferred_quote) = QuoteChar::from_style(preferred_style) { + if let Ok(preferred_quote) = Quote::try_from(preferred_style) { if let Some(first_quote_or_normalized_char_offset) = first_quote_or_normalized_char_offset { @@ -281,7 +282,7 @@ impl Format> for NormalizedString<'_> { fn choose_quotes_for_raw_string( input: &str, quotes: StringQuotes, - preferred_quote: QuoteChar, + preferred_quote: Quote, ) -> StringQuotes { let preferred_quote_char = preferred_quote.as_char(); let mut chars = input.chars().peekable(); @@ -337,11 +338,7 @@ fn choose_quotes_for_raw_string( /// For triple quoted strings, the preferred quote style is always used, unless the string contains /// a triplet of the quote character (e.g., if double quotes are preferred, double quotes will be /// used unless the string contains `"""`). -fn choose_quotes_impl( - input: &str, - quotes: StringQuotes, - preferred_quote: QuoteChar, -) -> StringQuotes { +fn choose_quotes_impl(input: &str, quotes: StringQuotes, preferred_quote: Quote) -> StringQuotes { let quote = if quotes.triple { // True if the string contains a triple quote sequence of the configured quote style. let mut uses_triple_quotes = false; @@ -419,18 +416,18 @@ fn choose_quotes_impl( } match preferred_quote { - QuoteChar::Single => { + Quote::Single => { if single_quotes > double_quotes { - QuoteChar::Double + Quote::Double } else { - QuoteChar::Single + Quote::Single } } - QuoteChar::Double => { + Quote::Double => { if double_quotes > single_quotes { - QuoteChar::Single + Quote::Single } else { - QuoteChar::Double + Quote::Double } } } @@ -462,7 +459,7 @@ pub(crate) fn normalize_string( let quote = quotes.quote_char; let preferred_quote = quote.as_char(); - let opposite_quote = quote.invert().as_char(); + let opposite_quote = quote.opposite().as_char(); let mut chars = CharIndicesWithOffset::new(input, start_offset).peekable(); @@ -707,7 +704,9 @@ impl UnicodeEscape { mod tests { use std::borrow::Cow; - use crate::string::{QuoteChar, StringPrefix, StringQuotes}; + use ruff_python_ast::str::Quote; + + use crate::string::{StringPrefix, StringQuotes}; use super::{normalize_string, UnicodeEscape}; @@ -730,7 +729,7 @@ mod tests { 0, StringQuotes { triple: false, - quote_char: QuoteChar::Double, + quote_char: Quote::Double, }, StringPrefix::BYTE, true, diff --git a/crates/ruff_python_literal/Cargo.toml b/crates/ruff_python_literal/Cargo.toml index 155ac57bbeb8fd..905aa3e58e4438 100644 --- a/crates/ruff_python_literal/Cargo.toml +++ b/crates/ruff_python_literal/Cargo.toml @@ -15,6 +15,8 @@ license = { workspace = true } doctest = false [dependencies] +ruff_python_ast = { path = "../ruff_python_ast" } + bitflags = { workspace = true } hexf-parse = { workspace = true } is-macro = { workspace = true } diff --git a/crates/ruff_python_literal/src/escape.rs b/crates/ruff_python_literal/src/escape.rs index 03fe3b9060f327..5d6abbf6716125 100644 --- a/crates/ruff_python_literal/src/escape.rs +++ b/crates/ruff_python_literal/src/escape.rs @@ -1,35 +1,4 @@ -#[derive(Debug, PartialEq, Eq, Copy, Clone, Hash, is_macro::Is)] -pub enum Quote { - Single, - Double, -} - -impl Quote { - #[inline] - #[must_use] - pub const fn swap(self) -> Self { - match self { - Quote::Single => Quote::Double, - Quote::Double => Quote::Single, - } - } - - #[inline] - pub const fn to_byte(&self) -> u8 { - match self { - Quote::Single => b'\'', - Quote::Double => b'"', - } - } - - #[inline] - pub const fn to_char(&self) -> char { - match self { - Quote::Single => '\'', - Quote::Double => '"', - } - } -} +use ruff_python_ast::str::Quote; pub struct EscapeLayout { pub quote: Quote, @@ -69,7 +38,7 @@ pub(crate) const fn choose_quote( // always use primary unless we have primary but no secondary let use_secondary = primary_count > 0 && secondary_count == 0; if use_secondary { - (preferred_quote.swap(), secondary_count) + (preferred_quote.opposite(), secondary_count) } else { (preferred_quote, primary_count) } @@ -105,7 +74,7 @@ pub struct StrRepr<'r, 'a>(&'r UnicodeEscape<'a>); impl StrRepr<'_, '_> { pub fn write(&self, formatter: &mut impl std::fmt::Write) -> std::fmt::Result { - let quote = self.0.layout().quote.to_char(); + let quote = self.0.layout().quote.as_char(); formatter.write_char(quote)?; self.0.write_body(formatter)?; formatter.write_char(quote) @@ -216,7 +185,7 @@ impl UnicodeEscape<'_> { // unicodedata lookup just for ascii characters '\x20'..='\x7e' => { // printable ascii range - if ch == quote.to_char() || ch == '\\' { + if ch == quote.as_char() || ch == '\\' { formatter.write_char('\\')?; } formatter.write_char(ch) @@ -379,7 +348,7 @@ impl AsciiEscape<'_> { b'\r' => formatter.write_str("\\r"), 0x20..=0x7e => { // printable ascii range - if ch == quote.to_byte() || ch == b'\\' { + if ch == quote.as_byte() || ch == b'\\' { formatter.write_char('\\')?; } formatter.write_char(ch as char) @@ -416,7 +385,7 @@ pub struct BytesRepr<'r, 'a>(&'r AsciiEscape<'a>); impl BytesRepr<'_, '_> { pub fn write(&self, formatter: &mut impl std::fmt::Write) -> std::fmt::Result { - let quote = self.0.layout().quote.to_char(); + let quote = self.0.layout().quote.as_char(); formatter.write_char('b')?; formatter.write_char(quote)?; self.0.write_body(formatter)?; diff --git a/crates/ruff_python_parser/src/string_token_flags.rs b/crates/ruff_python_parser/src/string_token_flags.rs index 18fed6a52f98ed..e0454e898b3973 100644 --- a/crates/ruff_python_parser/src/string_token_flags.rs +++ b/crates/ruff_python_parser/src/string_token_flags.rs @@ -2,7 +2,7 @@ use std::fmt; use bitflags::bitflags; -use ruff_python_ast::{str::QuoteStyle, StringLiteralPrefix}; +use ruff_python_ast::{str::Quote, StringLiteralPrefix}; use ruff_text_size::{TextLen, TextSize}; bitflags! { @@ -171,11 +171,11 @@ impl StringKind { } /// Does the string use single or double quotes in its opener and closer? - pub const fn quote_style(self) -> QuoteStyle { + pub const fn quote_style(self) -> Quote { if self.0.contains(StringFlags::DOUBLE) { - QuoteStyle::Double + Quote::Double } else { - QuoteStyle::Single + Quote::Single } } @@ -190,13 +190,13 @@ impl StringKind { pub const fn quote_str(self) -> &'static str { if self.is_triple_quoted() { match self.quote_style() { - QuoteStyle::Single => "'''", - QuoteStyle::Double => r#"""""#, + Quote::Single => "'''", + Quote::Double => r#"""""#, } } else { match self.quote_style() { - QuoteStyle::Single => "'", - QuoteStyle::Double => "\"", + Quote::Single => "'", + Quote::Double => "\"", } } }