enable case-insensitive php tests #3013
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 tasks
akuhlens
force-pushed
the
andre/enable-case-insensitive-php-tests
branch
from
e1f5a68
to
4b2fbd7
Compare
LewisArdern
approved these changes
Jul 27, 2023
|
@akuhlens the way it works is you make a PR to semgrep-rules, and don't care about the CI results (it will fail), then you make a PR in semgrep, with its semgrep-rules submodule updated to point to this PR, and there the CI results should all pass! then you merge in semgrep, wait a few hours, and rerun the CI in semgrep-rules and it should use the latest develop and pass. Feel free to update our Notion page for this process :) |
|
@akuhlens is this still waiting on Semgrep changes? |
|
yes, still waiting. This should not be merged. |
* Improve gitleaks generic api rule * Improve gitleaks generic api rule * fix indent * fix test case * fix test case * fix test todo * add original rule
|
Sorry that this is taking longer than it should. Due to some miscommunication on my part I went down a rabbit whole that was a little too big of scope for the original PR. I am trying to tidy things up and get the original PR and this diff merged as soon as possible. |
…returntocorp/semgrep-rules into andre/enable-case-insensitive-php-tests
|
@LewisArdern Just a heads up. I had to change some testing annotations for secrets to get semgrep's develop branch to run complete all the tests on this side. The changes are included in commit 1ac4d20. As far as I can tell these are all just cases of the test working when it wasn't expected to. |
aryx
pushed a commit
to semgrep/semgrep
that referenced
this pull request
Aug 14, 2023
## What Adds matching support for languages that have case insensitive identifiers and demonstrates their usage for Php. closes #7231 ## How Adds a boolean field to `id_info` fields and updates `Generic_vs_generic.ml` and `Matching_generic.ml` to respect these fields. I originally thought it would be easier to add a special case for Php in matching, but extending `Matching_generic.ml` to be language specific becomes troublesome because `equal_ast_bound_code` is called from outside this submodules in contexts that could possibly be addressing variables for multiple languages (or at least that was my take on the situation and types at play). ## Remaining Work To Do This was shared as a draft to communicate my work and get feedback about how to add the bitfield to id_info and update the submodule. - [x] Modify `id_info` to contain a `id_flags` field that contains a bitfield instead of having two separate boolean fields (id_case_insensitive, and id_hidden). - [x] Clean up code and document purpose. - [x] Add change log entry. - [x] Submit pull request for `semgrep-rules` submodule and update submodule to point to main branch again. (Not actually sure the correct order to do this without breaking things). [(ongoing here)](semgrep/semgrep-rules#3013) ## Testing Adds a few test cases matching against identifiers and metavariables in a case insensitive fashion and updates `tests/semgrep-rules` that were disabled due lack of support for this. Note, `tests/semgrep-rules` is currently pointing to a branch that I need to open a pull request for. 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/)
…returntocorp/semgrep-rules into andre/enable-case-insensitive-php-tests
cretoxyrhina
pushed a commit
to cretoxyrhina/semgrep
that referenced
this pull request
Oct 17, 2023
…ep#8356) ## What Adds matching support for languages that have case insensitive identifiers and demonstrates their usage for Php. closes semgrep#7231 ## How Adds a boolean field to `id_info` fields and updates `Generic_vs_generic.ml` and `Matching_generic.ml` to respect these fields. I originally thought it would be easier to add a special case for Php in matching, but extending `Matching_generic.ml` to be language specific becomes troublesome because `equal_ast_bound_code` is called from outside this submodules in contexts that could possibly be addressing variables for multiple languages (or at least that was my take on the situation and types at play). ## Remaining Work To Do This was shared as a draft to communicate my work and get feedback about how to add the bitfield to id_info and update the submodule. - [x] Modify `id_info` to contain a `id_flags` field that contains a bitfield instead of having two separate boolean fields (id_case_insensitive, and id_hidden). - [x] Clean up code and document purpose. - [x] Add change log entry. - [x] Submit pull request for `semgrep-rules` submodule and update submodule to point to main branch again. (Not actually sure the correct order to do this without breaking things). [(ongoing here)](semgrep/semgrep-rules#3013) ## Testing Adds a few test cases matching against identifiers and metavariables in a case insensitive fashion and updates `tests/semgrep-rules` that were disabled due lack of support for this. Note, `tests/semgrep-rules` is currently pointing to a branch that I need to open a pull request for. 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/)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates Php tests to allow case insensitive identifiers. This corresponds to the addition of the feature in PR #8356 in semgrep repo.
I am not aware of the best way to smoothly transition both repos in tandem. So I would love any assistance you could provide on that front.