[`pylint`] Add U+061C to `PLE2502` by kotx · Pull Request #20106 · astral-sh/ruff

kotx · 2025-08-26T21:52:05Z

Resolves #20058

ntBre

Thank you!

I'm a bit conflicted because this clearly seems within the original intent of the rule, but a conservative reading of our versioning guidelines suggests that this needs to be a preview change since it increases the scope of a stable rule.

Would you mind making this a preview change? You can find examples of our preview checks in this file:

ruff/crates/ruff_linter/src/preview.rs

Lines 254 to 257 in e5e217f

    
           // https://github.com/astral-sh/ruff/pull/19851 
        
           pub(crate) const fn is_maxsplit_without_separator_fix_enabled(settings: &LinterSettings) -> bool { 
        
               settings.preview.is_enabled() 
        
           }

and I'd recommend making a copy of the test function here with preview enabled (often called preview_rules):

ruff/crates/ruff_linter/src/rules/pylint/mod.rs

Line 237 in e5e217f

fn rules(rule_code: Rule, path: &Path) -> Result<()> {

with just one test_case for bidirectional_unicode.py.

We'll probably want to move the character out of BIDI_UNICODE for now and combine it with the preview check, but what you have here is perfect once we stabilize the behavior.

github-actions · 2025-08-27T19:08:16Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

kotx · 2025-08-27T19:43:39Z

@ntBre The guidelines say minor version is bumped when "The scope of a stable rule is significantly increased" (emphasis added). Is this significant enough to warrant a preview change? Will proceed if this is the case (thank you for the detailed instructions!).

ntBre · 2025-08-27T21:06:55Z

Yeah, that's why I was conflicted. I guess I consider it significant within the niche of the rule itself, and it just errs on the safe side to keep it in preview. It's also a departure from the upstream pylint rule based on a quick test I just did, which is another reason.

ntBre · 2025-08-28T15:48:13Z

Thank you! This looks good overall. I did try to push one small patch to put the new test case back in bidirectional_unicode.py instead of a separate _preview file. That will make it a little easier to update things when we stabilize the change. But I couldn't push to the PR branch for some reason. Would you mind applying this patch or making similar changes? This is good to go otherwise.

Patch

diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py b/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py
index 25accb3aad..f72695d0c4 100644
--- a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py
+++ b/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode.py
@@ -4,6 +4,9 @@ print("שלום‬")
 # E2502
 example = "x‏" * 100  #    "‏x" is assigned
 
+# E2502
+another = "x؜" * 50  #    "؜x" is assigned
+
 # E2502
 if access_level != "none‮⁦":  # Check if admin ⁩⁦' and access_level != 'user
     print("You are an admin.")
diff --git a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode_preview.py b/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode_preview.py
deleted file mode 100644
index 1445423c99..0000000000
--- a/crates/ruff_linter/resources/test/fixtures/pylint/bidirectional_unicode_preview.py
+++ /dev/null
@@ -1,11 +0,0 @@
-# E2502
-hello = "x؜" * 50  #    "؜x" is assigned
-
-# OK
-print("\u061C")
-
-# OK
-print("\N{ARABIC LETTER MARK}")
-
-# OK
-print("Hello World")
diff --git a/crates/ruff_linter/src/rules/pylint/mod.rs b/crates/ruff_linter/src/rules/pylint/mod.rs
index 9d2d26072e..e181c0a146 100644
--- a/crates/ruff_linter/src/rules/pylint/mod.rs
+++ b/crates/ruff_linter/src/rules/pylint/mod.rs
@@ -252,12 +252,13 @@ mod tests {
         Ok(())
     }
 
-    #[test_case(
-        Rule::BidirectionalUnicode,
-        Path::new("bidirectional_unicode_preview.py")
-    )]
+    #[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"))]
     fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
