New input type for semgrep-core allows taking scanning roots instead of target files #337
+1,636
−83
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
|
Backwards compatibility summary: |
aryx
approved these changes
Jan 14, 2025
| always_select_explicit_targets : bool; | ||
| (* This is a hash table in Find_targets.conf: *) | ||
| explicit_targets : string list; | ||
| (* osemgrep-only: option (see Git_project.ml and the force_root parameter) *) |
There was a problem hiding this comment.
if those are osemgrep-only, then we could remove those options from here if pysemgrep
will never generate them?
There was a problem hiding this comment.
Which would remove the need to have to define project_root above.
mjambon
force-pushed
the
martin/find-targets-in-core
branch
from
24d4b87 to
bd2cf34
Compare
|
BTW would be great after all this work to merge the semgrep-interfaces repo in semgrep-pro :) Those double PRs |
|
This is annoying: It's a legitimate incompatibility report but it's about the semgrep-core interface that we can break at will. It wouldn't happen if we could split the ATD file into multiple modules each concerned with a different interface. Maybe for now we could have some kind of hack to work around this. Atddiff offers an option to only check certain types so we could use this but it's not great for the long term (since adding a new interface would require adding the new root types to the list of types to check). By increasing difficulty (and cleanliness), we have:
|
|
Yes I've asked a few times for the ability to white list a few roots to not bother to check anything that descend from RPC or semgrep-core stuff. |
|
whitelist or blacklist. In this case we probably want atddiff --do-not-bother-about-those-roots 'function_call,function_return,core_output' Note that core_output also deeply use cli_output_extra, but this cli_output_extra is also used by cli_output so we should |
I added an option to atddiff to ensure that we don't miss future type definitions. atd PR coming soon. |
We'll need to add an option to make this safer. See TODO.
mjambon
force-pushed
the
martin/find-targets-in-core
branch
from
c0bf503 to
391c610
Compare
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.
This is used by https://github.com/semgrep/semgrep-proprietary/pull/2878.
make setup && maketo update the generated code after editing a.atdfile (TODO: have a CI check)For example, the Semgrep backend need to still be able to consume data
generated by Semgrep 1.50.0.
See https://atd.readthedocs.io/en/latest/atdgen-tutorial.html#smooth-protocol-upgrades
Note that the types related to the semgrep-core JSON output or the
semgrep-core RPC do not need to be backward compatible!