Skip to content

Fix CleanupRefactoring queuing duplicate rewrites for the same node - #177

Merged
pjljvandelaar merged 3 commits into
mainfrom
fix-cleanup-refactoring-dupe
Sep 23, 2026
Merged

pjljvandelaar merged 3 commits into
mainfrom
fix-cleanup-refactoring-dupe

Conversation

@FrancescoPezzella

Copy link
Copy Markdown
Collaborator

@FrancescoPezzella FrancescoPezzella self-assigned this Sep 11, 2026
@FrancescoPezzella
FrancescoPezzella marked this pull request as draft September 11, 2026 15:20
@FrancescoPezzella
FrancescoPezzella force-pushed the fix-cleanup-refactoring-dupe branch from 6d6a160 to 66b2132 Compare September 11, 2026 15:57
@FrancescoPezzella
FrancescoPezzella marked this pull request as ready for review September 14, 2026 09:28
@FrancescoPezzella FrancescoPezzella added the bug Something isn't working label Sep 14, 2026
@pjljvandelaar

Copy link
Copy Markdown
Collaborator

@rcorvino The root cause of this issue is your change on line 15 of renaissance/recipes/cleanup_refactoring.py

-         refs = flatten(find_ast_type(n, VariableDef) for n in find_ast_type(ast_refactor.node, CompoundStatement))
+        refs = flatten(
+          find_semantic_kind(n, SemanticKind.DECLARATION) for n in find_semantic_kind(ast_refactor.node, SemanticKind.STATEMENT)
+        )

As after this change a declaration will be added twice - once as part of the compound statement (that is the body of a function) and once as part of the declaration statement.

When navigation to the parent would be available, one can check for the direct containment and solve the problem.

@FrancescoPezzella
FrancescoPezzella force-pushed the fix-cleanup-refactoring-dupe branch 3 times, most recently from 2982691 to 9461a89 Compare September 23, 2026 08:30
@FrancescoPezzella

Copy link
Copy Markdown
Collaborator Author

any update?

@FrancescoPezzella
FrancescoPezzella force-pushed the fix-cleanup-refactoring-dupe branch from 9461a89 to 3e3cfc5 Compare September 23, 2026 13:23

@pjljvandelaar pjljvandelaar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good for now - we will revisit when making an explicit textRewriter and ASTRewriter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CleanupRefactoring.remove_unused_variables queues duplicate rewrites for the same node

2 participants