-
Notifications
You must be signed in to change notification settings - Fork 28
refactor: remove depends_on fields from heuristics and handle skips in problog #1133
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
base: main
Are you sure you want to change the base?
Conversation
15a0c52
to
24d224e
Compare
mock_defaults.__getitem__.side_effect = lambda section: ( | ||
MagicMock(get=lambda _: None) if section == "heuristic.pypi" else None | ||
) | ||
defaults["heuristic.pypi"].clear() |
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.
What is the scenario where this case happen? For instance, what would be the values in the user provided defaults.ini
(if any).
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.
Our defaults.ini
usually looks like this:
disabled_default_rulesets = exfiltration
disabled_rules =
custom_semgrep_rules_path =
disabled_custom_rulesets =
The reason I used .clear()
here is to make sure that the rules that are being tested aren't disabled. Particularly here, by default our exfiltration
ruleset is disabled, but it is tested in this unit test, so the test would fail if I didn't remove that setting.
Signed-off-by: Carl Flottmann <[email protected]>
…nges, modified docstrings.
Signed-off-by: Carl Flottmann <[email protected]>
0c5b3b7
to
08bcd50
Compare
Summary
The purpose of this PR is to address #1052, and implement appropriate skip and error handling in all existing heuristics. This PR removes the
depends_on
field from heuristics and makes it a responsibility of the heuristic code to determine if it should be skipped or not. All skip handling is then done my the ProbLog logic.Description of changes
The
BaseHeuristicAnalyzer
no longer has thedepends_on
field, and this has propagated to all other analyzers. The description for aSKIP
result has been updated in line with issue #1052, andHeuristicAnalyzerValueError
s have replaced someSKIP
results previously used when malformed data was encountered. Unit tests have been updated accordingly.Some refactoring to the
test_pypi_sourcecode_analyzer.py
unit tests have also been done in this PR to address #1108, adding in temporary.ini
config files to the unit tests as opposed to the existing mock objects.Related issues
Closes #1052 and #1108.
Checklist
verified
label should appear next to all of your commits on GitHub.