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

Ruby 3.1 features #7753

Merged
merged 12 commits into from
Feb 6, 2022
Merged

Ruby 3.1 features #7753

merged 12 commits into from
Feb 6, 2022

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Jan 26, 2022

This pull request updates the tree-sitter grammar with the following Ruby 3.1 features

  • Values in Hash literals and keyword arguments can be omitted
  • Anonymous block argument
  • Expressions and non-local variables allowed in pin operator ^

See also: tree-sitter/tree-sitter-ruby#201

@github-actions github-actions bot added the Ruby label Jan 26, 2022
@aibaars aibaars force-pushed the ruby-3.1 branch 7 times, most recently from 3b5b4f5 to a29d905 Compare January 27, 2022 16:17
@aibaars aibaars force-pushed the ruby-3.1 branch 5 times, most recently from 5809718 to e3e88bd Compare January 28, 2022 10:04
@aibaars aibaars marked this pull request as ready for review January 28, 2022 18:49
@aibaars aibaars requested a review from a team as a code owner January 28, 2022 18:49
Copy link
Contributor

@nickrolfe nickrolfe left a comment

Choose a reason for hiding this comment

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

Looks great overall - just a couple of questions. Also, I didn't feel very confident reviewing the changes to Synthesis.qll, so it might be good to take a second pair of eyes on those.

Copy link
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 great! Remember to do a DCA run.


/** Gets the variable access corresponding to this variable reference pattern. */
LocalVariableReadAccess getVariableAccess() { toGenerated(result) = g.getName() }
/** Gets the value this reference pattern matches against. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename to getExpr?
Also, I think we should add an example, something like For example `2 * x` in `^(2 * x)`


final override string toString() { result = "^..." }

final override AstNode getAChild(string pred) {
override AstNode getAChild(string pred) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can remain final.

}
}

private module AnonymousBlockParameterSynth {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add ql doc, something like

/**
 * ```rb
 * def foo(&)
 *   bar(&)
 * end
 * ```
 * desugars to,
 * ```rb
 * def foo(&__synth_0)
 *   bar(&__synth_0)
 * end
 * ```
 */

@aibaars
Copy link
Contributor Author

aibaars commented Feb 4, 2022

@hvitved Are you happy with this PR and the DCA results?

@nickrolfe
Copy link
Contributor

I had a quick look at the DCA results and it looks like there are no alert changes and the analysis times didn't change significantly.

@aibaars aibaars merged commit ac03fab into github:main Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants