Sitelet https://web.archive.org/web/20260605071249/https://github.com/github/codeql/pull/12540
Skip to content

Ruby: change evaluation order of destructured assignments#12540

Merged
aibaars merged 5 commits into
github:mainfrom
aibaars:destructured-assign
Mar 27, 2023
Merged

Ruby: change evaluation order of destructured assignments#12540
aibaars merged 5 commits into
github:mainfrom
aibaars:destructured-assign

Conversation

@aibaars
Copy link
Copy Markdown
Contributor

@aibaars aibaars commented Mar 15, 2023

No description provided.

@github-actions github-actions Bot added the Ruby label Mar 15, 2023
@aibaars aibaars marked this pull request as ready for review March 20, 2023 09:41
@aibaars aibaars requested a review from a team as a code owner March 20, 2023 09:41
@calumgrant calumgrant requested a review from hvitved March 20, 2023 09:44
@aibaars aibaars force-pushed the destructured-assign branch from a208919 to bb6cc3b Compare March 20, 2023 10:20
hvitved
hvitved previously approved these changes Mar 21, 2023
Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

Looks correct to me. Let's do a DCA run before merging.

Copy link
Copy Markdown
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

One minor comment, otherwise LGTM. We should also do another DCA run.

}
}

private class LhsScopedConstant extends LhsWithReceiver, TScopeResolutionConstantAccess {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be better to move the existing (private) ScopeResolutionConstantAccess class into internal/Constant.qll, and then reference that here.

Comment thread ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll Fixed
Comment thread ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll Fixed
Comment thread ruby/ql/lib/codeql/ruby/ast/internal/Synthesis.qll Outdated
@aibaars aibaars force-pushed the destructured-assign branch from 006ff98 to c4a98ff Compare March 24, 2023 09:47
hvitved
hvitved previously approved these changes Mar 24, 2023
@aibaars aibaars force-pushed the destructured-assign branch from 5ebf308 to 3b12ddf Compare March 27, 2023 09:01
@aibaars aibaars merged commit 4964f86 into github:main Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants