8000 [prework] Add types to struct initialize by azdavis · Pull Request #1024 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[prework] Add types to struct initialize #1024

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

Merged
merged 14 commits into from
Jun 24, 2019

Conversation

azdavis
Copy link
Contributor
@azdavis azdavis commented Jun 24, 2019

This is pre-work for #986, which was getting to be big. @DarkDimius requested that the work which doesn't change the observable behavior of the statics be pulled out into pre-work. When this is merged, #986 will be rebased on top of it.

Things changed:

  • Rm unused include
  • Avoid if !cond then ... else ...
  • Instead of returning a std::vector with empty meaning "no value", return an std::optional with std::nullopt meaning "no value" and ENFORCE that if the optional contains a vector, that vector is not empty
  • Introduce NodesAndProp (but don't put anything in Prop yet, that's for later)
  • Update an outdated comment
  • Fix misc ws and typos
  • Use a T helper to be more concise
  • Update vscode settings to treat .rbi as Ruby

Motivation

To prepare for #986.

Test plan

No changes to behavior. No tests.

azdavis added 10 commits June 24, 2019 09:37
It is ENFORCEd that if ret == SOME(x) then x != [].

This is in preparation for allowing ChalkODMProp::replaceDSL to return
something other than the vector<unique_ptr<ast::Expression>>.
Previously, because the only thing we were returning was that vector, we
could use the empty vector to signify "no desugaring occurred".
The old comment seemed to be inaccurate in multiple places. This
attempts to make it more accurate.

Of course, it's likely that the old comment was probably accurate at one
point and became inaccurate over time as the code changed. The same
could happen to this new comment. And since the inaccuracy of the old
comment wasn't noticed until now, it's arguable that we could remove the
comment altogether and not lose much.
@azdavis azdavis requested review from jez and a team June 24, 2019 17:01
@ghost ghost removed their request for review June 24, 2019 17:01
Copy link
Collaborator
@jez jez left a comment

Choose a reason for hiding this comment

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

Few comments, other than those, looks good.

Thanks for breaking this out!

@azdavis azdavis merged commit c553e08 into master Jun 24, 2019
@azdavis azdavis deleted the azdavis/add-types-to-struct-initialize-prework branch June 24, 2019 17:54
This was referenced Jun 24, 2019
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
0