Repository: protofire/solhint
Pull request #480 (2023-08-10 16:12:45)

Fix: no-unused-imports fix


Patch:
diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js
index 52133e8..e7f5228 100644
--- a/lib/rules/best-practises/index.js
+++ b/lib/rules/best-practises/index.js
@@ -12,7 +12,7 @@ const NoUnusedImportsChecker = require('./no-unused-import')
 const ExplicitTypesChecker = require('./explicit-types')
 const CustomErrorsChecker = require('./custom-errors')
 
-module.exports = function checkers(reporter, config, inputSrc) {
+module.exports = function checkers(reporter, config, inputSrc, tokens) {
   return [
     new CodeComplexityChecker(reporter, config),
     new FunctionMaxLinesChecker(reporter, config),
@@ -24,7 +24,7 @@ module.exports = function checkers(reporter, config, inputSrc) {
     new ReasonStringChecker(reporter, config),
     new NoConsoleLogChecker(reporter),
     new NoGlobalImportsChecker(reporter),
-    new NoUnusedImportsChecker(reporter),
+    new NoUnusedImportsChecker(reporter, tokens),
     new ExplicitTypesChecker(reporter, config),
     new CustomErrorsChecker(reporter),
   ]
diff --git a/lib/rules/best-practises/no-unused-import.js b/lib/rules/best-practises/no-unused-import.js
index 993ae14..297875f 100644
--- a/lib/rules/best-practises/no-unused-import.js
+++ b/lib/rules/best-practises/no-unused-import.js
@@ -43,9 +43,10 @@ function isImportIdentifier(node) {
 }
 
 class NoUnusedImportsChecker extends BaseChecker {
-  constructor(reporter) {
+  constructor(reporter, tokens) {
     super(reporter, ruleId, meta)
     this.importedNames = {}
+    this.tokens = tokens
   }
 
   registerUsage(rawName) {
@@ -60,14 +61,6 @@ class NoUnusedImportsChecker extends BaseChecker {
     }
   }
 
-  ArrayTypeName(node) {
-    this.registerUsage(node.baseTypeName.namePath)
-  }
-
-  NewExpression(node) {
-    this.registerUsage(node.typeName.namePath)
-  }
-
   Identifier(node) {
     if (!isImportIdentifier(node)) {
       this.registerUsage(node.name)
@@ -75,9 +68,6 @@ class NoUnusedImportsChecker extends BaseChecker {
   }
 
   FunctionDefinition(node) {
-    if (node.override) {
-      node.override.forEach((it) => this.registerUsage(it.namePath))
-    }
     if (node.isConstructor) {
       node.modifiers.forEach((it) => this.registerUsage(it.name))
     }
@@ -93,18 +83,8 @@ class NoUnusedImportsChecker extends BaseChecker {
     }
   }
 
-  VariableDeclaration(node) {
-    // this ignores builtin types, the check inside registerUsage ignores types
-    // defined in the same file
-    if (node.typeName.type === 'UserDefinedTypeName') {
-      this.registerUsage(node.typeName.namePath)
-    }
-  }
-
-  ContractDefinition(node) {
-    node.baseContracts.forEach((inheritanceSpecifier) => {
-      this.registerUsage(inheritanceSpecifier.baseName.namePath)
-    })
+  UserDefinedTypeName(node) {
+    this.registerUsage(node.namePath)
   }
 
   UsingForDeclaration(node) {
@@ -113,6 +93,16 @@ class NoUnusedImportsChecker extends BaseChecker {
   }
 
   'SourceUnit:exit'() {
+    const keywords = this.tokens.filter((it) => it.type === 'Keyword')
+    const inheritdocStatements = keywords.filter(
+      ({ value }) => /^\/\/\/ *@inheritdoc/.test(value) || /^\/\*\* *@inheritdoc/.test(value)
+    )
+    inheritdocStatements.forEach(({ value }) => {
+      const match = value.match(/@inheritdoc *([a-zA-Z0-9_]*)/)
+      if (match && match[1]) {
+        this.registerUsage(match[1])
+      }
+    })
     forIn(this.importedNames, (value, key) => {
       if (!value.used) {
         this.error(value.node, `imported name ${key} is not used`)
diff --git a/lib/rules/index.js b/lib/rules/index.js
index d7e78ab..7e8a3b5 100644
--- a/lib/rules/index.js
+++ b/lib/rules/index.js
@@ -58,7 +58,7 @@ function coreRules(meta) {
   const { reporter, config, inputSrc, tokens } = meta
 
   return [
-    ...bestPractises(reporter, config, inputSrc),
+    ...bestPractises(reporter, config, inputSrc, tokens),
     ...deprecations(reporter),
     ...miscellaneous(reporter, config, tokens),
     ...naming(reporter, config),


Test patch:
diff --git a/test/rules/best-practises/no-unused-import.js b/test/rules/best-practises/no-unused-import.js
index 49a2c49..ff1e332 100644
--- a/test/rules/best-practises/no-unused-import.js
+++ b/test/rules/best-practises/no-unused-import.js
@@ -27,6 +27,26 @@ describe('Linter - no-unused-import', () => {
     assertErrorMessage(report, 'imported name A is not used')
   })
 
+  it('should not crash on, but also not recognize, malformed inheritdoc statements', () => {
+    const code = `
+    import {A} from './A.sol';
+    // inheritdoc A
+    // @inheritdoc A
+    /// inheritdoc A
+    /// @ inherit A
+    /// @ inheritdoc A
+    /// @inheritdoc somethingelse
+    /// @inheritdoc
+    /// @inheritdoc 
+    `
+
+    const report = linter.processStr(code, {
+      rules: { 'no-unused-import': 'error' },
+    })
+    assertErrorCount(report, 1)
+    assertErrorMessage(report, 'imported name A is not used')
+  })
+
   it('should raise error when using solhint:recommended', () => {
     const code = `pragma solidity ^0.5.8; import {A} from "./A.sol";`
 
@@ -171,10 +191,11 @@ describe('Linter - no-unused-import', () => {
       description: 'Name is used in an override',
       code: `
         pragma solidity >=0.8.19;
+
         import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
         import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
+
         contract MyContract {
-            /// @inheritdoc ERC721
             function tokenURI(uint256 streamId) public view override(IERC721Metadata, ERC721) returns (string memory uri) {
                 uri = "example.com";
             }
@@ -185,7 +206,9 @@ describe('Linter - no-unused-import', () => {
       description: 'Type is used in a constructor initializator',
       code: `
         pragma solidity >=0.8.19;
+
         import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
+
         contract MyContract {
             constructor() ERC721("Sablier V2 Lockup Dynamic NFT", "SAB-V2-LOCKUP-DYN") { }
         }
@@ -195,7 +218,9 @@ describe('Linter - no-unused-import', () => {
       description: 'Import is used as value in a function parameter',
       code: `
         pragma solidity >=0.8.19 <0.9.0;
+
         import { UD60x18, ZERO } from "@prb/math/UD60x18.sol";
+
         contract SetProtocolFee_Integration_Fuzz_Test {
             function testFuzz_SetProtocolFee(UD60x18 newProtocolFee) external {
                 newProtocolFee = _bound(newProtocolFee, 1, MAX_FEE);
@@ -209,6 +234,7 @@ describe('Linter - no-unused-import', () => {
       description: 'Import is used as value in a binary expression',
       code: `
         import { ZERO } from "@prb/math/UD60x18.sol";
+
         contract Foo {
             function returnFifteen() public returns (uint){
               return ZERO + 15;
@@ -220,6 +246,7 @@ describe('Linter - no-unused-import', () => {
       description: 'Import is used as value in an assignment',
       code: `
         import { ZERO } from "@prb/math/UD60x18.sol";
+
         contract Foo {
             uint256 public howMuch;
             constructor () {
@@ -228,6 +255,69 @@ describe('Linter - no-unused-import', () => {
         }
       `,
     },
+    {
+      description: 'Import is used in the left side of a using for statement',
+      code: `
+        pragma solidity >=0.8.19;
+
+        import {EnumerableSetUD60x18, EnumerableSet} from "./libraries/EnumerableSetUD60x18.sol";
+
+        contract SolhintTest {
+            using EnumerableSetUD60x18 for EnumerableSet.Bytes32Set;
+        }
+      `,
+    },
+    {
+      description: 'Import is used as the type value of a mapping',
+      code: `
+        pragma solidity >=0.8.19;
+
+        import {UD60x18} from "@prb/math/UD60x18.sol";
+
+        contract SolhintTest {
+            mapping(address user => UD60x18 amount) internal balance;
+        }
+      `,
+    },
+    {
+      description: 'Import is used as the type value of a nested mapping',
+      code: `
+        pragma solidity >=0.8.19;
+        import {SD59x18} from "@prb/math/SD59x18.sol";
+
+        contract SolhintTest {
+            mapping(address user => mapping(address relayer => SD59x18 amount)) internal balance;
+        }
+      `,
+    },
+    {
+      description: 'Import is used in /// @inheritdoc',
+      code: `
+        import { IPRBProxyPlugin } from "@prb/proxy/interfaces/IPRBProxyPlugin.sol";
+
+        contract SablierV2ProxyPlugin {
+          /// @inheritdoc IPRBProxyPlugin
+          function getMethods() external pure returns (bytes4[] memory methods) {
+              methods = new bytes4[](1);
+              methods[0] = this.onStreamCanceled.selector;
+          }
+        }
+      `,
+    },
+    {
+      description: 'Import is used in /** @inheritdoc',
+      code: `
+        import { IPRBProxyPlugin } from "@prb/proxy/interfaces/IPRBProxyPlugin.sol";
+
+        contract SablierV2ProxyPlugin {
+          /** @inheritdoc IPRBProxyPlugin */
+          function getMethods() external pure returns (bytes4[] memory methods) {
+              methods = new bytes4[](1);
+              methods[0] = this.onStreamCanceled.selector;
+          }
+        }
+      `,
+    },
   ].forEach(({ description, code }) => {
     it(`should not raise when ${description}`, () => {
       const report = linter.processStr(code, {