From 2832d62ba430369d387bb599892acd8b822b0ef0 Mon Sep 17 00:00:00 2001 From: Josselin Date: Thu, 12 Sep 2019 09:24:59 +0200 Subject: [PATCH] Improve external function detector: merge together shadowed function --- .../detectors/functions/external_function.py | 24 +++++++++++++------ .../external_function.external-function.json | 10 ++++---- .../external_function.external-function.txt | 19 +++++++++------ .../external_function_2.external-function.txt | 2 +- 4 files changed, 35 insertions(+), 20 deletions(-) diff --git a/slither/detectors/functions/external_function.py b/slither/detectors/functions/external_function.py index e2250ba3a..732488780 100644 --- a/slither/detectors/functions/external_function.py +++ b/slither/detectors/functions/external_function.py @@ -172,14 +172,24 @@ class ExternalFunction(AbstractDetector): if is_called: continue - # Loop for each function definition, and recommend it be declared external. - for function_definition in all_function_definitions: - txt = "{} ({}) should be declared external\n" - info = txt.format(function_definition.canonical_name, - function_definition.source_mapping_str) - - json = self.generate_json_result(info) + # As we collect all shadowed functions in get_all_function_definitions + # Some function coming from a base might already been declared as external + all_function_definitions = [f for f in all_function_definitions if f.visibility == 'public' and + f.contract == f.contract_declarer] + if all_function_definitions: + function_definition = all_function_definitions[0] + all_function_definitions = all_function_definitions[1:] + + txt = f"{function_definition.full_name} should be declared external:\n" + txt += f"\t- {function_definition.canonical_name} ({function_definition.source_mapping_str})\n" + for other_function_definition in all_function_definitions: + txt += f"\t- {other_function_definition.canonical_name}" + txt += f" ({other_function_definition.source_mapping_str})\n" + + json = self.generate_json_result(txt) self.add_function_to_json(function_definition, json) + for other_function_definition in all_function_definitions: + self.add_function_to_json(other_function_definition, json) results.append(json) return results diff --git a/tests/expected_json/external_function.external-function.json b/tests/expected_json/external_function.external-function.json index d60c5ef49..026c45140 100644 --- a/tests/expected_json/external_function.external-function.json +++ b/tests/expected_json/external_function.external-function.json @@ -7,7 +7,7 @@ "check": "external-function", "impact": "Optimization", "confidence": "High", - "description": "ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15) should be declared external\n", + "description": "funcNotCalled3() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15)\n", "elements": [ { "type": "function", @@ -74,7 +74,7 @@ "check": "external-function", "impact": "Optimization", "confidence": "High", - "description": "ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) should be declared external\n", + "description": "funcNotCalled2() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19)\n", "elements": [ { "type": "function", @@ -141,7 +141,7 @@ "check": "external-function", "impact": "Optimization", "confidence": "High", - "description": "ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) should be declared external\n", + "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23)\n", "elements": [ { "type": "function", @@ -208,7 +208,7 @@ "check": "external-function", "impact": "Optimization", "confidence": "High", - "description": "ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) should be declared external\n", + "description": "funcNotCalled() should be declared external:\n\t- ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39)\n", "elements": [ { "type": "function", @@ -271,7 +271,7 @@ "check": "external-function", "impact": "Optimization", "confidence": "High", - "description": "FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) should be declared external\n", + "description": "parameter_read_ok_for_external(uint256) should be declared external:\n\t- FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76)\n", "elements": [ { "type": "function", diff --git a/tests/expected_json/external_function.external-function.txt b/tests/expected_json/external_function.external-function.txt index 82470ba0d..a0f3d6499 100644 --- a/tests/expected_json/external_function.external-function.txt +++ b/tests/expected_json/external_function.external-function.txt @@ -1,8 +1,13 @@ -INFO:Detectors: -ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15) should be declared external -ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) should be declared external -ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) should be declared external -ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) should be declared external -FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) should be declared external + +funcNotCalled3() should be declared external: + - ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15) +funcNotCalled2() should be declared external: + - ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) +funcNotCalled() should be declared external: + - ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) +funcNotCalled() should be declared external: + - ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) +parameter_read_ok_for_external(uint256) should be declared external: + - FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) Reference: https://github.com/crytic/slither/wiki/Detector-Documentation#public-function-that-could-be-declared-as-external -INFO:Slither:tests/external_function.sol analyzed (6 contracts), 5 result(s) found +tests/external_function.sol analyzed (6 contracts with 1 detectors), 5 result(s) found diff --git a/tests/expected_json/external_function_2.external-function.txt b/tests/expected_json/external_function_2.external-function.txt index 352324d7f..a61fb7600 100644 --- a/tests/expected_json/external_function_2.external-function.txt +++ b/tests/expected_json/external_function_2.external-function.txt @@ -1 +1 @@ -INFO:Slither:tests/external_function_2.sol analyzed (4 contracts), 0 result(s) found +tests/external_function_2.sol analyzed (4 contracts with 1 detectors), 0 result(s) found