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
Integrate fast v2 #2836
Integrate fast v2 #2836
Conversation
First step in #2773 |
error in CI: pkg_resources.DistributionNotFound: The 'ruamel.yaml.clib>=0.1.2; platform_python_implementation == "CPython" and python_version < "3.9"' distribution was not found and is required by ruamel.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great!
I actually also get the ruamel CI error in an unrelated PR: #2839 |
|
||
outputs: Dict[Rule, List[RuleMatch]] = defaultdict(list) | ||
errors: List[SemgrepError] = [] | ||
for rule, language in tuple( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Later on we can try to pass multiple rules at the same time if they have the same language and same set of target files
so we can start to dedup identical patterns and even even identical subformulas used in different rules (e.g., in nodejsscan, ajin reuse many times the same subformulas).
cmd = [SEMGREP_PATH] + [ | ||
"-lang", | ||
language, | ||
"-fast", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory -fast is another thing, but we can bundle it in --experimental.
I've merged develop @minusworld to fix some CI errors, so don't forget to pull the latest in your branch. |
@@ -324,6 +324,11 @@ def cli() -> None: | |||
"containing a 'nosem' comment at the end." | |||
), | |||
) | |||
output.add_argument( | |||
"--experimental", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we name this something less generic like --core-direct-rules
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's probably fine; this is really only meant for internal use. I think Pad was thinking that this would be an entrypoint for future experimental work, too. In any case, we can always change it later - it's just a string after all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, --core-direct-rules sounds good.
No description provided.