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

fix(gnovm): improve error message for nil assignment in variable declaration #3068

Merged
merged 20 commits into from
Dec 10, 2024

Conversation

omarsy
Copy link
Member

@omarsy omarsy commented Nov 4, 2024

…aration

Contributors' checklist...
  • Added new tests, or not needed, or not feasible
  • Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory
  • Updated the official documentation or not needed
  • No breaking changes were made, or a BREAKING CHANGE: xxx message was included in the description
  • Added references to related issues and PRs
  • Provided any useful hints for running manual tests

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Nov 4, 2024
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

Attention: Patch coverage is 88.96552% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
gnovm/pkg/gnolang/types.go 52.63% 9 Missing ⚠️
gnovm/pkg/gnolang/preprocess.go 93.61% 6 Missing ⚠️
gnovm/pkg/gnolang/type_check.go 96.87% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@omarsy omarsy force-pushed the fix/improve-nil-not-assignable branch from f8443e5 to a67d677 Compare November 4, 2024 22:57
gnovm/tests/files/assign29.gno Outdated Show resolved Hide resolved
@ltzmaxwell
Copy link
Contributor

ltzmaxwell commented Nov 5, 2024

can you add some more tests like a + nil, etc. ? I guess there would be no more side effect but it's still worth to be clearer.

@omarsy omarsy force-pushed the fix/improve-nil-not-assignable branch from 3cec7e2 to 5b171a4 Compare November 12, 2024 22:13
@omarsy omarsy force-pushed the fix/improve-nil-not-assignable branch from 5b171a4 to 6be8a9a Compare November 12, 2024 22:15
@omarsy omarsy marked this pull request as ready for review November 15, 2024 20:10
@Kouteki Kouteki requested a review from ltzmaxwell November 17, 2024 03:52
@Kouteki Kouteki added the in focus Core team is prioritizing this work label Nov 28, 2024
@omarsy omarsy changed the title fix(gnovm): improve error message for nil assignment in variable decl… fix(gnovm): improve error message for nil assignment in variable declaration Dec 3, 2024
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Dec 3, 2024

I'm a bot that assists the Gno Core team in maintaining this repository. My role is to ensure that contributors understand and follow our guidelines, helping to streamline the development process.

The following requirements must be fulfilled before a pull request can be merged.
Some requirement checks are automated and can be verified by the CI, while others need manual verification by a staff member.

These requirements are defined in this configuration file.

Automated Checks

🟢 Maintainers must be able to edit this pull request (more info)
🔴 The pull request head branch must be up-to-date with its base (more info)

Manual Checks

No manual checks match this pull request.

Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 The pull request was created from a fork (head branch repo: omarsy/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

The pull request head branch must be up-to-date with its base (more info)

If

🟢 Condition met
└── 🟢 On every pull request

Then

🔴 Requirement not satisfied
└── 🔴 Head branch (omarsy:fix/improve-nil-not-assignable) is up to date with base (master): behind by 17 / ahead by 19

Copy link
Member

@thehowl thehowl left a comment

Choose a reason for hiding this comment

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

leaving @ltzmaxwell for final review/merge

gnovm/pkg/gnolang/type_check_test.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/types.go Outdated Show resolved Hide resolved
Copy link
Contributor

@ltzmaxwell ltzmaxwell left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for fixing this.

gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/preprocess.go Outdated Show resolved Hide resolved
gnovm/pkg/gnolang/preprocess.go Show resolved Hide resolved
gnovm/pkg/gnolang/type_check.go Show resolved Hide resolved
@ltzmaxwell ltzmaxwell merged commit bb38fb1 into gnolang:master Dec 10, 2024
39 checks passed
r3v4s pushed a commit to gnoswap-labs/gno that referenced this pull request Dec 10, 2024
…aration (gnolang#3068)

…aration

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: Morgan <[email protected]>
Co-authored-by: ltzmaxwell <[email protected]>
r3v4s pushed a commit to gnoswap-labs/gno that referenced this pull request Dec 10, 2024
…aration (gnolang#3068)

…aration

<!-- please provide a detailed description of the changes made in this
pull request. -->

<details><summary>Contributors' checklist...</summary>

- [ ] Added new tests, or not needed, or not feasible
- [ ] Provided an example (e.g. screenshot) to aid review or the PR is
self-explanatory
- [ ] Updated the official documentation or not needed
- [ ] No breaking changes were made, or a `BREAKING CHANGE: xxx` message
was included in the description
- [ ] Added references to related issues and PRs
- [ ] Provided any useful hints for running manual tests
</details>

---------

Co-authored-by: Morgan <[email protected]>
Co-authored-by: ltzmaxwell <[email protected]>
@omarsy omarsy deleted the fix/improve-nil-not-assignable branch December 10, 2024 16:33
@Kouteki Kouteki removed the in focus Core team is prioritizing this work label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants