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

Go: Unable to match specific members within a struct initialization when using pattern-inside #5452

Closed
1 of 3 tasks
enncoded opened this issue Jun 7, 2022 · 5 comments · Fixed by #5498
Closed
1 of 3 tasks
Labels
bug Something isn't working lang:golang pattern:inside user:internal requested only by someone within Semgrep Inc.

Comments

@enncoded
Copy link
Contributor

enncoded commented Jun 7, 2022

Describe the bug
I'm having trouble matching specific MemberName: Value pairs when using a pattern-inside along with a pattern to match specific members of a struct.

To Reproduce
https://semgrep.dev/s/68B1

While playing around with it, I noticed that you can match the MemberNames, such as ServerName or InsecureSkipVerify, but not the Values or MemberName: Value pairs.

Expected behavior
The InsecureSkipVerify: true should match

What is the priority of the bug to you?

  • P0: blocking your adoption of Semgrep or workflow
  • P1: important to fix or quite annoying
  • P2: regular bug that should get fixed

Use case
Allow me to write more detailed Go rules

@enncoded enncoded added bug Something isn't working lang:golang pattern:inside user:internal requested only by someone within Semgrep Inc. labels Jun 7, 2022
@r2c-demo
Copy link
Collaborator

r2c-demo commented Jun 7, 2022

This issue is synced in Linear at https://linear.app/r2c/issue/PA-1453/go-unable-to-match-specific-members-within-a-struct-initialization. Note: this link is for r2c use only and is not accessible publicly.

@kopecs
Copy link
Contributor

kopecs commented Jun 8, 2022

Something like https://semgrep.dev/s/j44N should work---we probably don't try and parse a pattern as being a field right now, although we could change that if using a metavariable-pattern & focus-metavariable is an issue.

@enncoded
Copy link
Contributor Author

enncoded commented Jun 8, 2022

Oh thanks! I didn't think of this. I'll see how far I can go with this for now, but I definitely do think that this is important to fix, as narrowing scope as much as possible using pattern-inside and using pattern to match internal stuff is a very common rule pattern.

@aryx
Copy link
Collaborator

aryx commented Jun 12, 2022

We actually do parse the pattern as a field (see -dump_pattern with PartialSingleField). The problem is that the Composite Literal (the tls.Config {xxx}) is internally parsed as a function call with keyword arguments instead of a Record,
and so the PartialSingleField pattern does not match an ArgKwd.
I'll try to fix it by generating a Record. Not sure why we were not doing that in the first place.

@aryx
Copy link
Collaborator

aryx commented Jun 12, 2022

Hmm I think it's because the composite literal can also be written with the "fields" unnamed as long as they are in the same order.

aryx added a commit that referenced this issue Jun 12, 2022
This closes #5452

test plan:
test file included
aryx added a commit that referenced this issue Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lang:golang pattern:inside user:internal requested only by someone within Semgrep Inc.
Development

Successfully merging a pull request may close this issue.

4 participants