no-global-import rule

disallow imports that potentially pollute the namespace by importing
every defined name of the imported file.

~when registering an ImportDirective method on the NoGlobalImportsChecker
class, it's only called by imports that explicitly define a new
Identifier, the only way I found to find global imports is to search for
them in the SourceUnit nodes~

^ was recommended against in review and when trying to replicate it to
show the issue to fvictorio, I wasn't able to. So the cleaner way, using
the ImportDirective visitor, will remain.

docs/rules.md

@@ -14,6 +14,7 @@ title:       "Rule Index of Solhint"
 | [max-states-count](./rules/best-practises/max-states-count.md)     | Contract has "some count" states declarations but allowed no more than maxstates.                                     | ✔️          |
 | [no-console](./rules/best-practises/no-console.md)                 | No console.log/logInt/logBytesX/logString/etc & No hardhat and forge-std console.sol import statements                | ✔️          |
 | [no-empty-blocks](./rules/best-practises/no-empty-blocks.md)       | Code contains empty block.                                                                                            | ✔️          |
+| [no-global-import](./rules/best-practises/no-global-import.md)     | Import statement includes an entire file instead of selected symbols                                                  |             |
 | [no-unused-vars](./rules/best-practises/no-unused-vars.md)         | Variable "name" is unused.                                                                                            | ✔️          |
 | [payable-fallback](./rules/best-practises/payable-fallback.md)     | When fallback is not payable you will not be able to receive ethers.                                                  | ✔️          |
 | [reason-string](./rules/best-practises/reason-string.md)           | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | ✔️          |

docs/rules/best-practises/no-global-import.md

@@ -0,0 +1,56 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "no-global-import | Solhint"
+---
+
+# no-global-import
+![Category Badge](https://img.shields.io/badge/-Best%20Practise%20Rules-informational)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
+
+## Description
+Import statement includes an entire file instead of selected symbols
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+  "rules": {
+    "no-global-import": "warn"
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### import names explicitly
+
+```solidity
+import {A} from "./A.sol"
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### import all members from a file
+
+```solidity
+import * from "foo.sol"
+```
+
+#### import an entire file
+
+```solidity
+import "foo.sol"
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/no-global-import.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/no-global-import.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/no-global-import.js)

lib/rules/best-practises/index.js

@@ -7,6 +7,7 @@ const NoUnusedVarsChecker = require('./no-unused-vars')
 const PayableFallbackChecker = require('./payable-fallback')
 const ReasonStringChecker = require('./reason-string')
 const NoConsoleLogChecker = require('./no-console')
+const NoGlobalImportsChecker = require('./no-global-import')
 
 module.exports = function checkers(reporter, config, inputSrc) {
   return [
@@ -19,5 +20,6 @@ module.exports = function checkers(reporter, config, inputSrc) {
     new PayableFallbackChecker(reporter),
     new ReasonStringChecker(reporter, config),
     new NoConsoleLogChecker(reporter),
+    new NoGlobalImportsChecker(reporter),
   ]
 }

lib/rules/best-practises/no-global-import.js

@@ -0,0 +1,41 @@
+const BaseChecker = require('../base-checker')
+
+const ruleId = 'no-global-import'
+const meta = {
+  type: 'best-practises',
+
+  docs: {
+    description: 'Import statement includes an entire file instead of selected symbols',
+    category: 'Best Practise Rules',
+    examples: {
+      bad: [
+        { description: 'import all members from a file', code: 'import * from "foo.sol"' },
+        { description: 'import an entire file', code: 'import "foo.sol"' },
+      ],
+      good: [{ description: 'import names explicitly', code: 'import {A} from "./A.sol"' }],
+    },
+  },
+
+  isDefault: false,
+  recommended: false,
+  defaultSetup: 'warn',
+
+  schema: null,
+}
+
+class NoGlobalImportsChecker extends BaseChecker {
+  constructor(reporter) {
+    super(reporter, ruleId, meta)
+  }
+
+  ImportDirective(node) {
+    if (!node.symbolAliases) {
+      this.error(
+        node,
+        `global import of path ${node.path} is not allowed. Specify names to import individually`
+      )
+    }
+  }
+}
+
+module.exports = NoGlobalImportsChecker

test/rules/best-practises/no-global-import.js

@@ -0,0 +1,48 @@
+const linter = require('../../../lib/index')
+const {
+  assertNoWarnings,
+  assertLineNumber,
+  assertErrorMessage,
+  assertWarnsCount,
+} = require('../../common/asserts')
+
+describe('Linter - no-global-import', () => {
+  it('should raise on import * from "path"', () => {
+    const code = `import * from './A.sol';`
+
+    const report = linter.processStr(code, {
+      rules: { 'no-global-import': 'warn' },
+    })
+    assertWarnsCount(report, 1)
+    assertErrorMessage(report, 'global import')
+    assertErrorMessage(report, 'Specify names to import individually')
+  })
+  it('should raise on import "path"', () => {
+    const code = `import './A.sol';`
+
+    const report = linter.processStr(code, {
+      rules: { 'no-global-import': 'warn' },
+    })
+    assertWarnsCount(report, 1)
+    assertErrorMessage(report, 'global import')
+    assertErrorMessage(report, 'Specify names to import individually')
+  })
+  it('should report correct line', () => {
+    const code = `import {A} from './A.sol';
+    import './A.sol';`
+
+    const report = linter.processStr(code, {
+      rules: { 'no-global-import': 'warn' },
+    })
+    assertLineNumber(report.reports[0], 2)
+    assertWarnsCount(report, 1)
+  })
+  it('should not raise on import {identifier} from "path"', () => {
+    const code = `import {A} from './A.sol';`
+
+    const report = linter.processStr(code, {
+      rules: { 'no-global-import': 'warn' },
+    })
+    assertNoWarnings(report)
+  })
+})