Skip to content

Rust: Model namespaces in path resolution #18728

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

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Feb 10, 2025

This PR models namespaces when resolving paths. Values and types live in different namespaces, so their names are allowed to overlap without shadowing. The context in which a path occurs uniquely determines which namespace to lookup into, except for uses like use foo::bar where bar may refer to either a value or a type (or both).

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Feb 10, 2025
@hvitved hvitved marked this pull request as ready for review February 10, 2025 19:20
@hvitved hvitved requested review from Copilot and paldepind February 10, 2025 19:20
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (3)

rust/ql/test/library-tests/path-resolution/main.rs:218

  • [nitpick] Having a function named 'Foo' alongside a struct 'Foo' reinforces the distinct namespace resolution but may reduce readability. Consider adding a clarifying comment to indicate that this naming collision is intentional for testing purposes.
fn Foo() {} // I62

rust/ql/test/library-tests/path-resolution/main.rs:233

  • [nitpick] The function 'FooBar' shares its identifier with an enum variant 'FooBar' from 'Bar', which might cause confusion even though the namespaces differ. Consider adding a clarifying comment or renaming one of them to improve code clarity.
fn FooBar() {} // I65

rust/ql/test/library-tests/path-resolution/main.rs:273

  • [nitpick] This import statement brings in both a function and a struct named 'f' from module m13, which can be ambiguous in usage. To enhance clarity, consider aliasing one of the items or splitting the import into separate statements.
use crate::m13::f; // $ item=I71 item=I72

Tip: Copilot code review supports C#, Go, Java, JavaScript, Markdown, Python, Ruby and TypeScript, with more languages coming soon. Learn more

@hvitved hvitved merged commit eaaf510 into github:main Feb 11, 2025
16 checks passed
@hvitved hvitved deleted the rust/path-resolution-namespaces branch February 11, 2025 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants