Skip to content

Commit

Permalink
Avoid debug assertion around NFKC renames (#11249)
Browse files Browse the repository at this point in the history
## Summary

This assertion isn't quite correct, since with NFKC normalization, two
identifiers can have different lengths but map to the same binding.

Closes #11238.

Closes #11239.
  • Loading branch information
charliermarsh authored May 2, 2024
1 parent 1673bc4 commit e62fa4e
Show file tree
Hide file tree
Showing 5 changed files with 400 additions and 335 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ def bad_method(this):
pass

if False:

def extra_bad_method(this):
pass

Expand Down Expand Up @@ -94,6 +93,7 @@ def good(self):
def badstatic(foo):
pass


class SelfInArgsClass:
def self_as_argument(this, self):
pass
Expand All @@ -110,10 +110,16 @@ def self_as_varags(this, *self):
def self_as_kwargs(this, **self):
pass


class RenamingInMethodBodyClass:
def bad_method(this):
this = this
this

def bad_method(this):
self = this


class RenamingWithNFKC:
def formula(household):
hºusehold(1)
3 changes: 1 addition & 2 deletions crates/ruff_linter/src/renamer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use itertools::Itertools;
use ruff_diagnostics::Edit;
use ruff_python_codegen::Stylist;
use ruff_python_semantic::{Binding, BindingKind, Scope, ScopeId, SemanticModel};
use ruff_text_size::{Ranged, TextSize};
use ruff_text_size::Ranged;

pub(crate) struct Renamer;

Expand Down Expand Up @@ -215,7 +215,6 @@ impl Renamer {
let quote = stylist.quote();
format!("{quote}{target}{quote}")
} else {
debug_assert_eq!(TextSize::of(name), reference.range().len());
target.to_string()
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,160 +20,159 @@ N805.py:7:20: N805 [*] First argument of a method should be named `self`
9 9 |
10 10 | if False:

N805.py:12:30: N805 [*] First argument of a method should be named `self`
N805.py:11:30: N805 [*] First argument of a method should be named `self`
|
10 | if False:
11 |
12 | def extra_bad_method(this):
11 | def extra_bad_method(this):
| ^^^^ N805
13 | pass
12 | pass
|
= help: Rename `this` to `self`

Unsafe fix
8 8 | pass
9 9 |
10 10 | if False:
11 11 |
12 |- def extra_bad_method(this):
12 |+ def extra_bad_method(self):
13 13 | pass
14 14 |
15 15 | def good_method(self):
11 |- def extra_bad_method(this):
11 |+ def extra_bad_method(self):
12 12 | pass
13 13 |
14 14 | def good_method(self):

N805.py:31:15: N805 [*] First argument of a method should be named `self`
N805.py:30:15: N805 [*] First argument of a method should be named `self`
|
30 | @pydantic.validator
31 | def lower(cls, my_field: str) -> str:
29 | @pydantic.validator
30 | def lower(cls, my_field: str) -> str:
| ^^^ N805
32 | pass
31 | pass
|
= help: Rename `cls` to `self`

Unsafe fix
28 28 | return x
29 29 |
30 30 | @pydantic.validator
31 |- def lower(cls, my_field: str) -> str:
31 |+ def lower(self, my_field: str) -> str:
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
27 27 | return x
28 28 |
29 29 | @pydantic.validator
30 |- def lower(cls, my_field: str) -> str:
30 |+ def lower(self, my_field: str) -> str:
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")

N805.py:35:15: N805 [*] First argument of a method should be named `self`
N805.py:34:15: N805 [*] First argument of a method should be named `self`
|
34 | @pydantic.validator("my_field")
35 | def lower(cls, my_field: str) -> str:
33 | @pydantic.validator("my_field")
34 | def lower(cls, my_field: str) -> str:
| ^^^ N805
36 | pass
35 | pass
|
= help: Rename `cls` to `self`

Unsafe fix
32 32 | pass
33 33 |
34 34 | @pydantic.validator("my_field")
35 |- def lower(cls, my_field: str) -> str:
35 |+ def lower(self, my_field: str) -> str:
36 36 | pass
37 37 |
38 38 | def __init__(self):
31 31 | pass
32 32 |
33 33 | @pydantic.validator("my_field")
34 |- def lower(cls, my_field: str) -> str:
34 |+ def lower(self, my_field: str) -> str:
35 35 | pass
36 36 |
37 37 | def __init__(self):

N805.py:64:29: N805 [*] First argument of a method should be named `self`
N805.py:63:29: N805 [*] First argument of a method should be named `self`
|
62 | pass
63 |
64 | def bad_method_pos_only(this, blah, /, something: str):
61 | pass
62 |
63 | def bad_method_pos_only(this, blah, /, something: str):
| ^^^^ N805
65 | pass
64 | pass
|
= help: Rename `this` to `self`

Unsafe fix
61 61 | def good_method_pos_only(self, blah, /, something: str):
62 62 | pass
63 63 |
64 |- def bad_method_pos_only(this, blah, /, something: str):
64 |+ def bad_method_pos_only(self, blah, /, something: str):
65 65 | pass
60 60 | def good_method_pos_only(self, blah, /, something: str):
61 61 | pass
62 62 |
63 |- def bad_method_pos_only(this, blah, /, something: str):
63 |+ def bad_method_pos_only(self, blah, /, something: str):
64 64 | pass
65 65 |
66 66 |
67 67 |

N805.py:70:13: N805 [*] First argument of a method should be named `self`
N805.py:69:13: N805 [*] First argument of a method should be named `self`
|
68 | class ModelClass:
69 | @hybrid_property
70 | def bad(cls):
67 | class ModelClass:
68 | @hybrid_property
69 | def bad(cls):
| ^^^ N805
71 | pass
70 | pass
|
= help: Rename `cls` to `self`

Unsafe fix
67 67 |
68 68 | class ModelClass:
69 69 | @hybrid_property
70 |- def bad(cls):
70 |+ def bad(self):
71 71 | pass
72 72 |
73 73 | @bad.expression
66 66 |
67 67 | class ModelClass:
68 68 | @hybrid_property
69 |- def bad(cls):
69 |+ def bad(self):
70 70 | pass
71 71 |
72 72 | @bad.expression

N805.py:78:13: N805 [*] First argument of a method should be named `self`
N805.py:77:13: N805 [*] First argument of a method should be named `self`
|
77 | @bad.wtf
78 | def bad(cls):
76 | @bad.wtf
77 | def bad(cls):
| ^^^ N805
79 | pass
78 | pass
|
= help: Rename `cls` to `self`

Unsafe fix
75 75 | pass
76 76 |
77 77 | @bad.wtf
78 |- def bad(cls):
78 |+ def bad(self):
79 79 | pass
80 80 |
81 81 | @hybrid_property
74 74 | pass
75 75 |
76 76 | @bad.wtf
77 |- def bad(cls):
77 |+ def bad(self):
78 78 | pass
79 79 |
80 80 | @hybrid_property

N805.py:86:14: N805 [*] First argument of a method should be named `self`
N805.py:85:14: N805 [*] First argument of a method should be named `self`
|
85 | @good.expression
86 | def good(cls):
84 | @good.expression
85 | def good(cls):
| ^^^ N805
87 | pass
86 | pass
|
= help: Rename `cls` to `self`

Unsafe fix
83 83 | pass
84 84 |
85 85 | @good.expression
86 |- def good(cls):
86 |+ def good(self):
87 87 | pass
88 88 |
89 89 | @good.wtf
82 82 | pass
83 83 |
84 84 | @good.expression
85 |- def good(cls):
85 |+ def good(self):
86 86 | pass
87 87 |
88 88 | @good.wtf

N805.py:94:19: N805 [*] First argument of a method should be named `self`
N805.py:93:19: N805 [*] First argument of a method should be named `self`
|
93 | @foobar.thisisstatic
94 | def badstatic(foo):
92 | @foobar.thisisstatic
93 | def badstatic(foo):
| ^^^ N805
95 | pass
94 | pass
|
= help: Rename `foo` to `self`

Unsafe fix
91 91 | pass
92 92 |
93 93 | @foobar.thisisstatic
94 |- def badstatic(foo):
94 |+ def badstatic(self):
95 95 | pass
90 90 | pass
91 91 |
92 92 | @foobar.thisisstatic
93 |- def badstatic(foo):
93 |+ def badstatic(self):
94 94 | pass
95 95 |
96 96 |
97 97 | class SelfInArgsClass:

N805.py:98:26: N805 First argument of a method should be named `self`
|
Expand Down Expand Up @@ -224,45 +223,66 @@ N805.py:110:24: N805 First argument of a method should be named `self`
|
= help: Rename `this` to `self`

N805.py:114:20: N805 [*] First argument of a method should be named `self`
N805.py:115:20: N805 [*] First argument of a method should be named `self`
|
113 | class RenamingInMethodBodyClass:
114 | def bad_method(this):
114 | class RenamingInMethodBodyClass:
115 | def bad_method(this):
| ^^^^ N805
115 | this = this
116 | this
116 | this = this
117 | this
|
= help: Rename `this` to `self`

Unsafe fix
111 111 | pass
112 112 |
113 113 | class RenamingInMethodBodyClass:
114 |- def bad_method(this):
115 |- this = this
116 |- this
114 |+ def bad_method(self):
115 |+ self = self
116 |+ self
117 117 |
118 118 | def bad_method(this):
119 119 | self = this
113 113 |
114 114 | class RenamingInMethodBodyClass:
115 |- def bad_method(this):
116 |- this = this
117 |- this
115 |+ def bad_method(self):
116 |+ self = self
117 |+ self
118 118 |
119 119 | def bad_method(this):
120 120 | self = this

N805.py:118:20: N805 [*] First argument of a method should be named `self`
N805.py:119:20: N805 [*] First argument of a method should be named `self`
|
116 | this
117 |
118 | def bad_method(this):
117 | this
118 |
119 | def bad_method(this):
| ^^^^ N805
119 | self = this
120 | self = this
|
= help: Rename `this` to `self`

Unsafe fix
115 115 | this = this
116 116 | this
117 117 |
118 |- def bad_method(this):
119 |- self = this
118 |+ def bad_method(self):
119 |+ self = self
116 116 | this = this
117 117 | this
118 118 |
119 |- def bad_method(this):
120 |- self = this
119 |+ def bad_method(self):
120 |+ self = self
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:

N805.py:124:17: N805 [*] First argument of a method should be named `self`
|
123 | class RenamingWithNFKC:
124 | def formula(household):
| ^^^^^^^^^ N805
125 | hºusehold(1)
|
= help: Rename `household` to `self`

Unsafe fix
121 121 |
122 122 |
123 123 | class RenamingWithNFKC:
124 |- def formula(household):
125 |- hºusehold(1)
124 |+ def formula(self):
125 |+ self(1)
Loading

0 comments on commit e62fa4e

Please sign in to comment.