Skip to content

Don't skip over imports and other nodes containing nested statements in import collector#13521

Merged
MichaReiser merged 6 commits intomainfrom
micha/fix-import-collector-skip
Sep 26, 2024
Merged

Don't skip over imports and other nodes containing nested statements in import collector#13521
MichaReiser merged 6 commits intomainfrom
micha/fix-import-collector-skip

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Sep 26, 2024

Summary

This is a follow up for #13441

enter_node is called for every node and it's important that it returns true for any node between ModModule and an import statement, including the import statement itself.

Getting this right in enter_node seems hard and it's easy to forget adding a new "include" when a new compound statement gets added.

This PR moves the traversal logic into the visist_stmt by requiring explicit match arms for each statement. I think this is easier to understand
and less error prone.

Also... enter_node doesn't get call when a visitor overrides visit_stmt. That's an oversight from my side when designing the visitor.

@MichaReiser MichaReiser added the bug Something isn't working label Sep 26, 2024
@MichaReiser MichaReiser force-pushed the micha/fix-import-collector-skip branch from d1b2bda to e2e2a13 Compare September 26, 2024 10:08
@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@MichaReiser
Copy link
Member Author

Let me add a test for this

@MichaReiser MichaReiser force-pushed the micha/fix-import-collector-skip branch from e2e2a13 to c5b46f2 Compare September 26, 2024 11:43
@MichaReiser MichaReiser enabled auto-merge (squash) September 26, 2024 11:46
@MichaReiser MichaReiser force-pushed the micha/fix-import-collector-skip branch from 6c21504 to 8d77e0e Compare September 26, 2024 11:52
@MichaReiser MichaReiser merged commit ff2d214 into main Sep 26, 2024
@MichaReiser MichaReiser deleted the micha/fix-import-collector-skip branch September 26, 2024 11:57
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.

2 participants