diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js index eab50c55..8da5af3d 100644 --- a/conf/rulesets/solhint-all.js +++ b/conf/rulesets/solhint-all.js @@ -16,6 +16,7 @@ module.exports = Object.freeze({ 'no-global-import': 'warn', 'no-unused-import': 'warn', 'no-unused-vars': 'warn', + 'one-contract-per-file': 'warn', 'payable-fallback': 'warn', 'reason-string': [ 'warn', diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js index 072ea68e..56ff9ebe 100644 --- a/conf/rulesets/solhint-recommended.js +++ b/conf/rulesets/solhint-recommended.js @@ -13,6 +13,7 @@ module.exports = Object.freeze({ 'no-global-import': 'warn', 'no-unused-import': 'warn', 'no-unused-vars': 'warn', + 'one-contract-per-file': 'warn', 'payable-fallback': 'warn', 'reason-string': [ 'warn', diff --git a/docs/rules.md b/docs/rules.md index 1f88d8a4..66d12655 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -6,22 +6,23 @@ title: "Rule Index of Solhint" ## Best Practise Rules -| Rule Id | Error | Recommended | Deprecated | -| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- | -| [code-complexity](./rules/best-practises/code-complexity.md) | Function has cyclomatic complexity "current" but allowed no more than maxcompl. | | | -| [custom-errors](./rules/best-practises/custom-errors.md) | Enforces the use of Custom Errors over Require and Revert statements | $~~~~~~~~$✔️ | | -| [explicit-types](./rules/best-practises/explicit-types.md) | Forbid or enforce explicit types (like uint256) that have an alias (like uint). | $~~~~~~~~$✔️ | | -| [function-max-lines](./rules/best-practises/function-max-lines.md) | Function body contains "count" lines but allowed no more than maxlines. | | | -| [max-line-length](./rules/best-practises/max-line-length.md) | Line length must be no more than maxlen. | | | -| [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 block has zero statements inside. Exceptions apply. | $~~~~~~~~$✔️ | | -| [no-global-import](./rules/best-practises/no-global-import.md) | Import statement includes an entire file instead of selected symbols. | $~~~~~~~~$✔️ | | -| [no-unused-import](./rules/best-practises/no-unused-import.md) | Imported object name is not being used by the contract. | $~~~~~~~~$✔️ | | -| [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. | $~~~~~~~~$✔️ | | -| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword. | | | +| Rule Id | Error | Recommended | Deprecated | +| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------ | ------------ | ---------- | +| [code-complexity](./rules/best-practises/code-complexity.md) | Function has cyclomatic complexity "current" but allowed no more than maxcompl. | | | +| [custom-errors](./rules/best-practises/custom-errors.md) | Enforces the use of Custom Errors over Require and Revert statements | $~~~~~~~~$✔️ | | +| [explicit-types](./rules/best-practises/explicit-types.md) | Forbid or enforce explicit types (like uint256) that have an alias (like uint). | $~~~~~~~~$✔️ | | +| [function-max-lines](./rules/best-practises/function-max-lines.md) | Function body contains "count" lines but allowed no more than maxlines. | | | +| [max-line-length](./rules/best-practises/max-line-length.md) | Line length must be no more than maxlen. | | | +| [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 block has zero statements inside. Exceptions apply. | $~~~~~~~~$✔️ | | +| [no-global-import](./rules/best-practises/no-global-import.md) | Import statement includes an entire file instead of selected symbols. | $~~~~~~~~$✔️ | | +| [no-unused-import](./rules/best-practises/no-unused-import.md) | Imported object name is not being used by the contract. | $~~~~~~~~$✔️ | | +| [no-unused-vars](./rules/best-practises/no-unused-vars.md) | Variable "name" is unused. | $~~~~~~~~$✔️ | | +| [one-contract-per-file](./rules/best-practises/one-contract-per-file.md) | Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names) | $~~~~~~~~$✔️ | | +| [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. | $~~~~~~~~$✔️ | | +| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword. | | | ## Miscellaneous diff --git a/docs/rules/best-practises/one-contract-per-file.md b/docs/rules/best-practises/one-contract-per-file.md new file mode 100644 index 00000000..564bacaf --- /dev/null +++ b/docs/rules/best-practises/one-contract-per-file.md @@ -0,0 +1,39 @@ +--- +warning: "This is a dynamically generated file. Do not edit manually." +layout: "default" +title: "one-contract-per-file | Solhint" +--- + +# one-contract-per-file +![Recommended Badge](https://img.shields.io/badge/-Recommended-brightgreen) +![Category Badge](https://img.shields.io/badge/-Best%20Practise%20Rules-informational) +![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow) +> The {"extends": "solhint:recommended"} property in a configuration file enables this rule. + + +## Description +Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names) + +## Options +This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. + +### Example Config +```json +{ + "rules": { + "one-contract-per-file": "warn" + } +} +``` + + +## Examples +This rule does not have examples. + +## Version +This rule is introduced in the latest version. + +## Resources +- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/one-contract-per-file.js) +- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/one-contract-per-file.md) +- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/one-contract-per-file.js) diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js index e7f5228d..f077d6f5 100644 --- a/lib/rules/best-practises/index.js +++ b/lib/rules/best-practises/index.js @@ -11,6 +11,7 @@ const NoGlobalImportsChecker = require('./no-global-import') const NoUnusedImportsChecker = require('./no-unused-import') const ExplicitTypesChecker = require('./explicit-types') const CustomErrorsChecker = require('./custom-errors') +const OneContractPerFileChecker = require('./one-contract-per-file') module.exports = function checkers(reporter, config, inputSrc, tokens) { return [ @@ -27,5 +28,6 @@ module.exports = function checkers(reporter, config, inputSrc, tokens) { new NoUnusedImportsChecker(reporter, tokens), new ExplicitTypesChecker(reporter, config), new CustomErrorsChecker(reporter), + new OneContractPerFileChecker(reporter), ] } diff --git a/lib/rules/best-practises/one-contract-per-file.js b/lib/rules/best-practises/one-contract-per-file.js new file mode 100644 index 00000000..200ab771 --- /dev/null +++ b/lib/rules/best-practises/one-contract-per-file.js @@ -0,0 +1,51 @@ +const BaseChecker = require('../base-checker') +const { severityDescription } = require('../../doc/utils') + +const DEFAULT_SEVERITY = 'warn' + +const ruleId = 'one-contract-per-file' +const meta = { + type: 'best-practises', + + docs: { + description: + 'Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names)', + category: 'Best Practise Rules', + options: [ + { + description: severityDescription, + default: DEFAULT_SEVERITY, + }, + ], + }, + + isDefault: false, + recommended: true, + defaultSetup: DEFAULT_SEVERITY, + + schema: null, +} + +class OneContractPerFileChecker extends BaseChecker { + constructor(reporter) { + super(reporter, ruleId, meta) + } + + SourceUnit(node) { + const contractDefinitionCount = node.children.reduce((count, child) => { + if (child.type === 'ContractDefinition') { + return count + 1 + } + return count + }, 0) + + if (contractDefinitionCount > 1) { + this.error( + node, + `Found more than One contract per file. ${contractDefinitionCount} contracts found!` + ) + } + } +} + +module.exports = OneContractPerFileChecker diff --git a/test/fixtures/best-practises/one-contract-per-file.js b/test/fixtures/best-practises/one-contract-per-file.js new file mode 100644 index 00000000..afc74921 --- /dev/null +++ b/test/fixtures/best-practises/one-contract-per-file.js @@ -0,0 +1,36 @@ +const ONE_CONTRACT = ` + pragma solidity 0.8.0; + + contract A { + uint256 public constant TESTA = "testA"; + } + ` + +const TWO_CONTRACTS = ` + pragma solidity 0.8.0; + + contract A { + uint256 public constant TESTA = "testA"; + } + + contract B { + uint256 public constant TESTB = "testB"; + } + ` + +const THREE_CONTRACTS = ` + pragma solidity 0.8.0; + + contract A { + uint256 public constant TESTA = "testA"; + } + + contract B { + uint256 public constant TESTB = "testB"; + } + + contract C { + uint256 public constant TESTC = "testC"; + } + ` +module.exports = { ONE_CONTRACT, TWO_CONTRACTS, THREE_CONTRACTS } diff --git a/test/rules/best-practises/one-contract-per-file.js b/test/rules/best-practises/one-contract-per-file.js new file mode 100644 index 00000000..54b5876c --- /dev/null +++ b/test/rules/best-practises/one-contract-per-file.js @@ -0,0 +1,37 @@ +const { assertNoWarnings, assertErrorMessage, assertErrorCount } = require('../../common/asserts') +const linter = require('../../../lib/index') +const contracts = require('../../fixtures/best-practises/one-contract-per-file') + +describe('Linter - one-contract-per-file', () => { + it('should not raise error for ONE contract only', () => { + const code = contracts.ONE_CONTRACT + + const report = linter.processStr(code, { + rules: { 'one-contract-per-file': 'error' }, + }) + + assertNoWarnings(report) + }) + + it('should raise error for TWO contracts in same file', () => { + const code = contracts.TWO_CONTRACTS + + const report = linter.processStr(code, { + rules: { 'one-contract-per-file': 'error' }, + }) + + assertErrorCount(report, 1) + assertErrorMessage(report, 'Found more than One contract per file. 2 contracts found!') + }) + + it('should raise error for THREE contracts in same file', () => { + const code = contracts.THREE_CONTRACTS + + const report = linter.processStr(code, { + rules: { 'one-contract-per-file': 'error' }, + }) + + assertErrorCount(report, 1) + assertErrorMessage(report, 'Found more than One contract per file. 3 contracts found!') + }) +})