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.

Sign up for GitHub

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

Jump to bottom

Ruby: pattern matching #7154

Merged
merged 13 commits into from
Dec 1, 2021
Merged

Ruby: pattern matching #7154

merged 13 commits into from
Dec 1, 2021

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Nov 17, 2021 •
edited
Loading

This pull request updates the Ruby grammar to tree-sitter/tree-sitter-ruby#193

@github-actions github-actions bot added the Ruby label Nov 17, 2021
@aibaars aibaars force-pushed the ruby-pattern-matching branch 2 times, most recently from 2e9b57a to b602549 Compare November 17, 2021 11:07
@aibaars aibaars changed the title Ruby pattern matching Ruby: pattern matching Nov 17, 2021
@aibaars aibaars force-pushed the ruby-pattern-matching branch 6 times, most recently from 93ce829 to 157877e Compare November 22, 2021 17:52
@aibaars aibaars marked this pull request as ready for review November 23, 2021 14:57
@aibaars aibaars requested a review from a team as a code owner November 23, 2021 14:57
@aibaars aibaars force-pushed the ruby-pattern-matching branch 5 times, most recently from 5bd7bde to 5ca8fe7 Compare November 23, 2021 21:03
@aibaars aibaars force-pushed the ruby-pattern-matching branch from 5ca8fe7 to cec34a5 Compare November 23, 2021 22:04
@aibaars aibaars force-pushed the ruby-pattern-matching branch from cec34a5 to c3112c4 Compare November 24, 2021 16:18
nickrolfe
nickrolfe requested changes Nov 24, 2021
View reviewed changes
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.

I haven't reviewed the tests yet, but looks very sensible so far. I found an issue in the upgrade script, plus some comment suggestions.

ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Outdated Show resolved Hide resolved
ruby/ql/lib/upgrades/f36dd8a35ce55a3b93a178e38a79b8e8cbea7463/ruby_tokeninfo.ql Outdated Show resolved Hide resolved
hvitved
hvitved requested changes Nov 24, 2021
View reviewed changes
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 very solid to me!

As for the CFG inconsistencies: that is expected, so for now we should simply commit library-tests/ast/CONSISTENCY/CfgConsistency.expected and library-tests/ast/control/CONSISTENCY/CfgConsistency.ql. Then once we actually implement the CFG logic, those inconsistencies should disappear.

ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Pattern.qll Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Literal.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/internal/AST.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
@aibaars aibaars force-pushed the ruby-pattern-matching branch from 96562cd to 2086927 Compare November 25, 2021 11:32
@aibaars aibaars force-pushed the ruby-pattern-matching branch 2 times, most recently from e8d78e2 to de6f5d6 Compare November 25, 2021 11:58
@aibaars aibaars force-pushed the ruby-pattern-matching branch from de6f5d6 to 53011f6 Compare November 25, 2021 12:25
@aibaars aibaars force-pushed the ruby-pattern-matching branch from 53011f6 to f836b47 Compare November 25, 2021 12:46
@aibaars aibaars force-pushed the ruby-pattern-matching branch from f836b47 to 8b0bc67 Compare November 25, 2021 12:50
hvitved
hvitved reviewed Nov 29, 2021
View reviewed changes
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
ruby/ql/lib/codeql/ruby/ast/Control.qll Outdated Show resolved Hide resolved
@aibaars aibaars force-pushed the ruby-pattern-matching branch from b1c80fd to ea25dd9 Compare November 30, 2021 11:56
@aibaars aibaars force-pushed the ruby-pattern-matching branch from ea25dd9 to 830908b Compare November 30, 2021 13:06
@aibaars aibaars requested review from nickrolfe and removed request for nickrolfe December 1, 2021 11:08
@aibaars aibaars merged commit e41cd81 into github:main Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickrolfe nickrolfe nickrolfe approved these changes

@hvitved hvitved hvitved approved these changes

Assignees
No one assigned
Labels
documentation Ruby
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aibaars @nickrolfe @hvitved