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

LSP rename struct incorrectly renames self and Self #5412

Closed
asterite opened this issue Jul 4, 2024 · 0 comments · Fixed by #5411
Closed

LSP rename struct incorrectly renames self and Self #5412

asterite opened this issue Jul 4, 2024 · 0 comments · Fixed by #5411
Labels
bug Something isn't working

Comments

@asterite
Copy link
Collaborator

asterite commented Jul 4, 2024

Aim

Renaming a struct shouldn't rename self or Self.

Expected Behavior

When triggering a struct rename, whenever self refers to that struct type as a function argument, or when Self is used to reference that struct, they are incorrectly renamed.

Bug

self and Self are incorrectly renamed.

To Reproduce

No response

Project Impact

None

Impact Context

No response

Workaround

None

Workaround Description

No response

Additional Context

No response

Installation Method

None

Nargo Version

No response

NoirJS Version

No response

Would you like to submit a PR for this Issue?

Yes

Support Needs

No response

@asterite asterite added the bug Something isn't working label Jul 4, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 4, 2024
github-merge-queue bot pushed a commit that referenced this issue Jul 8, 2024
# Description

## Problem

Resolves #5412

## Summary

When a function parameter is `self` and it refers to a struct type, that
variable is linked to that type (through a synthetic "Self" type):

```rust
struct Foo {
}

impl Foo {
  fn foo(self) { // <-- here
  }
}
```

Then, when renaming `Foo`, those `self` occurrences were incorrectly
renamed. To fix that, in this PR we don't track these synthetic `Self`
types in the reference graph.

The second thing here is that when using `Self` to refer to a struct
type, that `Self` was also incorrectly renamed. To fix this we now track
whether a reference to a type came from `Self` or from the actual struct
name. Then we don't rename these `Self` references.

Note that we still keep these `Self` references in order for "find all
references" to return them (this is similar to how rust-analyzer works).


https://github.com/noir-lang/noir/assets/209371/8eeeebd7-816f-4e8f-a6cc-426a5172c1a7

## Additional Context

None.

## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Jul 8, 2024
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
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

1 participant