Improve external function detector: merge together shadowed function

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.
+                # As we collect all shadowed functions in get_all_function_definitions
-                for function_definition in all_function_definitions:
+                # Some function coming from a base might already been declared as external
-                    txt = "{} ({}) should be declared external\n"
+                all_function_definitions = [f for f in all_function_definitions if f.visibility == 'public' and
-                    info = txt.format(function_definition.canonical_name,
+                                            f.contract == f.contract_declarer]
-                                      function_definition.source_mapping_str)
+                if all_function_definitions:
-
+                    function_definition = all_function_definitions[0]
-                    json = self.generate_json_result(info)
+                    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

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",

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
+funcNotCalled3() should be declared external:
-ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19) should be declared external
+	- ContractWithFunctionNotCalled.funcNotCalled3() (tests/external_function.sol#13-15)
-ContractWithFunctionNotCalled.funcNotCalled() (tests/external_function.sol#21-23) should be declared external
+funcNotCalled2() should be declared external:
-ContractWithFunctionNotCalled2.funcNotCalled() (tests/external_function.sol#32-39) should be declared external
+	- ContractWithFunctionNotCalled.funcNotCalled2() (tests/external_function.sol#17-19)
-FunctionParameterWrite.parameter_read_ok_for_external(uint256) (tests/external_function.sol#74-76) should be declared external
+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

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