Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing file with rule F401 cause infinite loop #12570

Closed
qarmin opened this issue Jul 29, 2024 · 2 comments · Fixed by #12571
Closed

Fixing file with rule F401 cause infinite loop #12570

qarmin opened this issue Jul 29, 2024 · 2 comments · Fixed by #12571
Assignees
Labels
bug Something isn't working fuzzer Surfaced via fuzzing.

Comments

@qarmin
Copy link

qarmin commented Jul 29, 2024

ruff 0.5.5+353 (2f54d05 2024-07-29)

ruff check *.py --select F401 --no-cache --fix --unsafe-fixes --preview --output-format concise --isolated

file content(at the bottom should be attached raw, not formatted file - github removes some non-printable characters, so copying from here may not work):

from .main import MaµToMan

error

/tmp/tmp_folder/data/5737962126122819962.py:1:8: F401 `tima` imported but unused
Found 501 errors (500 fixed, 1 remaining).
[*] 1 fixable with the --fix option.

debug error: Failed to converge after 500 iterations in `/tmp/tmp_folder/data/5737962126122819962.py` with rule codes F401:---
import timª
---

Ruff build, that was used to reproduce problem(compiled on Ubuntu 22.04 with release mode + debug symbols + debug assertions + overflow checks) - https://github.com/qarmin/Automated-Fuzzer/releases/download/Nightly/ruff.7z

python_compressed.zip

@AlexWaygood
Copy link
Member

Cool bug, thanks!

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 29, 2024

This is an issue with NFKC normalization. If you apply this diff to Ruff:

diff --git a/crates/ruff_linter/src/fix/codemods.rs b/crates/ruff_linter/src/fix/codemods.rs
index c3c769172..ed4061435 100644
--- a/crates/ruff_linter/src/fix/codemods.rs
+++ b/crates/ruff_linter/src/fix/codemods.rs
@@ -80,7 +80,7 @@ pub(crate) fn remove_imports<'a>(
     for member in member_names {
         let alias_index = aliases
             .iter()
-            .position(|alias| member == qualified_name_from_name_or_attribute(&alias.name));
+            .position(|alias| dbg!(member) == dbg!(qualified_name_from_name_or_attribute(&alias.name)));
         if let Some(index) = alias_index {
             aliases.remove(index);
         }
diff --git a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
index bfa884801..4c86744db 100644
--- a/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
+++ b/crates/ruff_linter/src/rules/pyflakes/rules/unused_import.rs
@@ -397,6 +397,8 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
         }
         diagnostics.push(diagnostic);
     }
+
+    panic!()
 }

Then the output from the debug calls is as follows:

[crates/ruff_linter/src/fix/codemods.rs:83:31] member = "MaμToMan"
[crates/ruff_linter/src/fix/codemods.rs:83:47] qualified_name_from_name_or_attribute(&alias.name) = "MaµToMan"

And I'll also paste a screenshot because it looks like GitHub might actually normalize these confusables as well when it renders them in an issue comment?!):


image


If you look very closely at that screenshot, you can see that the μ on the first line is sliiiightly narrower in size than the µ on the second line. In other words, one of the two variables has been NFKC-normalized while the other has not, meaning the comparison here evaluates to false when it should be evaluating to true:

.position(|alias| member == qualified_name_from_name_or_attribute(&alias.name));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fuzzer Surfaced via fuzzing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants