Skip to content
This repository has been archived by the owner on Nov 11, 2019. It is now read-only.

staticanalysis/bot: Make reliability index more easy to be understood. #2127

Merged
merged 11 commits into from
May 28, 2019
14 changes: 14 additions & 0 deletions src/staticanalysis/bot/static_analysis_bot/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,20 @@ class Reliability(enum.Enum):
Medium = 'medium'
Low = 'low'

@property
def invert(self):
abpostelnicu marked this conversation as resolved.
Show resolved Hide resolved
'''
Verbalize the opposite of `value` of reliability to be used in coherent
sentences.
'''
inversions = {
'high': 'low',
'medium': 'medium',
'low': 'high'
}

return inversions.get(self.value, 'unknown')


# Create common stats instance
stats = Datadog()
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,8 @@ def as_text(self):
body += '\n{}'.format(self.reason)
# Also add the reliability of the checker
if self.reliability != Reliability.Unknown:
body += '\nChecker reliability is {} (false positive risk).'.format(self.reliability.value)
body += '\nChecker reliability is {0}, meaning that the false positive ratio is {1}.'.format(
self.reliability.value, self.reliability.invert)
return body

def as_markdown(self):
Expand Down Expand Up @@ -207,7 +208,8 @@ def as_phabricator_lint(self):

# Append to description the reliability index if any
if self.reliability != Reliability.Unknown:
description += '\nChecker reliability is {} (false positive risk).'.format(self.reliability.value)
description += '\nChecker reliability is {0}, meaning that the false positive ratio is {1}.'.format(
self.reliability.value, self.reliability.invert)

if self.body:
description += '\n\n > {}'.format(self.body)
Expand Down
4 changes: 2 additions & 2 deletions src/staticanalysis/bot/static_analysis_bot/tasks/coverity.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def as_text(self):
Build the text body published on reporters
'''
# If there is the reliability index use it
return f'Checker reliability is {self.reliability.value} (false positive risk).\n{self.message}' \
return f'Checker reliability is {self.reliability.value}, meaning that the false positive ratio is {self.reliability.invert}.\n{self.message}' \
if self.reliability != Reliability.Unknown \
else self.message

Expand Down Expand Up @@ -156,7 +156,7 @@ def as_phabricator_lint(self):
Outputs a Phabricator lint result
'''
# If there is the reliability index use it
message = f'Checker reliability is {self.reliability.value} (false positive risk).\n{self.message}' \
message = f'Checker reliability is {self.reliability.value}, meaning that the false positive ratio is {self.reliability.invert}.\n{self.message}'\
if self.reliability != Reliability.Unknown \
else self.message

Expand Down
2 changes: 1 addition & 1 deletion src/staticanalysis/bot/tests/test_clang.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def test_as_markdown(mock_revision):
'code': 'clang-tidy.dummy-check',
'line': 42,
'name': 'Clang-Tidy - dummy-check',
'description': 'dummy message\nChecker reliability is high (false positive risk).\n\n > Dummy body',
'description': 'dummy message\nChecker reliability is high, meaning that the false positive ratio is low.\n\n > Dummy body',
'path': 'test.cpp',
'severity': 'warning',
}
Expand Down
11 changes: 10 additions & 1 deletion src/staticanalysis/bot/tests/test_coverity.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_simple(mock_revision, mock_config):
assert issue.validates()
assert not issue.is_publishable()

assert issue.as_text() == '''Checker reliability is medium (false positive risk).
checker_desc = '''Checker reliability is medium, meaning that the false positive ratio is medium.
Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".
The path that leads to this defect is:

Expand All @@ -96,6 +96,15 @@ def test_simple(mock_revision, mock_config):
- ///builds/worker/checkouts/gecko/js/src/jit/BaselineCompiler.cpp:3703//:
-- `dereference: Dereferencing a pointer that might be "nullptr" "env" when calling "lookupImport".`.
'''
assert issue.as_phabricator_lint() == {
'code': 'coverity.NULL_RETURNS',
'line': 3703,
'name': checker_desc,
'path': 'js/src/jit/BaselineCompiler.cpp',
'severity': 'error',
}

assert issue.as_text() == checker_desc
assert issue.as_dict() == {
'analyzer': 'Coverity',
'body': None,
Expand Down
2 changes: 1 addition & 1 deletion src/staticanalysis/bot/tests/test_remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ def test_coverity_task(mock_config, mock_revision, mock_workflow):
assert issue.is_local()
assert not issue.is_clang_error()
assert issue.validates()
assert issue.as_text() == f'Checker reliability is high (false positive risk).\nSome error here'
assert issue.as_text() == f'Checker reliability is high, meaning that the false positive ratio is low.\nSome error here'


def test_infer_task(mock_config, mock_revision, mock_workflow):
Expand Down