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

Regression in matching vardefs with expression patterns in 0.18 #1497

Closed
DrewDennison opened this issue Aug 12, 2020 · 2 comments · Fixed by #1527
Closed

Regression in matching vardefs with expression patterns in 0.18 #1497

DrewDennison opened this issue Aug 12, 2020 · 2 comments · Fixed by #1527
Assignees
@nbrahms
Labels
bug Something isn't working user:external requested by someone outside of r2c

Comments

@DrewDennison
Copy link
Member

Describe the bug
Regression in the matching logic in 0.18 caught by @ajinabraham's integration tests: https://semgrep.dev/7g0Q?version=0.18.0

To Reproduce
Steps to reproduce the behavior, ideally a link to https://semgrep.dev: https://semgrep.dev/7g0Q?version=0.18.0 vs https://semgrep.dev/7g0Q?version=0.17.0

Expected behavior
$X = '...'; should match var $X = '...'; in javacript
@DrewDennison DrewDennison added bug Something isn't working user:external requested by someone outside of r2c labels Aug 12, 2020
@nbrahms nbrahms changed the title Regression in matching equivalent statements in 0.18 Regression in matching vardefs with expression patterns in 0.18 Aug 17, 2020
@nbrahms
Copy link
Contributor

nbrahms commented Aug 17, 2020

This appears to be due to semgrep/pfff@78f052659671. This converts ExprStmt nodes into pure expressions.

However, we don't convert VarDef nodes into Expr nodes when matching against the program, so we no longer match.

IMO, we should perform the VarDef conversion instead of reverting 78f052659671, as this would also allow us to match with or without the trailing semicolon.

cc. @aryx

nbrahms pushed a commit that referenced this issue Aug 17, 2020
Update pfff to match vardefs as exprs
c8af9ee 
Also add and update test cases.

Fixes #1497.
@nbrahms nbrahms mentioned this issue Aug 17, 2020
Closed
nbrahms pushed a commit that referenced this issue Aug 18, 2020
Update pfff to match vardefs as exprs
89c785b 
Also add and update test cases.

Fixes #1497.
@nbrahms nbrahms mentioned this issue Aug 18, 2020
Merged
nbrahms pushed a commit that referenced this issue Aug 18, 2020
Update pfff to match vardefs as exprs
7176930 
Also add and update test cases.

Fixes #1497.
nbrahms added a commit that referenced this issue Aug 18, 2020
@nbrahms
Update pfff to match vardefs as exprs (#1527)
5eff14f 
Also add and update test cases.

Fixes #1497.
@nbrahms
Copy link
Contributor

nbrahms commented Aug 18, 2020

This will be fixed in 0.20.0.

As a bonus, the trailing semicolon will no longer be required in this type of pattern:
https://semgrep.dev/7g0Q?version=develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nbrahms nbrahms

Labels
bug Something isn't working user:external requested by someone outside of r2c
Milestone
No milestone
2 participants
@DrewDennison @nbrahms