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

Allow assignment of immutable variables #838

Merged
merged 4 commits into from
Jul 10, 2023
Merged

Conversation

frangio
Copy link
Contributor

@frangio frangio commented Jul 5, 2023

Assignment to immutable variables currently requires both overrides state-variable-immutable and state-variable-assignment. I believe the second one should be implicit. Assignment is a problem when it's in storage.

@frangio frangio requested a review from ericglau July 5, 2023 22:01
msg: e => `Variable \`${e.name}\` is immutable and will be initialized on the implementation`,
hint: () =>
`If by design, annotate with '@custom:oz-upgrades-unsafe-allow state-variable-immutable'\n` +
`Otherwise, use a mutable variable instead`,
Copy link
Member

Choose a reason for hiding this comment

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

Why is "use a constant" no longer one of the suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No strong reason, I've added it back.

@ericglau
Copy link
Member

ericglau commented Jul 7, 2023

Can you also update the changelog in core?

@frangio frangio requested a review from ericglau July 7, 2023 23:48
@frangio
Copy link
Contributor Author

frangio commented Jul 10, 2023

@ericglau Let's merge this and release this when you can. I want to make the equivalent change in the transpiler (override flags are used to control transpilation).

@ericglau ericglau merged commit ceeafbb into master Jul 10, 2023
7 checks passed
@ericglau ericglau deleted the immutable-assignment branch July 10, 2023 19:11
@ericglau
Copy link
Member

@frangio Released in @openzeppelin/[email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants