Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Rule one contract per file #487

Merged
merged 1 commit into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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!')
})
})