From d66063bb33b15da4bf1ca432c4356110c48b4880 Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Fri, 1 Dec 2023 17:18:52 +0000 Subject: [PATCH 01/18] [`flake8-pyi`] Check for kwarg and vararg `NoReturn` type annotations (#8948) ## Summary Triggers `no-return-argument-annotation-in-stub` (`PYI050`) for vararg and kwarg `NoReturn` type annotations. Related to #8771. ## Test Plan `cargo test` --- .../test/fixtures/flake8_pyi/PYI050.pyi | 11 ++++ .../rules/no_return_argument_annotation.rs | 45 ++++++++++---- ..._flake8_pyi__tests__PYI050_PYI050.pyi.snap | 61 +++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI050.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI050.pyi index 4720ee7756b6a..583c96e71b59b 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI050.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI050.pyi @@ -10,3 +10,14 @@ def foo_no_return_typing_extensions( def foo_no_return_kwarg(arg: int, *, arg2: NoReturn): ... # Error: PYI050 def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ... # Error: PYI050 def foo_never(arg: Never): ... +def foo_args(*args: NoReturn): ... # Error: PYI050 +def foo_kwargs(**kwargs: NoReturn): ... # Error: PYI050 +def foo_args_kwargs(*args: NoReturn, **kwargs: NoReturn): ... # Error: PYI050 +def foo_int_args(*args: int): ... +def foo_int_kwargs(**kwargs: int): ... +def foo_int_args_kwargs(*args: int, **kwargs: int): ... +def foo_int_args_no_return(*args: int, **kwargs: NoReturn): ... # Error: PYI050 +def foo_int_kwargs_no_return(*args: NoReturn, **kwargs: int): ... # Error: PYI050 +def foo_args_never(*args: Never): ... +def foo_kwargs_never(**kwargs: Never): ... +def foo_args_kwargs_never(*args: Never, **kwargs: Never): ... diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs index f10946236be2d..01350b9501cc0 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/no_return_argument_annotation.rs @@ -2,7 +2,7 @@ use std::fmt; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::Parameters; +use ruff_python_ast::{Expr, Parameters}; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; @@ -51,6 +51,9 @@ impl Violation for NoReturnArgumentAnnotationInStub { /// PYI050 pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: &Parameters) { + // Ex) def func(arg: NoReturn): ... + // Ex) def func(arg: NoReturn, /): ... + // Ex) def func(*, arg: NoReturn): ... for annotation in parameters .posonlyargs .iter() @@ -58,21 +61,39 @@ pub(crate) fn no_return_argument_annotation(checker: &mut Checker, parameters: & .chain(¶meters.kwonlyargs) .filter_map(|arg| arg.parameter.annotation.as_ref()) { - if checker.semantic().match_typing_expr(annotation, "NoReturn") { - checker.diagnostics.push(Diagnostic::new( - NoReturnArgumentAnnotationInStub { - module: if checker.settings.target_version >= Py311 { - TypingModule::Typing - } else { - TypingModule::TypingExtensions - }, - }, - annotation.range(), - )); + check_no_return_argument_annotation(checker, annotation); + } + + // Ex) def func(*args: NoReturn): ... + if let Some(arg) = ¶meters.vararg { + if let Some(annotation) = &arg.annotation { + check_no_return_argument_annotation(checker, annotation); + } + } + + // Ex) def func(**kwargs: NoReturn): ... + if let Some(arg) = ¶meters.kwarg { + if let Some(annotation) = &arg.annotation { + check_no_return_argument_annotation(checker, annotation); } } } +fn check_no_return_argument_annotation(checker: &mut Checker, annotation: &Expr) { + if checker.semantic().match_typing_expr(annotation, "NoReturn") { + checker.diagnostics.push(Diagnostic::new( + NoReturnArgumentAnnotationInStub { + module: if checker.settings.target_version >= Py311 { + TypingModule::Typing + } else { + TypingModule::TypingExtensions + }, + }, + annotation.range(), + )); + } +} + #[derive(Debug, PartialEq, Eq)] enum TypingModule { Typing, diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap index a0cb503548fca..caafc0254ea8c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI050_PYI050.pyi.snap @@ -28,6 +28,67 @@ PYI050.pyi:11:47: PYI050 Prefer `typing.Never` over `NoReturn` for argument anno 11 | def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ... # Error: PYI050 | ^^^^^^^^ PYI050 12 | def foo_never(arg: Never): ... +13 | def foo_args(*args: NoReturn): ... # Error: PYI050 + | + +PYI050.pyi:13:21: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +11 | def foo_no_return_pos_only(arg: int, /, arg2: NoReturn): ... # Error: PYI050 +12 | def foo_never(arg: Never): ... +13 | def foo_args(*args: NoReturn): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +14 | def foo_kwargs(**kwargs: NoReturn): ... # Error: PYI050 +15 | def foo_args_kwargs(*args: NoReturn, **kwargs: NoReturn): ... # Error: PYI050 + | + +PYI050.pyi:14:26: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +12 | def foo_never(arg: Never): ... +13 | def foo_args(*args: NoReturn): ... # Error: PYI050 +14 | def foo_kwargs(**kwargs: NoReturn): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +15 | def foo_args_kwargs(*args: NoReturn, **kwargs: NoReturn): ... # Error: PYI050 +16 | def foo_int_args(*args: int): ... + | + +PYI050.pyi:15:28: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +13 | def foo_args(*args: NoReturn): ... # Error: PYI050 +14 | def foo_kwargs(**kwargs: NoReturn): ... # Error: PYI050 +15 | def foo_args_kwargs(*args: NoReturn, **kwargs: NoReturn): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +16 | def foo_int_args(*args: int): ... +17 | def foo_int_kwargs(**kwargs: int): ... + | + +PYI050.pyi:15:48: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +13 | def foo_args(*args: NoReturn): ... # Error: PYI050 +14 | def foo_kwargs(**kwargs: NoReturn): ... # Error: PYI050 +15 | def foo_args_kwargs(*args: NoReturn, **kwargs: NoReturn): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +16 | def foo_int_args(*args: int): ... +17 | def foo_int_kwargs(**kwargs: int): ... + | + +PYI050.pyi:19:50: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +17 | def foo_int_kwargs(**kwargs: int): ... +18 | def foo_int_args_kwargs(*args: int, **kwargs: int): ... +19 | def foo_int_args_no_return(*args: int, **kwargs: NoReturn): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +20 | def foo_int_kwargs_no_return(*args: NoReturn, **kwargs: int): ... # Error: PYI050 +21 | def foo_args_never(*args: Never): ... + | + +PYI050.pyi:20:37: PYI050 Prefer `typing.Never` over `NoReturn` for argument annotations + | +18 | def foo_int_args_kwargs(*args: int, **kwargs: int): ... +19 | def foo_int_args_no_return(*args: int, **kwargs: NoReturn): ... # Error: PYI050 +20 | def foo_int_kwargs_no_return(*args: NoReturn, **kwargs: int): ... # Error: PYI050 + | ^^^^^^^^ PYI050 +21 | def foo_args_never(*args: Never): ... +22 | def foo_kwargs_never(**kwargs: Never): ... | From e5db72459e1ee9c2f83aaf0e4653b33e4a85bed3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Dec 2023 12:35:01 -0500 Subject: [PATCH 02/18] Detect implicit returns in auto-return-types (#8952) ## Summary Adds detection for branches without a `return` or `raise`, so that we can properly `Optional` the return types. I'd like to remove this and replace it with our code graph analysis from the `unreachable.rs` rule, but it at least fixes the worst offenders. Closes #8942. --- .../flake8_annotations/auto_return_type.py | 84 ++++++ .../src/rules/flake8_annotations/helpers.rs | 23 +- ..._annotations__tests__auto_return_type.snap | 227 +++++++++++++++ ...tations__tests__auto_return_type_py38.snap | 262 ++++++++++++++++++ ...__flake8_annotations__tests__defaults.snap | 56 +++- ...otations__tests__ignore_fully_untyped.snap | 40 ++- ..._annotations__tests__mypy_init_return.snap | 14 +- crates/ruff_python_ast/src/helpers.rs | 174 ++++++++++++ 8 files changed, 860 insertions(+), 20 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py b/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py index 81ce953ed11e0..ab75a50da712c 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_annotations/auto_return_type.py @@ -63,3 +63,87 @@ def func(x: int): return "str" else: return None + + +def func(x: int): + if x: + return 1 + + +def func(): + x = 1 + + +def func(x: int): + if x > 0: + return 1 + + +def func(x: int): + match x: + case [1, 2, 3]: + return 1 + case 4 as y: + return "foo" + + +def func(x: int): + for i in range(5): + if i > 0: + return 1 + + +def func(x: int): + for i in range(5): + if i > 0: + return 1 + else: + return 4 + + +def func(x: int): + for i in range(5): + if i > 0: + break + else: + return 4 + + +def func(x: int): + try: + pass + except: + return 1 + + +def func(x: int): + try: + pass + except: + return 1 + finally: + return 2 + + +def func(x: int): + try: + pass + except: + return 1 + else: + return 2 + + +def func(x: int): + try: + return 1 + except: + return 2 + else: + pass + + +def func(x: int): + while x > 0: + break + return 1 diff --git a/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs b/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs index 9dc7e896dd106..41cff286d79c5 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs +++ b/crates/ruff_linter/src/rules/flake8_annotations/helpers.rs @@ -1,10 +1,9 @@ use itertools::Itertools; -use ruff_diagnostics::Edit; use rustc_hash::FxHashSet; -use crate::importer::{ImportRequest, Importer}; +use ruff_diagnostics::Edit; use ruff_python_ast::helpers::{ - pep_604_union, typing_optional, typing_union, ReturnStatementVisitor, + implicit_return, pep_604_union, typing_optional, typing_union, ReturnStatementVisitor, }; use ruff_python_ast::visitor::Visitor; use ruff_python_ast::{self as ast, Expr, ExprContext}; @@ -13,6 +12,7 @@ use ruff_python_semantic::analyze::visibility; use ruff_python_semantic::{Definition, SemanticModel}; use ruff_text_size::{TextRange, TextSize}; +use crate::importer::{ImportRequest, Importer}; use crate::settings::types::PythonVersion; /// Return the name of the function, if it's overloaded. @@ -48,14 +48,19 @@ pub(crate) fn auto_return_type(function: &ast::StmtFunctionDef) -> Option Option 0: + // return 1 + // ``` + if implicit_return(function) { + return_type = return_type.union(ResolvedPythonType::Atom(PythonType::None)); + } + match return_type { ResolvedPythonType::Atom(python_type) => Some(AutoPythonType::Atom(python_type)), ResolvedPythonType::Union(python_types) => Some(AutoPythonType::Union(python_types)), diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type.snap index 021e1b520766a..6fcbb95d029f6 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type.snap +++ b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type.snap @@ -200,4 +200,231 @@ auto_return_type.py:59:5: ANN201 [*] Missing return type annotation for public f 61 61 | return 1 62 62 | elif x > 5: +auto_return_type.py:68:5: ANN201 [*] Missing return type annotation for public function `func` + | +68 | def func(x: int): + | ^^^^ ANN201 +69 | if x: +70 | return 1 + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +65 65 | return None +66 66 | +67 67 | +68 |-def func(x: int): + 68 |+def func(x: int) -> int | None: +69 69 | if x: +70 70 | return 1 +71 71 | + +auto_return_type.py:73:5: ANN201 [*] Missing return type annotation for public function `func` + | +73 | def func(): + | ^^^^ ANN201 +74 | x = 1 + | + = help: Add return type annotation: `None` + +ℹ Unsafe fix +70 70 | return 1 +71 71 | +72 72 | +73 |-def func(): + 73 |+def func() -> None: +74 74 | x = 1 +75 75 | +76 76 | + +auto_return_type.py:77:5: ANN201 [*] Missing return type annotation for public function `func` + | +77 | def func(x: int): + | ^^^^ ANN201 +78 | if x > 0: +79 | return 1 + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +74 74 | x = 1 +75 75 | +76 76 | +77 |-def func(x: int): + 77 |+def func(x: int) -> int | None: +78 78 | if x > 0: +79 79 | return 1 +80 80 | + +auto_return_type.py:82:5: ANN201 [*] Missing return type annotation for public function `func` + | +82 | def func(x: int): + | ^^^^ ANN201 +83 | match x: +84 | case [1, 2, 3]: + | + = help: Add return type annotation: `str | int` + +ℹ Unsafe fix +79 79 | return 1 +80 80 | +81 81 | +82 |-def func(x: int): + 82 |+def func(x: int) -> str | int: +83 83 | match x: +84 84 | case [1, 2, 3]: +85 85 | return 1 + +auto_return_type.py:90:5: ANN201 [*] Missing return type annotation for public function `func` + | +90 | def func(x: int): + | ^^^^ ANN201 +91 | for i in range(5): +92 | if i > 0: + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +87 87 | return "foo" +88 88 | +89 89 | +90 |-def func(x: int): + 90 |+def func(x: int) -> int | None: +91 91 | for i in range(5): +92 92 | if i > 0: +93 93 | return 1 + +auto_return_type.py:96:5: ANN201 [*] Missing return type annotation for public function `func` + | +96 | def func(x: int): + | ^^^^ ANN201 +97 | for i in range(5): +98 | if i > 0: + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +93 93 | return 1 +94 94 | +95 95 | +96 |-def func(x: int): + 96 |+def func(x: int) -> int: +97 97 | for i in range(5): +98 98 | if i > 0: +99 99 | return 1 + +auto_return_type.py:104:5: ANN201 [*] Missing return type annotation for public function `func` + | +104 | def func(x: int): + | ^^^^ ANN201 +105 | for i in range(5): +106 | if i > 0: + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +101 101 | return 4 +102 102 | +103 103 | +104 |-def func(x: int): + 104 |+def func(x: int) -> int | None: +105 105 | for i in range(5): +106 106 | if i > 0: +107 107 | break + +auto_return_type.py:112:5: ANN201 [*] Missing return type annotation for public function `func` + | +112 | def func(x: int): + | ^^^^ ANN201 +113 | try: +114 | pass + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +109 109 | return 4 +110 110 | +111 111 | +112 |-def func(x: int): + 112 |+def func(x: int) -> int | None: +113 113 | try: +114 114 | pass +115 115 | except: + +auto_return_type.py:119:5: ANN201 [*] Missing return type annotation for public function `func` + | +119 | def func(x: int): + | ^^^^ ANN201 +120 | try: +121 | pass + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +116 116 | return 1 +117 117 | +118 118 | +119 |-def func(x: int): + 119 |+def func(x: int) -> int: +120 120 | try: +121 121 | pass +122 122 | except: + +auto_return_type.py:128:5: ANN201 [*] Missing return type annotation for public function `func` + | +128 | def func(x: int): + | ^^^^ ANN201 +129 | try: +130 | pass + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +125 125 | return 2 +126 126 | +127 127 | +128 |-def func(x: int): + 128 |+def func(x: int) -> int: +129 129 | try: +130 130 | pass +131 131 | except: + +auto_return_type.py:137:5: ANN201 [*] Missing return type annotation for public function `func` + | +137 | def func(x: int): + | ^^^^ ANN201 +138 | try: +139 | return 1 + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +134 134 | return 2 +135 135 | +136 136 | +137 |-def func(x: int): + 137 |+def func(x: int) -> int: +138 138 | try: +139 139 | return 1 +140 140 | except: + +auto_return_type.py:146:5: ANN201 [*] Missing return type annotation for public function `func` + | +146 | def func(x: int): + | ^^^^ ANN201 +147 | while x > 0: +148 | break + | + = help: Add return type annotation: `int | None` + +ℹ Unsafe fix +143 143 | pass +144 144 | +145 145 | +146 |-def func(x: int): + 146 |+def func(x: int) -> int | None: +147 147 | while x > 0: +148 148 | break +149 149 | return 1 + diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap index 0831fcffa3e09..d91484ca3fe71 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap +++ b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__auto_return_type_py38.snap @@ -220,4 +220,266 @@ auto_return_type.py:59:5: ANN201 [*] Missing return type annotation for public f 61 62 | return 1 62 63 | elif x > 5: +auto_return_type.py:68:5: ANN201 [*] Missing return type annotation for public function `func` + | +68 | def func(x: int): + | ^^^^ ANN201 +69 | if x: +70 | return 1 + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +65 66 | return None +66 67 | +67 68 | +68 |-def func(x: int): + 69 |+def func(x: int) -> Optional[int]: +69 70 | if x: +70 71 | return 1 +71 72 | + +auto_return_type.py:73:5: ANN201 [*] Missing return type annotation for public function `func` + | +73 | def func(): + | ^^^^ ANN201 +74 | x = 1 + | + = help: Add return type annotation: `None` + +ℹ Unsafe fix +70 70 | return 1 +71 71 | +72 72 | +73 |-def func(): + 73 |+def func() -> None: +74 74 | x = 1 +75 75 | +76 76 | + +auto_return_type.py:77:5: ANN201 [*] Missing return type annotation for public function `func` + | +77 | def func(x: int): + | ^^^^ ANN201 +78 | if x > 0: +79 | return 1 + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +74 75 | x = 1 +75 76 | +76 77 | +77 |-def func(x: int): + 78 |+def func(x: int) -> Optional[int]: +78 79 | if x > 0: +79 80 | return 1 +80 81 | + +auto_return_type.py:82:5: ANN201 [*] Missing return type annotation for public function `func` + | +82 | def func(x: int): + | ^^^^ ANN201 +83 | match x: +84 | case [1, 2, 3]: + | + = help: Add return type annotation: `Union[str | int]` + +ℹ Unsafe fix + 1 |+from typing import Union +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +79 80 | return 1 +80 81 | +81 82 | +82 |-def func(x: int): + 83 |+def func(x: int) -> Union[str | int]: +83 84 | match x: +84 85 | case [1, 2, 3]: +85 86 | return 1 + +auto_return_type.py:90:5: ANN201 [*] Missing return type annotation for public function `func` + | +90 | def func(x: int): + | ^^^^ ANN201 +91 | for i in range(5): +92 | if i > 0: + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +87 88 | return "foo" +88 89 | +89 90 | +90 |-def func(x: int): + 91 |+def func(x: int) -> Optional[int]: +91 92 | for i in range(5): +92 93 | if i > 0: +93 94 | return 1 + +auto_return_type.py:96:5: ANN201 [*] Missing return type annotation for public function `func` + | +96 | def func(x: int): + | ^^^^ ANN201 +97 | for i in range(5): +98 | if i > 0: + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +93 93 | return 1 +94 94 | +95 95 | +96 |-def func(x: int): + 96 |+def func(x: int) -> int: +97 97 | for i in range(5): +98 98 | if i > 0: +99 99 | return 1 + +auto_return_type.py:104:5: ANN201 [*] Missing return type annotation for public function `func` + | +104 | def func(x: int): + | ^^^^ ANN201 +105 | for i in range(5): +106 | if i > 0: + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +101 102 | return 4 +102 103 | +103 104 | +104 |-def func(x: int): + 105 |+def func(x: int) -> Optional[int]: +105 106 | for i in range(5): +106 107 | if i > 0: +107 108 | break + +auto_return_type.py:112:5: ANN201 [*] Missing return type annotation for public function `func` + | +112 | def func(x: int): + | ^^^^ ANN201 +113 | try: +114 | pass + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +109 110 | return 4 +110 111 | +111 112 | +112 |-def func(x: int): + 113 |+def func(x: int) -> Optional[int]: +113 114 | try: +114 115 | pass +115 116 | except: + +auto_return_type.py:119:5: ANN201 [*] Missing return type annotation for public function `func` + | +119 | def func(x: int): + | ^^^^ ANN201 +120 | try: +121 | pass + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +116 116 | return 1 +117 117 | +118 118 | +119 |-def func(x: int): + 119 |+def func(x: int) -> int: +120 120 | try: +121 121 | pass +122 122 | except: + +auto_return_type.py:128:5: ANN201 [*] Missing return type annotation for public function `func` + | +128 | def func(x: int): + | ^^^^ ANN201 +129 | try: +130 | pass + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +125 125 | return 2 +126 126 | +127 127 | +128 |-def func(x: int): + 128 |+def func(x: int) -> int: +129 129 | try: +130 130 | pass +131 131 | except: + +auto_return_type.py:137:5: ANN201 [*] Missing return type annotation for public function `func` + | +137 | def func(x: int): + | ^^^^ ANN201 +138 | try: +139 | return 1 + | + = help: Add return type annotation: `int` + +ℹ Unsafe fix +134 134 | return 2 +135 135 | +136 136 | +137 |-def func(x: int): + 137 |+def func(x: int) -> int: +138 138 | try: +139 139 | return 1 +140 140 | except: + +auto_return_type.py:146:5: ANN201 [*] Missing return type annotation for public function `func` + | +146 | def func(x: int): + | ^^^^ ANN201 +147 | while x > 0: +148 | break + | + = help: Add return type annotation: `Optional[int]` + +ℹ Unsafe fix + 1 |+from typing import Optional +1 2 | def func(): +2 3 | return 1 +3 4 | +-------------------------------------------------------------------------------- +143 144 | pass +144 145 | +145 146 | +146 |-def func(x: int): + 147 |+def func(x: int) -> Optional[int]: +147 148 | while x > 0: +148 149 | break +149 150 | return 1 + diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__defaults.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__defaults.snap index 8665fb92ce192..9be33117659ce 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__defaults.snap +++ b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__defaults.snap @@ -1,14 +1,24 @@ --- source: crates/ruff_linter/src/rules/flake8_annotations/mod.rs --- -annotation_presence.py:5:5: ANN201 Missing return type annotation for public function `foo` +annotation_presence.py:5:5: ANN201 [*] Missing return type annotation for public function `foo` | 4 | # Error 5 | def foo(a, b): | ^^^ ANN201 6 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +2 2 | from typing_extensions import override +3 3 | +4 4 | # Error +5 |-def foo(a, b): + 5 |+def foo(a, b) -> None: +6 6 | pass +7 7 | +8 8 | annotation_presence.py:5:9: ANN001 Missing type annotation for function argument `a` | @@ -26,14 +36,24 @@ annotation_presence.py:5:12: ANN001 Missing type annotation for function argumen 6 | pass | -annotation_presence.py:10:5: ANN201 Missing return type annotation for public function `foo` +annotation_presence.py:10:5: ANN201 [*] Missing return type annotation for public function `foo` | 9 | # Error 10 | def foo(a: int, b): | ^^^ ANN201 11 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +7 7 | +8 8 | +9 9 | # Error +10 |-def foo(a: int, b): + 10 |+def foo(a: int, b) -> None: +11 11 | pass +12 12 | +13 13 | annotation_presence.py:10:17: ANN001 Missing type annotation for function argument `b` | @@ -51,23 +71,43 @@ annotation_presence.py:15:17: ANN001 Missing type annotation for function argume 16 | pass | -annotation_presence.py:20:5: ANN201 Missing return type annotation for public function `foo` +annotation_presence.py:20:5: ANN201 [*] Missing return type annotation for public function `foo` | 19 | # Error 20 | def foo(a: int, b: int): | ^^^ ANN201 21 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` -annotation_presence.py:25:5: ANN201 Missing return type annotation for public function `foo` +ℹ Unsafe fix +17 17 | +18 18 | +19 19 | # Error +20 |-def foo(a: int, b: int): + 20 |+def foo(a: int, b: int) -> None: +21 21 | pass +22 22 | +23 23 | + +annotation_presence.py:25:5: ANN201 [*] Missing return type annotation for public function `foo` | 24 | # Error 25 | def foo(): | ^^^ ANN201 26 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +22 22 | +23 23 | +24 24 | # Error +25 |-def foo(): + 25 |+def foo() -> None: +26 26 | pass +27 27 | +28 28 | annotation_presence.py:45:12: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` | diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__ignore_fully_untyped.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__ignore_fully_untyped.snap index a93908081cec1..d806d641b4caa 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__ignore_fully_untyped.snap +++ b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__ignore_fully_untyped.snap @@ -1,13 +1,23 @@ --- source: crates/ruff_linter/src/rules/flake8_annotations/mod.rs --- -ignore_fully_untyped.py:24:5: ANN201 Missing return type annotation for public function `error_partially_typed_1` +ignore_fully_untyped.py:24:5: ANN201 [*] Missing return type annotation for public function `error_partially_typed_1` | 24 | def error_partially_typed_1(a: int, b): | ^^^^^^^^^^^^^^^^^^^^^^^ ANN201 25 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +21 21 | pass +22 22 | +23 23 | +24 |-def error_partially_typed_1(a: int, b): + 24 |+def error_partially_typed_1(a: int, b) -> None: +25 25 | pass +26 26 | +27 27 | ignore_fully_untyped.py:24:37: ANN001 Missing type annotation for function argument `b` | @@ -23,15 +33,25 @@ ignore_fully_untyped.py:28:37: ANN001 Missing type annotation for function argum 29 | pass | -ignore_fully_untyped.py:32:5: ANN201 Missing return type annotation for public function `error_partially_typed_3` +ignore_fully_untyped.py:32:5: ANN201 [*] Missing return type annotation for public function `error_partially_typed_3` | 32 | def error_partially_typed_3(a: int, b: int): | ^^^^^^^^^^^^^^^^^^^^^^^ ANN201 33 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +29 29 | pass +30 30 | +31 31 | +32 |-def error_partially_typed_3(a: int, b: int): + 32 |+def error_partially_typed_3(a: int, b: int) -> None: +33 33 | pass +34 34 | +35 35 | -ignore_fully_untyped.py:43:9: ANN201 Missing return type annotation for public function `error_typed_self` +ignore_fully_untyped.py:43:9: ANN201 [*] Missing return type annotation for public function `error_typed_self` | 41 | pass 42 | @@ -39,6 +59,14 @@ ignore_fully_untyped.py:43:9: ANN201 Missing return type annotation for public f | ^^^^^^^^^^^^^^^^ ANN201 44 | pass | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +40 40 | def ok_untyped_method(self): +41 41 | pass +42 42 | +43 |- def error_typed_self(self: X): + 43 |+ def error_typed_self(self: X) -> None: +44 44 | pass diff --git a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__mypy_init_return.snap b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__mypy_init_return.snap index 2ce0c94524eb4..f25fc530bc8cf 100644 --- a/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__mypy_init_return.snap +++ b/crates/ruff_linter/src/rules/flake8_annotations/snapshots/ruff_linter__rules__flake8_annotations__tests__mypy_init_return.snap @@ -41,14 +41,24 @@ mypy_init_return.py:11:9: ANN204 [*] Missing return type annotation for special 13 13 | 14 14 | -mypy_init_return.py:40:5: ANN202 Missing return type annotation for private function `__init__` +mypy_init_return.py:40:5: ANN202 [*] Missing return type annotation for private function `__init__` | 39 | # Error 40 | def __init__(self, foo: int): | ^^^^^^^^ ANN202 41 | ... | - = help: Add return type annotation + = help: Add return type annotation: `None` + +ℹ Unsafe fix +37 37 | +38 38 | +39 39 | # Error +40 |-def __init__(self, foo: int): + 40 |+def __init__(self, foo: int) -> None: +41 41 | ... +42 42 | +43 43 | mypy_init_return.py:47:9: ANN204 [*] Missing return type annotation for special method `__init__` | diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 5d065fa30fc11..cca590903244b 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -911,6 +911,180 @@ where } } +/// Returns `true` if the function has an implicit return. +pub fn implicit_return(function: &ast::StmtFunctionDef) -> bool { + /// Returns `true` if the body may break via a `break` statement. + fn sometimes_breaks(stmts: &[Stmt]) -> bool { + for stmt in stmts { + match stmt { + Stmt::For(ast::StmtFor { body, orelse, .. }) => { + if returns(body) { + return false; + } + if sometimes_breaks(orelse) { + return true; + } + } + Stmt::While(ast::StmtWhile { body, orelse, .. }) => { + if returns(body) { + return false; + } + if sometimes_breaks(orelse) { + return true; + } + } + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + if std::iter::once(body) + .chain(elif_else_clauses.iter().map(|clause| &clause.body)) + .any(|body| sometimes_breaks(body)) + { + return true; + } + } + Stmt::Match(ast::StmtMatch { cases, .. }) => { + if cases.iter().any(|case| sometimes_breaks(&case.body)) { + return true; + } + } + Stmt::Try(ast::StmtTry { + body, + handlers, + orelse, + finalbody, + .. + }) => { + if sometimes_breaks(body) + || handlers.iter().any(|handler| { + let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + body, + .. + }) = handler; + sometimes_breaks(body) + }) + || sometimes_breaks(orelse) + || sometimes_breaks(finalbody) + { + return true; + } + } + Stmt::With(ast::StmtWith { body, .. }) => { + if sometimes_breaks(body) { + return true; + } + } + Stmt::Break(_) => return true, + Stmt::Return(_) => return false, + Stmt::Raise(_) => return false, + _ => {} + } + } + false + } + + /// Returns `true` if the body may break via a `break` statement. + fn always_breaks(stmts: &[Stmt]) -> bool { + for stmt in stmts { + match stmt { + Stmt::Break(_) => return true, + Stmt::Return(_) => return false, + Stmt::Raise(_) => return false, + _ => {} + } + } + false + } + + /// Returns `true` if the body contains a branch that ends without an explicit `return` or + /// `raise` statement. + fn returns(stmts: &[Stmt]) -> bool { + for stmt in stmts.iter().rev() { + match stmt { + Stmt::For(ast::StmtFor { body, orelse, .. }) => { + if always_breaks(body) { + return false; + } + if returns(body) { + return true; + } + if returns(orelse) && !sometimes_breaks(body) { + return true; + } + } + Stmt::While(ast::StmtWhile { body, orelse, .. }) => { + if always_breaks(body) { + return false; + } + if returns(body) { + return true; + } + if returns(orelse) && !sometimes_breaks(body) { + return true; + } + } + Stmt::If(ast::StmtIf { + body, + elif_else_clauses, + .. + }) => { + if elif_else_clauses.iter().any(|clause| clause.test.is_none()) + && std::iter::once(body) + .chain(elif_else_clauses.iter().map(|clause| &clause.body)) + .all(|body| returns(body)) + { + return true; + } + } + Stmt::Match(ast::StmtMatch { cases, .. }) => { + // Note: we assume the `match` is exhaustive. + if cases.iter().all(|case| returns(&case.body)) { + return true; + } + } + Stmt::Try(ast::StmtTry { + body, + handlers, + orelse, + finalbody, + .. + }) => { + // If the `finally` block returns, the `try` block must also return. + if returns(finalbody) { + return true; + } + + // If the `body` or the `else` block returns, the `try` block must also return. + if (returns(body) || returns(orelse)) + && handlers.iter().all(|handler| { + let ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { + body, + .. + }) = handler; + returns(body) + }) + { + return true; + } + } + Stmt::With(ast::StmtWith { body, .. }) => { + if returns(body) { + return true; + } + } + Stmt::Return(_) => return true, + Stmt::Raise(_) => return true, + _ => {} + } + } + false + } + + !returns(&function.body) +} + /// A [`StatementVisitor`] that collects all `raise` statements in a function or method. #[derive(Default)] pub struct RaiseStatementVisitor<'a> { From 5510a6131e56346957064ccf41e4e83a3bf46647 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Dec 2023 13:22:53 -0500 Subject: [PATCH 03/18] Ignore `@overload` and `@override` methods for too-many-arguments checks (#8954) Closes https://github.com/astral-sh/ruff/issues/8945. --- .../fixtures/pylint/too_many_arguments.py | 13 ++++++++++ .../src/checkers/ast/analyze/statement.rs | 2 +- .../rules/pylint/rules/too_many_arguments.rs | 24 +++++++++++++------ 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_arguments.py b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_arguments.py index 5271910cc53a4..43b2b178c04e0 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/too_many_arguments.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/too_many_arguments.py @@ -32,3 +32,16 @@ def f(x, y, z, *, u, v, w): # Too many arguments (6/5) def f(x, y, z, a, b, c, *, u, v, w): # Too many arguments (9/5) pass + + +from typing import override, overload + + +@override +def f(x, y, z, a, b, c, *, u, v, w): # OK + pass + + +@overload +def f(x, y, z, a, b, c, *, u, v, w): # OK + pass diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index d4e6162221938..99ea30595f0a6 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -248,7 +248,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pylint::rules::property_with_parameters(checker, stmt, decorator_list, parameters); } if checker.enabled(Rule::TooManyArguments) { - pylint::rules::too_many_arguments(checker, parameters, stmt); + pylint::rules::too_many_arguments(checker, function_def); } if checker.enabled(Rule::TooManyReturnStatements) { if let Some(diagnostic) = pylint::rules::too_many_return_statements( diff --git a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs index 5ddd80ecbc37c..84e5740f6d987 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/too_many_arguments.rs @@ -1,8 +1,8 @@ -use ruff_python_ast::{Parameters, Stmt}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast as ast; use ruff_python_ast::identifier::Identifier; +use ruff_python_semantic::analyze::visibility; use crate::checkers::ast::Checker; @@ -58,12 +58,13 @@ impl Violation for TooManyArguments { } /// PLR0913 -pub(crate) fn too_many_arguments(checker: &mut Checker, parameters: &Parameters, stmt: &Stmt) { - let num_arguments = parameters +pub(crate) fn too_many_arguments(checker: &mut Checker, function_def: &ast::StmtFunctionDef) { + let num_arguments = function_def + .parameters .args .iter() - .chain(¶meters.kwonlyargs) - .chain(¶meters.posonlyargs) + .chain(&function_def.parameters.kwonlyargs) + .chain(&function_def.parameters.posonlyargs) .filter(|arg| { !checker .settings @@ -71,13 +72,22 @@ pub(crate) fn too_many_arguments(checker: &mut Checker, parameters: &Parameters, .is_match(&arg.parameter.name) }) .count(); + if num_arguments > checker.settings.pylint.max_args { + // Allow excessive arguments in `@override` or `@overload` methods, since they're required + // to adhere to the parent signature. + if visibility::is_override(&function_def.decorator_list, checker.semantic()) + || visibility::is_overload(&function_def.decorator_list, checker.semantic()) + { + return; + } + checker.diagnostics.push(Diagnostic::new( TooManyArguments { c_args: num_arguments, max_args: checker.settings.pylint.max_args, }, - stmt.identifier(), + function_def.identifier(), )); } } From 64c2535e28a121d67ad0f162ef35d31b7b3c2290 Mon Sep 17 00:00:00 2001 From: qdegraaf <34540841+qdegraaf@users.noreply.github.com> Date: Fri, 1 Dec 2023 19:23:56 +0100 Subject: [PATCH 04/18] [`pylint`] Add `add_argument` utility and autofix for `PLW1514` (#8928) ## Summary - Adds `add_argument` similar to existing `remove_argument` utility to safely add arguments to functions. - Adds autofix for `PLW1514` as per specs requested in https://github.com/astral-sh/ruff/issues/8883 as a test ## Test Plan Checks on existing fixtures as well as additional test and fixture for Python 3.9 and lower fix ## Issue Link Closes: https://github.com/astral-sh/ruff/issues/8883 --- .../fixtures/pylint/unspecified_encoding.py | 27 + crates/ruff_linter/src/fix/edits.rs | 32 +- crates/ruff_linter/src/rules/pylint/mod.rs | 11 + .../pylint/rules/unspecified_encoding.rs | 81 ++- ...ests__PLW1514_unspecified_encoding.py.snap | 298 ++++++++++- ...nspecified_encoding_python39_or_lower.snap | 477 ++++++++++++++++++ 6 files changed, 904 insertions(+), 22 deletions(-) create mode 100644 crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__unspecified_encoding_python39_or_lower.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py b/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py index 216fd83513a9c..01c12c9fc4c77 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/unspecified_encoding.py @@ -42,3 +42,30 @@ def func(*args, **kwargs): tempfile.SpooledTemporaryFile(0, "w", -1, "utf-8") tempfile.SpooledTemporaryFile(0, "wb") tempfile.SpooledTemporaryFile(0, ) + +open("test.txt",) +open() +open( + "test.txt", # comment +) +open( + "test.txt", + # comment +) +open(("test.txt"),) +open( + ("test.txt"), # comment +) +open( + ("test.txt"), + # comment +) + +open((("test.txt")),) +open( + (("test.txt")), # comment +) +open( + (("test.txt")), + # comment +) diff --git a/crates/ruff_linter/src/fix/edits.rs b/crates/ruff_linter/src/fix/edits.rs index 76257f7f5967b..89d6db3f39d38 100644 --- a/crates/ruff_linter/src/fix/edits.rs +++ b/crates/ruff_linter/src/fix/edits.rs @@ -3,12 +3,14 @@ use anyhow::{Context, Result}; use ruff_diagnostics::Edit; -use ruff_python_ast::AnyNodeRef; +use ruff_python_ast::parenthesize::parenthesized_range; use ruff_python_ast::{self as ast, Arguments, ExceptHandler, Stmt}; +use ruff_python_ast::{AnyNodeRef, ArgOrKeyword}; use ruff_python_codegen::Stylist; use ruff_python_index::Indexer; use ruff_python_trivia::{ - has_leading_content, is_python_whitespace, PythonWhitespace, SimpleTokenKind, SimpleTokenizer, + has_leading_content, is_python_whitespace, CommentRanges, PythonWhitespace, SimpleTokenKind, + SimpleTokenizer, }; use ruff_source_file::{Locator, NewlineWithTrailingNewline, UniversalNewlines}; use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; @@ -138,6 +140,32 @@ pub(crate) fn remove_argument( } } +/// Generic function to add arguments or keyword arguments to function calls. +pub(crate) fn add_argument( + argument: &str, + arguments: &Arguments, + comment_ranges: &CommentRanges, + source: &str, +) -> Edit { + if let Some(last) = arguments.arguments_source_order().last() { + // Case 1: existing arguments, so append after the last argument. + let last = parenthesized_range( + match last { + ArgOrKeyword::Arg(arg) => arg.into(), + ArgOrKeyword::Keyword(keyword) => (&keyword.value).into(), + }, + arguments.into(), + comment_ranges, + source, + ) + .unwrap_or(last.range()); + Edit::insertion(format!(", {argument}"), last.end()) + } else { + // Case 2: no arguments. Add argument, without any trailing comma. + Edit::insertion(argument.to_string(), arguments.start() + TextSize::from(1)) + } +} + /// Determine if a vector contains only one, specific element. fn is_only(vec: &[T], value: &T) -> bool { vec.len() == 1 && vec[0] == *value diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs index ba4536eaf9f49..20920043f97b6 100644 --- a/crates/ruff_linter/src/rules/pylint/mod.rs +++ b/crates/ruff_linter/src/rules/pylint/mod.rs @@ -328,4 +328,15 @@ mod tests { assert_messages!(diagnostics); Ok(()) } + + #[test] + fn unspecified_encoding_python39_or_lower() -> Result<()> { + let diagnostics = test_path( + Path::new("pylint/unspecified_encoding.py"), + &LinterSettings::for_rule(Rule::UnspecifiedEncoding) + .with_target_version(PythonVersion::Py39), + )?; + assert_messages!(diagnostics); + Ok(()) + } } diff --git a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs index 0559919d8e905..b6728df692415 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unspecified_encoding.rs @@ -1,10 +1,16 @@ -use ruff_diagnostics::{Diagnostic, Violation}; +use anyhow::Result; + +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast as ast; use ruff_python_ast::call_path::{format_call_path, CallPath}; -use ruff_text_size::Ranged; +use ruff_python_ast::Expr; +use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; +use crate::fix::edits::add_argument; +use crate::importer::ImportRequest; +use crate::settings::types::PythonVersion; /// ## What it does /// Checks for uses of `open` and related calls without an explicit `encoding` @@ -15,7 +21,9 @@ use crate::checkers::ast::Checker; /// non-portable code, with differing behavior across platforms. /// /// Instead, consider using the `encoding` parameter to enforce a specific -/// encoding. +/// encoding. [PEP 597] recommends using `locale.getpreferredencoding(False)` +/// as the default encoding on versions earlier than Python 3.10, and +/// `encoding="locale"` on Python 3.10 and later. /// /// ## Example /// ```python @@ -29,13 +37,15 @@ use crate::checkers::ast::Checker; /// /// ## References /// - [Python documentation: `open`](https://docs.python.org/3/library/functions.html#open) +/// +/// [PEP 597]: https://peps.python.org/pep-0597/ #[violation] pub struct UnspecifiedEncoding { function_name: String, mode: Mode, } -impl Violation for UnspecifiedEncoding { +impl AlwaysFixableViolation for UnspecifiedEncoding { #[derive_message_formats] fn message(&self) -> String { let UnspecifiedEncoding { @@ -52,6 +62,10 @@ impl Violation for UnspecifiedEncoding { } } } + + fn fix_title(&self) -> String { + format!("Add explicit `encoding` argument") + } } /// PLW1514 @@ -70,17 +84,63 @@ pub(crate) fn unspecified_encoding(checker: &mut Checker, call: &ast::ExprCall) return; }; - checker.diagnostics.push(Diagnostic::new( + let mut diagnostic = Diagnostic::new( UnspecifiedEncoding { function_name, mode, }, call.func.range(), - )); + ); + + if checker.settings.target_version >= PythonVersion::Py310 { + diagnostic.set_fix(generate_keyword_fix(checker, call)); + } else { + diagnostic.try_set_fix(|| generate_import_fix(checker, call)); + } + + checker.diagnostics.push(diagnostic); +} + +/// Generate an [`Edit`] for Python 3.10 and later. +fn generate_keyword_fix(checker: &Checker, call: &ast::ExprCall) -> Fix { + Fix::unsafe_edit(add_argument( + &format!( + "encoding={}", + checker + .generator() + .expr(&Expr::StringLiteral(ast::ExprStringLiteral { + value: ast::StringLiteralValue::single(ast::StringLiteral { + value: "locale".to_string(), + unicode: false, + range: TextRange::default(), + }), + range: TextRange::default(), + })) + ), + &call.arguments, + checker.indexer().comment_ranges(), + checker.locator().contents(), + )) +} + +/// Generate an [`Edit`] for Python 3.9 and earlier. +fn generate_import_fix(checker: &Checker, call: &ast::ExprCall) -> Result { + let (import_edit, binding) = checker.importer().get_or_import_symbol( + &ImportRequest::import("locale", "getpreferredencoding"), + call.start(), + checker.semantic(), + )?; + let argument_edit = add_argument( + &format!("encoding={binding}(False)"), + &call.arguments, + checker.indexer().comment_ranges(), + checker.locator().contents(), + ); + Ok(Fix::unsafe_edits(import_edit, [argument_edit])) } /// Returns `true` if the given expression is a string literal containing a `b` character. -fn is_binary_mode(expr: &ast::Expr) -> Option { +fn is_binary_mode(expr: &Expr) -> Option { Some( expr.as_string_literal_expr()? .value @@ -92,12 +152,7 @@ fn is_binary_mode(expr: &ast::Expr) -> Option { /// Returns `true` if the given call lacks an explicit `encoding`. fn is_violation(call: &ast::ExprCall, call_path: &CallPath) -> bool { // If we have something like `*args`, which might contain the encoding argument, abort. - if call - .arguments - .args - .iter() - .any(ruff_python_ast::Expr::is_starred_expr) - { + if call.arguments.args.iter().any(Expr::is_starred_expr) { return false; } // If we have something like `**kwargs`, which might contain the encoding argument, abort. diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap index d10f3d3b633fd..9ceaf09daf820 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW1514_unspecified_encoding.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/pylint/mod.rs --- -unspecified_encoding.py:8:1: PLW1514 `open` in text mode without explicit `encoding` argument +unspecified_encoding.py:8:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument | 7 | # Errors. 8 | open("test.txt") @@ -9,8 +9,19 @@ unspecified_encoding.py:8:1: PLW1514 `open` in text mode without explicit `encod 9 | io.TextIOWrapper(io.FileIO("test.txt")) 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) | + = help: Add explicit `encoding` argument -unspecified_encoding.py:9:1: PLW1514 `io.TextIOWrapper` without explicit `encoding` argument +ℹ Unsafe fix +5 5 | import codecs +6 6 | +7 7 | # Errors. +8 |-open("test.txt") + 8 |+open("test.txt", encoding="locale") +9 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 11 | tempfile.NamedTemporaryFile("w") + +unspecified_encoding.py:9:1: PLW1514 [*] `io.TextIOWrapper` without explicit `encoding` argument | 7 | # Errors. 8 | open("test.txt") @@ -19,8 +30,19 @@ unspecified_encoding.py:9:1: PLW1514 `io.TextIOWrapper` without explicit `encodi 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) 11 | tempfile.NamedTemporaryFile("w") | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +6 6 | +7 7 | # Errors. +8 8 | open("test.txt") +9 |-io.TextIOWrapper(io.FileIO("test.txt")) + 9 |+io.TextIOWrapper(io.FileIO("test.txt"), encoding="locale") +10 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 11 | tempfile.NamedTemporaryFile("w") +12 12 | tempfile.TemporaryFile("w") -unspecified_encoding.py:10:1: PLW1514 `io.TextIOWrapper` without explicit `encoding` argument +unspecified_encoding.py:10:1: PLW1514 [*] `io.TextIOWrapper` without explicit `encoding` argument | 8 | open("test.txt") 9 | io.TextIOWrapper(io.FileIO("test.txt")) @@ -29,8 +51,19 @@ unspecified_encoding.py:10:1: PLW1514 `io.TextIOWrapper` without explicit `encod 11 | tempfile.NamedTemporaryFile("w") 12 | tempfile.TemporaryFile("w") | + = help: Add explicit `encoding` argument -unspecified_encoding.py:11:1: PLW1514 `tempfile.NamedTemporaryFile` in text mode without explicit `encoding` argument +ℹ Unsafe fix +7 7 | # Errors. +8 8 | open("test.txt") +9 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 |-hugo.TextIOWrapper(hugo.FileIO("test.txt")) + 10 |+hugo.TextIOWrapper(hugo.FileIO("test.txt"), encoding="locale") +11 11 | tempfile.NamedTemporaryFile("w") +12 12 | tempfile.TemporaryFile("w") +13 13 | codecs.open("test.txt") + +unspecified_encoding.py:11:1: PLW1514 [*] `tempfile.NamedTemporaryFile` in text mode without explicit `encoding` argument | 9 | io.TextIOWrapper(io.FileIO("test.txt")) 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) @@ -39,8 +72,19 @@ unspecified_encoding.py:11:1: PLW1514 `tempfile.NamedTemporaryFile` in text mode 12 | tempfile.TemporaryFile("w") 13 | codecs.open("test.txt") | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +8 8 | open("test.txt") +9 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 |-tempfile.NamedTemporaryFile("w") + 11 |+tempfile.NamedTemporaryFile("w", encoding="locale") +12 12 | tempfile.TemporaryFile("w") +13 13 | codecs.open("test.txt") +14 14 | tempfile.SpooledTemporaryFile(0, "w") -unspecified_encoding.py:12:1: PLW1514 `tempfile.TemporaryFile` in text mode without explicit `encoding` argument +unspecified_encoding.py:12:1: PLW1514 [*] `tempfile.TemporaryFile` in text mode without explicit `encoding` argument | 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) 11 | tempfile.NamedTemporaryFile("w") @@ -49,8 +93,19 @@ unspecified_encoding.py:12:1: PLW1514 `tempfile.TemporaryFile` in text mode with 13 | codecs.open("test.txt") 14 | tempfile.SpooledTemporaryFile(0, "w") | + = help: Add explicit `encoding` argument -unspecified_encoding.py:13:1: PLW1514 `codecs.open` in text mode without explicit `encoding` argument +ℹ Unsafe fix +9 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 11 | tempfile.NamedTemporaryFile("w") +12 |-tempfile.TemporaryFile("w") + 12 |+tempfile.TemporaryFile("w", encoding="locale") +13 13 | codecs.open("test.txt") +14 14 | tempfile.SpooledTemporaryFile(0, "w") +15 15 | + +unspecified_encoding.py:13:1: PLW1514 [*] `codecs.open` in text mode without explicit `encoding` argument | 11 | tempfile.NamedTemporaryFile("w") 12 | tempfile.TemporaryFile("w") @@ -58,8 +113,19 @@ unspecified_encoding.py:13:1: PLW1514 `codecs.open` in text mode without explici | ^^^^^^^^^^^ PLW1514 14 | tempfile.SpooledTemporaryFile(0, "w") | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +10 10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 11 | tempfile.NamedTemporaryFile("w") +12 12 | tempfile.TemporaryFile("w") +13 |-codecs.open("test.txt") + 13 |+codecs.open("test.txt", encoding="locale") +14 14 | tempfile.SpooledTemporaryFile(0, "w") +15 15 | +16 16 | # Non-errors. -unspecified_encoding.py:14:1: PLW1514 `tempfile.SpooledTemporaryFile` in text mode without explicit `encoding` argument +unspecified_encoding.py:14:1: PLW1514 [*] `tempfile.SpooledTemporaryFile` in text mode without explicit `encoding` argument | 12 | tempfile.TemporaryFile("w") 13 | codecs.open("test.txt") @@ -68,5 +134,223 @@ unspecified_encoding.py:14:1: PLW1514 `tempfile.SpooledTemporaryFile` in text mo 15 | 16 | # Non-errors. | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +11 11 | tempfile.NamedTemporaryFile("w") +12 12 | tempfile.TemporaryFile("w") +13 13 | codecs.open("test.txt") +14 |-tempfile.SpooledTemporaryFile(0, "w") + 14 |+tempfile.SpooledTemporaryFile(0, "w", encoding="locale") +15 15 | +16 16 | # Non-errors. +17 17 | open("test.txt", encoding="utf-8") + +unspecified_encoding.py:46:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +44 | tempfile.SpooledTemporaryFile(0, ) +45 | +46 | open("test.txt",) + | ^^^^ PLW1514 +47 | open() +48 | open( + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +43 43 | tempfile.SpooledTemporaryFile(0, "wb") +44 44 | tempfile.SpooledTemporaryFile(0, ) +45 45 | +46 |-open("test.txt",) + 46 |+open("test.txt", encoding="locale",) +47 47 | open() +48 48 | open( +49 49 | "test.txt", # comment + +unspecified_encoding.py:47:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +46 | open("test.txt",) +47 | open() + | ^^^^ PLW1514 +48 | open( +49 | "test.txt", # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +44 44 | tempfile.SpooledTemporaryFile(0, ) +45 45 | +46 46 | open("test.txt",) +47 |-open() + 47 |+open(encoding="locale") +48 48 | open( +49 49 | "test.txt", # comment +50 50 | ) + +unspecified_encoding.py:48:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +46 | open("test.txt",) +47 | open() +48 | open( + | ^^^^ PLW1514 +49 | "test.txt", # comment +50 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +46 46 | open("test.txt",) +47 47 | open() +48 48 | open( +49 |- "test.txt", # comment + 49 |+ "test.txt", encoding="locale", # comment +50 50 | ) +51 51 | open( +52 52 | "test.txt", + +unspecified_encoding.py:51:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +49 | "test.txt", # comment +50 | ) +51 | open( + | ^^^^ PLW1514 +52 | "test.txt", +53 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +49 49 | "test.txt", # comment +50 50 | ) +51 51 | open( +52 |- "test.txt", + 52 |+ "test.txt", encoding="locale", +53 53 | # comment +54 54 | ) +55 55 | open(("test.txt"),) + +unspecified_encoding.py:55:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +53 | # comment +54 | ) +55 | open(("test.txt"),) + | ^^^^ PLW1514 +56 | open( +57 | ("test.txt"), # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +52 52 | "test.txt", +53 53 | # comment +54 54 | ) +55 |-open(("test.txt"),) + 55 |+open(("test.txt"), encoding="locale",) +56 56 | open( +57 57 | ("test.txt"), # comment +58 58 | ) + +unspecified_encoding.py:56:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +54 | ) +55 | open(("test.txt"),) +56 | open( + | ^^^^ PLW1514 +57 | ("test.txt"), # comment +58 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +54 54 | ) +55 55 | open(("test.txt"),) +56 56 | open( +57 |- ("test.txt"), # comment + 57 |+ ("test.txt"), encoding="locale", # comment +58 58 | ) +59 59 | open( +60 60 | ("test.txt"), + +unspecified_encoding.py:59:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +57 | ("test.txt"), # comment +58 | ) +59 | open( + | ^^^^ PLW1514 +60 | ("test.txt"), +61 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +57 57 | ("test.txt"), # comment +58 58 | ) +59 59 | open( +60 |- ("test.txt"), + 60 |+ ("test.txt"), encoding="locale", +61 61 | # comment +62 62 | ) +63 63 | + +unspecified_encoding.py:64:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +62 | ) +63 | +64 | open((("test.txt")),) + | ^^^^ PLW1514 +65 | open( +66 | (("test.txt")), # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +61 61 | # comment +62 62 | ) +63 63 | +64 |-open((("test.txt")),) + 64 |+open((("test.txt")), encoding="locale",) +65 65 | open( +66 66 | (("test.txt")), # comment +67 67 | ) + +unspecified_encoding.py:65:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +64 | open((("test.txt")),) +65 | open( + | ^^^^ PLW1514 +66 | (("test.txt")), # comment +67 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +63 63 | +64 64 | open((("test.txt")),) +65 65 | open( +66 |- (("test.txt")), # comment + 66 |+ (("test.txt")), encoding="locale", # comment +67 67 | ) +68 68 | open( +69 69 | (("test.txt")), + +unspecified_encoding.py:68:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +66 | (("test.txt")), # comment +67 | ) +68 | open( + | ^^^^ PLW1514 +69 | (("test.txt")), +70 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +66 66 | (("test.txt")), # comment +67 67 | ) +68 68 | open( +69 |- (("test.txt")), + 69 |+ (("test.txt")), encoding="locale", +70 70 | # comment +71 71 | ) diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__unspecified_encoding_python39_or_lower.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__unspecified_encoding_python39_or_lower.snap new file mode 100644 index 0000000000000..1390eeede0cf3 --- /dev/null +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__unspecified_encoding_python39_or_lower.snap @@ -0,0 +1,477 @@ +--- +source: crates/ruff_linter/src/rules/pylint/mod.rs +--- +unspecified_encoding.py:8:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | + 7 | # Errors. + 8 | open("test.txt") + | ^^^^ PLW1514 + 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 |-open("test.txt") + 9 |+open("test.txt", encoding=locale.getpreferredencoding(False)) +9 10 | io.TextIOWrapper(io.FileIO("test.txt")) +10 11 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 12 | tempfile.NamedTemporaryFile("w") + +unspecified_encoding.py:9:1: PLW1514 [*] `io.TextIOWrapper` without explicit `encoding` argument + | + 7 | # Errors. + 8 | open("test.txt") + 9 | io.TextIOWrapper(io.FileIO("test.txt")) + | ^^^^^^^^^^^^^^^^ PLW1514 +10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 | tempfile.NamedTemporaryFile("w") + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +9 |-io.TextIOWrapper(io.FileIO("test.txt")) + 10 |+io.TextIOWrapper(io.FileIO("test.txt"), encoding=locale.getpreferredencoding(False)) +10 11 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 12 | tempfile.NamedTemporaryFile("w") +12 13 | tempfile.TemporaryFile("w") + +unspecified_encoding.py:10:1: PLW1514 [*] `io.TextIOWrapper` without explicit `encoding` argument + | + 8 | open("test.txt") + 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) + | ^^^^^^^^^^^^^^^^^^ PLW1514 +11 | tempfile.NamedTemporaryFile("w") +12 | tempfile.TemporaryFile("w") + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +9 10 | io.TextIOWrapper(io.FileIO("test.txt")) +10 |-hugo.TextIOWrapper(hugo.FileIO("test.txt")) + 11 |+hugo.TextIOWrapper(hugo.FileIO("test.txt"), encoding=locale.getpreferredencoding(False)) +11 12 | tempfile.NamedTemporaryFile("w") +12 13 | tempfile.TemporaryFile("w") +13 14 | codecs.open("test.txt") + +unspecified_encoding.py:11:1: PLW1514 [*] `tempfile.NamedTemporaryFile` in text mode without explicit `encoding` argument + | + 9 | io.TextIOWrapper(io.FileIO("test.txt")) +10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 | tempfile.NamedTemporaryFile("w") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW1514 +12 | tempfile.TemporaryFile("w") +13 | codecs.open("test.txt") + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +9 10 | io.TextIOWrapper(io.FileIO("test.txt")) +10 11 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 |-tempfile.NamedTemporaryFile("w") + 12 |+tempfile.NamedTemporaryFile("w", encoding=locale.getpreferredencoding(False)) +12 13 | tempfile.TemporaryFile("w") +13 14 | codecs.open("test.txt") +14 15 | tempfile.SpooledTemporaryFile(0, "w") + +unspecified_encoding.py:12:1: PLW1514 [*] `tempfile.TemporaryFile` in text mode without explicit `encoding` argument + | +10 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 | tempfile.NamedTemporaryFile("w") +12 | tempfile.TemporaryFile("w") + | ^^^^^^^^^^^^^^^^^^^^^^ PLW1514 +13 | codecs.open("test.txt") +14 | tempfile.SpooledTemporaryFile(0, "w") + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +9 10 | io.TextIOWrapper(io.FileIO("test.txt")) +10 11 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 12 | tempfile.NamedTemporaryFile("w") +12 |-tempfile.TemporaryFile("w") + 13 |+tempfile.TemporaryFile("w", encoding=locale.getpreferredencoding(False)) +13 14 | codecs.open("test.txt") +14 15 | tempfile.SpooledTemporaryFile(0, "w") +15 16 | + +unspecified_encoding.py:13:1: PLW1514 [*] `codecs.open` in text mode without explicit `encoding` argument + | +11 | tempfile.NamedTemporaryFile("w") +12 | tempfile.TemporaryFile("w") +13 | codecs.open("test.txt") + | ^^^^^^^^^^^ PLW1514 +14 | tempfile.SpooledTemporaryFile(0, "w") + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +10 11 | hugo.TextIOWrapper(hugo.FileIO("test.txt")) +11 12 | tempfile.NamedTemporaryFile("w") +12 13 | tempfile.TemporaryFile("w") +13 |-codecs.open("test.txt") + 14 |+codecs.open("test.txt", encoding=locale.getpreferredencoding(False)) +14 15 | tempfile.SpooledTemporaryFile(0, "w") +15 16 | +16 17 | # Non-errors. + +unspecified_encoding.py:14:1: PLW1514 [*] `tempfile.SpooledTemporaryFile` in text mode without explicit `encoding` argument + | +12 | tempfile.TemporaryFile("w") +13 | codecs.open("test.txt") +14 | tempfile.SpooledTemporaryFile(0, "w") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW1514 +15 | +16 | # Non-errors. + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +11 12 | tempfile.NamedTemporaryFile("w") +12 13 | tempfile.TemporaryFile("w") +13 14 | codecs.open("test.txt") +14 |-tempfile.SpooledTemporaryFile(0, "w") + 15 |+tempfile.SpooledTemporaryFile(0, "w", encoding=locale.getpreferredencoding(False)) +15 16 | +16 17 | # Non-errors. +17 18 | open("test.txt", encoding="utf-8") + +unspecified_encoding.py:46:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +44 | tempfile.SpooledTemporaryFile(0, ) +45 | +46 | open("test.txt",) + | ^^^^ PLW1514 +47 | open() +48 | open( + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +43 44 | tempfile.SpooledTemporaryFile(0, "wb") +44 45 | tempfile.SpooledTemporaryFile(0, ) +45 46 | +46 |-open("test.txt",) + 47 |+open("test.txt", encoding=locale.getpreferredencoding(False),) +47 48 | open() +48 49 | open( +49 50 | "test.txt", # comment + +unspecified_encoding.py:47:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +46 | open("test.txt",) +47 | open() + | ^^^^ PLW1514 +48 | open( +49 | "test.txt", # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +44 45 | tempfile.SpooledTemporaryFile(0, ) +45 46 | +46 47 | open("test.txt",) +47 |-open() + 48 |+open(encoding=locale.getpreferredencoding(False)) +48 49 | open( +49 50 | "test.txt", # comment +50 51 | ) + +unspecified_encoding.py:48:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +46 | open("test.txt",) +47 | open() +48 | open( + | ^^^^ PLW1514 +49 | "test.txt", # comment +50 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +46 47 | open("test.txt",) +47 48 | open() +48 49 | open( +49 |- "test.txt", # comment + 50 |+ "test.txt", encoding=locale.getpreferredencoding(False), # comment +50 51 | ) +51 52 | open( +52 53 | "test.txt", + +unspecified_encoding.py:51:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +49 | "test.txt", # comment +50 | ) +51 | open( + | ^^^^ PLW1514 +52 | "test.txt", +53 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +49 50 | "test.txt", # comment +50 51 | ) +51 52 | open( +52 |- "test.txt", + 53 |+ "test.txt", encoding=locale.getpreferredencoding(False), +53 54 | # comment +54 55 | ) +55 56 | open(("test.txt"),) + +unspecified_encoding.py:55:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +53 | # comment +54 | ) +55 | open(("test.txt"),) + | ^^^^ PLW1514 +56 | open( +57 | ("test.txt"), # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +52 53 | "test.txt", +53 54 | # comment +54 55 | ) +55 |-open(("test.txt"),) + 56 |+open(("test.txt"), encoding=locale.getpreferredencoding(False),) +56 57 | open( +57 58 | ("test.txt"), # comment +58 59 | ) + +unspecified_encoding.py:56:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +54 | ) +55 | open(("test.txt"),) +56 | open( + | ^^^^ PLW1514 +57 | ("test.txt"), # comment +58 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +54 55 | ) +55 56 | open(("test.txt"),) +56 57 | open( +57 |- ("test.txt"), # comment + 58 |+ ("test.txt"), encoding=locale.getpreferredencoding(False), # comment +58 59 | ) +59 60 | open( +60 61 | ("test.txt"), + +unspecified_encoding.py:59:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +57 | ("test.txt"), # comment +58 | ) +59 | open( + | ^^^^ PLW1514 +60 | ("test.txt"), +61 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +57 58 | ("test.txt"), # comment +58 59 | ) +59 60 | open( +60 |- ("test.txt"), + 61 |+ ("test.txt"), encoding=locale.getpreferredencoding(False), +61 62 | # comment +62 63 | ) +63 64 | + +unspecified_encoding.py:64:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +62 | ) +63 | +64 | open((("test.txt")),) + | ^^^^ PLW1514 +65 | open( +66 | (("test.txt")), # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +61 62 | # comment +62 63 | ) +63 64 | +64 |-open((("test.txt")),) + 65 |+open((("test.txt")), encoding=locale.getpreferredencoding(False),) +65 66 | open( +66 67 | (("test.txt")), # comment +67 68 | ) + +unspecified_encoding.py:65:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +64 | open((("test.txt")),) +65 | open( + | ^^^^ PLW1514 +66 | (("test.txt")), # comment +67 | ) + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +63 64 | +64 65 | open((("test.txt")),) +65 66 | open( +66 |- (("test.txt")), # comment + 67 |+ (("test.txt")), encoding=locale.getpreferredencoding(False), # comment +67 68 | ) +68 69 | open( +69 70 | (("test.txt")), + +unspecified_encoding.py:68:1: PLW1514 [*] `open` in text mode without explicit `encoding` argument + | +66 | (("test.txt")), # comment +67 | ) +68 | open( + | ^^^^ PLW1514 +69 | (("test.txt")), +70 | # comment + | + = help: Add explicit `encoding` argument + +ℹ Unsafe fix +3 3 | import tempfile +4 4 | import io as hugo +5 5 | import codecs + 6 |+import locale +6 7 | +7 8 | # Errors. +8 9 | open("test.txt") +-------------------------------------------------------------------------------- +66 67 | (("test.txt")), # comment +67 68 | ) +68 69 | open( +69 |- (("test.txt")), + 70 |+ (("test.txt")), encoding=locale.getpreferredencoding(False), +70 71 | # comment +71 72 | ) + + From 0b1a36f8c8a26549beadeae90e8f65781b1a6240 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Fri, 1 Dec 2023 14:46:39 -0500 Subject: [PATCH 05/18] ruff_python_formatter: light refactoring of code snippet formatting in docstrings (#8950) In the source of working on #8859, I made a number of smallish refactors to how code snippet formatting works. Most or all of these were motivated by writing in support for reStructuredText blocks. They have some fundamentally different requirements than doctests, and there are a lot more ways for reStructuredText blocks to become invalid. (Commit-by-commit review is recommended as the commit messages provide further context on each change. I split this off from ongoing work to make review more manageable.) --- .../fixtures/ruff/docstring_code_examples.py | 9 + .../src/expression/string/docstring.rs | 320 +++++++++++------- .../format@docstring_code_examples.py.snap | 81 +++++ 3 files changed, 291 insertions(+), 119 deletions(-) diff --git a/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples.py b/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples.py index 1050c71a31847..6f5fc0b30bb61 100644 --- a/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples.py +++ b/crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_code_examples.py @@ -1,3 +1,12 @@ +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ diff --git a/crates/ruff_python_formatter/src/expression/string/docstring.rs b/crates/ruff_python_formatter/src/expression/string/docstring.rs index 3155fa1a0f532..bed24c34191c0 100644 --- a/crates/ruff_python_formatter/src/expression/string/docstring.rs +++ b/crates/ruff_python_formatter/src/expression/string/docstring.rs @@ -244,10 +244,10 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { mut lines: std::iter::Peekable>, ) -> FormatResult<()> { while let Some(line) = lines.next() { - let line = DocstringLine { - line: Cow::Borrowed(line), + let line = InputDocstringLine { + line, offset: self.offset, - is_last: lines.peek().is_none(), + next: lines.peek().copied(), }; // We know that the normalized string has \n line endings. self.offset += line.line.text_len() + "\n".text_len(); @@ -262,7 +262,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { /// immediately to the underlying buffer. If the line starts or is part /// of an existing code snippet, then the lines will get buffered until /// the code snippet is complete. - fn add_one(&mut self, line: DocstringLine<'src>) -> FormatResult<()> { + fn add_one(&mut self, line: InputDocstringLine<'src>) -> FormatResult<()> { // Just pass through the line as-is without looking for a code snippet // when docstring code formatting is disabled. And also when we are // formatting a code snippet so as to avoid arbitrarily nested code @@ -271,45 +271,49 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { // not clear that it's worth the effort to support. if !self.f.options().docstring_code().is_enabled() || self.f.context().docstring().is_some() { - return self.print_one(&line); + return self.print_one(&line.as_output()); } match self.code_example.add(line) { - CodeExampleAddAction::Print { original } => self.print_one(&original)?, + CodeExampleAddAction::Print { original } => self.print_one(&original.as_output())?, CodeExampleAddAction::Kept => {} - CodeExampleAddAction::Format { - kind, - code, - original, - } => { - let Some(formatted_lines) = self.format(&code)? else { + CodeExampleAddAction::Reset { code, original } => { + for codeline in code { + self.print_one(&codeline.original.as_output())?; + } + self.print_one(&original.as_output())?; + } + CodeExampleAddAction::Format { mut kind, original } => { + let Some(formatted_lines) = self.format(kind.code())? else { // If formatting failed in a way that should not be // allowed, we back out what we're doing and print the // original lines we found as-is as if we did nothing. - for codeline in code { - self.print_one(&codeline.original)?; + for codeline in kind.code() { + self.print_one(&codeline.original.as_output())?; } if let Some(original) = original { - self.print_one(&original)?; + self.print_one(&original.as_output())?; } return Ok(()); }; self.already_normalized = false; match kind { - CodeExampleKind::Doctest(CodeExampleDoctest { indent }) => { + CodeExampleKind::Doctest(CodeExampleDoctest { ps1_indent, .. }) => { let mut lines = formatted_lines.into_iter(); if let Some(first) = lines.next() { - self.print_one(&first.map(|line| std::format!("{indent}>>> {line}")))?; + self.print_one( + &first.map(|line| std::format!("{ps1_indent}>>> {line}")), + )?; for docline in lines { self.print_one( - &docline.map(|line| std::format!("{indent}... {line}")), + &docline.map(|line| std::format!("{ps1_indent}... {line}")), )?; } } } } if let Some(original) = original { - self.print_one(&original)?; + self.print_one(&original.as_output())?; } } } @@ -321,7 +325,7 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { /// This mostly just handles indentation and ensuring line breaks are /// inserted as appropriate before passing it on to the formatter to /// print to the buffer. - fn print_one(&mut self, line: &DocstringLine<'_>) -> FormatResult<()> { + fn print_one(&mut self, line: &OutputDocstringLine<'_>) -> FormatResult<()> { let trim_end = line.line.trim_end(); if trim_end.is_empty() { return if line.is_last { @@ -391,10 +395,14 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { /// routine is silent about it. So from the user's perspective, this will /// fail silently. Ideally, this would at least emit a warning message, /// but at time of writing, it wasn't clear to me how to best do that. + /// + /// # Panics + /// + /// This panics when the given slice is empty. fn format( &mut self, - code: &[CodeExampleLine<'src>], - ) -> FormatResult>>> { + code: &[CodeExampleLine<'_>], + ) -> FormatResult>>> { use ruff_python_parser::AsMode; let offset = code @@ -406,10 +414,10 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { .last() .expect("code blob must be non-empty") .original - .is_last; + .is_last(); let codeblob = code .iter() - .map(|line| &*line.code) + .map(|line| line.code) .collect::>() .join("\n"); let printed = match docstring_format_source(self.f.options(), self.quote_style, &codeblob) { @@ -451,8 +459,8 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { let mut lines = printed .as_code() .lines() - .map(|line| DocstringLine { - line: Cow::Owned(line.into()), + .map(|line| OutputDocstringLine { + line: Cow::Owned(line.to_string()), offset, is_last: false, }) @@ -466,29 +474,72 @@ impl<'ast, 'buf, 'fmt, 'src> DocstringLinePrinter<'ast, 'buf, 'fmt, 'src> { /// Represents a single line in a docstring. /// -/// This type is used to both represent the original lines in a docstring -/// (the line will be borrowed) and also the newly formatted lines from code -/// snippets (the line will be owned). -#[derive(Clone, Debug)] -struct DocstringLine<'src> { +/// This type is only used to represent the original lines in a docstring. +/// Specifically, the line contained in this type has no changes from the input +/// source. +#[derive(Clone, Copy, Debug)] +struct InputDocstringLine<'src> { /// The actual text of the line, not including the line terminator. /// /// In practice, this line is borrowed when it corresponds to an original /// unformatted line in a docstring, and owned when it corresponds to a /// reformatted line (e.g., from a code snippet) in a docstring. + line: &'src str, + /// The offset into the source document which this line corresponds to. + offset: TextSize, + /// For any input line that isn't the last line, this contains a reference + /// to the line immediately following this one. + /// + /// This is `None` if and only if this is the last line in the docstring. + next: Option<&'src str>, +} + +impl<'src> InputDocstringLine<'src> { + /// Borrow this input docstring line as an output docstring line. + fn as_output(&self) -> OutputDocstringLine<'src> { + OutputDocstringLine { + line: Cow::Borrowed(self.line), + offset: self.offset, + is_last: self.is_last(), + } + } + + /// Whether this is the last line in the docstring or not. + fn is_last(&self) -> bool { + self.next.is_none() + } +} + +/// Represents a single reformatted code line in a docstring. +/// +/// An input source line may be cheaply converted to an output source line. +/// This is the common case: an input source line is printed pretty much as it +/// is, with perhaps some whitespace normalization applied. The less common +/// case is that the output docstring line owns its `line` because it was +/// produced by reformatting a code snippet. +#[derive(Clone, Debug)] +struct OutputDocstringLine<'src> { + /// The output line. + /// + /// This is an owned variant in precisely the cases where it corresponds to + /// a line from a reformatted code snippet. In other cases, it is borrowed + /// from the input docstring line as-is. line: Cow<'src, str>, /// The offset into the source document which this line corresponds to. + /// Currently, this is an estimate. offset: TextSize, - /// Whether this is the last line in a docstring or not. "Last" lines have - /// some special treatment when printing. + /// Whether this is the last line in a docstring or not. This is determined + /// by whether the last line in the code snippet was also the last line in + /// the docstring. If it was, then it follows that the last line in the + /// reformatted code snippet is also the last line in the docstring. is_last: bool, } -impl<'src> DocstringLine<'src> { - /// Return this line, but with the given function applied to the text of - /// the line. - fn map(self, mut map: impl FnMut(&str) -> String) -> DocstringLine<'static> { - DocstringLine { +impl<'src> OutputDocstringLine<'src> { + /// Return this reformatted line, but with the given function applied to + /// the text of the line. + fn map(self, mut map: impl FnMut(&str) -> String) -> OutputDocstringLine<'static> { + OutputDocstringLine { line: Cow::Owned(map(&self.line)), ..self } @@ -507,9 +558,10 @@ impl<'src> DocstringLine<'src> { struct CodeExample<'src> { /// The kind of code example being collected, or `None` if no code example /// has been observed. - kind: Option, - /// The lines that have been seen so far that make up the code example. - lines: Vec>, + /// + /// The kind is split out into a separate type so that we can pass it + /// around and have a guarantee that a code example actually exists. + kind: Option>, } impl<'src> CodeExample<'src> { @@ -520,7 +572,7 @@ impl<'src> CodeExample<'src> { /// the caller to perform. The typical case is a "print" action, which /// instructs the caller to just print the line as though it were not part /// of a code snippet. - fn add(&mut self, original: DocstringLine<'src>) -> CodeExampleAddAction<'src> { + fn add(&mut self, original: InputDocstringLine<'src>) -> CodeExampleAddAction<'src> { match self.kind.take() { // There's no existing code example being built, so we look for // the start of one or otherwise tell the caller we couldn't find @@ -529,19 +581,15 @@ impl<'src> CodeExample<'src> { None => CodeExampleAddAction::Kept, Some(original) => CodeExampleAddAction::Print { original }, }, - Some(CodeExampleKind::Doctest(doctest)) => { - if let Some(code) = doctest_find_ps2_prompt(&doctest.indent, &original.line) { - let code = code.to_string(); - self.lines.push(CodeExampleLine { original, code }); + Some(CodeExampleKind::Doctest(mut doctest)) => { + if doctest.add_code_line(original) { // Stay with the doctest kind while we accumulate all // PS2 prompts. self.kind = Some(CodeExampleKind::Doctest(doctest)); return CodeExampleAddAction::Kept; } - let code = std::mem::take(&mut self.lines); let original = self.add_start(original); CodeExampleAddAction::Format { - code, kind: CodeExampleKind::Doctest(doctest), original, } @@ -558,13 +606,13 @@ impl<'src> CodeExample<'src> { /// This panics when the existing code-example is any non-None value. That /// is, this routine assumes that there is no ongoing code example being /// collected and looks for the beginning of another code example. - fn add_start(&mut self, original: DocstringLine<'src>) -> Option> { - assert_eq!(None, self.kind, "expected no existing code example"); - if let Some((indent, code)) = doctest_find_ps1_prompt(&original.line) { - let indent = indent.to_string(); - let code = code.to_string(); - self.lines.push(CodeExampleLine { original, code }); - self.kind = Some(CodeExampleKind::Doctest(CodeExampleDoctest { indent })); + fn add_start( + &mut self, + original: InputDocstringLine<'src>, + ) -> Option> { + assert!(self.kind.is_none(), "expected no existing code example"); + if let Some(doctest) = CodeExampleDoctest::new(original) { + self.kind = Some(CodeExampleKind::Doctest(doctest)); return None; } Some(original) @@ -572,8 +620,8 @@ impl<'src> CodeExample<'src> { } /// The kind of code example observed in a docstring. -#[derive(Clone, Debug, Eq, PartialEq)] -enum CodeExampleKind { +#[derive(Debug)] +enum CodeExampleKind<'src> { /// Code found in Python "doctests." /// /// Documentation describing doctests and how they're recognized can be @@ -584,17 +632,90 @@ enum CodeExampleKind { /// doctest module to determine more precisely how it works.) /// /// [regex matching]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L611-L622 - Doctest(CodeExampleDoctest), + Doctest(CodeExampleDoctest<'src>), +} + +impl<'src> CodeExampleKind<'src> { + /// Return the lines of code collected so far for this example. + /// + /// This is borrowed mutably because it may need to mutate the code lines + /// based on the state accrued so far. + fn code(&mut self) -> &[CodeExampleLine<'src>] { + match *self { + CodeExampleKind::Doctest(ref doctest) => &doctest.lines, + } + } } /// State corresponding to a single doctest code example found in a docstring. -#[derive(Clone, Debug, Eq, PartialEq)] -struct CodeExampleDoctest { +#[derive(Debug)] +struct CodeExampleDoctest<'src> { + /// The lines that have been seen so far that make up the doctest. + lines: Vec>, /// The indent observed in the first doctest line. /// /// More precisely, this corresponds to the whitespace observed before /// the starting `>>> ` (the "PS1 prompt"). - indent: String, + ps1_indent: &'src str, +} + +impl<'src> CodeExampleDoctest<'src> { + /// Looks for a valid doctest PS1 prompt in the line given. + /// + /// If one was found, then state for a new doctest code example is + /// returned, along with the code example line. + fn new(original: InputDocstringLine<'src>) -> Option> { + let trim_start = original.line.trim_start(); + // Prompts must be followed by an ASCII space character[1]. + // + // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L809-L812 + let code = trim_start.strip_prefix(">>> ")?; + let indent_len = original + .line + .len() + .checked_sub(trim_start.len()) + .expect("suffix is <= original"); + let lines = vec![CodeExampleLine { original, code }]; + let ps1_indent = &original.line[..indent_len]; + let doctest = CodeExampleDoctest { lines, ps1_indent }; + Some(doctest) + } + + /// Looks for a valid doctest PS2 prompt in the line given. + /// + /// If one is found, then the code portion of the line following the PS2 prompt + /// is returned. + /// + /// Callers must provide a string containing the original indentation of the + /// PS1 prompt that started the doctest containing the potential PS2 prompt + /// in the line given. If the line contains a PS2 prompt, its indentation must + /// match the indentation used for the corresponding PS1 prompt (otherwise + /// `None` will be returned). + fn add_code_line(&mut self, original: InputDocstringLine<'src>) -> bool { + let Some((ps2_indent, ps2_after)) = original.line.split_once("...") else { + return false; + }; + // PS2 prompts must have the same indentation as their + // corresponding PS1 prompt.[1] While the 'doctest' Python + // module will error in this case, we just treat this line as a + // non-doctest line. + // + // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L733 + if self.ps1_indent != ps2_indent { + return false; + } + // PS2 prompts must be followed by an ASCII space character unless + // it's an otherwise empty line[1]. + // + // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L809-L812 + let code = match ps2_after.strip_prefix(' ') { + None if ps2_after.is_empty() => "", + None => return false, + Some(code) => code, + }; + self.lines.push(CodeExampleLine { original, code }); + true + } } /// A single line in a code example found in a docstring. @@ -615,9 +736,9 @@ struct CodeExampleLine<'src> { /// The normalized (but original) line from the doc string. This might, for /// example, contain a `>>> ` or `... ` prefix if this code example is a /// doctest. - original: DocstringLine<'src>, + original: InputDocstringLine<'src>, /// The code extracted from the line. - code: String, + code: &'src str, } /// An action that a caller should perform after attempting to add a line from @@ -633,7 +754,7 @@ enum CodeExampleAddAction<'src> { /// /// This is the common case. That is, most lines in most docstrings are not /// part of a code example. - Print { original: DocstringLine<'src> }, + Print { original: InputDocstringLine<'src> }, /// The line added was kept by `CodeExample` as part of a new or existing /// code example. /// @@ -647,68 +768,29 @@ enum CodeExampleAddAction<'src> { /// callers should pass it through to the formatter as-is. Format { /// The kind of code example that was found. - kind: CodeExampleKind, - /// The Python code that should be formatted, indented and printed. /// - /// This is guaranteed to be non-empty. - code: Vec>, + /// This is guaranteed to have a non-empty code snippet. + kind: CodeExampleKind<'src>, /// When set, the line is considered not part of any code example and /// should be formatted as if the [`Print`] action were returned. /// Otherwise, if there is no line, then either one does not exist /// or it is part of another code example and should be treated as a /// [`Kept`] action. - original: Option>, + original: Option>, + }, + /// This occurs when adding a line to an existing code example + /// results in that code example becoming invalid. In this case, + /// we don't want to treat it as a code example, but instead write + /// back the lines to the docstring unchanged. + #[allow(dead_code)] // FIXME: remove when reStructuredText support is added + Reset { + /// The lines of code that we collected but should be printed back to + /// the docstring as-is and not formatted. + code: Vec>, + /// The line that was added and triggered this reset to occur. It + /// should be written back to the docstring as-is after the code lines. + original: InputDocstringLine<'src>, }, -} - -/// Looks for a valid doctest PS1 prompt in the line given. -/// -/// If one was found, then the indentation prior to the prompt is returned -/// along with the code portion of the line. -fn doctest_find_ps1_prompt(line: &str) -> Option<(&str, &str)> { - let trim_start = line.trim_start(); - // Prompts must be followed by an ASCII space character[1]. - // - // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L809-L812 - let code = trim_start.strip_prefix(">>> ")?; - let indent_len = line - .len() - .checked_sub(trim_start.len()) - .expect("suffix is <= original"); - let indent = &line[..indent_len]; - Some((indent, code)) -} - -/// Looks for a valid doctest PS2 prompt in the line given. -/// -/// If one is found, then the code portion of the line following the PS2 prompt -/// is returned. -/// -/// Callers must provide a string containing the original indentation of the -/// PS1 prompt that started the doctest containing the potential PS2 prompt -/// in the line given. If the line contains a PS2 prompt, its indentation must -/// match the indentation used for the corresponding PS1 prompt (otherwise -/// `None` will be returned). -fn doctest_find_ps2_prompt<'src>(ps1_indent: &str, line: &'src str) -> Option<&'src str> { - let (ps2_indent, ps2_after) = line.split_once("...")?; - // PS2 prompts must have the same indentation as their - // corresponding PS1 prompt.[1] While the 'doctest' Python - // module will error in this case, we just treat this line as a - // non-doctest line. - // - // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L733 - if ps1_indent != ps2_indent { - return None; - } - // PS2 prompts must be followed by an ASCII space character unless - // it's an otherwise empty line[1]. - // - // [1]: https://github.com/python/cpython/blob/0ff6368519ed7542ad8b443de01108690102420a/Lib/doctest.py#L809-L812 - match ps2_after.strip_prefix(' ') { - None if ps2_after.is_empty() => Some(""), - None => None, - Some(code) => Some(code), - } } /// Formats the given source code using the given options. diff --git a/crates/ruff_python_formatter/tests/snapshots/format@docstring_code_examples.py.snap b/crates/ruff_python_formatter/tests/snapshots/format@docstring_code_examples.py.snap index 3a83075f29975..5eeafa914fb93 100644 --- a/crates/ruff_python_formatter/tests/snapshots/format@docstring_code_examples.py.snap +++ b/crates/ruff_python_formatter/tests/snapshots/format@docstring_code_examples.py.snap @@ -4,6 +4,15 @@ input_file: crates/ruff_python_formatter/resources/test/fixtures/ruff/docstring_ --- ## Input ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -336,6 +345,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -671,6 +689,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -1006,6 +1033,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -1341,6 +1377,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -1676,6 +1721,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -2011,6 +2065,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -2346,6 +2409,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ @@ -2681,6 +2753,15 @@ preview = Disabled ``` ```python +############################################################################### +# DOCTEST CODE EXAMPLES +# +# This section shows examples of docstrings that contain code snippets in +# Python's "doctest" format. +# +# See: https://docs.python.org/3/library/doctest.html +############################################################################### + # The simplest doctest to ensure basic formatting works. def doctest_simple(): """ From 4af3f43e5ec63a1a246d3acb2f04ed5e5195fb6a Mon Sep 17 00:00:00 2001 From: Michael Essiet Date: Fri, 1 Dec 2023 21:43:01 +0100 Subject: [PATCH 06/18] Added the command to run ruff using pkgx to the installation.md (#8955) ## Summary This PR adds the command to run ruff using [pkgx](https://pkgx.sh). It's just showing that ruff is supported in one more package manager. ## Test Plan You can run `pkgx ruff` if you have pkgx installed or run `sh <(curl https://pkgx.sh) +github.com/charliermarsh/ruff sh ` --- docs/installation.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/installation.md b/docs/installation.md index a822bdf4b0230..e0a1e882e93f9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -27,6 +27,13 @@ For **Conda** users, Ruff is also available as [`ruff`](https://anaconda.org/con conda install -c conda-forge ruff ``` +For **pkgx** users, Ruff is also available as [`ruff`](https://pkgx.dev/pkgs/github.com/charliermarsh/ruff/) +on the `pkgx` registry: + +```shell +pkgx install ruff +``` + For **Arch Linux** users, Ruff is also available as [`ruff`](https://archlinux.org/packages/extra/x86_64/ruff/) on the official repositories: From 20a40771a51658f1b7d90a8e8595be8e60c41c97 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Dec 2023 17:58:32 -0500 Subject: [PATCH 07/18] Consider more wildcards in control flow graph matches (#8956) --- .../test/fixtures/control-flow-graph/match.py | 8 + ...ules__unreachable__tests__match.py.md.snap | 39 + ...__unreachable__tests__match.py.md.snap.new | 816 ++++++++++++++++++ .../src/rules/ruff/rules/unreachable.rs | 32 +- 4 files changed, 883 insertions(+), 12 deletions(-) create mode 100644 crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new diff --git a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/match.py b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/match.py index cce019e3085e9..d83726268991f 100644 --- a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/match.py +++ b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/match.py @@ -129,3 +129,11 @@ class Color(Enum): print("Grass is green") case Color.BLUE: print("I'm feeling the blues :(") + + +def func(point): + match point: + case (0, 0): + print("Origin") + case foo: + raise ValueError("oops") diff --git a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap index 54cb336e95510..d74b08116f936 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap +++ b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap @@ -773,4 +773,43 @@ flowchart TD block0 --> return ``` +## Function 14 +### Source +```python +def func(point): + match point: + case (0, 0): + print("Origin") + case foo: + raise ValueError("oops") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["raise ValueError(#quot;oops#quot;)\n"] + block2["match point: + case (0, 0): + print(#quot;Origin#quot;) + case foo: + raise ValueError(#quot;oops#quot;)\n"] + block3["print(#quot;Origin#quot;)\n"] + block4["match point: + case (0, 0): + print(#quot;Origin#quot;) + case foo: + raise ValueError(#quot;oops#quot;)\n"] + + start --> block4 + block4 -- "case (0, 0)" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> return + block0 --> return +``` + diff --git a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new new file mode 100644 index 0000000000000..2dfe0793d895d --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new @@ -0,0 +1,816 @@ +--- +source: crates/ruff_linter/src/rules/ruff/rules/unreachable.rs +assertion_line: 1112 +description: "This is a Mermaid graph. You can use https://mermaid.live to visualize it as a diagram." +--- +## Function 0 +### Source +```python +def func(status): + match status: + case _: + return 0 + return "unreachable" +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return #quot;unreachable#quot;\n"] + block1["return 0\n"] + block2["match status: + case _: + return 0\n"] + + start --> block2 + block2 --> block1 + block1 --> return + block0 --> return +``` + +## Function 1 +### Source +```python +def func(status): + match status: + case 1: + return 1 + return 0 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return 0\n"] + block1["return 1\n"] + block2["match status: + case 1: + return 1\n"] + + start --> block2 + block2 -- "case 1" --> block1 + block2 -- "else" --> block0 + block1 --> return + block0 --> return +``` + +## Function 2 +### Source +```python +def func(status): + match status: + case 1: + return 1 + case _: + return 0 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return 0\n"] + block1["match status: + case 1: + return 1 + case _: + return 0\n"] + block2["return 1\n"] + block3["match status: + case 1: + return 1 + case _: + return 0\n"] + + start --> block3 + block3 -- "case 1" --> block2 + block3 -- "else" --> block1 + block2 --> return + block1 --> block0 + block0 --> return +``` + +## Function 3 +### Source +```python +def func(status): + match status: + case 1 | 2 | 3: + return 5 + return 6 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return 6\n"] + block1["return 5\n"] + block2["match status: + case 1 | 2 | 3: + return 5\n"] + + start --> block2 + block2 -- "case 1 | 2 | 3" --> block1 + block2 -- "else" --> block0 + block1 --> return + block0 --> return +``` + +## Function 4 +### Source +```python +def func(status): + match status: + case 1 | 2 | 3: + return 5 + case _: + return 10 + return 0 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return 0\n"] + block1["return 10\n"] + block2["match status: + case 1 | 2 | 3: + return 5 + case _: + return 10\n"] + block3["return 5\n"] + block4["match status: + case 1 | 2 | 3: + return 5 + case _: + return 10\n"] + + start --> block4 + block4 -- "case 1 | 2 | 3" --> block3 + block4 -- "else" --> block2 + block3 --> return + block2 --> block1 + block1 --> return + block0 --> return +``` + +## Function 5 +### Source +```python +def func(status): + match status: + case 0: + return 0 + case 1: + return 1 + case 1: + return "1 again" + case _: + return 3 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["return 3\n"] + block1["match status: + case 0: + return 0 + case 1: + return 1 + case 1: + return #quot;1 again#quot; + case _: + return 3\n"] + block2["return #quot;1 again#quot;\n"] + block3["match status: + case 0: + return 0 + case 1: + return 1 + case 1: + return #quot;1 again#quot; + case _: + return 3\n"] + block4["return 1\n"] + block5["match status: + case 0: + return 0 + case 1: + return 1 + case 1: + return #quot;1 again#quot; + case _: + return 3\n"] + block6["return 0\n"] + block7["match status: + case 0: + return 0 + case 1: + return 1 + case 1: + return #quot;1 again#quot; + case _: + return 3\n"] + + start --> block7 + block7 -- "case 0" --> block6 + block7 -- "else" --> block5 + block6 --> return + block5 -- "case 1" --> block4 + block5 -- "else" --> block3 + block4 --> return + block3 -- "case 1" --> block2 + block3 -- "else" --> block1 + block2 --> return + block1 --> block0 + block0 --> return +``` + +## Function 6 +### Source +```python +def func(status): + i = 0 + match status, i: + case _, _: + return 0 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["return 0\n"] + block2["match status, i: + case _, _: + return 0\n"] + block3["i = 0\n"] + + start --> block3 + block3 --> block2 + block2 -- "case _, _" --> block1 + block2 -- "else" --> block0 + block1 --> return + block0 --> return +``` + +## Function 7 +### Source +```python +def func(status): + i = 0 + match status, i: + case _, 0: + return 0 + case _, 2: + return 0 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["return 0\n"] + block2["match status, i: + case _, 0: + return 0 + case _, 2: + return 0\n"] + block3["return 0\n"] + block4["match status, i: + case _, 0: + return 0 + case _, 2: + return 0\n"] + block5["i = 0\n"] + + start --> block5 + block5 --> block4 + block4 -- "case _, 0" --> block3 + block4 -- "else" --> block2 + block3 --> return + block2 -- "case _, 2" --> block1 + block2 -- "else" --> block0 + block1 --> return + block0 --> return +``` + +## Function 8 +### Source +```python +def func(point): + match point: + case (0, 0): + print("Origin") + case _: + raise ValueError("oops") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["raise ValueError(#quot;oops#quot;)\n"] + block2["match point: + case (0, 0): + print(#quot;Origin#quot;) + case _: + raise ValueError(#quot;oops#quot;)\n"] + block3["print(#quot;Origin#quot;)\n"] + block4["match point: + case (0, 0): + print(#quot;Origin#quot;) + case _: + raise ValueError(#quot;oops#quot;)\n"] + + start --> block4 + block4 -- "case (0, 0)" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> return + block0 --> return +``` + +## Function 9 +### Source +```python +def func(point): + match point: + case (0, 0): + print("Origin") + case (0, y): + print(f"Y={y}") + case (x, 0): + print(f"X={x}") + case (x, y): + print(f"X={x}, Y={y}") + case _: + raise ValueError("Not a point") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["raise ValueError(#quot;Not a point#quot;)\n"] + block2["match point: + case (0, 0): + print(#quot;Origin#quot;) + case (0, y): + print(f#quot;Y={y}#quot;) + case (x, 0): + print(f#quot;X={x}#quot;) + case (x, y): + print(f#quot;X={x}, Y={y}#quot;) + case _: + raise ValueError(#quot;Not a point#quot;)\n"] + block3["print(f#quot;X={x}, Y={y}#quot;)\n"] + block4["match point: + case (0, 0): + print(#quot;Origin#quot;) + case (0, y): + print(f#quot;Y={y}#quot;) + case (x, 0): + print(f#quot;X={x}#quot;) + case (x, y): + print(f#quot;X={x}, Y={y}#quot;) + case _: + raise ValueError(#quot;Not a point#quot;)\n"] + block5["print(f#quot;X={x}#quot;)\n"] + block6["match point: + case (0, 0): + print(#quot;Origin#quot;) + case (0, y): + print(f#quot;Y={y}#quot;) + case (x, 0): + print(f#quot;X={x}#quot;) + case (x, y): + print(f#quot;X={x}, Y={y}#quot;) + case _: + raise ValueError(#quot;Not a point#quot;)\n"] + block7["print(f#quot;Y={y}#quot;)\n"] + block8["match point: + case (0, 0): + print(#quot;Origin#quot;) + case (0, y): + print(f#quot;Y={y}#quot;) + case (x, 0): + print(f#quot;X={x}#quot;) + case (x, y): + print(f#quot;X={x}, Y={y}#quot;) + case _: + raise ValueError(#quot;Not a point#quot;)\n"] + block9["print(#quot;Origin#quot;)\n"] + block10["match point: + case (0, 0): + print(#quot;Origin#quot;) + case (0, y): + print(f#quot;Y={y}#quot;) + case (x, 0): + print(f#quot;X={x}#quot;) + case (x, y): + print(f#quot;X={x}, Y={y}#quot;) + case _: + raise ValueError(#quot;Not a point#quot;)\n"] + + start --> block10 + block10 -- "case (0, 0)" --> block9 + block10 -- "else" --> block8 + block9 --> block0 + block8 -- "case (0, y)" --> block7 + block8 -- "else" --> block6 + block7 --> block0 + block6 -- "case (x, 0)" --> block5 + block6 -- "else" --> block4 + block5 --> block0 + block4 -- "case (x, y)" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> return + block0 --> return +``` + +## Function 10 +### Source +```python +def where_is(point): + class Point: + x: int + y: int + + match point: + case Point(x=0, y=0): + print("Origin") + case Point(x=0, y=y): + print(f"Y={y}") + case Point(x=x, y=0): + print(f"X={x}") + case Point(): + print("Somewhere else") + case _: + print("Not a point") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["print(#quot;Not a point#quot;)\n"] + block2["match point: + case Point(x=0, y=0): + print(#quot;Origin#quot;) + case Point(x=0, y=y): + print(f#quot;Y={y}#quot;) + case Point(x=x, y=0): + print(f#quot;X={x}#quot;) + case Point(): + print(#quot;Somewhere else#quot;) + case _: + print(#quot;Not a point#quot;)\n"] + block3["print(#quot;Somewhere else#quot;)\n"] + block4["match point: + case Point(x=0, y=0): + print(#quot;Origin#quot;) + case Point(x=0, y=y): + print(f#quot;Y={y}#quot;) + case Point(x=x, y=0): + print(f#quot;X={x}#quot;) + case Point(): + print(#quot;Somewhere else#quot;) + case _: + print(#quot;Not a point#quot;)\n"] + block5["print(f#quot;X={x}#quot;)\n"] + block6["match point: + case Point(x=0, y=0): + print(#quot;Origin#quot;) + case Point(x=0, y=y): + print(f#quot;Y={y}#quot;) + case Point(x=x, y=0): + print(f#quot;X={x}#quot;) + case Point(): + print(#quot;Somewhere else#quot;) + case _: + print(#quot;Not a point#quot;)\n"] + block7["print(f#quot;Y={y}#quot;)\n"] + block8["match point: + case Point(x=0, y=0): + print(#quot;Origin#quot;) + case Point(x=0, y=y): + print(f#quot;Y={y}#quot;) + case Point(x=x, y=0): + print(f#quot;X={x}#quot;) + case Point(): + print(#quot;Somewhere else#quot;) + case _: + print(#quot;Not a point#quot;)\n"] + block9["print(#quot;Origin#quot;)\n"] + block10["match point: + case Point(x=0, y=0): + print(#quot;Origin#quot;) + case Point(x=0, y=y): + print(f#quot;Y={y}#quot;) + case Point(x=x, y=0): + print(f#quot;X={x}#quot;) + case Point(): + print(#quot;Somewhere else#quot;) + case _: + print(#quot;Not a point#quot;)\n"] + block11["class Point: + x: int + y: int\n"] + + start --> block11 + block11 --> block10 + block10 -- "case Point(x=0, y=0)" --> block9 + block10 -- "else" --> block8 + block9 --> block0 + block8 -- "case Point(x=0, y=y)" --> block7 + block8 -- "else" --> block6 + block7 --> block0 + block6 -- "case Point(x=x, y=0)" --> block5 + block6 -- "else" --> block4 + block5 --> block0 + block4 -- "case Point()" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> block0 + block0 --> return +``` + +## Function 11 +### Source +```python +def func(points): + match points: + case []: + print("No points") + case [Point(0, 0)]: + print("The origin") + case [Point(x, y)]: + print(f"Single point {x}, {y}") + case [Point(0, y1), Point(0, y2)]: + print(f"Two on the Y axis at {y1}, {y2}") + case _: + print("Something else") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["print(#quot;Something else#quot;)\n"] + block2["match points: + case []: + print(#quot;No points#quot;) + case [Point(0, 0)]: + print(#quot;The origin#quot;) + case [Point(x, y)]: + print(f#quot;Single point {x}, {y}#quot;) + case [Point(0, y1), Point(0, y2)]: + print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) + case _: + print(#quot;Something else#quot;)\n"] + block3["print(f#quot;Two on the Y axis at {y1}, {y2}#quot;)\n"] + block4["match points: + case []: + print(#quot;No points#quot;) + case [Point(0, 0)]: + print(#quot;The origin#quot;) + case [Point(x, y)]: + print(f#quot;Single point {x}, {y}#quot;) + case [Point(0, y1), Point(0, y2)]: + print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) + case _: + print(#quot;Something else#quot;)\n"] + block5["print(f#quot;Single point {x}, {y}#quot;)\n"] + block6["match points: + case []: + print(#quot;No points#quot;) + case [Point(0, 0)]: + print(#quot;The origin#quot;) + case [Point(x, y)]: + print(f#quot;Single point {x}, {y}#quot;) + case [Point(0, y1), Point(0, y2)]: + print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) + case _: + print(#quot;Something else#quot;)\n"] + block7["print(#quot;The origin#quot;)\n"] + block8["match points: + case []: + print(#quot;No points#quot;) + case [Point(0, 0)]: + print(#quot;The origin#quot;) + case [Point(x, y)]: + print(f#quot;Single point {x}, {y}#quot;) + case [Point(0, y1), Point(0, y2)]: + print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) + case _: + print(#quot;Something else#quot;)\n"] + block9["print(#quot;No points#quot;)\n"] + block10["match points: + case []: + print(#quot;No points#quot;) + case [Point(0, 0)]: + print(#quot;The origin#quot;) + case [Point(x, y)]: + print(f#quot;Single point {x}, {y}#quot;) + case [Point(0, y1), Point(0, y2)]: + print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) + case _: + print(#quot;Something else#quot;)\n"] + + start --> block10 + block10 -- "case []" --> block9 + block10 -- "else" --> block8 + block9 --> block0 + block8 -- "case [Point(0, 0)]" --> block7 + block8 -- "else" --> block6 + block7 --> block0 + block6 -- "case [Point(x, y)]" --> block5 + block6 -- "else" --> block4 + block5 --> block0 + block4 -- "case [Point(0, y1), Point(0, y2)]" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> block0 + block0 --> return +``` + +## Function 12 +### Source +```python +def func(point): + match point: + case Point(x, y) if x == y: + print(f"Y=X at {x}") + case Point(x, y): + print(f"Not on the diagonal") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["print(f#quot;Not on the diagonal#quot;)\n"] + block2["match point: + case Point(x, y) if x == y: + print(f#quot;Y=X at {x}#quot;) + case Point(x, y): + print(f#quot;Not on the diagonal#quot;)\n"] + block3["print(f#quot;Y=X at {x}#quot;)\n"] + block4["match point: + case Point(x, y) if x == y: + print(f#quot;Y=X at {x}#quot;) + case Point(x, y): + print(f#quot;Not on the diagonal#quot;)\n"] + + start --> block4 + block4 -- "case Point(x, y) if x == y" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 -- "case Point(x, y)" --> block1 + block2 -- "else" --> block0 + block1 --> block0 + block0 --> return +``` + +## Function 13 +### Source +```python +def func(): + from enum import Enum + class Color(Enum): + RED = 'red' + GREEN = 'green' + BLUE = 'blue' + + color = Color(input("Enter your choice of 'red', 'blue' or 'green': ")) + + match color: + case Color.RED: + print("I see red!") + case Color.GREEN: + print("Grass is green") + case Color.BLUE: + print("I'm feeling the blues :(") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["print(#quot;I'm feeling the blues :(#quot;)\n"] + block2["match color: + case Color.RED: + print(#quot;I see red!#quot;) + case Color.GREEN: + print(#quot;Grass is green#quot;) + case Color.BLUE: + print(#quot;I'm feeling the blues :(#quot;)\n"] + block3["print(#quot;Grass is green#quot;)\n"] + block4["match color: + case Color.RED: + print(#quot;I see red!#quot;) + case Color.GREEN: + print(#quot;Grass is green#quot;) + case Color.BLUE: + print(#quot;I'm feeling the blues :(#quot;)\n"] + block5["print(#quot;I see red!#quot;)\n"] + block6["match color: + case Color.RED: + print(#quot;I see red!#quot;) + case Color.GREEN: + print(#quot;Grass is green#quot;) + case Color.BLUE: + print(#quot;I'm feeling the blues :(#quot;)\n"] + block7["from enum import Enum\nclass Color(Enum): + RED = 'red' + GREEN = 'green' + BLUE = 'blue'\ncolor = Color(input(#quot;Enter your choice of 'red', 'blue' or 'green': #quot;))\n"] + + start --> block7 + block7 --> block6 + block6 -- "case Color.RED" --> block5 + block6 -- "else" --> block4 + block5 --> block0 + block4 -- "case Color.GREEN" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 -- "case Color.BLUE" --> block1 + block2 -- "else" --> block0 + block1 --> block0 + block0 --> return +``` + +## Function 14 +### Source +```python +def func(point): + match point: + case (0, 0): + print("Origin") + case _: + raise ValueError("oops") +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0[["`*(empty)*`"]] + block1["raise ValueError(#quot;oops#quot;)\n"] + block2["match point: + case (0, 0): + print(#quot;Origin#quot;) + case _: + raise ValueError(#quot;oops#quot;)\n"] + block3["print(#quot;Origin#quot;)\n"] + block4["match point: + case (0, 0): + print(#quot;Origin#quot;) + case _: + raise ValueError(#quot;oops#quot;)\n"] + + start --> block4 + block4 -- "case (0, 0)" --> block3 + block4 -- "else" --> block2 + block3 --> block0 + block2 --> block1 + block1 --> return + block0 --> return +``` + + diff --git a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs index 8f5db5a35bc32..51a75f817e0e7 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs @@ -2,8 +2,8 @@ use std::{fmt, iter, usize}; use log::error; use ruff_python_ast::{ - Expr, ExprBooleanLiteral, Identifier, MatchCase, Pattern, PatternMatchAs, Stmt, StmtFor, - StmtMatch, StmtReturn, StmtTry, StmtWhile, StmtWith, + Expr, ExprBooleanLiteral, Identifier, MatchCase, Pattern, PatternMatchAs, PatternMatchOr, Stmt, + StmtFor, StmtMatch, StmtReturn, StmtTry, StmtWhile, StmtWith, }; use ruff_text_size::{Ranged, TextRange, TextSize}; @@ -416,13 +416,6 @@ fn match_case<'stmt>( } last_statement_index }; - // TODO: handle named arguments, e.g. - // ```python - // match $subject: - // case $binding: - // print($binding) - // ``` - // These should also return `NextBlock::Always`. let next = if is_wildcard(case) { // Wildcard case is always taken. NextBlock::Always(next_block_index) @@ -436,10 +429,25 @@ fn match_case<'stmt>( BasicBlock { stmts, next } } -/// Returns true if `pattern` is a wildcard (`_`) pattern. +/// Returns true if the [`MatchCase`] is a wildcard pattern. fn is_wildcard(pattern: &MatchCase) -> bool { - pattern.guard.is_none() - && matches!(&pattern.pattern, Pattern::MatchAs(PatternMatchAs { pattern, name, .. }) if pattern.is_none() && name.is_none()) + /// Returns true if the [`Pattern`] is a wildcard pattern. + fn is_wildcard_pattern(pattern: &Pattern) -> bool { + match pattern { + Pattern::MatchValue(_) + | Pattern::MatchSingleton(_) + | Pattern::MatchSequence(_) + | Pattern::MatchMapping(_) + | Pattern::MatchClass(_) + | Pattern::MatchStar(_) => false, + Pattern::MatchAs(PatternMatchAs { pattern, .. }) => pattern.is_none(), + Pattern::MatchOr(PatternMatchOr { patterns, .. }) => { + patterns.iter().all(is_wildcard_pattern) + } + } + } + + pattern.guard.is_none() && is_wildcard_pattern(&pattern.pattern) } #[derive(Debug, Default)] From 277cd80175ae52c5d78331fa8dc3ed632999606a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Dec 2023 18:11:42 -0500 Subject: [PATCH 08/18] Add erroneous for-loop test case for CFG (#8957) --- .../test/fixtures/control-flow-graph/for.py | 15 + ..._rules__unreachable__tests__for.py.md.snap | 61 ++ ...__unreachable__tests__match.py.md.snap.new | 816 ------------------ 3 files changed, 76 insertions(+), 816 deletions(-) delete mode 100644 crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new diff --git a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/for.py b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/for.py index a5807a635a412..9aef74d5d027f 100644 --- a/crates/ruff_linter/resources/test/fixtures/control-flow-graph/for.py +++ b/crates/ruff_linter/resources/test/fixtures/control-flow-graph/for.py @@ -39,3 +39,18 @@ def func(): for i in range(1110): if True: break + +# TODO(charlie): The `pass` here does not get properly redirected to the top of the +# loop, unlike below. +def func(): + for i in range(5): + pass + else: + return 1 + +def func(): + for i in range(5): + pass + else: + return 1 + x = 1 diff --git a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__for.py.md.snap b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__for.py.md.snap index e93710e8eda53..6850c5f69b04b 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__for.py.md.snap +++ b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__for.py.md.snap @@ -238,4 +238,65 @@ flowchart TD block0 --> return ``` +## Function 8 +### Source +```python +def func(): + for i in range(5): + pass + else: + return 1 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["pass\n"] + block1["return 1\n"] + block2["for i in range(5): + pass + else: + return 1\n"] + + start --> block2 + block2 -- "range(5)" --> block0 + block2 -- "else" --> block1 + block1 --> return + block0 --> return +``` + +## Function 9 +### Source +```python +def func(): + for i in range(5): + pass + else: + return 1 + x = 1 +``` + +### Control Flow Graph +```mermaid +flowchart TD + start(("Start")) + return(("End")) + block0["x = 1\n"] + block1["pass\n"] + block2["return 1\n"] + block3["for i in range(5): + pass + else: + return 1\n"] + + start --> block3 + block3 -- "range(5)" --> block1 + block3 -- "else" --> block2 + block2 --> return + block1 --> block3 + block0 --> return +``` + diff --git a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new b/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new deleted file mode 100644 index 2dfe0793d895d..0000000000000 --- a/crates/ruff_linter/src/rules/ruff/rules/snapshots/ruff_linter__rules__ruff__rules__unreachable__tests__match.py.md.snap.new +++ /dev/null @@ -1,816 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/ruff/rules/unreachable.rs -assertion_line: 1112 -description: "This is a Mermaid graph. You can use https://mermaid.live to visualize it as a diagram." ---- -## Function 0 -### Source -```python -def func(status): - match status: - case _: - return 0 - return "unreachable" -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return #quot;unreachable#quot;\n"] - block1["return 0\n"] - block2["match status: - case _: - return 0\n"] - - start --> block2 - block2 --> block1 - block1 --> return - block0 --> return -``` - -## Function 1 -### Source -```python -def func(status): - match status: - case 1: - return 1 - return 0 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return 0\n"] - block1["return 1\n"] - block2["match status: - case 1: - return 1\n"] - - start --> block2 - block2 -- "case 1" --> block1 - block2 -- "else" --> block0 - block1 --> return - block0 --> return -``` - -## Function 2 -### Source -```python -def func(status): - match status: - case 1: - return 1 - case _: - return 0 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return 0\n"] - block1["match status: - case 1: - return 1 - case _: - return 0\n"] - block2["return 1\n"] - block3["match status: - case 1: - return 1 - case _: - return 0\n"] - - start --> block3 - block3 -- "case 1" --> block2 - block3 -- "else" --> block1 - block2 --> return - block1 --> block0 - block0 --> return -``` - -## Function 3 -### Source -```python -def func(status): - match status: - case 1 | 2 | 3: - return 5 - return 6 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return 6\n"] - block1["return 5\n"] - block2["match status: - case 1 | 2 | 3: - return 5\n"] - - start --> block2 - block2 -- "case 1 | 2 | 3" --> block1 - block2 -- "else" --> block0 - block1 --> return - block0 --> return -``` - -## Function 4 -### Source -```python -def func(status): - match status: - case 1 | 2 | 3: - return 5 - case _: - return 10 - return 0 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return 0\n"] - block1["return 10\n"] - block2["match status: - case 1 | 2 | 3: - return 5 - case _: - return 10\n"] - block3["return 5\n"] - block4["match status: - case 1 | 2 | 3: - return 5 - case _: - return 10\n"] - - start --> block4 - block4 -- "case 1 | 2 | 3" --> block3 - block4 -- "else" --> block2 - block3 --> return - block2 --> block1 - block1 --> return - block0 --> return -``` - -## Function 5 -### Source -```python -def func(status): - match status: - case 0: - return 0 - case 1: - return 1 - case 1: - return "1 again" - case _: - return 3 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0["return 3\n"] - block1["match status: - case 0: - return 0 - case 1: - return 1 - case 1: - return #quot;1 again#quot; - case _: - return 3\n"] - block2["return #quot;1 again#quot;\n"] - block3["match status: - case 0: - return 0 - case 1: - return 1 - case 1: - return #quot;1 again#quot; - case _: - return 3\n"] - block4["return 1\n"] - block5["match status: - case 0: - return 0 - case 1: - return 1 - case 1: - return #quot;1 again#quot; - case _: - return 3\n"] - block6["return 0\n"] - block7["match status: - case 0: - return 0 - case 1: - return 1 - case 1: - return #quot;1 again#quot; - case _: - return 3\n"] - - start --> block7 - block7 -- "case 0" --> block6 - block7 -- "else" --> block5 - block6 --> return - block5 -- "case 1" --> block4 - block5 -- "else" --> block3 - block4 --> return - block3 -- "case 1" --> block2 - block3 -- "else" --> block1 - block2 --> return - block1 --> block0 - block0 --> return -``` - -## Function 6 -### Source -```python -def func(status): - i = 0 - match status, i: - case _, _: - return 0 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["return 0\n"] - block2["match status, i: - case _, _: - return 0\n"] - block3["i = 0\n"] - - start --> block3 - block3 --> block2 - block2 -- "case _, _" --> block1 - block2 -- "else" --> block0 - block1 --> return - block0 --> return -``` - -## Function 7 -### Source -```python -def func(status): - i = 0 - match status, i: - case _, 0: - return 0 - case _, 2: - return 0 -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["return 0\n"] - block2["match status, i: - case _, 0: - return 0 - case _, 2: - return 0\n"] - block3["return 0\n"] - block4["match status, i: - case _, 0: - return 0 - case _, 2: - return 0\n"] - block5["i = 0\n"] - - start --> block5 - block5 --> block4 - block4 -- "case _, 0" --> block3 - block4 -- "else" --> block2 - block3 --> return - block2 -- "case _, 2" --> block1 - block2 -- "else" --> block0 - block1 --> return - block0 --> return -``` - -## Function 8 -### Source -```python -def func(point): - match point: - case (0, 0): - print("Origin") - case _: - raise ValueError("oops") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["raise ValueError(#quot;oops#quot;)\n"] - block2["match point: - case (0, 0): - print(#quot;Origin#quot;) - case _: - raise ValueError(#quot;oops#quot;)\n"] - block3["print(#quot;Origin#quot;)\n"] - block4["match point: - case (0, 0): - print(#quot;Origin#quot;) - case _: - raise ValueError(#quot;oops#quot;)\n"] - - start --> block4 - block4 -- "case (0, 0)" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 --> block1 - block1 --> return - block0 --> return -``` - -## Function 9 -### Source -```python -def func(point): - match point: - case (0, 0): - print("Origin") - case (0, y): - print(f"Y={y}") - case (x, 0): - print(f"X={x}") - case (x, y): - print(f"X={x}, Y={y}") - case _: - raise ValueError("Not a point") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["raise ValueError(#quot;Not a point#quot;)\n"] - block2["match point: - case (0, 0): - print(#quot;Origin#quot;) - case (0, y): - print(f#quot;Y={y}#quot;) - case (x, 0): - print(f#quot;X={x}#quot;) - case (x, y): - print(f#quot;X={x}, Y={y}#quot;) - case _: - raise ValueError(#quot;Not a point#quot;)\n"] - block3["print(f#quot;X={x}, Y={y}#quot;)\n"] - block4["match point: - case (0, 0): - print(#quot;Origin#quot;) - case (0, y): - print(f#quot;Y={y}#quot;) - case (x, 0): - print(f#quot;X={x}#quot;) - case (x, y): - print(f#quot;X={x}, Y={y}#quot;) - case _: - raise ValueError(#quot;Not a point#quot;)\n"] - block5["print(f#quot;X={x}#quot;)\n"] - block6["match point: - case (0, 0): - print(#quot;Origin#quot;) - case (0, y): - print(f#quot;Y={y}#quot;) - case (x, 0): - print(f#quot;X={x}#quot;) - case (x, y): - print(f#quot;X={x}, Y={y}#quot;) - case _: - raise ValueError(#quot;Not a point#quot;)\n"] - block7["print(f#quot;Y={y}#quot;)\n"] - block8["match point: - case (0, 0): - print(#quot;Origin#quot;) - case (0, y): - print(f#quot;Y={y}#quot;) - case (x, 0): - print(f#quot;X={x}#quot;) - case (x, y): - print(f#quot;X={x}, Y={y}#quot;) - case _: - raise ValueError(#quot;Not a point#quot;)\n"] - block9["print(#quot;Origin#quot;)\n"] - block10["match point: - case (0, 0): - print(#quot;Origin#quot;) - case (0, y): - print(f#quot;Y={y}#quot;) - case (x, 0): - print(f#quot;X={x}#quot;) - case (x, y): - print(f#quot;X={x}, Y={y}#quot;) - case _: - raise ValueError(#quot;Not a point#quot;)\n"] - - start --> block10 - block10 -- "case (0, 0)" --> block9 - block10 -- "else" --> block8 - block9 --> block0 - block8 -- "case (0, y)" --> block7 - block8 -- "else" --> block6 - block7 --> block0 - block6 -- "case (x, 0)" --> block5 - block6 -- "else" --> block4 - block5 --> block0 - block4 -- "case (x, y)" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 --> block1 - block1 --> return - block0 --> return -``` - -## Function 10 -### Source -```python -def where_is(point): - class Point: - x: int - y: int - - match point: - case Point(x=0, y=0): - print("Origin") - case Point(x=0, y=y): - print(f"Y={y}") - case Point(x=x, y=0): - print(f"X={x}") - case Point(): - print("Somewhere else") - case _: - print("Not a point") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["print(#quot;Not a point#quot;)\n"] - block2["match point: - case Point(x=0, y=0): - print(#quot;Origin#quot;) - case Point(x=0, y=y): - print(f#quot;Y={y}#quot;) - case Point(x=x, y=0): - print(f#quot;X={x}#quot;) - case Point(): - print(#quot;Somewhere else#quot;) - case _: - print(#quot;Not a point#quot;)\n"] - block3["print(#quot;Somewhere else#quot;)\n"] - block4["match point: - case Point(x=0, y=0): - print(#quot;Origin#quot;) - case Point(x=0, y=y): - print(f#quot;Y={y}#quot;) - case Point(x=x, y=0): - print(f#quot;X={x}#quot;) - case Point(): - print(#quot;Somewhere else#quot;) - case _: - print(#quot;Not a point#quot;)\n"] - block5["print(f#quot;X={x}#quot;)\n"] - block6["match point: - case Point(x=0, y=0): - print(#quot;Origin#quot;) - case Point(x=0, y=y): - print(f#quot;Y={y}#quot;) - case Point(x=x, y=0): - print(f#quot;X={x}#quot;) - case Point(): - print(#quot;Somewhere else#quot;) - case _: - print(#quot;Not a point#quot;)\n"] - block7["print(f#quot;Y={y}#quot;)\n"] - block8["match point: - case Point(x=0, y=0): - print(#quot;Origin#quot;) - case Point(x=0, y=y): - print(f#quot;Y={y}#quot;) - case Point(x=x, y=0): - print(f#quot;X={x}#quot;) - case Point(): - print(#quot;Somewhere else#quot;) - case _: - print(#quot;Not a point#quot;)\n"] - block9["print(#quot;Origin#quot;)\n"] - block10["match point: - case Point(x=0, y=0): - print(#quot;Origin#quot;) - case Point(x=0, y=y): - print(f#quot;Y={y}#quot;) - case Point(x=x, y=0): - print(f#quot;X={x}#quot;) - case Point(): - print(#quot;Somewhere else#quot;) - case _: - print(#quot;Not a point#quot;)\n"] - block11["class Point: - x: int - y: int\n"] - - start --> block11 - block11 --> block10 - block10 -- "case Point(x=0, y=0)" --> block9 - block10 -- "else" --> block8 - block9 --> block0 - block8 -- "case Point(x=0, y=y)" --> block7 - block8 -- "else" --> block6 - block7 --> block0 - block6 -- "case Point(x=x, y=0)" --> block5 - block6 -- "else" --> block4 - block5 --> block0 - block4 -- "case Point()" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 --> block1 - block1 --> block0 - block0 --> return -``` - -## Function 11 -### Source -```python -def func(points): - match points: - case []: - print("No points") - case [Point(0, 0)]: - print("The origin") - case [Point(x, y)]: - print(f"Single point {x}, {y}") - case [Point(0, y1), Point(0, y2)]: - print(f"Two on the Y axis at {y1}, {y2}") - case _: - print("Something else") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["print(#quot;Something else#quot;)\n"] - block2["match points: - case []: - print(#quot;No points#quot;) - case [Point(0, 0)]: - print(#quot;The origin#quot;) - case [Point(x, y)]: - print(f#quot;Single point {x}, {y}#quot;) - case [Point(0, y1), Point(0, y2)]: - print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) - case _: - print(#quot;Something else#quot;)\n"] - block3["print(f#quot;Two on the Y axis at {y1}, {y2}#quot;)\n"] - block4["match points: - case []: - print(#quot;No points#quot;) - case [Point(0, 0)]: - print(#quot;The origin#quot;) - case [Point(x, y)]: - print(f#quot;Single point {x}, {y}#quot;) - case [Point(0, y1), Point(0, y2)]: - print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) - case _: - print(#quot;Something else#quot;)\n"] - block5["print(f#quot;Single point {x}, {y}#quot;)\n"] - block6["match points: - case []: - print(#quot;No points#quot;) - case [Point(0, 0)]: - print(#quot;The origin#quot;) - case [Point(x, y)]: - print(f#quot;Single point {x}, {y}#quot;) - case [Point(0, y1), Point(0, y2)]: - print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) - case _: - print(#quot;Something else#quot;)\n"] - block7["print(#quot;The origin#quot;)\n"] - block8["match points: - case []: - print(#quot;No points#quot;) - case [Point(0, 0)]: - print(#quot;The origin#quot;) - case [Point(x, y)]: - print(f#quot;Single point {x}, {y}#quot;) - case [Point(0, y1), Point(0, y2)]: - print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) - case _: - print(#quot;Something else#quot;)\n"] - block9["print(#quot;No points#quot;)\n"] - block10["match points: - case []: - print(#quot;No points#quot;) - case [Point(0, 0)]: - print(#quot;The origin#quot;) - case [Point(x, y)]: - print(f#quot;Single point {x}, {y}#quot;) - case [Point(0, y1), Point(0, y2)]: - print(f#quot;Two on the Y axis at {y1}, {y2}#quot;) - case _: - print(#quot;Something else#quot;)\n"] - - start --> block10 - block10 -- "case []" --> block9 - block10 -- "else" --> block8 - block9 --> block0 - block8 -- "case [Point(0, 0)]" --> block7 - block8 -- "else" --> block6 - block7 --> block0 - block6 -- "case [Point(x, y)]" --> block5 - block6 -- "else" --> block4 - block5 --> block0 - block4 -- "case [Point(0, y1), Point(0, y2)]" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 --> block1 - block1 --> block0 - block0 --> return -``` - -## Function 12 -### Source -```python -def func(point): - match point: - case Point(x, y) if x == y: - print(f"Y=X at {x}") - case Point(x, y): - print(f"Not on the diagonal") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["print(f#quot;Not on the diagonal#quot;)\n"] - block2["match point: - case Point(x, y) if x == y: - print(f#quot;Y=X at {x}#quot;) - case Point(x, y): - print(f#quot;Not on the diagonal#quot;)\n"] - block3["print(f#quot;Y=X at {x}#quot;)\n"] - block4["match point: - case Point(x, y) if x == y: - print(f#quot;Y=X at {x}#quot;) - case Point(x, y): - print(f#quot;Not on the diagonal#quot;)\n"] - - start --> block4 - block4 -- "case Point(x, y) if x == y" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 -- "case Point(x, y)" --> block1 - block2 -- "else" --> block0 - block1 --> block0 - block0 --> return -``` - -## Function 13 -### Source -```python -def func(): - from enum import Enum - class Color(Enum): - RED = 'red' - GREEN = 'green' - BLUE = 'blue' - - color = Color(input("Enter your choice of 'red', 'blue' or 'green': ")) - - match color: - case Color.RED: - print("I see red!") - case Color.GREEN: - print("Grass is green") - case Color.BLUE: - print("I'm feeling the blues :(") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["print(#quot;I'm feeling the blues :(#quot;)\n"] - block2["match color: - case Color.RED: - print(#quot;I see red!#quot;) - case Color.GREEN: - print(#quot;Grass is green#quot;) - case Color.BLUE: - print(#quot;I'm feeling the blues :(#quot;)\n"] - block3["print(#quot;Grass is green#quot;)\n"] - block4["match color: - case Color.RED: - print(#quot;I see red!#quot;) - case Color.GREEN: - print(#quot;Grass is green#quot;) - case Color.BLUE: - print(#quot;I'm feeling the blues :(#quot;)\n"] - block5["print(#quot;I see red!#quot;)\n"] - block6["match color: - case Color.RED: - print(#quot;I see red!#quot;) - case Color.GREEN: - print(#quot;Grass is green#quot;) - case Color.BLUE: - print(#quot;I'm feeling the blues :(#quot;)\n"] - block7["from enum import Enum\nclass Color(Enum): - RED = 'red' - GREEN = 'green' - BLUE = 'blue'\ncolor = Color(input(#quot;Enter your choice of 'red', 'blue' or 'green': #quot;))\n"] - - start --> block7 - block7 --> block6 - block6 -- "case Color.RED" --> block5 - block6 -- "else" --> block4 - block5 --> block0 - block4 -- "case Color.GREEN" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 -- "case Color.BLUE" --> block1 - block2 -- "else" --> block0 - block1 --> block0 - block0 --> return -``` - -## Function 14 -### Source -```python -def func(point): - match point: - case (0, 0): - print("Origin") - case _: - raise ValueError("oops") -``` - -### Control Flow Graph -```mermaid -flowchart TD - start(("Start")) - return(("End")) - block0[["`*(empty)*`"]] - block1["raise ValueError(#quot;oops#quot;)\n"] - block2["match point: - case (0, 0): - print(#quot;Origin#quot;) - case _: - raise ValueError(#quot;oops#quot;)\n"] - block3["print(#quot;Origin#quot;)\n"] - block4["match point: - case (0, 0): - print(#quot;Origin#quot;) - case _: - raise ValueError(#quot;oops#quot;)\n"] - - start --> block4 - block4 -- "case (0, 0)" --> block3 - block4 -- "else" --> block2 - block3 --> block0 - block2 --> block1 - block1 --> return - block0 --> return -``` - - From 58bf6f57626afe5f9d5b8635ae831f346913b5f2 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 1 Dec 2023 18:46:50 -0500 Subject: [PATCH 09/18] Remove todo branches from control-flow graph (#8960) --- .../src/rules/ruff/rules/unreachable.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs index 51a75f817e0e7..e94cb4179d3e9 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/unreachable.rs @@ -485,6 +485,8 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { | Stmt::AugAssign(_) | Stmt::AnnAssign(_) | Stmt::Break(_) + | Stmt::TypeAlias(_) + | Stmt::IpyEscapeCommand(_) | Stmt::Pass(_) => self.unconditional_next_block(after), Stmt::Continue(_) => { // NOTE: the next branch gets fixed up in `change_next_block`. @@ -646,6 +648,7 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { | Expr::Starred(_) | Expr::Name(_) | Expr::List(_) + | Expr::IpyEscapeCommand(_) | Expr::Tuple(_) | Expr::Slice(_) => self.unconditional_next_block(after), // TODO: handle these expressions. @@ -659,13 +662,10 @@ impl<'stmt> BasicBlocksBuilder<'stmt> { | Expr::Await(_) | Expr::Yield(_) | Expr::YieldFrom(_) => self.unconditional_next_block(after), - Expr::IpyEscapeCommand(_) => todo!(), } } // The tough branches are done, here is an easy one. Stmt::Return(_) => NextBlock::Terminate, - Stmt::TypeAlias(_) => todo!(), - Stmt::IpyEscapeCommand(_) => todo!(), }; // Include any statements in the block that don't divert the control flow. @@ -898,6 +898,8 @@ fn needs_next_block(stmts: &[Stmt]) -> bool { | Stmt::AnnAssign(_) | Stmt::Expr(_) | Stmt::Pass(_) + | Stmt::TypeAlias(_) + | Stmt::IpyEscapeCommand(_) // TODO: check below. | Stmt::Break(_) | Stmt::Continue(_) @@ -907,8 +909,6 @@ fn needs_next_block(stmts: &[Stmt]) -> bool { | Stmt::Match(_) | Stmt::Try(_) | Stmt::Assert(_) => true, - Stmt::TypeAlias(_) => todo!(), - Stmt::IpyEscapeCommand(_) => todo!(), } } @@ -927,6 +927,8 @@ fn is_control_flow_stmt(stmt: &Stmt) -> bool { | Stmt::AugAssign(_) | Stmt::AnnAssign(_) | Stmt::Expr(_) + | Stmt::TypeAlias(_) + | Stmt::IpyEscapeCommand(_) | Stmt::Pass(_) => false, Stmt::Return(_) | Stmt::For(_) @@ -939,8 +941,6 @@ fn is_control_flow_stmt(stmt: &Stmt) -> bool { | Stmt::Assert(_) | Stmt::Break(_) | Stmt::Continue(_) => true, - Stmt::TypeAlias(_) => todo!(), - Stmt::IpyEscapeCommand(_) => todo!(), } } From 5aaf99b856bdd26cd281ab1b99b8d7227cd6185d Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Sat, 2 Dec 2023 09:35:34 +0900 Subject: [PATCH 10/18] Implement the `fix_power_op_line_length` preview style (#8947) --- .../src/expression/binary_like.rs | 16 +- .../src/expression/parentheses.rs | 20 + ...ty@cases__preview_power_op_spacing.py.snap | 354 ------------------ 3 files changed, 30 insertions(+), 360 deletions(-) delete mode 100644 crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_power_op_spacing.py.snap diff --git a/crates/ruff_python_formatter/src/expression/binary_like.rs b/crates/ruff_python_formatter/src/expression/binary_like.rs index 6c88db34f6e0f..149449d61cb04 100644 --- a/crates/ruff_python_formatter/src/expression/binary_like.rs +++ b/crates/ruff_python_formatter/src/expression/binary_like.rs @@ -3,7 +3,7 @@ use std::ops::{Deref, Index}; use smallvec::SmallVec; -use ruff_formatter::write; +use ruff_formatter::{write, FormatContext}; use ruff_python_ast::{ Expr, ExprAttribute, ExprBinOp, ExprBoolOp, ExprCompare, ExprUnaryOp, UnaryOp, }; @@ -13,10 +13,10 @@ use ruff_text_size::{Ranged, TextRange}; use crate::comments::{leading_comments, trailing_comments, Comments, SourceComment}; use crate::expression::parentheses::{ - in_parentheses_only_group, in_parentheses_only_soft_line_break, - in_parentheses_only_soft_line_break_or_space, is_expression_parenthesized, - write_in_parentheses_only_group_end_tag, write_in_parentheses_only_group_start_tag, - Parentheses, + in_parentheses_only_group, in_parentheses_only_if_group_breaks, + in_parentheses_only_soft_line_break, in_parentheses_only_soft_line_break_or_space, + is_expression_parenthesized, write_in_parentheses_only_group_end_tag, + write_in_parentheses_only_group_start_tag, Parentheses, }; use crate::expression::string::{AnyString, FormatString, StringLayout}; use crate::expression::OperatorPrecedence; @@ -718,7 +718,11 @@ impl Format> for FlatBinaryExpressionSlice<'_> { ) { hard_line_break().fmt(f)?; - } else if !is_pow { + } else if is_pow { + if f.context().options().preview().is_enabled() { + in_parentheses_only_if_group_breaks(&space()).fmt(f)?; + } + } else { space().fmt(f)?; } diff --git a/crates/ruff_python_formatter/src/expression/parentheses.rs b/crates/ruff_python_formatter/src/expression/parentheses.rs index 971d913146609..d6a40f213b6ad 100644 --- a/crates/ruff_python_formatter/src/expression/parentheses.rs +++ b/crates/ruff_python_formatter/src/expression/parentheses.rs @@ -355,6 +355,26 @@ pub(super) fn write_in_parentheses_only_group_end_tag(f: &mut PyFormatter) { } } +/// Shows prints `content` only if the expression is enclosed by (optional) parentheses (`()`, `[]`, or `{}`) +/// and splits across multiple lines. +pub(super) fn in_parentheses_only_if_group_breaks<'a, T>( + content: T, +) -> impl Format> +where + T: Format>, +{ + format_with(move |f: &mut PyFormatter| match f.context().node_level() { + NodeLevel::TopLevel(_) | NodeLevel::CompoundStatement | NodeLevel::Expression(None) => { + // no-op, not parenthesized + Ok(()) + } + NodeLevel::Expression(Some(parentheses_id)) => if_group_breaks(&content) + .with_group_id(Some(parentheses_id)) + .fmt(f), + NodeLevel::ParenthesizedExpression => if_group_breaks(&content).fmt(f), + }) +} + /// Format comments inside empty parentheses, brackets or curly braces. /// /// Empty `()`, `[]` and `{}` are special because there can be dangling comments, and they can be in diff --git a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_power_op_spacing.py.snap b/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_power_op_spacing.py.snap deleted file mode 100644 index f7cb0914c0304..0000000000000 --- a/crates/ruff_python_formatter/tests/snapshots/black_compatibility@cases__preview_power_op_spacing.py.snap +++ /dev/null @@ -1,354 +0,0 @@ ---- -source: crates/ruff_python_formatter/tests/fixtures.rs -input_file: crates/ruff_python_formatter/resources/test/fixtures/black/cases/preview_power_op_spacing.py ---- -## Input - -```python -a = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -b = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -c = 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 ** 1 -d = 1**1 ** 1**1 ** 1**1 ** 1**1 ** 1**1**1 ** 1 ** 1**1 ** 1**1**1**1**1 ** 1 ** 1**1**1 **1**1** 1 ** 1 ** 1 -e = 𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟 -f = 𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟 - -a = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -b = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -c = 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 ** 1.0 -d = 1.0**1.0 ** 1.0**1.0 ** 1.0**1.0 ** 1.0**1.0 ** 1.0**1.0**1.0 ** 1.0 ** 1.0**1.0 ** 1.0**1.0**1.0 -``` - -## Black Differences - -```diff ---- Black -+++ Ruff -@@ -1,83 +1,83 @@ - a = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 - b = ( - 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -- ** 1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 -+ **1 - ) - c = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 - d = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 - e = 𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟 - f = ( - 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -- ** 𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 -+ **𨉟 - ) - - a = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 - b = ( - 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -- ** 1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 -+ **1.0 - ) - c = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 - d = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -``` - -## Ruff Output - -```python -a = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -b = ( - 1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 - **1 -) -c = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -d = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -e = 𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟 -f = ( - 𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 - **𨉟 -) - -a = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -b = ( - 1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 - **1.0 -) -c = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -d = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -``` - -## Black Output - -```python -a = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -b = ( - 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 - ** 1 -) -c = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -d = 1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1**1 -e = 𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟**𨉟 -f = ( - 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 - ** 𨉟 -) - -a = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -b = ( - 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 - ** 1.0 -) -c = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -d = 1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0**1.0 -``` - - From 35082b28cdd7c7e9a9fa587149df657ca740a0be Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Sat, 2 Dec 2023 13:52:50 +0000 Subject: [PATCH 11/18] Fix error in `t-suffixed-type-alias` (`PYI043`) example (#8963) ## Summary For `t-suffixed-type-alias` to trigger, the type alias needs to be marked as such using the `typing.TypeAlias` annotation and the name of the alias must be marked as private using a leading underscore. The documentation example was of an unannotated type alias that was not marked as private, which was misleading. ## Test Plan The current example doesn't trigger the rule; the example in this merge request does. --- .../src/rules/flake8_pyi/rules/type_alias_naming.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/type_alias_naming.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/type_alias_naming.rs index e240bd3bd8191..4e60317211c2c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/type_alias_naming.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/type_alias_naming.rs @@ -46,12 +46,16 @@ impl Violation for SnakeCaseTypeAlias { /// /// ## Example /// ```python -/// MyTypeT = int +/// from typing import TypeAlias +/// +/// _MyTypeT: TypeAlias = int /// ``` /// /// Use instead: /// ```python -/// MyType = int +/// from typing import TypeAlias +/// +/// _MyType: TypeAlias = int /// ``` #[violation] pub struct TSuffixedTypeAlias { From 22d8a989d48caf3e4bd558400cc3bc19f472001d Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 2 Dec 2023 08:56:57 -0500 Subject: [PATCH 12/18] Avoid underflow in `get_model` matching (#8965) Closes https://github.com/astral-sh/ruff/issues/8962. --- .../resources/test/fixtures/pep8_naming/N806.py | 3 +++ .../ruff_linter/src/rules/pep8_naming/helpers.rs | 6 +++++- ..._rules__pep8_naming__tests__N806_N806.py.snap | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N806.py b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N806.py index bbaf2b785fc90..d3a0585cb46a1 100644 --- a/crates/ruff_linter/resources/test/fixtures/pep8_naming/N806.py +++ b/crates/ruff_linter/resources/test/fixtures/pep8_naming/N806.py @@ -52,3 +52,6 @@ def model_assign() -> None: Bad = import_string("django.core.exceptions.ValidationError") # N806 ValidationError = import_string("django.core.exceptions.ValidationError") # OK + + Bad = apps.get_model() # N806 + Bad = apps.get_model(model_name="Stream") # N806 diff --git a/crates/ruff_linter/src/rules/pep8_naming/helpers.rs b/crates/ruff_linter/src/rules/pep8_naming/helpers.rs index fc8f6568af827..00ad10b77f4da 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/helpers.rs +++ b/crates/ruff_linter/src/rules/pep8_naming/helpers.rs @@ -101,11 +101,15 @@ pub(super) fn is_django_model_import(name: &str, stmt: &Stmt, semantic: &Semanti return false; }; + if arguments.is_empty() { + return false; + } + // Match against, e.g., `apps.get_model("zerver", "Attachment")`. if let Some(call_path) = collect_call_path(func.as_ref()) { if matches!(call_path.as_slice(), [.., "get_model"]) { if let Some(argument) = - arguments.find_argument("model_name", arguments.args.len() - 1) + arguments.find_argument("model_name", arguments.args.len().saturating_sub(1)) { if let Some(string_literal) = argument.as_string_literal_expr() { return string_literal.value.to_str() == name; diff --git a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N806_N806.py.snap b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N806_N806.py.snap index e572eecccb0ad..d53fb46f9df72 100644 --- a/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N806_N806.py.snap +++ b/crates/ruff_linter/src/rules/pep8_naming/snapshots/ruff_linter__rules__pep8_naming__tests__N806_N806.py.snap @@ -38,4 +38,20 @@ N806.py:53:5: N806 Variable `Bad` in function should be lowercase 54 | ValidationError = import_string("django.core.exceptions.ValidationError") # OK | +N806.py:56:5: N806 Variable `Bad` in function should be lowercase + | +54 | ValidationError = import_string("django.core.exceptions.ValidationError") # OK +55 | +56 | Bad = apps.get_model() # N806 + | ^^^ N806 +57 | Bad = apps.get_model(model_name="Stream") # N806 + | + +N806.py:57:5: N806 Variable `Bad` in function should be lowercase + | +56 | Bad = apps.get_model() # N806 +57 | Bad = apps.get_model(model_name="Stream") # N806 + | ^^^ N806 + | + From 20ab14e3548ffcc2da2dd35562a1365145fb9f87 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 2 Dec 2023 13:17:17 -0500 Subject: [PATCH 13/18] Avoid unnecessary index diagnostics when value is modified (#8970) Closes https://github.com/astral-sh/ruff/issues/8969. --- .../pylint/unnecessary_dict_index_lookup.py | 27 ++-- .../pylint/unnecessary_list_index_lookup.py | 49 ++++---- .../ruff_linter/src/rules/pylint/helpers.rs | 117 +++++++++++++++++- .../rules/unnecessary_dict_index_lookup.rs | 106 +--------------- .../rules/unnecessary_list_index_lookup.rs | 115 +---------------- ...1736_unnecessary_list_index_lookup.py.snap | 12 +- 6 files changed, 180 insertions(+), 246 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dict_index_lookup.py b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dict_index_lookup.py index cfdd9fc42ae04..d3daeb83c97b8 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dict_index_lookup.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_dict_index_lookup.py @@ -14,16 +14,27 @@ def fix_these(): def dont_fix_these(): # once there is an assignment to the dict[index], we stop emitting diagnostics for fruit_name, fruit_count in FRUITS.items(): - FRUITS[fruit_name] = 0 # Ok - assert FRUITS[fruit_name] == 0 # Ok + FRUITS[fruit_name] = 0 # OK + assert FRUITS[fruit_name] == 0 # OK + + # once there is an assignment to the key, we stop emitting diagnostics + for fruit_name, fruit_count in FRUITS.items(): + fruit_name = 0 # OK + assert FRUITS[fruit_name] == 0 # OK + + # once there is an assignment to the value, we stop emitting diagnostics + for fruit_name, fruit_count in FRUITS.items(): + if fruit_count < 5: + fruit_count = -fruit_count + assert FRUITS[fruit_name] == 0 # OK def value_intentionally_unused(): - [FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()] # Ok - {FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()} # Ok - {fruit_name: FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()} # Ok + [FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()] # OK + {FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()} # OK + {fruit_name: FRUITS[fruit_name] for fruit_name, _ in FRUITS.items()} # OK for fruit_name, _ in FRUITS.items(): - print(FRUITS[fruit_name]) # Ok - blah = FRUITS[fruit_name] # Ok - assert FRUITS[fruit_name] == "pear" # Ok + print(FRUITS[fruit_name]) # OK + blah = FRUITS[fruit_name] # OK + assert FRUITS[fruit_name] == "pear" # OK diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_list_index_lookup.py b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_list_index_lookup.py index 182e63cb7b913..8911c8bd26c96 100644 --- a/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_list_index_lookup.py +++ b/crates/ruff_linter/resources/test/fixtures/pylint/unnecessary_list_index_lookup.py @@ -12,7 +12,7 @@ def fix_these(): print(letters[index]) # PLR1736 blah = letters[index] # PLR1736 assert letters[index] == "d" # PLR1736 - + for index, letter in builtins.enumerate(letters): print(letters[index]) # PLR1736 blah = letters[index] # PLR1736 @@ -22,38 +22,43 @@ def fix_these(): def dont_fix_these(): # once there is an assignment to the sequence[index], we stop emitting diagnostics for index, letter in enumerate(letters): - letters[index] = "d" # Ok - letters[index] += "e" # Ok - assert letters[index] == "de" # Ok - + letters[index] = "d" # OK + letters[index] += "e" # OK + assert letters[index] == "de" # OK + # once there is an assignment to the index, we stop emitting diagnostics for index, letter in enumerate(letters): - index += 1 # Ok - print(letters[index]) # Ok - + index += 1 # OK + print(letters[index]) # OK + # once there is an assignment to the sequence, we stop emitting diagnostics for index, letter in enumerate(letters): - letters = ["d", "e", "f"] # Ok - print(letters[index]) # Ok + letters = ["d", "e", "f"] # OK + print(letters[index]) # OK + + # once there is an assignment to the value, we stop emitting diagnostics + for index, letter in enumerate(letters): + letter = "d" + print(letters[index]) # OK # once there is an deletion from or of the sequence or index, we stop emitting diagnostics for index, letter in enumerate(letters): - del letters[index] # Ok - print(letters[index]) # Ok + del letters[index] # OK + print(letters[index]) # OK for index, letter in enumerate(letters): - del letters # Ok - print(letters[index]) # Ok + del letters # OK + print(letters[index]) # OK for index, letter in enumerate(letters): - del index # Ok - print(letters[index]) # Ok + del index # OK + print(letters[index]) # OK def value_intentionally_unused(): - [letters[index] for index, _ in enumerate(letters)] # Ok - {letters[index] for index, _ in enumerate(letters)} # Ok - {index: letters[index] for index, _ in enumerate(letters)} # Ok + [letters[index] for index, _ in enumerate(letters)] # OK + {letters[index] for index, _ in enumerate(letters)} # OK + {index: letters[index] for index, _ in enumerate(letters)} # OK for index, _ in enumerate(letters): - print(letters[index]) # Ok - blah = letters[index] # Ok - letters[index] = "d" # Ok + print(letters[index]) # OK + blah = letters[index] # OK + letters[index] = "d" # OK diff --git a/crates/ruff_linter/src/rules/pylint/helpers.rs b/crates/ruff_linter/src/rules/pylint/helpers.rs index 027272d03c004..d68e639b59f56 100644 --- a/crates/ruff_linter/src/rules/pylint/helpers.rs +++ b/crates/ruff_linter/src/rules/pylint/helpers.rs @@ -1,9 +1,11 @@ use std::fmt; use ruff_python_ast as ast; -use ruff_python_ast::{Arguments, CmpOp, Expr}; +use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::{visitor, Arguments, CmpOp, Expr, Stmt}; use ruff_python_semantic::analyze::function_type; use ruff_python_semantic::{ScopeKind, SemanticModel}; +use ruff_text_size::TextRange; use crate::settings::LinterSettings; @@ -82,3 +84,116 @@ impl fmt::Display for CmpOpExt { write!(f, "{representation}") } } + +/// Visitor to track reads from an iterable in a loop. +#[derive(Debug)] +pub(crate) struct SequenceIndexVisitor<'a> { + /// `letters`, given `for index, letter in enumerate(letters)`. + sequence_name: &'a str, + /// `index`, given `for index, letter in enumerate(letters)`. + index_name: &'a str, + /// `letter`, given `for index, letter in enumerate(letters)`. + value_name: &'a str, + /// The ranges of any `letters[index]` accesses. + accesses: Vec, + /// Whether any of the variables have been modified. + modified: bool, +} + +impl<'a> SequenceIndexVisitor<'a> { + pub(crate) fn new(sequence_name: &'a str, index_name: &'a str, value_name: &'a str) -> Self { + Self { + sequence_name, + index_name, + value_name, + accesses: Vec::new(), + modified: false, + } + } + + pub(crate) fn into_accesses(self) -> Vec { + self.accesses + } +} + +impl SequenceIndexVisitor<'_> { + fn is_assignment(&self, expr: &Expr) -> bool { + // If we see the sequence, a subscript, or the index being modified, we'll stop emitting + // diagnostics. + match expr { + Expr::Name(ast::ExprName { id, .. }) => { + id == self.sequence_name || id == self.index_name || id == self.value_name + } + Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { + let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { + return false; + }; + if id == self.sequence_name { + let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { + return false; + }; + if id == self.index_name { + return true; + } + } + false + } + _ => false, + } + } +} + +impl<'a> Visitor<'_> for SequenceIndexVisitor<'a> { + fn visit_stmt(&mut self, stmt: &Stmt) { + if self.modified { + return; + } + match stmt { + Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { + self.modified = targets.iter().any(|target| self.is_assignment(target)); + self.visit_expr(value); + } + Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => { + if let Some(value) = value { + self.modified = self.is_assignment(target); + self.visit_expr(value); + } + } + Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { + self.modified = self.is_assignment(target); + self.visit_expr(value); + } + Stmt::Delete(ast::StmtDelete { targets, .. }) => { + self.modified = targets.iter().any(|target| self.is_assignment(target)); + } + _ => visitor::walk_stmt(self, stmt), + } + } + + fn visit_expr(&mut self, expr: &Expr) { + if self.modified { + return; + } + match expr { + Expr::Subscript(ast::ExprSubscript { + value, + slice, + range, + .. + }) => { + let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { + return; + }; + if id == self.sequence_name { + let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { + return; + }; + if id == self.index_name { + self.accesses.push(*range); + } + } + } + _ => visitor::walk_expr(self, expr), + } + } +} diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs index 16dc733d61d80..88a842b7d6b4b 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_dict_index_lookup.rs @@ -1,13 +1,10 @@ -use ast::Stmt; -use ruff_python_ast::{self as ast, Expr, StmtFor}; - use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; -use ruff_text_size::TextRange; +use ruff_python_ast::{self as ast, Expr, StmtFor}; use crate::checkers::ast::Checker; +use crate::rules::pylint::helpers::SequenceIndexVisitor; /// ## What it does /// Checks for key-based dict accesses during `.items()` iterations. @@ -54,10 +51,10 @@ pub(crate) fn unnecessary_dict_index_lookup(checker: &mut Checker, stmt_for: &St }; let ranges = { - let mut visitor = SubscriptVisitor::new(dict_name, index_name); + let mut visitor = SequenceIndexVisitor::new(dict_name, index_name, value_name); visitor.visit_body(&stmt_for.body); visitor.visit_body(&stmt_for.orelse); - visitor.diagnostic_ranges + visitor.into_accesses() }; for range in ranges { @@ -96,12 +93,12 @@ pub(crate) fn unnecessary_dict_index_lookup_comprehension(checker: &mut Checker, }; let ranges = { - let mut visitor = SubscriptVisitor::new(dict_name, index_name); + let mut visitor = SequenceIndexVisitor::new(dict_name, index_name, value_name); visitor.visit_expr(elt.as_ref()); for expr in &comp.ifs { visitor.visit_expr(expr); } - visitor.diagnostic_ranges + visitor.into_accesses() }; for range in ranges { @@ -161,94 +158,3 @@ fn dict_items<'a>( Some((dict_name, index_name, value_name)) } - -#[derive(Debug)] -struct SubscriptVisitor<'a> { - dict_name: &'a str, - index_name: &'a str, - diagnostic_ranges: Vec, - modified: bool, -} - -impl<'a> SubscriptVisitor<'a> { - fn new(dict_name: &'a str, index_name: &'a str) -> Self { - Self { - dict_name, - index_name, - diagnostic_ranges: Vec::new(), - modified: false, - } - } -} - -impl SubscriptVisitor<'_> { - fn is_assignment(&self, expr: &Expr) -> bool { - let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr else { - return false; - }; - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return false; - }; - if id == self.dict_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return false; - }; - if id == self.index_name { - return true; - } - } - false - } -} - -impl<'a> Visitor<'_> for SubscriptVisitor<'a> { - fn visit_stmt(&mut self, stmt: &Stmt) { - if self.modified { - return; - } - match stmt { - Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { - self.modified = targets.iter().any(|target| self.is_assignment(target)); - self.visit_expr(value); - } - Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => { - if let Some(value) = value { - self.modified = self.is_assignment(target); - self.visit_expr(value); - } - } - Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { - self.modified = self.is_assignment(target); - self.visit_expr(value); - } - _ => visitor::walk_stmt(self, stmt), - } - } - - fn visit_expr(&mut self, expr: &Expr) { - if self.modified { - return; - } - match expr { - Expr::Subscript(ast::ExprSubscript { - value, - slice, - range, - .. - }) => { - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return; - }; - if id == self.dict_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return; - }; - if id == self.index_name { - self.diagnostic_ranges.push(*range); - } - } - } - _ => visitor::walk_expr(self, expr), - } - } -} diff --git a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs index 17a42b29e25a2..920d2442e9b5a 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/unnecessary_list_index_lookup.rs @@ -1,13 +1,11 @@ -use ruff_python_ast::{self as ast, Expr, Stmt, StmtFor}; - use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::visitor; use ruff_python_ast::visitor::Visitor; +use ruff_python_ast::{self as ast, Expr, StmtFor}; use ruff_python_semantic::SemanticModel; -use ruff_text_size::TextRange; use crate::checkers::ast::Checker; +use crate::rules::pylint::helpers::SequenceIndexVisitor; /// ## What it does /// Checks for index-based list accesses during `enumerate` iterations. @@ -55,10 +53,10 @@ pub(crate) fn unnecessary_list_index_lookup(checker: &mut Checker, stmt_for: &St }; let ranges = { - let mut visitor = SubscriptVisitor::new(sequence, index_name); + let mut visitor = SequenceIndexVisitor::new(sequence, index_name, value_name); visitor.visit_body(&stmt_for.body); visitor.visit_body(&stmt_for.orelse); - visitor.diagnostic_ranges + visitor.into_accesses() }; for range in ranges { @@ -99,9 +97,9 @@ pub(crate) fn unnecessary_list_index_lookup_comprehension(checker: &mut Checker, }; let ranges = { - let mut visitor = SubscriptVisitor::new(sequence, index_name); + let mut visitor = SequenceIndexVisitor::new(sequence, index_name, value_name); visitor.visit_expr(elt.as_ref()); - visitor.diagnostic_ranges + visitor.into_accesses() }; for range in ranges { @@ -161,104 +159,3 @@ fn enumerate_items<'a>( Some((sequence, index_name, value_name)) } - -#[derive(Debug)] -struct SubscriptVisitor<'a> { - sequence_name: &'a str, - index_name: &'a str, - diagnostic_ranges: Vec, - modified: bool, -} - -impl<'a> SubscriptVisitor<'a> { - fn new(sequence_name: &'a str, index_name: &'a str) -> Self { - Self { - sequence_name, - index_name, - diagnostic_ranges: Vec::new(), - modified: false, - } - } -} - -impl SubscriptVisitor<'_> { - fn is_assignment(&self, expr: &Expr) -> bool { - // If we see the sequence, a subscript, or the index being modified, we'll stop emitting - // diagnostics. - match expr { - Expr::Name(ast::ExprName { id, .. }) => { - id == self.sequence_name || id == self.index_name - } - Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return false; - }; - if id == self.sequence_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return false; - }; - if id == self.index_name { - return true; - } - } - false - } - _ => false, - } - } -} - -impl<'a> Visitor<'_> for SubscriptVisitor<'a> { - fn visit_stmt(&mut self, stmt: &Stmt) { - if self.modified { - return; - } - match stmt { - Stmt::Assign(ast::StmtAssign { targets, value, .. }) => { - self.modified = targets.iter().any(|target| self.is_assignment(target)); - self.visit_expr(value); - } - Stmt::AnnAssign(ast::StmtAnnAssign { target, value, .. }) => { - if let Some(value) = value { - self.modified = self.is_assignment(target); - self.visit_expr(value); - } - } - Stmt::AugAssign(ast::StmtAugAssign { target, value, .. }) => { - self.modified = self.is_assignment(target); - self.visit_expr(value); - } - Stmt::Delete(ast::StmtDelete { targets, .. }) => { - self.modified = targets.iter().any(|target| self.is_assignment(target)); - } - _ => visitor::walk_stmt(self, stmt), - } - } - - fn visit_expr(&mut self, expr: &Expr) { - if self.modified { - return; - } - match expr { - Expr::Subscript(ast::ExprSubscript { - value, - slice, - range, - .. - }) => { - let Expr::Name(ast::ExprName { id, .. }) = value.as_ref() else { - return; - }; - if id == self.sequence_name { - let Expr::Name(ast::ExprName { id, .. }) = slice.as_ref() else { - return; - }; - if id == self.index_name { - self.diagnostic_ranges.push(*range); - } - } - } - _ => visitor::walk_expr(self, expr), - } - } -} diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap index 8e4d22472df3d..880422eab6674 100644 --- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap +++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLR1736_unnecessary_list_index_lookup.py.snap @@ -80,7 +80,7 @@ unnecessary_list_index_lookup.py:12:15: PLR1736 [*] Unnecessary lookup of list i 12 |+ print(letter) # PLR1736 13 13 | blah = letters[index] # PLR1736 14 14 | assert letters[index] == "d" # PLR1736 -15 15 | +15 15 | unnecessary_list_index_lookup.py:13:16: PLR1736 [*] Unnecessary lookup of list item by index | @@ -99,7 +99,7 @@ unnecessary_list_index_lookup.py:13:16: PLR1736 [*] Unnecessary lookup of list i 13 |- blah = letters[index] # PLR1736 13 |+ blah = letter # PLR1736 14 14 | assert letters[index] == "d" # PLR1736 -15 15 | +15 15 | 16 16 | for index, letter in builtins.enumerate(letters): unnecessary_list_index_lookup.py:14:16: PLR1736 [*] Unnecessary lookup of list item by index @@ -108,7 +108,7 @@ unnecessary_list_index_lookup.py:14:16: PLR1736 [*] Unnecessary lookup of list i 13 | blah = letters[index] # PLR1736 14 | assert letters[index] == "d" # PLR1736 | ^^^^^^^^^^^^^^ PLR1736 -15 | +15 | 16 | for index, letter in builtins.enumerate(letters): | = help: Use existing variable @@ -119,7 +119,7 @@ unnecessary_list_index_lookup.py:14:16: PLR1736 [*] Unnecessary lookup of list i 13 13 | blah = letters[index] # PLR1736 14 |- assert letters[index] == "d" # PLR1736 14 |+ assert letter == "d" # PLR1736 -15 15 | +15 15 | 16 16 | for index, letter in builtins.enumerate(letters): 17 17 | print(letters[index]) # PLR1736 @@ -135,7 +135,7 @@ unnecessary_list_index_lookup.py:17:15: PLR1736 [*] Unnecessary lookup of list i ℹ Safe fix 14 14 | assert letters[index] == "d" # PLR1736 -15 15 | +15 15 | 16 16 | for index, letter in builtins.enumerate(letters): 17 |- print(letters[index]) # PLR1736 17 |+ print(letter) # PLR1736 @@ -154,7 +154,7 @@ unnecessary_list_index_lookup.py:18:16: PLR1736 [*] Unnecessary lookup of list i = help: Use existing variable ℹ Safe fix -15 15 | +15 15 | 16 16 | for index, letter in builtins.enumerate(letters): 17 17 | print(letters[index]) # PLR1736 18 |- blah = letters[index] # PLR1736 From 3fbabfe12690c27c8a0d16230ed925afa7ed6c74 Mon Sep 17 00:00:00 2001 From: Tom Kuson Date: Sat, 2 Dec 2023 18:26:43 +0000 Subject: [PATCH 14/18] [`flake8-pyi`] Check PEP 695 type aliases for `snake-case-type-alias` and `t-suffixed-type-alias` (#8966) ## Summary Check PEP 695 type alias definitions for `snake-case-type-alias` (`PYI042`) and `t-suffixed-type-alias` (`PYI043`) Related to #8771. ## Test Plan `cargo test` --- .../resources/test/fixtures/flake8_pyi/PYI042.py | 4 ++++ .../resources/test/fixtures/flake8_pyi/PYI042.pyi | 4 ++++ .../resources/test/fixtures/flake8_pyi/PYI043.py | 4 ++++ .../resources/test/fixtures/flake8_pyi/PYI043.pyi | 4 ++++ crates/ruff_linter/src/checkers/ast/analyze/statement.rs | 8 ++++++++ ...inter__rules__flake8_pyi__tests__PYI042_PYI042.py.snap | 8 ++++++++ ...nter__rules__flake8_pyi__tests__PYI042_PYI042.pyi.snap | 8 ++++++++ ...inter__rules__flake8_pyi__tests__PYI043_PYI043.py.snap | 8 ++++++++ ...nter__rules__flake8_pyi__tests__PYI043_PYI043.pyi.snap | 8 ++++++++ 9 files changed, 56 insertions(+) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.py index 2936c39c6c836..2d2b932e42c8a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.py @@ -22,3 +22,7 @@ # check that this edge case doesn't crash _: TypeAlias = str | int + +# PEP 695 +type foo_bar = int | str +type FooBar = int | str diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.pyi index 2936c39c6c836..2d2b932e42c8a 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI042.pyi @@ -22,3 +22,7 @@ Snake_case_alias: TypeAlias = int | float # PYI042, since not camel case # check that this edge case doesn't crash _: TypeAlias = str | int + +# PEP 695 +type foo_bar = int | str +type FooBar = int | str diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.py b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.py index b48f5e0fa8c47..b8107662f9931 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.py @@ -21,3 +21,7 @@ # check that this edge case doesn't crash _: TypeAlias = str | int + +# PEP 695 +type _FooT = str | int +type Foo = str | int diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.pyi b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.pyi index b48f5e0fa8c47..b8107662f9931 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.pyi +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI043.pyi @@ -21,3 +21,7 @@ _PrivateAliasS2: TypeAlias = Annotated[str, "also okay"] # check that this edge case doesn't crash _: TypeAlias = str | int + +# PEP 695 +type _FooT = str | int +type Foo = str | int diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 99ea30595f0a6..4b79a9ec7da14 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1534,6 +1534,14 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } } + Stmt::TypeAlias(ast::StmtTypeAlias { name, .. }) => { + if checker.enabled(Rule::SnakeCaseTypeAlias) { + flake8_pyi::rules::snake_case_type_alias(checker, name); + } + if checker.enabled(Rule::TSuffixedTypeAlias) { + flake8_pyi::rules::t_suffixed_type_alias(checker, name); + } + } Stmt::Delete(delete @ ast::StmtDelete { targets, range: _ }) => { if checker.enabled(Rule::GlobalStatement) { for target in targets { diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.py.snap index 1f80b9c9af969..539eb12d99a7f 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.py.snap @@ -29,4 +29,12 @@ PYI042.py:20:1: PYI042 Type alias `_snake_case_alias2` should be CamelCase 21 | Snake_case_alias: TypeAlias = int | float # PYI042, since not camel case | +PYI042.py:27:6: PYI042 Type alias `foo_bar` should be CamelCase + | +26 | # PEP 695 +27 | type foo_bar = int | str + | ^^^^^^^ PYI042 +28 | type FooBar = int | str + | + diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.pyi.snap index ab3c2fe98eeed..9d1d034c0018e 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI042_PYI042.pyi.snap @@ -29,4 +29,12 @@ PYI042.pyi:20:1: PYI042 Type alias `_snake_case_alias2` should be CamelCase 21 | Snake_case_alias: TypeAlias = int | float # PYI042, since not camel case | +PYI042.pyi:27:6: PYI042 Type alias `foo_bar` should be CamelCase + | +26 | # PEP 695 +27 | type foo_bar = int | str + | ^^^^^^^ PYI042 +28 | type FooBar = int | str + | + diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.py.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.py.snap index 3dfc9c819b5e8..550ad122d17de 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.py.snap @@ -30,4 +30,12 @@ PYI043.py:12:1: PYI043 Private type alias `_PrivateAliasT3` should not be suffix 14 | ] # PYI043, since this ends in a T | +PYI043.py:26:6: PYI043 Private type alias `_FooT` should not be suffixed with `T` (the `T` suffix implies that an object is a `TypeVar`) + | +25 | # PEP 695 +26 | type _FooT = str | int + | ^^^^^ PYI043 +27 | type Foo = str | int + | + diff --git a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.pyi.snap b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.pyi.snap index f856dc86ad88a..80fb8b4c19182 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.pyi.snap +++ b/crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI043_PYI043.pyi.snap @@ -30,4 +30,12 @@ PYI043.pyi:12:1: PYI043 Private type alias `_PrivateAliasT3` should not be suffi 14 | ] # PYI043, since this ends in a T | +PYI043.pyi:26:6: PYI043 Private type alias `_FooT` should not be suffixed with `T` (the `T` suffix implies that an object is a `TypeVar`) + | +25 | # PEP 695 +26 | type _FooT = str | int + | ^^^^^ PYI043 +27 | type Foo = str | int + | + From 1dda669f9a680ed55ff91d8c36c20560b6cab054 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 2 Dec 2023 13:37:49 -0500 Subject: [PATCH 15/18] Avoid syntax error via invalid ur string prefix (#8971) ## Summary If a string has a Unicode prefix, we can't add the `r` prefix on top of that -- we need to remove and replace it. (The Unicode prefix is redundant anyway in Python 3.) Closes https://github.com/astral-sh/ruff/issues/8967. --- .../test/fixtures/pycodestyle/W605_0.py | 3 + .../test/fixtures/pycodestyle/W605_1.py | 42 ++- .../test/fixtures/pycodestyle/W605_2.py | 54 ---- .../ruff_linter/src/rules/pycodestyle/mod.rs | 1 - .../rules/invalid_escape_sequence.rs | 84 ++++-- ...s__pycodestyle__tests__W605_W605_0.py.snap | 20 ++ ...s__pycodestyle__tests__W605_W605_1.py.snap | 273 +++++++++++++----- ...s__pycodestyle__tests__W605_W605_2.py.snap | 227 --------------- 8 files changed, 303 insertions(+), 401 deletions(-) delete mode 100644 crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_2.py delete mode 100644 crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_2.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_0.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_0.py index 967ed5bc4d334..85ec535e22d19 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_0.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_0.py @@ -43,3 +43,6 @@ def f(): ''' # noqa regex = '\\\_' + +#: W605:1:7 +u'foo\ bar' diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_1.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_1.py index 20bf0ea14c8f7..b34ad587c46d5 100644 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_1.py +++ b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_1.py @@ -1,40 +1,54 @@ +# Same as `W605_0.py` but using f-strings instead. + #: W605:1:10 -regex = '\.png$' +regex = f'\.png$' #: W605:2:1 -regex = ''' +regex = f''' \.png$ ''' #: W605:2:6 f( - '\_' + f'\_' ) #: W605:4:6 -""" +f""" multi-line literal with \_ somewhere in the middle """ +#: W605:1:38 +value = f'new line\nand invalid escape \_ here' -def f(): - #: W605:1:11 - return'\.png$' #: Okay -regex = r'\.png$' -regex = '\\.png$' -regex = r''' +regex = fr'\.png$' +regex = f'\\.png$' +regex = fr''' \.png$ ''' -regex = r''' +regex = fr''' \\.png$ ''' -s = '\\' -regex = '\w' # noqa -regex = ''' +s = f'\\' +regex = f'\w' # noqa +regex = f''' \w ''' # noqa + +regex = f'\\\_' +value = f'\{{1}}' +value = f'\{1}' +value = f'{1:\}' +value = f"{f"\{1}"}" +value = rf"{f"\{1}"}" + +# Okay +value = rf'\{{1}}' +value = rf'\{1}' +value = rf'{1:\}' +value = f"{rf"\{1}"}" diff --git a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_2.py b/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_2.py deleted file mode 100644 index b34ad587c46d5..0000000000000 --- a/crates/ruff_linter/resources/test/fixtures/pycodestyle/W605_2.py +++ /dev/null @@ -1,54 +0,0 @@ -# Same as `W605_0.py` but using f-strings instead. - -#: W605:1:10 -regex = f'\.png$' - -#: W605:2:1 -regex = f''' -\.png$ -''' - -#: W605:2:6 -f( - f'\_' -) - -#: W605:4:6 -f""" -multi-line -literal -with \_ somewhere -in the middle -""" - -#: W605:1:38 -value = f'new line\nand invalid escape \_ here' - - -#: Okay -regex = fr'\.png$' -regex = f'\\.png$' -regex = fr''' -\.png$ -''' -regex = fr''' -\\.png$ -''' -s = f'\\' -regex = f'\w' # noqa -regex = f''' -\w -''' # noqa - -regex = f'\\\_' -value = f'\{{1}}' -value = f'\{1}' -value = f'{1:\}' -value = f"{f"\{1}"}" -value = rf"{f"\{1}"}" - -# Okay -value = rf'\{{1}}' -value = rf'\{1}' -value = rf'{1:\}' -value = f"{rf"\{1}"}" diff --git a/crates/ruff_linter/src/rules/pycodestyle/mod.rs b/crates/ruff_linter/src/rules/pycodestyle/mod.rs index b2a02f5e23c55..3f6aa412bf179 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/mod.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/mod.rs @@ -31,7 +31,6 @@ mod tests { #[test_case(Rule::BlankLineWithWhitespace, Path::new("W29.py"))] #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_0.py"))] #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_1.py"))] - #[test_case(Rule::InvalidEscapeSequence, Path::new("W605_2.py"))] #[test_case(Rule::LineTooLong, Path::new("E501.py"))] #[test_case(Rule::LineTooLong, Path::new("E501_3.py"))] #[test_case(Rule::MixedSpacesAndTabs, Path::new("E101.py"))] diff --git a/crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs b/crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs index b4d100aa9d6f3..77a3d6ba621df 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs +++ b/crates/ruff_linter/src/rules/pycodestyle/rules/invalid_escape_sequence.rs @@ -3,9 +3,9 @@ use memchr::memchr_iter; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_index::Indexer; -use ruff_python_parser::Tok; +use ruff_python_parser::{StringKind, Tok}; use ruff_source_file::Locator; -use ruff_text_size::{TextLen, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; use crate::fix::edits::pad_start; @@ -58,18 +58,6 @@ impl AlwaysFixableViolation for InvalidEscapeSequence { } } -#[derive(Debug, PartialEq, Eq)] -enum FixTitle { - AddBackslash, - UseRawStringLiteral, -} - -#[derive(Debug)] -struct InvalidEscapeChar { - ch: char, - range: TextRange, -} - /// W605 pub(crate) fn invalid_escape_sequence( diagnostics: &mut Vec, @@ -195,41 +183,77 @@ pub(crate) fn invalid_escape_sequence( if contains_valid_escape_sequence { // Escape with backslash. for invalid_escape_char in &invalid_escape_chars { - let diagnostic = Diagnostic::new( + let mut diagnostic = Diagnostic::new( InvalidEscapeSequence { ch: invalid_escape_char.ch, fix_title: FixTitle::AddBackslash, }, - invalid_escape_char.range, - ) - .with_fix(Fix::safe_edit(Edit::insertion( + invalid_escape_char.range(), + ); + diagnostic.set_fix(Fix::safe_edit(Edit::insertion( r"\".to_string(), - invalid_escape_char.range.start() + TextSize::from(1), + invalid_escape_char.start() + TextSize::from(1), ))); invalid_escape_sequence.push(diagnostic); } } else { // Turn into raw string. for invalid_escape_char in &invalid_escape_chars { - let diagnostic = Diagnostic::new( + let mut diagnostic = Diagnostic::new( InvalidEscapeSequence { ch: invalid_escape_char.ch, fix_title: FixTitle::UseRawStringLiteral, }, - invalid_escape_char.range, - ) - .with_fix( - // If necessary, add a space between any leading keyword (`return`, `yield`, - // `assert`, etc.) and the string. For example, `return"foo"` is valid, but - // `returnr"foo"` is not. - Fix::safe_edit(Edit::insertion( - pad_start("r".to_string(), string_start_location, locator), - string_start_location, - )), + invalid_escape_char.range(), ); + + if matches!( + token, + Tok::String { + kind: StringKind::Unicode, + .. + } + ) { + // Replace the Unicode prefix with `r`. + diagnostic.set_fix(Fix::safe_edit(Edit::replacement( + "r".to_string(), + string_start_location, + string_start_location + TextSize::from(1), + ))); + } else { + // Insert the `r` prefix. + diagnostic.set_fix( + // If necessary, add a space between any leading keyword (`return`, `yield`, + // `assert`, etc.) and the string. For example, `return"foo"` is valid, but + // `returnr"foo"` is not. + Fix::safe_edit(Edit::insertion( + pad_start("r".to_string(), string_start_location, locator), + string_start_location, + )), + ); + } + invalid_escape_sequence.push(diagnostic); } } diagnostics.extend(invalid_escape_sequence); } + +#[derive(Debug, PartialEq, Eq)] +enum FixTitle { + AddBackslash, + UseRawStringLiteral, +} + +#[derive(Debug)] +struct InvalidEscapeChar { + ch: char, + range: TextRange, +} + +impl Ranged for InvalidEscapeChar { + fn range(&self) -> TextRange { + self.range + } +} diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_0.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_0.py.snap index 98da5fff80c25..e4630487b7500 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_0.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_0.py.snap @@ -125,6 +125,8 @@ W605_0.py:45:12: W605 [*] Invalid escape sequence: `\_` 44 | 45 | regex = '\\\_' | ^^ W605 +46 | +47 | #: W605:1:7 | = help: Add backslash to escape sequence @@ -134,5 +136,23 @@ W605_0.py:45:12: W605 [*] Invalid escape sequence: `\_` 44 44 | 45 |-regex = '\\\_' 45 |+regex = '\\\\_' +46 46 | +47 47 | #: W605:1:7 +48 48 | u'foo\ bar' + +W605_0.py:48:6: W605 [*] Invalid escape sequence: `\ ` + | +47 | #: W605:1:7 +48 | u'foo\ bar' + | ^^ W605 + | + = help: Use a raw string literal + +ℹ Safe fix +45 45 | regex = '\\\_' +46 46 | +47 47 | #: W605:1:7 +48 |-u'foo\ bar' + 48 |+r'foo\ bar' diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_1.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_1.py.snap index 2fee83a5fee08..c47507e0ccc88 100644 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_1.py.snap +++ b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_1.py.snap @@ -1,104 +1,227 @@ --- source: crates/ruff_linter/src/rules/pycodestyle/mod.rs --- -W605_1.py:2:10: W605 [*] Invalid escape sequence: `\.` +W605_1.py:4:11: W605 [*] Invalid escape sequence: `\.` | -1 | #: W605:1:10 -2 | regex = '\.png$' - | ^^ W605 -3 | -4 | #: W605:2:1 +3 | #: W605:1:10 +4 | regex = f'\.png$' + | ^^ W605 +5 | +6 | #: W605:2:1 | = help: Use a raw string literal ℹ Safe fix -1 1 | #: W605:1:10 -2 |-regex = '\.png$' - 2 |+regex = r'\.png$' -3 3 | -4 4 | #: W605:2:1 -5 5 | regex = ''' - -W605_1.py:6:1: W605 [*] Invalid escape sequence: `\.` +1 1 | # Same as `W605_0.py` but using f-strings instead. +2 2 | +3 3 | #: W605:1:10 +4 |-regex = f'\.png$' + 4 |+regex = rf'\.png$' +5 5 | +6 6 | #: W605:2:1 +7 7 | regex = f''' + +W605_1.py:8:1: W605 [*] Invalid escape sequence: `\.` | -4 | #: W605:2:1 -5 | regex = ''' -6 | \.png$ +6 | #: W605:2:1 +7 | regex = f''' +8 | \.png$ | ^^ W605 -7 | ''' +9 | ''' | = help: Use a raw string literal ℹ Safe fix -2 2 | regex = '\.png$' -3 3 | -4 4 | #: W605:2:1 -5 |-regex = ''' - 5 |+regex = r''' -6 6 | \.png$ -7 7 | ''' -8 8 | - -W605_1.py:11:6: W605 [*] Invalid escape sequence: `\_` - | - 9 | #: W605:2:6 -10 | f( -11 | '\_' - | ^^ W605 -12 | ) +4 4 | regex = f'\.png$' +5 5 | +6 6 | #: W605:2:1 +7 |-regex = f''' + 7 |+regex = rf''' +8 8 | \.png$ +9 9 | ''' +10 10 | + +W605_1.py:13:7: W605 [*] Invalid escape sequence: `\_` + | +11 | #: W605:2:6 +12 | f( +13 | f'\_' + | ^^ W605 +14 | ) | = help: Use a raw string literal ℹ Safe fix -8 8 | -9 9 | #: W605:2:6 -10 10 | f( -11 |- '\_' - 11 |+ r'\_' -12 12 | ) -13 13 | -14 14 | #: W605:4:6 - -W605_1.py:18:6: W605 [*] Invalid escape sequence: `\_` - | -16 | multi-line -17 | literal -18 | with \_ somewhere +10 10 | +11 11 | #: W605:2:6 +12 12 | f( +13 |- f'\_' + 13 |+ rf'\_' +14 14 | ) +15 15 | +16 16 | #: W605:4:6 + +W605_1.py:20:6: W605 [*] Invalid escape sequence: `\_` + | +18 | multi-line +19 | literal +20 | with \_ somewhere | ^^ W605 -19 | in the middle -20 | """ +21 | in the middle +22 | """ | = help: Use a raw string literal ℹ Safe fix -12 12 | ) -13 13 | -14 14 | #: W605:4:6 -15 |-""" - 15 |+r""" -16 16 | multi-line -17 17 | literal -18 18 | with \_ somewhere - -W605_1.py:25:12: W605 [*] Invalid escape sequence: `\.` - | -23 | def f(): -24 | #: W605:1:11 -25 | return'\.png$' - | ^^ W605 -26 | -27 | #: Okay +14 14 | ) +15 15 | +16 16 | #: W605:4:6 +17 |-f""" + 17 |+rf""" +18 18 | multi-line +19 19 | literal +20 20 | with \_ somewhere + +W605_1.py:25:40: W605 [*] Invalid escape sequence: `\_` | - = help: Use a raw string literal +24 | #: W605:1:38 +25 | value = f'new line\nand invalid escape \_ here' + | ^^ W605 + | + = help: Add backslash to escape sequence ℹ Safe fix -22 22 | -23 23 | def f(): -24 24 | #: W605:1:11 -25 |- return'\.png$' - 25 |+ return r'\.png$' +22 22 | """ +23 23 | +24 24 | #: W605:1:38 +25 |-value = f'new line\nand invalid escape \_ here' + 25 |+value = f'new line\nand invalid escape \\_ here' 26 26 | -27 27 | #: Okay -28 28 | regex = r'\.png$' +27 27 | +28 28 | #: Okay + +W605_1.py:43:13: W605 [*] Invalid escape sequence: `\_` + | +41 | ''' # noqa +42 | +43 | regex = f'\\\_' + | ^^ W605 +44 | value = f'\{{1}}' +45 | value = f'\{1}' + | + = help: Add backslash to escape sequence + +ℹ Safe fix +40 40 | \w +41 41 | ''' # noqa +42 42 | +43 |-regex = f'\\\_' + 43 |+regex = f'\\\\_' +44 44 | value = f'\{{1}}' +45 45 | value = f'\{1}' +46 46 | value = f'{1:\}' + +W605_1.py:44:11: W605 [*] Invalid escape sequence: `\{` + | +43 | regex = f'\\\_' +44 | value = f'\{{1}}' + | ^^ W605 +45 | value = f'\{1}' +46 | value = f'{1:\}' + | + = help: Use a raw string literal + +ℹ Safe fix +41 41 | ''' # noqa +42 42 | +43 43 | regex = f'\\\_' +44 |-value = f'\{{1}}' + 44 |+value = rf'\{{1}}' +45 45 | value = f'\{1}' +46 46 | value = f'{1:\}' +47 47 | value = f"{f"\{1}"}" + +W605_1.py:45:11: W605 [*] Invalid escape sequence: `\{` + | +43 | regex = f'\\\_' +44 | value = f'\{{1}}' +45 | value = f'\{1}' + | ^^ W605 +46 | value = f'{1:\}' +47 | value = f"{f"\{1}"}" + | + = help: Use a raw string literal + +ℹ Safe fix +42 42 | +43 43 | regex = f'\\\_' +44 44 | value = f'\{{1}}' +45 |-value = f'\{1}' + 45 |+value = rf'\{1}' +46 46 | value = f'{1:\}' +47 47 | value = f"{f"\{1}"}" +48 48 | value = rf"{f"\{1}"}" + +W605_1.py:46:14: W605 [*] Invalid escape sequence: `\}` + | +44 | value = f'\{{1}}' +45 | value = f'\{1}' +46 | value = f'{1:\}' + | ^^ W605 +47 | value = f"{f"\{1}"}" +48 | value = rf"{f"\{1}"}" + | + = help: Use a raw string literal + +ℹ Safe fix +43 43 | regex = f'\\\_' +44 44 | value = f'\{{1}}' +45 45 | value = f'\{1}' +46 |-value = f'{1:\}' + 46 |+value = rf'{1:\}' +47 47 | value = f"{f"\{1}"}" +48 48 | value = rf"{f"\{1}"}" +49 49 | + +W605_1.py:47:14: W605 [*] Invalid escape sequence: `\{` + | +45 | value = f'\{1}' +46 | value = f'{1:\}' +47 | value = f"{f"\{1}"}" + | ^^ W605 +48 | value = rf"{f"\{1}"}" + | + = help: Use a raw string literal + +ℹ Safe fix +44 44 | value = f'\{{1}}' +45 45 | value = f'\{1}' +46 46 | value = f'{1:\}' +47 |-value = f"{f"\{1}"}" + 47 |+value = f"{rf"\{1}"}" +48 48 | value = rf"{f"\{1}"}" +49 49 | +50 50 | # Okay + +W605_1.py:48:15: W605 [*] Invalid escape sequence: `\{` + | +46 | value = f'{1:\}' +47 | value = f"{f"\{1}"}" +48 | value = rf"{f"\{1}"}" + | ^^ W605 +49 | +50 | # Okay + | + = help: Use a raw string literal + +ℹ Safe fix +45 45 | value = f'\{1}' +46 46 | value = f'{1:\}' +47 47 | value = f"{f"\{1}"}" +48 |-value = rf"{f"\{1}"}" + 48 |+value = rf"{rf"\{1}"}" +49 49 | +50 50 | # Okay +51 51 | value = rf'\{{1}}' diff --git a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_2.py.snap b/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_2.py.snap deleted file mode 100644 index 9f1016ae835e3..0000000000000 --- a/crates/ruff_linter/src/rules/pycodestyle/snapshots/ruff_linter__rules__pycodestyle__tests__W605_W605_2.py.snap +++ /dev/null @@ -1,227 +0,0 @@ ---- -source: crates/ruff_linter/src/rules/pycodestyle/mod.rs ---- -W605_2.py:4:11: W605 [*] Invalid escape sequence: `\.` - | -3 | #: W605:1:10 -4 | regex = f'\.png$' - | ^^ W605 -5 | -6 | #: W605:2:1 - | - = help: Use a raw string literal - -ℹ Safe fix -1 1 | # Same as `W605_0.py` but using f-strings instead. -2 2 | -3 3 | #: W605:1:10 -4 |-regex = f'\.png$' - 4 |+regex = rf'\.png$' -5 5 | -6 6 | #: W605:2:1 -7 7 | regex = f''' - -W605_2.py:8:1: W605 [*] Invalid escape sequence: `\.` - | -6 | #: W605:2:1 -7 | regex = f''' -8 | \.png$ - | ^^ W605 -9 | ''' - | - = help: Use a raw string literal - -ℹ Safe fix -4 4 | regex = f'\.png$' -5 5 | -6 6 | #: W605:2:1 -7 |-regex = f''' - 7 |+regex = rf''' -8 8 | \.png$ -9 9 | ''' -10 10 | - -W605_2.py:13:7: W605 [*] Invalid escape sequence: `\_` - | -11 | #: W605:2:6 -12 | f( -13 | f'\_' - | ^^ W605 -14 | ) - | - = help: Use a raw string literal - -ℹ Safe fix -10 10 | -11 11 | #: W605:2:6 -12 12 | f( -13 |- f'\_' - 13 |+ rf'\_' -14 14 | ) -15 15 | -16 16 | #: W605:4:6 - -W605_2.py:20:6: W605 [*] Invalid escape sequence: `\_` - | -18 | multi-line -19 | literal -20 | with \_ somewhere - | ^^ W605 -21 | in the middle -22 | """ - | - = help: Use a raw string literal - -ℹ Safe fix -14 14 | ) -15 15 | -16 16 | #: W605:4:6 -17 |-f""" - 17 |+rf""" -18 18 | multi-line -19 19 | literal -20 20 | with \_ somewhere - -W605_2.py:25:40: W605 [*] Invalid escape sequence: `\_` - | -24 | #: W605:1:38 -25 | value = f'new line\nand invalid escape \_ here' - | ^^ W605 - | - = help: Add backslash to escape sequence - -ℹ Safe fix -22 22 | """ -23 23 | -24 24 | #: W605:1:38 -25 |-value = f'new line\nand invalid escape \_ here' - 25 |+value = f'new line\nand invalid escape \\_ here' -26 26 | -27 27 | -28 28 | #: Okay - -W605_2.py:43:13: W605 [*] Invalid escape sequence: `\_` - | -41 | ''' # noqa -42 | -43 | regex = f'\\\_' - | ^^ W605 -44 | value = f'\{{1}}' -45 | value = f'\{1}' - | - = help: Add backslash to escape sequence - -ℹ Safe fix -40 40 | \w -41 41 | ''' # noqa -42 42 | -43 |-regex = f'\\\_' - 43 |+regex = f'\\\\_' -44 44 | value = f'\{{1}}' -45 45 | value = f'\{1}' -46 46 | value = f'{1:\}' - -W605_2.py:44:11: W605 [*] Invalid escape sequence: `\{` - | -43 | regex = f'\\\_' -44 | value = f'\{{1}}' - | ^^ W605 -45 | value = f'\{1}' -46 | value = f'{1:\}' - | - = help: Use a raw string literal - -ℹ Safe fix -41 41 | ''' # noqa -42 42 | -43 43 | regex = f'\\\_' -44 |-value = f'\{{1}}' - 44 |+value = rf'\{{1}}' -45 45 | value = f'\{1}' -46 46 | value = f'{1:\}' -47 47 | value = f"{f"\{1}"}" - -W605_2.py:45:11: W605 [*] Invalid escape sequence: `\{` - | -43 | regex = f'\\\_' -44 | value = f'\{{1}}' -45 | value = f'\{1}' - | ^^ W605 -46 | value = f'{1:\}' -47 | value = f"{f"\{1}"}" - | - = help: Use a raw string literal - -ℹ Safe fix -42 42 | -43 43 | regex = f'\\\_' -44 44 | value = f'\{{1}}' -45 |-value = f'\{1}' - 45 |+value = rf'\{1}' -46 46 | value = f'{1:\}' -47 47 | value = f"{f"\{1}"}" -48 48 | value = rf"{f"\{1}"}" - -W605_2.py:46:14: W605 [*] Invalid escape sequence: `\}` - | -44 | value = f'\{{1}}' -45 | value = f'\{1}' -46 | value = f'{1:\}' - | ^^ W605 -47 | value = f"{f"\{1}"}" -48 | value = rf"{f"\{1}"}" - | - = help: Use a raw string literal - -ℹ Safe fix -43 43 | regex = f'\\\_' -44 44 | value = f'\{{1}}' -45 45 | value = f'\{1}' -46 |-value = f'{1:\}' - 46 |+value = rf'{1:\}' -47 47 | value = f"{f"\{1}"}" -48 48 | value = rf"{f"\{1}"}" -49 49 | - -W605_2.py:47:14: W605 [*] Invalid escape sequence: `\{` - | -45 | value = f'\{1}' -46 | value = f'{1:\}' -47 | value = f"{f"\{1}"}" - | ^^ W605 -48 | value = rf"{f"\{1}"}" - | - = help: Use a raw string literal - -ℹ Safe fix -44 44 | value = f'\{{1}}' -45 45 | value = f'\{1}' -46 46 | value = f'{1:\}' -47 |-value = f"{f"\{1}"}" - 47 |+value = f"{rf"\{1}"}" -48 48 | value = rf"{f"\{1}"}" -49 49 | -50 50 | # Okay - -W605_2.py:48:15: W605 [*] Invalid escape sequence: `\{` - | -46 | value = f'{1:\}' -47 | value = f"{f"\{1}"}" -48 | value = rf"{f"\{1}"}" - | ^^ W605 -49 | -50 | # Okay - | - = help: Use a raw string literal - -ℹ Safe fix -45 45 | value = f'\{1}' -46 46 | value = f'{1:\}' -47 47 | value = f"{f"\{1}"}" -48 |-value = rf"{f"\{1}"}" - 48 |+value = rf"{rf"\{1}"}" -49 49 | -50 50 | # Okay -51 51 | value = rf'\{{1}}' - - From 17c88176952ddb390c3a9d860704092d135ab00e Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Dec 2023 11:01:58 -0500 Subject: [PATCH 16/18] Avoid off-by-one error in stripping noqa following multi-byte char (#8979) Closes https://github.com/astral-sh/ruff/issues/8976. --- .../resources/test/fixtures/ruff/RUF100_3.py | 3 ++ crates/ruff_linter/src/checkers/noqa.rs | 14 ++---- ..._linter__rules__ruff__tests__ruf100_3.snap | 50 +++++++++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF100_3.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF100_3.py index 64f9cd12c49a4..17e6367ad0592 100644 --- a/crates/ruff_linter/resources/test/fixtures/ruff/RUF100_3.py +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF100_3.py @@ -23,3 +23,6 @@ print(a) # noqa: E501, F821 # comment print(a) # noqa: E501, F821 comment print(a) # noqa: E501, F821 comment + +print(a) # comment with unicode µ # noqa: E501 +print(a) # comment with unicode µ # noqa: E501, F821 diff --git a/crates/ruff_linter/src/checkers/noqa.rs b/crates/ruff_linter/src/checkers/noqa.rs index 7b4224c4e1e99..055f802ccc7c8 100644 --- a/crates/ruff_linter/src/checkers/noqa.rs +++ b/crates/ruff_linter/src/checkers/noqa.rs @@ -3,10 +3,10 @@ use std::path::Path; use itertools::Itertools; -use ruff_text_size::{Ranged, TextLen, TextRange, TextSize}; +use ruff_text_size::{Ranged, TextLen, TextRange}; use ruff_diagnostics::{Diagnostic, Edit, Fix}; -use ruff_python_trivia::CommentRanges; +use ruff_python_trivia::{CommentRanges, PythonWhitespace}; use ruff_source_file::Locator; use crate::noqa; @@ -200,17 +200,11 @@ fn delete_noqa(range: TextRange, locator: &Locator) -> Edit { // Compute the leading space. let prefix = locator.slice(TextRange::new(line_range.start(), range.start())); - let leading_space = prefix - .rfind(|c: char| !c.is_whitespace()) - .map_or(prefix.len(), |i| prefix.len() - i - 1); - let leading_space_len = TextSize::try_from(leading_space).unwrap(); + let leading_space_len = prefix.text_len() - prefix.trim_whitespace_end().text_len(); // Compute the trailing space. let suffix = locator.slice(TextRange::new(range.end(), line_range.end())); - let trailing_space = suffix - .find(|c: char| !c.is_whitespace()) - .map_or(suffix.len(), |i| i); - let trailing_space_len = TextSize::try_from(trailing_space).unwrap(); + let trailing_space_len = suffix.text_len() - suffix.trim_whitespace_start().text_len(); // Ex) `# noqa` if line_range diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap index 019c140aecbc9..5f38b489e2ab9 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__ruf100_3.snap @@ -344,6 +344,7 @@ RUF100_3.py:23:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) 23 |+print(a) # noqa: F821 # comment 24 24 | print(a) # noqa: E501, F821 comment 25 25 | print(a) # noqa: E501, F821 comment +26 26 | RUF100_3.py:24:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) | @@ -362,6 +363,8 @@ RUF100_3.py:24:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) 24 |-print(a) # noqa: E501, F821 comment 24 |+print(a) # noqa: F821 comment 25 25 | print(a) # noqa: E501, F821 comment +26 26 | +27 27 | print(a) # comment with unicode µ # noqa: E501 RUF100_3.py:25:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) | @@ -369,6 +372,8 @@ RUF100_3.py:25:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) 24 | print(a) # noqa: E501, F821 comment 25 | print(a) # noqa: E501, F821 comment | ^^^^^^^^^^^^^^^^^^ RUF100 +26 | +27 | print(a) # comment with unicode µ # noqa: E501 | = help: Remove unused `noqa` directive @@ -378,5 +383,50 @@ RUF100_3.py:25:11: RUF100 [*] Unused `noqa` directive (unused: `E501`) 24 24 | print(a) # noqa: E501, F821 comment 25 |-print(a) # noqa: E501, F821 comment 25 |+print(a) # noqa: F821 comment +26 26 | +27 27 | print(a) # comment with unicode µ # noqa: E501 +28 28 | print(a) # comment with unicode µ # noqa: E501, F821 + +RUF100_3.py:27:7: F821 Undefined name `a` + | +25 | print(a) # noqa: E501, F821 comment +26 | +27 | print(a) # comment with unicode µ # noqa: E501 + | ^ F821 +28 | print(a) # comment with unicode µ # noqa: E501, F821 + | + +RUF100_3.py:27:39: RUF100 [*] Unused `noqa` directive (unused: `E501`) + | +25 | print(a) # noqa: E501, F821 comment +26 | +27 | print(a) # comment with unicode µ # noqa: E501 + | ^^^^^^^^^^^^ RUF100 +28 | print(a) # comment with unicode µ # noqa: E501, F821 + | + = help: Remove unused `noqa` directive + +ℹ Safe fix +24 24 | print(a) # noqa: E501, F821 comment +25 25 | print(a) # noqa: E501, F821 comment +26 26 | +27 |-print(a) # comment with unicode µ # noqa: E501 + 27 |+print(a) # comment with unicode µ +28 28 | print(a) # comment with unicode µ # noqa: E501, F821 + +RUF100_3.py:28:39: RUF100 [*] Unused `noqa` directive (unused: `E501`) + | +27 | print(a) # comment with unicode µ # noqa: E501 +28 | print(a) # comment with unicode µ # noqa: E501, F821 + | ^^^^^^^^^^^^^^^^^^ RUF100 + | + = help: Remove unused `noqa` directive + +ℹ Safe fix +25 25 | print(a) # noqa: E501, F821 comment +26 26 | +27 27 | print(a) # comment with unicode µ # noqa: E501 +28 |-print(a) # comment with unicode µ # noqa: E501, F821 + 28 |+print(a) # comment with unicode µ # noqa: F821 From b358cbf398b85cbb8cfcf7c49749cd291b62f421 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Dec 2023 15:19:43 -0500 Subject: [PATCH 17/18] Fix start >= end error in over-indentation (#8982) Closes https://github.com/astral-sh/ruff/issues/8977. --- .../test/fixtures/pydocstyle/D208.py | 5 + .../src/rules/pydocstyle/rules/indent.rs | 3 +- ...ules__pydocstyle__tests__D208_D208.py.snap | 102 ++++++++++-------- 3 files changed, 67 insertions(+), 43 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py index f0515248abb5c..27cc67d056c30 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py @@ -1,3 +1,8 @@ +""" + Author +""" + + class Platform: """ Remove sampler Args: diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs index f56bc1bed47b2..c7220cb0b6181 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs @@ -251,7 +251,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::empty(line.start())); let edit = if indent.is_empty() { - Edit::deletion(line.start(), line_indent.text_len()) + // Delete the entire indent. + Edit::range_deletion(TextRange::at(line.start(), line_indent.text_len())) } else { // Convert the character count to an offset within the source. let offset = checker diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap index ca8b68bcbeee5..944cbb9d520b0 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap @@ -1,57 +1,75 @@ --- source: crates/ruff_linter/src/rules/pydocstyle/mod.rs --- -D208.py:3:1: D208 [*] Docstring is over-indented +D208.py:2:1: D208 [*] Docstring is over-indented | -1 | class Platform: -2 | """ Remove sampler -3 | Args: +1 | """ +2 | Author | D208 -4 |     Returns: -5 | """ +3 | """ | = help: Remove over-indentation ℹ Safe fix -1 1 | class Platform: -2 2 | """ Remove sampler -3 |- Args: - 3 |+ Args: -4 4 |     Returns: -5 5 | """ - -D208.py:4:1: D208 [*] Docstring is over-indented - | -2 | """ Remove sampler -3 | Args: -4 |     Returns: - | D208 -5 | """ - | - = help: Remove over-indentation +1 1 | """ +2 |- Author + 2 |+Author +3 3 | """ +4 4 | +5 5 | + +D208.py:8:1: D208 [*] Docstring is over-indented + | + 6 | class Platform: + 7 | """ Remove sampler + 8 | Args: + | D208 + 9 |     Returns: +10 | """ + | + = help: Remove over-indentation ℹ Safe fix -1 1 | class Platform: -2 2 | """ Remove sampler -3 3 | Args: -4 |-     Returns: - 4 |+ Returns: -5 5 | """ - -D208.py:5:1: D208 [*] Docstring is over-indented - | -3 | Args: -4 |     Returns: -5 | """ - | D208 - | - = help: Remove over-indentation +5 5 | +6 6 | class Platform: +7 7 | """ Remove sampler +8 |- Args: + 8 |+ Args: +9 9 |     Returns: +10 10 | """ + +D208.py:9:1: D208 [*] Docstring is over-indented + | + 7 | """ Remove sampler + 8 | Args: + 9 |     Returns: + | D208 +10 | """ + | + = help: Remove over-indentation + +ℹ Safe fix +6 6 | class Platform: +7 7 | """ Remove sampler +8 8 | Args: +9 |-     Returns: + 9 |+ Returns: +10 10 | """ + +D208.py:10:1: D208 [*] Docstring is over-indented + | + 8 | Args: + 9 |     Returns: +10 | """ + | D208 + | + = help: Remove over-indentation ℹ Safe fix -2 2 | """ Remove sampler -3 3 | Args: -4 4 |     Returns: -5 |- """ - 5 |+ """ +7 7 | """ Remove sampler +8 8 | Args: +9 9 |     Returns: +10 |- """ + 10 |+ """ From bfae1f1412d60fec43c3b0b7123d64e5505c82f8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Dec 2023 15:45:30 -0500 Subject: [PATCH 18/18] Convert over-indentation rule to use number of characters (#8983) Closes https://github.com/astral-sh/ruff/issues/8978. --- .../test/fixtures/pydocstyle/D208.py | 6 ++++ .../src/rules/pydocstyle/rules/indent.rs | 28 +++++++++---------- ...ules__pydocstyle__tests__D208_D208.py.snap | 6 ++++ 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py index 27cc67d056c30..4e99cf4b7fdf5 100644 --- a/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py +++ b/crates/ruff_linter/resources/test/fixtures/pydocstyle/D208.py @@ -8,3 +8,9 @@ class Platform: Args:     Returns: """ + + +def memory_test(): + """ +　　　参数含义：precision:精确到小数点后几位 + """ diff --git a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs index c7220cb0b6181..1c2ced8bd1b0d 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs +++ b/crates/ruff_linter/src/rules/pydocstyle/rules/indent.rs @@ -172,8 +172,9 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { let mut has_seen_tab = docstring.indentation.contains('\t'); let mut is_over_indented = true; let mut over_indented_lines = vec![]; - let mut over_indented_offset = usize::MAX; + let mut over_indented_size = usize::MAX; + let docstring_indent_size = docstring.indentation.chars().count(); for i in 0..lines.len() { // First lines and continuations doesn't need any indentation. if i == 0 || lines[i - 1].ends_with('\\') { @@ -189,6 +190,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { } let line_indent = leading_space(line); + let line_indent_size = line_indent.chars().count(); // We only report tab indentation once, so only check if we haven't seen a tab // yet. @@ -197,9 +199,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { if checker.enabled(Rule::UnderIndentation) { // We report under-indentation on every line. This isn't great, but enables // fix. - if (i == lines.len() - 1 || !is_blank) - && line_indent.len() < docstring.indentation.len() - { + if (i == lines.len() - 1 || !is_blank) && line_indent_size < docstring_indent_size { let mut diagnostic = Diagnostic::new(UnderIndentation, TextRange::empty(line.start())); diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement( @@ -217,14 +217,12 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { // until we've viewed all the lines, so for now, just track // the over-indentation status of every line. if i < lines.len() - 1 { - if line_indent.len() > docstring.indentation.len() { + if line_indent_size > docstring_indent_size { over_indented_lines.push(line); // Track the _smallest_ offset we see, in terms of characters. - over_indented_offset = std::cmp::min( - line_indent.chars().count() - docstring.indentation.chars().count(), - over_indented_offset, - ); + over_indented_size = + std::cmp::min(line_indent_size - docstring_indent_size, over_indented_size); } else { is_over_indented = false; } @@ -250,6 +248,7 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { // enables the fix capability. let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::empty(line.start())); + let edit = if indent.is_empty() { // Delete the entire indent. Edit::range_deletion(TextRange::at(line.start(), line_indent.text_len())) @@ -259,13 +258,11 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { .locator() .after(line.start() + indent.text_len()) .chars() - .take(over_indented_offset) + .take(over_indented_size) .map(TextLen::text_len) .sum::(); - Edit::range_replacement( - indent.clone(), - TextRange::at(line.start(), indent.text_len() + offset), - ) + let range = TextRange::at(line.start(), indent.text_len() + offset); + Edit::range_replacement(indent, range) }; diagnostic.set_fix(Fix::safe_edit(edit)); checker.diagnostics.push(diagnostic); @@ -275,7 +272,8 @@ pub(crate) fn indent(checker: &mut Checker, docstring: &Docstring) { // If the last line is over-indented... if let Some(last) = lines.last() { let line_indent = leading_space(last); - if line_indent.len() > docstring.indentation.len() { + let line_indent_size = line_indent.chars().count(); + if line_indent_size > docstring_indent_size { let mut diagnostic = Diagnostic::new(OverIndentation, TextRange::empty(last.start())); let indent = clean_space(docstring.indentation); diff --git a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap index 944cbb9d520b0..517dc3802e1ec 100644 --- a/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap +++ b/crates/ruff_linter/src/rules/pydocstyle/snapshots/ruff_linter__rules__pydocstyle__tests__D208_D208.py.snap @@ -37,6 +37,7 @@ D208.py:8:1: D208 [*] Docstring is over-indented 8 |+ Args: 9 9 |     Returns: 10 10 | """ +11 11 | D208.py:9:1: D208 [*] Docstring is over-indented | @@ -55,6 +56,8 @@ D208.py:9:1: D208 [*] Docstring is over-indented 9 |-     Returns: 9 |+ Returns: 10 10 | """ +11 11 | +12 12 | D208.py:10:1: D208 [*] Docstring is over-indented | @@ -71,5 +74,8 @@ D208.py:10:1: D208 [*] Docstring is over-indented 9 9 |     Returns: 10 |- """ 10 |+ """ +11 11 | +12 12 | +13 13 | def memory_test():