Skip to content

Conversation

tnowacki
Copy link
Contributor

Description

  • Improve error message for if without an else
  • Persisting through typing for lints

Test plan

  • New test

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Oct 23, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 23, 2024 11:28pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 11:28pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 11:28pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Oct 23, 2024 11:28pm

Vector(Loc, usize, Box<Type>, Box<Exp>),

IfElse(Box<Exp>, Box<Exp>, Box<Exp>),
IfElse(Box<Exp>, Box<Exp>, Option<Box<Exp>>),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need this in typing just for the error message, but I am persisting it for lints

Copy link
Contributor

@tzakian tzakian left a comment

Choose a reason for hiding this comment

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

LGTM. Small nit on error message wording.

subtype(
context,
eloc,
|| "Invalid 'if'. The body of an 'if' without an 'else' must be type '()'",
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nitty nit:

Suggested change
|| "Invalid 'if'. The body of an 'if' without an 'else' must be type '()'",
|| "Invalid 'if'. The body of an 'if' without an 'else' must be of type '()'",

Copy link
Contributor

Choose a reason for hiding this comment

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

or ... must have type '()'

@tnowacki tnowacki enabled auto-merge (squash) October 23, 2024 23:41
@tnowacki tnowacki merged commit 6b69012 into MystenLabs:main Oct 23, 2024
47 checks passed
@tnowacki tnowacki deleted the else-opt branch October 24, 2024 17:08
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