Merge pull request #2392 from crytic/feat/unused-import

feat: add detector for unused imports

slither/detectors/all_detectors.py

@@ -98,3 +98,4 @@
 from .statements.tautological_compare import TautologicalCompare
 from .statements.return_bomb import ReturnBomb
 from .functions.out_of_order_retryable import OutOfOrderRetryable
+from .statements.unused_import import UnusedImport

slither/detectors/statements/unused_import.py

@@ -0,0 +1,75 @@
+from typing import List
+from slither.detectors.abstract_detector import AbstractDetector, DetectorClassification, Output
+
+# pylint: disable=protected-access,too-many-nested-blocks
+class UnusedImport(AbstractDetector):
+    """
+    Detector unused imports.
+    """
+
+    ARGUMENT = "unused-import"
+    HELP = "Detects unused imports"
+    IMPACT = DetectorClassification.INFORMATIONAL
+    CONFIDENCE = DetectorClassification.HIGH
+
+    WIKI = "https://github.com/crytic/slither/wiki/Detector-Documentation#unused-imports"
+
+    WIKI_TITLE = "Unused Imports"
+    WIKI_DESCRIPTION = "Importing a file that is not used in the contract likely indicates a mistake. The import should be removed until it is needed."
+
+    # region wiki_exploit_scenario
+    WIKI_EXPLOIT_SCENARIO = """
+    ```solidity
+    import {A} from "./A.sol";
+    contract B {}
+    ```
+    B either should import from A and it was forgotten or the import is not needed and should be removed.
+    """
+    # endregion wiki_exploit_scenario
+    WIKI_RECOMMENDATION = (
+        "Remove the unused import. If the import is needed later, it can be added back."
+    )
+
+    def _detect(self) -> List[Output]:
+        results: List[Output] = []
+        # This is computed lazily and then memoized so we need to trigger the computation.
+        self.slither._compute_offsets_to_ref_impl_decl()
+
+        for unit in self.slither.compilation_units:
+            for filename, scope in unit.scopes.items():
+                unused = []
+                for i in scope.imports:
+                    # `scope.imports` contains all transitive imports so we need to filter out imports not explicitly imported in the file.
+                    # Otherwise, we would recommend removing an import that is used by a leaf contract and cause compilation errors.
+                    if i.scope != scope:
+                        continue
+
+                    import_path = self.slither.crytic_compile.filename_lookup(i.filename)
+
+                    use_found = False
+                    # Search through all references to the imported file
+                    for _, refs in self.slither._offset_to_references[import_path].items():
+                        for ref in refs:
+                            # If there is a reference in this file to the imported file, it is used.
+                            if ref.filename == filename:
+                                use_found = True
+                                break
+
+                        if use_found:
+                            break
+
+                    if not use_found:
+                        unused.append(f"{i.source_mapping.content} ({i.source_mapping})")
+
+                if len(unused) > 0:
+                    unused_list = "\n\t-" + "\n\t-".join(unused)
+
+                    results.append(
+                        self.generate_result(
+                            [
+                                f"The following unused import(s) in {filename.used} should be removed: {unused_list}\n",
+                            ]
+                        )
+                    )
+
+        return results

tests/e2e/detectors/snapshots/detectors__detector_UnusedImport_0_8_16_C_sol__0.txt