-        let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
+        let snapshot = format!(
+            "preview__{}_{}",
+            rule_code.noqa_code(),
+            path.to_string_lossy()
+        );
         let diagnostics = test_path(
             Path::new("pylint").join(path).as_path(),
             &LinterSettings {
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap
index 18356831d7..0d584e8108 100644
--- a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap
+++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode.py.snap
@@ -22,21 +22,21 @@ PLE2502 Contains control characters that can permit obfuscated code
   |
 
 PLE2502 Contains control characters that can permit obfuscated code
- --> bidirectional_unicode.py:8:1
-  |
-7 | # E2502
-8 | if access_level != "none":  # Check if admin ' and access_level != 'user
-  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-9 |     print("You are an admin.")
-  |
+  --> bidirectional_unicode.py:11:1
+   |
+10 | # E2502
+11 | if access_level != "none":  # Check if admin ' and access_level != 'user
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+12 |     print("You are an admin.")
+   |
 
 PLE2502 Contains control characters that can permit obfuscated code
-  --> bidirectional_unicode.py:14:1
+  --> bidirectional_unicode.py:17:1
    |
-12 | # E2502
-13 | def subtract_funds(account: str, amount: int):
-14 |     """Subtract funds from bank account then """
+15 | # E2502
+16 | def subtract_funds(account: str, amount: int):
+17 |     """Subtract funds from bank account then """
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-15 |     return
-16 |     bank[account] -= amount
+18 |     return
+19 |     bank[account] -= amount
    |
diff --git a/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap
new file mode 100644
index 0000000000..4a1555ebe8
--- /dev/null
+++ b/crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__preview__PLE2502_bidirectional_unicode.py.snap
@@ -0,0 +1,52 @@
+---
+source: crates/ruff_linter/src/rules/pylint/mod.rs
+---
+PLE2502 Contains control characters that can permit obfuscated code
+ --> bidirectional_unicode.py:2:1
+  |
+1 | # E2502
+2 | print("שלום")
+  | ^^^^^^^^^^^^^
+3 |
+4 | # E2502
+  |
+
+PLE2502 Contains control characters that can permit obfuscated code
+ --> bidirectional_unicode.py:5:1
+  |
+4 | # E2502
+5 | example = "x‏" * 100  #    "‏x" is assigned
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+6 |
+7 | # E2502
+  |
+
+PLE2502 Contains control characters that can permit obfuscated code
+  --> bidirectional_unicode.py:8:1
+   |
+ 7 | # E2502
+ 8 | another = "x؜" * 50  #    "؜x" is assigned
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ 9 |
+10 | # E2502
+   |
+
+PLE2502 Contains control characters that can permit obfuscated code
+  --> bidirectional_unicode.py:11:1
+   |
+10 | # E2502
+11 | if access_level != "none":  # Check if admin ' and access_level != 'user
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+12 |     print("You are an admin.")
+   |
+
+PLE2502 Contains control characters that can permit obfuscated code
+  --> bidirectional_unicode.py:17:1
+   |
+15 | # E2502
+16 | def subtract_funds(account: str, amount: int):
+17 |     """Subtract funds from bank account then """
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+18 |     return
+19 |     bank[account] -= amount
+   |

This is mostly restoring the test case you had before, which is the only reason I was going to push it quickly and then merge instead of bothering you again :)

kotx · 2025-08-28T19:01:42Z

@ntBre pushed!

...t/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode_preview.py.snap

ntBre · 2025-08-28T20:01:40Z

Did you fix the clippy error too? I think it just wanted you to combine your if branches with an ||. I noticed that but wasn't going to comment since I think it's easier to read this way, but clippy wasn't so generous 😆

kotx · 2025-08-28T20:14:40Z

You're right, I had totally missed that! Fixed

ntBre

Thanks!

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

Resolves astral-sh#20058

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

Add U+061C to PLE2502

87e8386

ntBre reviewed Aug 27, 2025

View reviewed changes

ntBre added rule Implementing or modifying a lint rule preview Related to preview mode features labels Aug 27, 2025

kotx added 3 commits August 27, 2025 16:58

Move PLE2502 U+061C lint and tests to preview

48fb194

Remove U+061C from bidi test

8ac6d70

Make preview rules actually preview

fda3b21

kotx requested a review from ntBre August 28, 2025 02:02

Move test from preview file

44cf625

ntBre reviewed Aug 28, 2025

View reviewed changes

...t/snapshots/ruff_linter__rules__pylint__tests__PLE2502_bidirectional_unicode_preview.py.snap Outdated Show resolved Hide resolved

Remove unused snapshot

2646df6

Fix clippy lint

0fb4472

ntBre approved these changes Aug 28, 2025

View reviewed changes

ntBre changed the title ~~Add U+061C to PLE2502~~ [pylint] Add U+061C to PLE2502 Aug 28, 2025

ntBre merged commit b6522cb into astral-sh:main Aug 28, 2025
35 checks passed

BrewTestBot mentioned this pull request Sep 4, 2025

ruff 0.12.12 Homebrew/homebrew-core#236182

Merged

dylwil3 mentioned this pull request Sep 5, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (PLE2502) #20276

Merged

dylwil3 added a commit that referenced this pull request Sep 5, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (`PLE25…

fcb2d6d

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

ntBre pushed a commit that referenced this pull request Sep 8, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (`PLE25…

f726a73

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025

[pylint] Add U+061C to PLE2502 (astral-sh#20106)

5a94fd7

Resolves astral-sh#20058

ntBre pushed a commit that referenced this pull request Sep 10, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (`PLE25…

31402ab

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

ntBre pushed a commit that referenced this pull request Sep 10, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (`PLE25…

427413d

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

ntBre pushed a commit that referenced this pull request Sep 10, 2025

[pylint] Stabilize adding U+061C to bidirectional-unicode (`PLE25…

4bda9da

…02`) (#20276) Introduced in #20106. Removed gating. Updated tests. No documentation to update.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[`pylint`] Add U+061C to `PLE2502`#20106

[`pylint`] Add U+061C to `PLE2502`#20106
ntBre merged 7 commits intoastral-sh:mainfrom
kotx:main

kotx commented Aug 26, 2025

Uh oh!

ntBre left a comment

Uh oh!

github-actions bot commented Aug 27, 2025 •

edited

Loading

Uh oh!

kotx commented Aug 27, 2025 •

edited

Loading

Uh oh!

ntBre commented Aug 27, 2025

Uh oh!

ntBre commented Aug 28, 2025

Uh oh!

kotx commented Aug 28, 2025

Uh oh!

Uh oh!

ntBre commented Aug 28, 2025

Uh oh!

kotx commented Aug 28, 2025 •

edited

Loading

Uh oh!

ntBre left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	// https://github.com/astral-sh/ruff/pull/19851
	pub(crate) const fn is_maxsplit_without_separator_fix_enabled(settings: &LinterSettings) -> bool {
	settings.preview.is_enabled()
	}

Comments

Conversation

kotx commented Aug 26, 2025

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Linter (stable)

Linter (preview)

Uh oh!

kotx commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre commented Aug 27, 2025

Uh oh!

ntBre commented Aug 28, 2025

Uh oh!

kotx commented Aug 28, 2025

Uh oh!

Uh oh!

ntBre commented Aug 28, 2025

Uh oh!

kotx commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ntBre left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Aug 27, 2025 •

edited

Loading

`ruff-ecosystem` results

kotx commented Aug 27, 2025 •

edited

Loading

kotx commented Aug 28, 2025 •

edited

Loading