Skip to content

Commit

Permalink
Merge pull request #487 from protofire/i154-one-contract-per-file
Browse files Browse the repository at this point in the history
New Rule one contract per file
  • Loading branch information
dbale-altoros authored Aug 17, 2023
2 parents 2e8bd7e + ae30025 commit 54aade1
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 16 deletions.
1 change: 1 addition & 0 deletions conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions conf/rulesets/solhint-recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
33 changes: 17 additions & 16 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions docs/rules/best-practises/one-contract-per-file.md
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 2 additions & 0 deletions lib/rules/best-practises/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 [
Expand All @@ -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),
]
}
51 changes: 51 additions & 0 deletions lib/rules/best-practises/one-contract-per-file.js
Original file line number Diff line number Diff line change
@@ -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
36 changes: 36 additions & 0 deletions test/fixtures/best-practises/one-contract-per-file.js
Original file line number Diff line number Diff line change
@@ -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 }
37 changes: 37 additions & 0 deletions test/rules/best-practises/one-contract-per-file.js
Original file line number Diff line number Diff line change
@@ -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!')
})
})

0 comments on commit 54aade1

Please sign in to comment.