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

Can't use autofix to remove some code #6318

Closed
aryx opened this issue Oct 14, 2022 · 5 comments
Closed

Can't use autofix to remove some code #6318

aryx opened this issue Oct 14, 2022 · 5 comments
Labels
bug Something isn't working feature:autofix priority:low user:internal requested only by someone within Semgrep Inc.

Comments

@aryx
Copy link
Collaborator

aryx commented Oct 14, 2022

with this rule:

 - id: python-typing-osemgrep
   pattern: from typing import $X
   fix: ""
   languages: [ python ]
   message: found one
   severity: ERROR

I can't use it on some codebase to remove all the typing imports.
semgrep --config ~/refactoring.yaml . --autofix does not apply any fixes.
@aryx aryx added priority:low feature:autofix user:internal requested only by someone within Semgrep Inc. labels Oct 14, 2022
@r2c-demo
Copy link
Collaborator

This issue is synced in Linear at https://linear.app/r2c/issue/PA-2000/cant-autofix-to-remove-some-code. Note: this link is for r2c use only and is not accessible publicly.

@aryx aryx changed the title Can't autofix to remove some code Can't use autofix to remove some code Oct 16, 2022
@IagoAbal IagoAbal added the bug Something isn't working label Oct 18, 2022
@alanisaac
Copy link

I'm also looking for the same feature. I think the thread in this issue is related: #5674

@joshlory
Copy link

joshlory commented Apr 3, 2023

Without this it's hard to use Semgrep for code cleanup. @nmote I think the line you're looking for is here?

https://github.com/returntocorp/semgrep/blob/33d49d5d351601598c882eb3fa9f758c441ea101/cli/src/semgrep/core_output.py#L168

I don't think AST-based autofix would affect it. It doesn't affect fix-regex at all, since fix-regex is by definition text-based. I bet this is due to a Python truthiness test somewhere like if replacement:, since the empty string is falsy. I bet that finding that spot and replacing if replacement: with if replacement is not None: would address this issue.

#5674 (comment)

@nmote
Copy link
Collaborator

nmote commented Apr 3, 2023

@joshlory that looks pretty likely. Are you interested in putting up a PR?

ihji added a commit to ihji/semgrep that referenced this issue May 9, 2023
@ihji
cli: Allow empty string in the fix field semgrep#6318
005daed
nmote pushed a commit that referenced this issue May 11, 2023
@ihji
cli: Allow empty string in the fix field #6318 (#7772)
f7ebf72 
PR checklist:

- [x] Purpose of the code is [evident to future
readers](https://semgrep.dev/docs/contributing/contributing-code/#explaining-code)
- [x] Tests included or PR comment includes a reproducible test plan
- [x] Documentation is up-to-date
- [x] A changelog entry was [added to
changelog.d](https://semgrep.dev/docs/contributing/contributing-code/#adding-a-changelog-entry)
for any user-facing change
- [x] Change has no security implications (otherwise, ping security
team)

If you're unsure about any of this, please see:

- [Contribution
guidelines](https://semgrep.dev/docs/contributing/contributing-code)!
- [One of the more specific guides located
here](https://semgrep.dev/docs/contributing/contributing/)
@nmote
Copy link
Collaborator

nmote commented May 11, 2023

Fixed by #7772

@nmote nmote closed this as completed May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working feature:autofix priority:low user:internal requested only by someone within Semgrep Inc.
Milestone
No milestone
Development

No branches or pull requests
7 participants
@aryx @joshlory @IagoAbal @nmote @alanisaac @r2c-demo and others