@@ -0,0 +1,6 @@
+The following unused import(s) in tests/e2e/detectors/test_data/unused-imports/0.8.16/B.sol should be removed: 
+	-import "./A.sol"; (tests/e2e/detectors/test_data/unused-imports/0.8.16/B.sol#4)
+
+The following unused import(s) in tests/e2e/detectors/test_data/unused-imports/0.8.16/C.sol should be removed: 
+	-import "./B.sol"; (tests/e2e/detectors/test_data/unused-imports/0.8.16/C.sol#4)
+

tests/e2e/detectors/test_data/unused-import/0.8.16/A.sol

@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+library A
+{
+    function a() public
+    {
+
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/B.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./A.sol";
+
+contract B
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/C.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./B.sol";
+
+contract C
+{
+    constructor()
+    {
+        A.a();
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantContractLevel.sol

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+library ConstantContractLevel
+{
+    uint constant public CONSTANT = 0;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantContractLevelUsedInContractTest.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./ConstantContractLevel.sol";
+
+contract ConstantContractLevelUsedInContractTest
+{
+    uint private v = ConstantContractLevel.CONSTANT;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantContractLevelUsedTopLevelTest.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./ConstantContractLevel.sol";
+
+uint constant __ = ConstantContractLevel.CONSTANT;
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantTopLevel.sol

@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+uint constant ConstantTopLevel = 0;
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantTopLevelUsedInContractTest.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./ConstantTopLevel.sol";
+
+contract ConstantTopLevelUsedInContractTest
+{
+    uint private v = ConstantTopLevel;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ConstantTopLevelUsedTopLevelTest.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./ConstantTopLevel.sol";
+
+uint constant __ = ConstantTopLevel;
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy_
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/Contract.sol

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+contract Contract
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ContractUsedInContractTest1.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./Contract.sol";
+
+contract ContractUsedInContractTest1
+{
+    Contract c;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ContractUsedInContractTest2.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./Contract.sol";
+
+contract ContractUsedInContractTest2 is Contract
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/ContractUsedTopLevelTest.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./Contract.sol";
+
+Contract constant c = Contract(address(0x0));
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomErrorTopLevel.sol

@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+error err();
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomErrorTopLevelUsedInContractTest.sol

@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomErrorTopLevel.sol";
+
+contract CustomErrorTopLevelUsedInContractTest
+{
+    constructor()
+    {
+        f();
+    }
+
+    function f() private pure
+    {
+        revert err();
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomEventContractLevel.sol

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+library CustomEventContractLevel
+{
+    event CustomEvent();
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomEventContractLevelUsedInContractTest.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomEventContractLevel.sol";
+
+contract CustomEventContractLevelUsedInContractTest
+{
+    function f() public 
+    {
+        emit CustomEventContractLevel.CustomEvent();
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomEventContractLevelUsedTopLevelTest.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomEventContractLevel.sol";
+
+function f()
+{
+    emit CustomEventContractLevel.CustomEvent();
+}
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevel.sol

@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+contract CustomTypeContractLevel
+{
+    type CustomType is uint;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevelUsedInContractTest1.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomTypeContractLevel.sol";
+
+contract CustomTypeContractLevelUsedInContractTest1
+{
+    CustomTypeContractLevel.CustomType private v;
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevelUsedInContractTest2.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomTypeContractLevel.sol";
+
+contract CustomTypeContractLevelUsedInContractTest2
+{
+    function f(CustomTypeContractLevel.CustomType) public 
+    {
+
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevelUsedInContractTest3.sol

@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomTypeContractLevel.sol";
+
+contract CustomTypeContractLevelUsedInContractTest3
+{
+    modifier m()
+    {
+        CustomTypeContractLevel.CustomType ___;
+        _;
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevelUsedInContractTest4.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomTypeContractLevel.sol";
+
+contract CustomTypeContractLevelUsedInContractTest4
+{
+    struct CustomStruct
+    {
+        CustomTypeContractLevel.CustomType ___;
+    }
+}

tests/e2e/detectors/test_data/unused-import/0.8.16/CustomTypeContractLevelUsedTopLevelTest1.sol

@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: Unlicense
+pragma solidity 0.8.16;
+
+import "./CustomTypeContractLevel.sol";
+
+struct CustomTypeContractLevelUsedTopLevelTest1
+{
+    CustomTypeContractLevel.CustomType __;
+}
+
+// dummy contract, so that "No contract were found ..." message is not being thrown by Slither
+contract Dummy
+{
+
+}