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

Do not remove findings with same range but different metavariables #3310

Merged
merged 4 commits into from Jun 29, 2021

Conversation

aryx
Copy link
Collaborator

@aryx aryx commented Jun 8, 2021

This closes #3279

test plan:
test file included
and
semgrep --config ~/yy/tests/OTHER/rules/misc_field.yaml
~/yy/tests/OTHER/rules/misc_field.js

returns now 22 findings (same range though).

PR checklist:

  • changelog is up to date

This closes #3279

test plan:
test file included
and
semgrep --config ~/yy/tests/OTHER/rules/misc_field.yaml
~/yy/tests/OTHER/rules/misc_field.js

returns now 22 findings (same range though).
@aryx aryx requested review from emjin, IagoAbal and mjambon June 8, 2021 16:11
@aryx
Copy link
Collaborator Author

aryx commented Jun 8, 2021

(semgrep) [pad@yrax semgrep (no_dedup)]$ semgrep --config ~/yy/tests/OTHER/rules/misc_field.yaml ~/yy/tests/OTHER/rules/misc_field.js
running 1 rules...
/home/pad/yy/tests/OTHER/rules/misc_field.js
severity:warning rule:home.pad.yy.tests.OTHER.rules.my_pattern_id: "web-vitals" looks like a new dependency! Please check this on https://deps.dev/npm/"web-vitals"/"^0.2.4" to make sure its license is legit, doesn't have lots of known security issues, and that it has enough users.

2:{
3:  "name": "frontend",
4:  "version": "0.1.0",
5:  "private": true,
6:  "dependencies": {
7:    "@fortawesome/fontawesome-svg-core": "^1.2.32",
8:    "@fortawesome/free-regular-svg-icons": "^5.15.1",
9:    "@fortawesome/free-solid-svg-icons": "^5.15.1",
10:    "@fortawesome/react-fontawesome": "^0.1.12",
11:    "@testing-library/jest-dom": "^5.11.4",
-------- [hid 19 additional lines, adjust with --max-lines-per-finding] --------
severity:warning rule:home.pad.yy.tests.OTHER.rules.my_pattern_id: "typescript" looks like a new dependency! Please check this on https://deps.dev/npm/"typescript"/"^4.0.3" to make sure its license is legit, doesn't have lots of known security issues, and that it has enough users.

2:{
3:  "name": "frontend",
4:  "version": "0.1.0",
5:  "private": true,
6:  "dependencies": {
7:    "@fortawesome/fontawesome-svg-core": "^1.2.32",
8:    "@fortawesome/free-regular-svg-icons": "^5.15.1",
9:    "@fortawesome/free-solid-svg-icons": "^5.15.1",
10:    "@fortawesome/react-fontawesome": "^0.1.12",
11:    "@testing-library/jest-dom": "^5.11.4",
-------- [hid 19 additional lines, adjust with --max-lines-per-finding] --------
...

@aryx aryx requested a review from ievans June 8, 2021 16:12
@emjin
Copy link
Contributor

emjin commented Jun 8, 2021

@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2021

Potential non-blocking slowdown latest time 3.996 is over 12 percent slower than baseline 3.519. See run output for more details

@aryx
Copy link
Collaborator Author

aryx commented Jun 8, 2021

We got a big release, so This is potentially a big change so I wait for the release cut to merge this.

@aryx aryx changed the title Do not remove findings with same range but different metavariables [DO NOT MERGE YET] Do not remove findings with same range but different metavariables Jun 9, 2021
@stale
Copy link

stale bot commented Jun 23, 2021

This pull request is being marked stale because there hasn't been any activity in 30 days. Please leave a comment on the pull request to keep it open. Otherwise, we'll close this pull request in 7 days (you can always reopen it later).

@stale stale bot added the stale needs prioritization; label applied by stalebot label Jun 23, 2021
Copy link
Member

@ievans ievans left a comment

Choose a reason for hiding this comment

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

I don't see any test that has 22 extra findings @aryx, am I missing something?

@stale stale bot removed the stale needs prioritization; label applied by stalebot label Jun 23, 2021
@aryx aryx changed the title [DO NOT MERGE YET] Do not remove findings with same range but different metavariables Do not remove findings with same range but different metavariables Jun 29, 2021
@aryx aryx merged commit e207b31 into develop Jun 29, 2021
@aryx aryx deleted the no_dedup branch June 29, 2021 14:02
@aryx
Copy link
Collaborator Author

aryx commented Jun 30, 2021

This works now on develop: https://semgrep.dev/s/2b25?version=develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

matching keys and values in JavaScript objects
4 participants