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

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,43 @@
+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)
+  }
+
+  SourceUnit(node) {
+    node.children.forEach((it) => {
+      if (it.type === 'ImportDirective' && !it.symbolAliases) {
+        this.error(
+          it,
+          `global import of path ${it.path} is not allowed. Specify names to import individually`
+        )
+      }
+    })
+  }
+}
+
+module.exports = NoGlobalImportsChecker

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

@@ -0,0 +1,47 @@
+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)
+  })
+  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)
+  })
+})