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 underscores as unused variable identifiers #338

Merged
merged 5 commits into from
Jun 14, 2024

Conversation

Gusarich
Copy link
Member

@Gusarich Gusarich commented May 15, 2024

Closes #319

  • I have updated CHANGELOG.md
  • I have added tests to demonstrate the contribution is correctly implemented: this usually includes both positive and negative tests, showing the happy path(s) and featuring intentionally broken cases
  • I have run all the tests locally and no test failure was reported
  • I did not do unrelated and/or undiscussed refactorings

@Gusarich Gusarich added this to the v1.4.0 milestone May 15, 2024
@Gusarich Gusarich requested a review from anton-trunov May 15, 2024 13:58
Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

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

Let's fix the merge conflicts here

@novusnota
Copy link
Member

As of now, it only works for foreach and catch, right?

@Gusarich
Copy link
Member Author

As of now, it only works for foreach and catch, right?

Are there any other cases where we need it?

@novusnota
Copy link
Member

novusnota commented Jun 10, 2024

Are there any other cases where we need it?

I don't think so, at least now. When we'll have #298 in the future, identifiers in let declarations would need them though.

@anton-trunov
Copy link
Member

anton-trunov commented Jun 10, 2024

When we'll have #298 in the future, identifiers in let declarations would need them though.

Actually, let-statements should have underscores even now. This can be used as a clear indication that some function gets called not for its return value but for its side-effects:

let _ : Foo = effectfulFun(x, y, z);

When we have a bit of type inference, the snippet above will become

let _ = effectfulFun(x, y, z);

In this case a static analyzer won't complain about unused values, whereas it should issue a warning for

effectfulFun(x, y, z);

if effectfulFun is a non-void function.

@Gusarich
Copy link
Member Author

Added support for let-statements and also resolved all merge conflicts

@Gusarich Gusarich requested a review from anton-trunov June 11, 2024 10:00
@anton-trunov anton-trunov self-assigned this Jun 12, 2024
@Gusarich
Copy link
Member Author

resolved merge conflicts

@Gusarich
Copy link
Member Author

@anton-trunov resolved merge conflicts yet again after merge of #198

Copy link
Member

@anton-trunov anton-trunov left a comment

Choose a reason for hiding this comment

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

Almost there!

src/types/__snapshots__/resolveStatements.spec.ts.snap Outdated Show resolved Hide resolved
src/types/resolveStatements.ts Outdated Show resolved Hide resolved
src/types/resolveStatements.ts Outdated Show resolved Hide resolved
src/types/resolveStatements.ts Outdated Show resolved Hide resolved
@Gusarich
Copy link
Member Author

Gusarich commented Jun 14, 2024

@anton-trunov have a look

@Gusarich Gusarich requested a review from anton-trunov June 14, 2024 10:40
@anton-trunov anton-trunov merged commit c4ffbf0 into tact-lang:main Jun 14, 2024
3 checks passed
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.

Allow underscores as unused variable identifiers
3 participants