diff --git a/.gitignore b/.gitignore
index 77dd2e5e..063bb153 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,4 +9,4 @@ convertLib.sol
antlr4.jar
/docs/.sass-cache/
_temp/
-.solhint.json
+
diff --git a/CHANGELOG.md b/CHANGELOG.md
index ecc118fb..92070ed1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,43 @@
+## [3.6.1] - 2023-08-11
+
+### BREAKING CHANGE
+- RULE: `not-rely-on-time` was REMOVED from RECOMMENDED ruleset
+This was long overdue.
+Beware!! If you are relying on this rule and it is not explicitly configured (meaning there's only `solhint:recommended` option).
+You should add this rule manually:
+```json
+ {
+ "extends": "solhint:recommended",
+ "rules": {
+ "not-rely-on-time": "warn",
+ "compiler-version": "off"
+ },
+ }
+```
+If not explicitly added, this rule will not be executed.
+
+### SPECIAL ATTENTION
+- RULE: `compiler-version` default was updated from ^0.5.2 to ^0.8.0
+
+### Added
+- New Rule: Enforces the use of Custom Errors over Require and Revert statements [#475](https://github.com/protofire/solhint/pull/475)
+- New Rule: Enforces the test_ prefix on a file for Foundry users [#476](https://github.com/protofire/solhint/pull/476)
+- New Rule: Enforces the naming of function return values [#478](https://github.com/protofire/solhint/pull/478)
+- `Notes` option on docs to add more information of each rule. See `foundry-test-functions`. [#476](https://github.com/protofire/solhint/pull/476)
+
+### Fixed
+- `func-named-parameters` - false positives on builtin functions [#472](https://github.com/protofire/solhint/pull/472)
+- `ordering` - treat initializer weight same as constructor [#474](https://github.com/protofire/solhint/pull/474)
+- `check-send-result` - false positive on `erc777.send()`` function [#477](https://github.com/protofire/solhint/pull/477)
+- `explicit-types` - default value is now taking into account when no value is specified in config [#481](https://github.com/protofire/solhint/pull/481)
+- `compiler-version` - default value is now taking into account when no value is specified in config [#483](https://github.com/protofire/solhint/pull/483)
+
+### Updates
+- Rule: `check-send-result` added config clarification in the new `Notes` section [#482](https://github.com/protofire/solhint/pull/482)
+- Rule: `compiler-version` default was updated from ^0.5.2 to ^0.8.0 [#483](https://github.com/protofire/solhint/pull/483)
+
+
+
## [3.5.1] - 2023-08-04
### Updated
- Support `ignoreConstructors` option for `no-empty-blocks` [#418](https://github.com/protofire/solhint/pull/418)
@@ -30,7 +70,6 @@
-
## [3.4.1] - 2023-03-06
### Updated
- Updated solidity parser to 0.16.0 [#420](https://github.com/protofire/solhint/pull/420)
@@ -46,10 +85,6 @@
- Fix named-parameters-mapping to not enforce on nested mappings [#421](https://github.com/protofire/solhint/pull/421)
-
-
-
-
## [3.4.0] - 2023-02-17
### Updated
- Solhint dependencies to support newer versions [#380](https://github.com/protofire/solhint/pull/380)
@@ -73,10 +108,6 @@
- False positive on no-unused-vars for payable arguments without name [#399](https://github.com/protofire/solhint/pull/399)
-
-
-
-
## [3.3.8] - 2023-01-17
### Fixed Docs and Typos
- [#292](https://github.com/protofire/solhint/pull/292)
diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
index 6ceced1e..eab50c55 100644
--- a/conf/rulesets/solhint-all.js
+++ b/conf/rulesets/solhint-all.js
@@ -6,6 +6,7 @@
module.exports = Object.freeze({
rules: {
'code-complexity': ['warn', 7],
+ 'custom-errors': 'warn',
'explicit-types': ['warn', 'explicit'],
'function-max-lines': ['warn', 50],
'max-line-length': ['error', 120],
@@ -28,8 +29,9 @@ module.exports = Object.freeze({
'const-name-snakecase': 'warn',
'contract-name-camelcase': 'warn',
'event-name-camelcase': 'warn',
+ 'foundry-test-functions': ['off', ['setUp']],
'func-name-mixedcase': 'warn',
- 'func-named-parameters': ['warn', 2],
+ 'func-named-parameters': ['warn', 4],
'func-param-name-mixedcase': 'warn',
'immutable-vars-naming': [
'warn',
@@ -39,6 +41,7 @@ module.exports = Object.freeze({
],
'modifier-name-mixedcase': 'warn',
'named-parameters-mapping': 'off',
+ 'named-return-values': 'warn',
'private-vars-leading-underscore': [
'warn',
{
@@ -57,7 +60,7 @@ module.exports = Object.freeze({
'avoid-throw': 'warn',
'avoid-tx-origin': 'warn',
'check-send-result': 'warn',
- 'compiler-version': ['error', '^0.5.8'],
+ 'compiler-version': ['error', '^0.8.0'],
'func-visibility': [
'warn',
{
diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js
index dbd05e77..072ea68e 100644
--- a/conf/rulesets/solhint-recommended.js
+++ b/conf/rulesets/solhint-recommended.js
@@ -5,6 +5,7 @@
module.exports = Object.freeze({
rules: {
+ 'custom-errors': 'warn',
'explicit-types': ['warn', 'explicit'],
'max-states-count': ['warn', 15],
'no-console': 'error',
@@ -24,7 +25,6 @@ module.exports = Object.freeze({
'contract-name-camelcase': 'warn',
'event-name-camelcase': 'warn',
'func-name-mixedcase': 'warn',
- 'func-named-parameters': ['warn', 2],
'immutable-vars-naming': [
'warn',
{
@@ -42,7 +42,7 @@ module.exports = Object.freeze({
'avoid-throw': 'warn',
'avoid-tx-origin': 'warn',
'check-send-result': 'warn',
- 'compiler-version': ['error', '^0.5.8'],
+ 'compiler-version': ['error', '^0.8.0'],
'func-visibility': [
'warn',
{
@@ -53,7 +53,6 @@ module.exports = Object.freeze({
'no-complex-fallback': 'warn',
'no-inline-assembly': 'warn',
'not-rely-on-block-hash': 'warn',
- 'not-rely-on-time': 'warn',
reentrancy: 'warn',
'state-visibility': 'warn',
},
diff --git a/docs/rules.md b/docs/rules.md
index 24d799a8..00b344b9 100644
--- a/docs/rules.md
+++ b/docs/rules.md
@@ -9,6 +9,7 @@ title: "Rule Index of Solhint"
| 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. | | |
@@ -33,24 +34,26 @@ title: "Rule Index of Solhint"
## Style Guide Rules
-| Rule Id | Error | Recommended | Deprecated |
-| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
-| [const-name-snakecase](./rules/naming/const-name-snakecase.md) | Constant name must be in capitalized SNAKE_CASE. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | |
-| [contract-name-camelcase](./rules/naming/contract-name-camelcase.md) | Contract name must be in CamelCase. | $~~~~~~~~$✔️ | |
-| [event-name-camelcase](./rules/naming/event-name-camelcase.md) | Event name must be in CamelCase. | $~~~~~~~~$✔️ | |
-| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md) | Function name must be in mixedCase. | $~~~~~~~~$✔️ | |
-| [func-named-parameters](./rules/naming/func-named-parameters.md) | Enforce function calls with Named Parameters when containing more than the configured qty | $~~~~~~~~$✔️ | |
-| [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md) | Function param name must be in mixedCase. | | |
-| [immutable-vars-naming](./rules/naming/immutable-vars-naming.md) | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration. | $~~~~~~~~$✔️ | |
-| [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md) | Modifier name must be in mixedCase. | | |
-| [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | |
-| [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Private and internal names must start with a single underscore. | | |
-| [use-forbidden-name](./rules/naming/use-forbidden-name.md) | Avoid to use letters 'I', 'l', 'O' as identifiers. | $~~~~~~~~$✔️ | |
-| [var-name-mixedcase](./rules/naming/var-name-mixedcase.md) | Variable name must be in mixedCase. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | |
-| [func-order](./rules/order/func-order.md) | Function order is incorrect. | | $~~~~~~~$✔️ |
-| [imports-on-top](./rules/order/imports-on-top.md) | Import statements must be on top. | $~~~~~~~~$✔️ | |
-| [ordering](./rules/order/ordering.md) | Check order of elements in file and inside each contract, according to the style guide. | | |
-| [visibility-modifier-order](./rules/order/visibility-modifier-order.md) | Visibility modifier must be first in list of modifiers. | $~~~~~~~~$✔️ | |
+| Rule Id | Error | Recommended | Deprecated |
+| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
+| [const-name-snakecase](./rules/naming/const-name-snakecase.md) | Constant name must be in capitalized SNAKE_CASE. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | |
+| [contract-name-camelcase](./rules/naming/contract-name-camelcase.md) | Contract name must be in CamelCase. | $~~~~~~~~$✔️ | |
+| [event-name-camelcase](./rules/naming/event-name-camelcase.md) | Event name must be in CamelCase. | $~~~~~~~~$✔️ | |
+| [foundry-test-functions](./rules/naming/foundry-test-functions.md) | Enforce naming convention on functions for Foundry test cases | | |
+| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md) | Function name must be in mixedCase. | $~~~~~~~~$✔️ | |
+| [func-named-parameters](./rules/naming/func-named-parameters.md) | Enforce named parameters for function calls with 4 or more arguments. This rule may have some false positives | | |
+| [func-param-name-mixedcase](./rules/naming/func-param-name-mixedcase.md) | Function param name must be in mixedCase. | | |
+| [immutable-vars-naming](./rules/naming/immutable-vars-naming.md) | Check Immutable variables. Capitalized SNAKE_CASE or mixedCase depending on configuration. | $~~~~~~~~$✔️ | |
+| [modifier-name-mixedcase](./rules/naming/modifier-name-mixedcase.md) | Modifier name must be in mixedCase. | | |
+| [named-parameters-mapping](./rules/naming/named-parameters-mapping.md) | Solidity v0.8.18 introduced named parameters on the mappings definition. | | |
+| [named-return-values](./rules/naming/named-return-values.md) | Enforce the return values of a function to be named | | |
+| [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Private and internal names must start with a single underscore. | | |
+| [use-forbidden-name](./rules/naming/use-forbidden-name.md) | Avoid to use letters 'I', 'l', 'O' as identifiers. | $~~~~~~~~$✔️ | |
+| [var-name-mixedcase](./rules/naming/var-name-mixedcase.md) | Variable name must be in mixedCase. (Does not check IMMUTABLES, use immutable-vars-naming) | $~~~~~~~~$✔️ | |
+| [func-order](./rules/order/func-order.md) | Function order is incorrect. | | $~~~~~~~$✔️ |
+| [imports-on-top](./rules/order/imports-on-top.md) | Import statements must be on top. | $~~~~~~~~$✔️ | |
+| [ordering](./rules/order/ordering.md) | Check order of elements in file and inside each contract, according to the style guide. | | |
+| [visibility-modifier-order](./rules/order/visibility-modifier-order.md) | Visibility modifier must be first in list of modifiers. | $~~~~~~~~$✔️ | |
## Security Rules
@@ -71,7 +74,7 @@ title: "Rule Index of Solhint"
| [no-complex-fallback](./rules/security/no-complex-fallback.md) | Fallback function must be simple. | $~~~~~~~~$✔️ | |
| [no-inline-assembly](./rules/security/no-inline-assembly.md) | Avoid to use inline assembly. It is acceptable only in rare cases. | $~~~~~~~~$✔️ | |
| [not-rely-on-block-hash](./rules/security/not-rely-on-block-hash.md) | Do not rely on "block.blockhash". Miners can influence its value. | $~~~~~~~~$✔️ | |
-| [not-rely-on-time](./rules/security/not-rely-on-time.md) | Avoid making time-based decisions in your business logic. | $~~~~~~~~$✔️ | |
+| [not-rely-on-time](./rules/security/not-rely-on-time.md) | Avoid making time-based decisions in your business logic. | | |
| [reentrancy](./rules/security/reentrancy.md) | Possible reentrancy vulnerabilities. Avoid state changes after transfer. | $~~~~~~~~$✔️ | |
| [state-visibility](./rules/security/state-visibility.md) | Explicitly mark visibility of state. | $~~~~~~~~$✔️ | |
diff --git a/docs/rules/best-practises/custom-errors.md b/docs/rules/best-practises/custom-errors.md
new file mode 100644
index 00000000..afe5b3c7
--- /dev/null
+++ b/docs/rules/best-practises/custom-errors.md
@@ -0,0 +1,71 @@
+---
+warning: "This is a dynamically generated file. Do not edit manually."
+layout: "default"
+title: "custom-errors | Solhint"
+---
+
+# custom-errors
+
+
+
+> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.
+
+
+## Description
+Enforces the use of Custom Errors over Require and Revert statements
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+ "rules": {
+ "custom-errors": "warn"
+ }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Use of Custom Errors
+
+```solidity
+revert CustomErrorFunction();
+```
+
+#### Use of Custom Errors with arguments
+
+```solidity
+revert CustomErrorFunction({ msg: "Insufficient Balance" });
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Use of require statement
+
+```solidity
+require(userBalance >= availableAmount, "Insufficient Balance");
+```
+
+#### Use of plain revert statement
+
+```solidity
+revert();
+```
+
+#### Use of revert statement with message
+
+```solidity
+revert("Insufficient Balance");
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/custom-errors.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/custom-errors.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/custom-errors.js)
diff --git a/docs/rules/best-practises/explicit-types.md b/docs/rules/best-practises/explicit-types.md
index 96cb06a5..8448bc6e 100644
--- a/docs/rules/best-practises/explicit-types.md
+++ b/docs/rules/best-practises/explicit-types.md
@@ -63,7 +63,7 @@ uint256 public variableName
```
## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/explicit-types.js)
diff --git a/docs/rules/best-practises/no-unused-import.md b/docs/rules/best-practises/no-unused-import.md
index 8d13e897..8d567c91 100644
--- a/docs/rules/best-practises/no-unused-import.md
+++ b/docs/rules/best-practises/no-unused-import.md
@@ -51,7 +51,7 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
```
## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/no-unused-import.js)
diff --git a/docs/rules/naming/foundry-test-functions.md b/docs/rules/naming/foundry-test-functions.md
new file mode 100644
index 00000000..534ad6e5
--- /dev/null
+++ b/docs/rules/naming/foundry-test-functions.md
@@ -0,0 +1,73 @@
+---
+warning: "This is a dynamically generated file. Do not edit manually."
+layout: "default"
+title: "foundry-test-functions | Solhint"
+---
+
+# foundry-test-functions
+
+
+
+## Description
+Enforce naming convention on functions for Foundry test cases
+
+## Options
+This rule accepts an array of options:
+
+| Index | Description | Default Value |
+| ----- | ----------------------------------------------------- | ------------- |
+| 0 | Rule severity. Must be one of "error", "warn", "off". | off |
+
+
+### Example Config
+```json
+{
+ "rules": {
+ "foundry-test-functions": ["off",["setUp"]]
+ }
+}
+```
+
+### Notes
+- This rule can be configured to skip certain function names in the SKIP array. In Example Config. ```setUp``` function will be skipped
+- Supported Regex: ```test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}```
+- This rule should be executed in a separate folder with a separate .solhint.json => ```solhint --config .solhint.json testFolder/**/*.sol```
+- This rule applies only to `external` and `public` functions
+- This rule skips the `setUp()` function
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Foundry test case with correct Function declaration
+
+```solidity
+function test_NumberIs42() public {}
+```
+
+#### Foundry test case with correct Function declaration
+
+```solidity
+function testFail_Subtract43() public {}
+```
+
+#### Foundry test case with correct Function declaration
+
+```solidity
+function testFuzz_FuzzyTest() public {}
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Foundry test case with incorrect Function declaration
+
+```solidity
+function numberIs42() public {}
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/foundry-test-functions.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/foundry-test-functions.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/foundry-test-functions.js)
diff --git a/docs/rules/naming/func-named-parameters.md b/docs/rules/naming/func-named-parameters.md
index 3c6710ce..44fe7da6 100644
--- a/docs/rules/naming/func-named-parameters.md
+++ b/docs/rules/naming/func-named-parameters.md
@@ -5,29 +5,26 @@ title: "func-named-parameters | Solhint"
---
# func-named-parameters
-
-> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.
-
## Description
-Enforce function calls with Named Parameters when containing more than the configured qty
+Enforce named parameters for function calls with 4 or more arguments. This rule may have some false positives
## Options
This rule accepts an array of options:
-| Index | Description | Default Value |
-| ----- | ------------------------------------------------------ | ------------- |
-| 0 | Rule severity. Must be one of "error", "warn", "off". | warn |
-| 1 | Maximum qty of unnamed parameters for a function call. | 2 |
+| Index | Description | Default Value |
+| ----- | -------------------------------------------------------------------------------------------------------- | ------------- |
+| 0 | Rule severity. Must be one of "error", "warn", "off". | warn |
+| 1 | Minimum qty of unnamed parameters for a function call (to prevent false positives on builtin functions). | 4 |
### Example Config
```json
{
"rules": {
- "func-named-parameters": ["warn",2]
+ "func-named-parameters": ["warn",4]
}
}
```
@@ -36,7 +33,7 @@ This rule accepts an array of options:
## Examples
### 👍 Examples of **correct** code for this rule
-#### Function call with two UNNAMED parameters (default is max 2)
+#### Function call with two UNNAMED parameters (default is 4)
```solidity
functionName('0xA81705c8C247C413a19A244938ae7f4A0393944e', 1e18)
@@ -56,14 +53,14 @@ functionName({ sender: _senderAddress, amount: 1e18, token: _tokenAddress, recei
### 👎 Examples of **incorrect** code for this rule
-#### Function call with four UNNAMED parameters (default is max 2)
+#### Function call with four UNNAMED parameters (default 4)
```solidity
functionName(_senderAddress, 1e18, _tokenAddress, _receiverAddress )
```
## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/func-named-parameters.js)
diff --git a/docs/rules/naming/immutable-vars-naming.md b/docs/rules/naming/immutable-vars-naming.md
index 8705d7ec..a12c2291 100644
--- a/docs/rules/naming/immutable-vars-naming.md
+++ b/docs/rules/naming/immutable-vars-naming.md
@@ -37,7 +37,7 @@ This rule accepts an array of options:
This rule does not have examples.
## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.5.1](https://github.com/protofire/solhint/tree/v3.5.1)
## Resources
- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/immutable-vars-naming.js)
diff --git a/docs/rules/naming/named-return-values.md b/docs/rules/naming/named-return-values.md
new file mode 100644
index 00000000..be9ce471
--- /dev/null
+++ b/docs/rules/naming/named-return-values.md
@@ -0,0 +1,50 @@
+---
+warning: "This is a dynamically generated file. Do not edit manually."
+layout: "default"
+title: "named-return-values | Solhint"
+---
+
+# named-return-values
+
+
+
+## Description
+Enforce the return values of a function to be named
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+ "rules": {
+ "named-return-values": "warn"
+ }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### Function definition with named return values
+
+```solidity
+function checkBalance(address wallet) external view returns(uint256 retBalance) {}
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### Function definition with UNNAMED return values
+
+```solidity
+function checkBalance(address wallet) external view returns(uint256) {}
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/named-return-values.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/naming/named-return-values.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/naming/named-return-values.js)
diff --git a/docs/rules/security/check-send-result.md b/docs/rules/security/check-send-result.md
index 7f8521bb..18db9a94 100644
--- a/docs/rules/security/check-send-result.md
+++ b/docs/rules/security/check-send-result.md
@@ -26,6 +26,9 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
}
```
+### Notes
+- Rule will rise false positive on this: `bool success = walletAddress.send(amount); require(success, "Failed to send"); `
+- Rule will skip ERC777 "send" function to prevent false positives
## Examples
### 👍 Examples of **correct** code for this rule
@@ -36,6 +39,12 @@ This rule accepts a string option of rule severity. Must be one of "error", "war
if(x.send(55)) {}
```
+#### result of "send" call checked within a require
+
+```solidity
+require(payable(walletAddress).send(moneyAmount), "Failed to send moneyAmount");
+```
+
### 👎 Examples of **incorrect** code for this rule
#### result of "send" call ignored
diff --git a/docs/rules/security/compiler-version.md b/docs/rules/security/compiler-version.md
index e930e93d..d7bc39de 100644
--- a/docs/rules/security/compiler-version.md
+++ b/docs/rules/security/compiler-version.md
@@ -20,14 +20,14 @@ This rule accepts an array of options:
| Index | Description | Default Value |
| ----- | ----------------------------------------------------- | ------------- |
| 0 | Rule severity. Must be one of "error", "warn", "off". | error |
-| 1 | Semver requirement | ^0.5.8 |
+| 1 | Semver requirement | ^0.8.0 |
### Example Config
```json
{
"rules": {
- "compiler-version": ["error","^0.5.8"]
+ "compiler-version": ["error","^0.8.0"]
}
}
```
diff --git a/docs/rules/security/not-rely-on-time.md b/docs/rules/security/not-rely-on-time.md
index f9998d73..dcc24251 100644
--- a/docs/rules/security/not-rely-on-time.md
+++ b/docs/rules/security/not-rely-on-time.md
@@ -5,11 +5,8 @@ title: "not-rely-on-time | Solhint"
---
# not-rely-on-time
-
-> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.
-
## Description
Avoid making time-based decisions in your business logic.
diff --git a/e2e/05-max-warnings/contracts/Foo.sol b/e2e/05-max-warnings/contracts/Foo.sol
index 659755d7..1c045747 100644
--- a/e2e/05-max-warnings/contracts/Foo.sol
+++ b/e2e/05-max-warnings/contracts/Foo.sol
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
-pragma solidity >=0.5.8;
+pragma solidity >=0.8.0;
contract Foo {
uint256 public constant test1 = 1;
diff --git a/e2e/06-formatters/contracts/Foo2.sol b/e2e/06-formatters/contracts/Foo2.sol
index 9fb98013..4a5c2af2 100644
--- a/e2e/06-formatters/contracts/Foo2.sol
+++ b/e2e/06-formatters/contracts/Foo2.sol
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
-pragma solidity >=0.5.8;
+pragma solidity >=0.8.0;
contract Foo {
uint256 public constant test1 = 1;
diff --git a/e2e/06-formatters/contracts/Foo3.sol b/e2e/06-formatters/contracts/Foo3.sol
index f89c6912..8c1c1e0a 100644
--- a/e2e/06-formatters/contracts/Foo3.sol
+++ b/e2e/06-formatters/contracts/Foo3.sol
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
-pragma solidity >=0.5.8;
+pragma solidity >=0.8.0;
contract Foo {
uint256 public constant TEST1 = 1;
diff --git a/e2e/06-formatters/helpers/helpers.js b/e2e/06-formatters/helpers/helpers.js
index 05089917..85d43c6e 100644
--- a/e2e/06-formatters/helpers/helpers.js
+++ b/e2e/06-formatters/helpers/helpers.js
@@ -3,7 +3,7 @@ const foo1Output = [
line: 2,
column: 1,
severity: 'Error',
- message: 'Compiler version >=0.6.0 does not satisfy the ^0.5.8 semver requirement',
+ message: 'Compiler version >=0.6.0 does not satisfy the ^0.8.0 semver requirement',
ruleId: 'compiler-version',
fix: null,
filePath: 'contracts/Foo.sol',
diff --git a/e2e/07-foundry-test/.solhint.json b/e2e/07-foundry-test/.solhint.json
new file mode 100644
index 00000000..92277f3d
--- /dev/null
+++ b/e2e/07-foundry-test/.solhint.json
@@ -0,0 +1,7 @@
+{
+ "extends": "solhint:recommended",
+ "rules": {
+ "compiler-version": "off",
+ "func-visibility": "off"
+ }
+}
diff --git a/e2e/07-foundry-test/contracts/Foo.sol b/e2e/07-foundry-test/contracts/Foo.sol
new file mode 100644
index 00000000..3ffecf60
--- /dev/null
+++ b/e2e/07-foundry-test/contracts/Foo.sol
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.0;
+
+contract Foo {
+ uint256 public constant TEST = 1;
+
+ constructor() {
+
+ }
+}
diff --git a/e2e/07-foundry-test/test/.solhint.json b/e2e/07-foundry-test/test/.solhint.json
new file mode 100644
index 00000000..da87cc47
--- /dev/null
+++ b/e2e/07-foundry-test/test/.solhint.json
@@ -0,0 +1,5 @@
+{
+ "rules": {
+ "foundry-test-functions": ["error", ["setUp", "finish"]]
+ }
+}
diff --git a/e2e/07-foundry-test/test/FooTest.sol b/e2e/07-foundry-test/test/FooTest.sol
new file mode 100644
index 00000000..be603e53
--- /dev/null
+++ b/e2e/07-foundry-test/test/FooTest.sol
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.6.0;
+
+import "./Test.sol";
+
+contract FooTest is Test {
+ uint256 testNumber;
+
+ function setUp() public {
+ testNumber = 42;
+ }
+
+ function testFail_Subtract43() public {
+ testNumber -= 43;
+ }
+
+ function wrongFunctionDefinitionName() public {
+ testNumber -= 43;
+ }
+}
\ No newline at end of file
diff --git a/e2e/07-foundry-test/test/Test.sol b/e2e/07-foundry-test/test/Test.sol
new file mode 100644
index 00000000..d7d16242
--- /dev/null
+++ b/e2e/07-foundry-test/test/Test.sol
@@ -0,0 +1,4 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.6.0;
+
+contract Test {}
diff --git a/e2e/test.js b/e2e/test.js
index bf3910f3..c218dae1 100644
--- a/e2e/test.js
+++ b/e2e/test.js
@@ -147,4 +147,26 @@ describe('e2e', function () {
expect(stdout.trim()).to.not.contain(errorFound)
})
})
+
+ describe('Linter - foundry-test-functions with shell', () => {
+ // Foo contract has 1 warning
+ // FooTest contract has 1 error
+ useFixture('07-foundry-test')
+
+ it(`should raise error for empty blocks only`, () => {
+ const { code, stdout } = shell.exec('solhint contracts/Foo.sol')
+
+ expect(code).to.equal(0)
+ expect(stdout.trim()).to.contain('Code contains empty blocks')
+ })
+
+ it(`should raise error for wrongFunctionDefinitionName() only`, () => {
+ const { code, stdout } = shell.exec('solhint -c test/.solhint.json test/FooTest.sol')
+
+ expect(code).to.equal(1)
+ expect(stdout.trim()).to.contain(
+ 'Function wrongFunctionDefinitionName() must match Foundry test naming convention'
+ )
+ })
+ })
})
diff --git a/lib/common/identifier-naming.js b/lib/common/identifier-naming.js
index 1444d398..efc7e3f5 100644
--- a/lib/common/identifier-naming.js
+++ b/lib/common/identifier-naming.js
@@ -30,4 +30,9 @@ module.exports = {
hasLeadingUnderscore(text) {
return text && text[0] === '_'
},
+
+ isNotFoundryTestCase(text) {
+ const regex = /^test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}$/
+ return !match(text, regex)
+ },
}
diff --git a/lib/config.js b/lib/config.js
index 44b4c420..69b3dad0 100644
--- a/lib/config.js
+++ b/lib/config.js
@@ -15,11 +15,6 @@ module.exports = {
return _.isBoolean(configVal) ? configVal : defaultValue
},
- getStringByPath(path, defaultValue) {
- const configVal = _.get(this, path)
- return _.isString(configVal) ? configVal : defaultValue
- },
-
getNumber(ruleName, defaultValue) {
return this.getNumberByPath(`rules["${ruleName}"][1]`, defaultValue)
},
@@ -32,7 +27,18 @@ module.exports = {
return this.getBooleanByPath(`rules["${ruleName}"][1][${ruleProperty}]`, defaultValue)
},
- getString(ruleName, defaultValue) {
- return this.getStringByPath(`rules["${ruleName}"][1]`, defaultValue)
+ getStringFromArray(ruleName, defaultValue) {
+ if (this.rules && this.rules[ruleName]) {
+ const ruleValue = this.rules[ruleName]
+ return Array.isArray(ruleValue) ? ruleValue[1] : defaultValue
+ } else {
+ return defaultValue
+ }
+ },
+
+ getArray(ruleName, defaultValue) {
+ const path = `rules["${ruleName}"][1]`
+ const configVal = _.get(this, path)
+ return _.isArray(configVal) ? configVal : defaultValue
},
}
diff --git a/lib/rules/best-practises/custom-errors.js b/lib/rules/best-practises/custom-errors.js
new file mode 100644
index 00000000..ce4304af
--- /dev/null
+++ b/lib/rules/best-practises/custom-errors.js
@@ -0,0 +1,76 @@
+const BaseChecker = require('../base-checker')
+const { severityDescription } = require('../../doc/utils')
+
+const DEFAULT_SEVERITY = 'warn'
+
+const ruleId = 'custom-errors'
+const meta = {
+ type: 'best-practises',
+
+ docs: {
+ description: 'Enforces the use of Custom Errors over Require and Revert statements',
+ category: 'Best Practise Rules',
+ options: [
+ {
+ description: severityDescription,
+ default: DEFAULT_SEVERITY,
+ },
+ ],
+ examples: {
+ good: [
+ {
+ description: 'Use of Custom Errors',
+ code: 'revert CustomErrorFunction();',
+ },
+ {
+ description: 'Use of Custom Errors with arguments',
+ code: 'revert CustomErrorFunction({ msg: "Insufficient Balance" });',
+ },
+ ],
+ bad: [
+ {
+ description: 'Use of require statement',
+ code: 'require(userBalance >= availableAmount, "Insufficient Balance");',
+ },
+ {
+ description: 'Use of plain revert statement',
+ code: 'revert();',
+ },
+ {
+ description: 'Use of revert statement with message',
+ code: 'revert("Insufficient Balance");',
+ },
+ ],
+ },
+ },
+
+ isDefault: false,
+ recommended: true,
+ defaultSetup: DEFAULT_SEVERITY,
+
+ schema: null,
+}
+
+class CustomErrorsChecker extends BaseChecker {
+ constructor(reporter) {
+ super(reporter, ruleId, meta)
+ }
+
+ FunctionCall(node) {
+ let errorStr = ''
+ if (node.expression.name === 'require') {
+ errorStr = 'require'
+ } else if (
+ node.expression.name === 'revert' &&
+ (node.arguments.length === 0 || node.arguments[0].type === 'StringLiteral')
+ ) {
+ errorStr = 'revert'
+ }
+
+ if (errorStr !== '') {
+ this.error(node, `Use Custom Errors instead of ${errorStr} statements`)
+ }
+ }
+}
+
+module.exports = CustomErrorsChecker
diff --git a/lib/rules/best-practises/explicit-types.js b/lib/rules/best-practises/explicit-types.js
index 4f2a6149..ebf7b60d 100644
--- a/lib/rules/best-practises/explicit-types.js
+++ b/lib/rules/best-practises/explicit-types.js
@@ -62,13 +62,17 @@ const meta = {
class ExplicitTypesChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)
- this.configOption = (config && config.getString(ruleId, DEFAULT_OPTION)) || DEFAULT_OPTION
+ this.configOption =
+ (config && config.getStringFromArray(ruleId, DEFAULT_OPTION)) || DEFAULT_OPTION
this.isExplicit = this.configOption === 'explicit'
-
- this.validateConfigOption(this.configOption)
}
VariableDeclaration(node) {
+ if (!VALID_CONFIGURATION_OPTIONS.includes(this.configOption)) {
+ this.error(node, 'Error: Config error on [explicit-types]. See explicit-types.md.')
+ return
+ }
+
const typeToFind = 'ElementaryTypeName'
const onlyTypes = this.findNamesOfElementaryTypeName(node, typeToFind)
@@ -89,12 +93,6 @@ class ExplicitTypesChecker extends BaseChecker {
}
}
- validateConfigOption(configOption) {
- const configOk = VALID_CONFIGURATION_OPTIONS.includes(configOption)
- if (!configOk)
- throw new Error('Error: Config error on [explicit-types]. See explicit-types.md.')
- }
-
validateVariables(configType, node, varsToBeChecked) {
const errorVars = varsToBeChecked.filter((type) => configType.includes(type))
diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js
index 6b2ac780..e7f5228d 100644
--- a/lib/rules/best-practises/index.js
+++ b/lib/rules/best-practises/index.js
@@ -10,8 +10,9 @@ const NoConsoleLogChecker = require('./no-console')
const NoGlobalImportsChecker = require('./no-global-import')
const NoUnusedImportsChecker = require('./no-unused-import')
const ExplicitTypesChecker = require('./explicit-types')
+const CustomErrorsChecker = require('./custom-errors')
-module.exports = function checkers(reporter, config, inputSrc) {
+module.exports = function checkers(reporter, config, inputSrc, tokens) {
return [
new CodeComplexityChecker(reporter, config),
new FunctionMaxLinesChecker(reporter, config),
@@ -23,7 +24,8 @@ module.exports = function checkers(reporter, config, inputSrc) {
new ReasonStringChecker(reporter, config),
new NoConsoleLogChecker(reporter),
new NoGlobalImportsChecker(reporter),
- new NoUnusedImportsChecker(reporter),
+ new NoUnusedImportsChecker(reporter, tokens),
new ExplicitTypesChecker(reporter, config),
+ new CustomErrorsChecker(reporter),
]
}
diff --git a/lib/rules/best-practises/no-unused-import.js b/lib/rules/best-practises/no-unused-import.js
index 993ae144..297875ff 100644
--- a/lib/rules/best-practises/no-unused-import.js
+++ b/lib/rules/best-practises/no-unused-import.js
@@ -43,9 +43,10 @@ function isImportIdentifier(node) {
}
class NoUnusedImportsChecker extends BaseChecker {
- constructor(reporter) {
+ constructor(reporter, tokens) {
super(reporter, ruleId, meta)
this.importedNames = {}
+ this.tokens = tokens
}
registerUsage(rawName) {
@@ -60,14 +61,6 @@ class NoUnusedImportsChecker extends BaseChecker {
}
}
- ArrayTypeName(node) {
- this.registerUsage(node.baseTypeName.namePath)
- }
-
- NewExpression(node) {
- this.registerUsage(node.typeName.namePath)
- }
-
Identifier(node) {
if (!isImportIdentifier(node)) {
this.registerUsage(node.name)
@@ -75,9 +68,6 @@ class NoUnusedImportsChecker extends BaseChecker {
}
FunctionDefinition(node) {
- if (node.override) {
- node.override.forEach((it) => this.registerUsage(it.namePath))
- }
if (node.isConstructor) {
node.modifiers.forEach((it) => this.registerUsage(it.name))
}
@@ -93,18 +83,8 @@ class NoUnusedImportsChecker extends BaseChecker {
}
}
- VariableDeclaration(node) {
- // this ignores builtin types, the check inside registerUsage ignores types
- // defined in the same file
- if (node.typeName.type === 'UserDefinedTypeName') {
- this.registerUsage(node.typeName.namePath)
- }
- }
-
- ContractDefinition(node) {
- node.baseContracts.forEach((inheritanceSpecifier) => {
- this.registerUsage(inheritanceSpecifier.baseName.namePath)
- })
+ UserDefinedTypeName(node) {
+ this.registerUsage(node.namePath)
}
UsingForDeclaration(node) {
@@ -113,6 +93,16 @@ class NoUnusedImportsChecker extends BaseChecker {
}
'SourceUnit:exit'() {
+ const keywords = this.tokens.filter((it) => it.type === 'Keyword')
+ const inheritdocStatements = keywords.filter(
+ ({ value }) => /^\/\/\/ *@inheritdoc/.test(value) || /^\/\*\* *@inheritdoc/.test(value)
+ )
+ inheritdocStatements.forEach(({ value }) => {
+ const match = value.match(/@inheritdoc *([a-zA-Z0-9_]*)/)
+ if (match && match[1]) {
+ this.registerUsage(match[1])
+ }
+ })
forIn(this.importedNames, (value, key) => {
if (!value.used) {
this.error(value.node, `imported name ${key} is not used`)
diff --git a/lib/rules/index.js b/lib/rules/index.js
index d7e78ab8..7e8a3b55 100644
--- a/lib/rules/index.js
+++ b/lib/rules/index.js
@@ -58,7 +58,7 @@ function coreRules(meta) {
const { reporter, config, inputSrc, tokens } = meta
return [
- ...bestPractises(reporter, config, inputSrc),
+ ...bestPractises(reporter, config, inputSrc, tokens),
...deprecations(reporter),
...miscellaneous(reporter, config, tokens),
...naming(reporter, config),
diff --git a/lib/rules/naming/foundry-test-functions.js b/lib/rules/naming/foundry-test-functions.js
new file mode 100644
index 00000000..1cc367a6
--- /dev/null
+++ b/lib/rules/naming/foundry-test-functions.js
@@ -0,0 +1,93 @@
+const BaseChecker = require('../base-checker')
+const naming = require('../../common/identifier-naming')
+const { severityDescription } = require('../../doc/utils')
+
+const DEFAULT_SEVERITY = 'off'
+const DEFAULT_SKIP_FUNCTIONS = ['setUp']
+
+const ruleId = 'foundry-test-functions'
+const meta = {
+ type: 'naming',
+
+ docs: {
+ description: `Enforce naming convention on functions for Foundry test cases`,
+ category: 'Style Guide Rules',
+ options: [
+ {
+ description: severityDescription,
+ default: DEFAULT_SEVERITY,
+ },
+ ],
+ examples: {
+ good: [
+ {
+ description: 'Foundry test case with correct Function declaration',
+ code: 'function test_NumberIs42() public {}',
+ },
+ {
+ description: 'Foundry test case with correct Function declaration',
+ code: 'function testFail_Subtract43() public {}',
+ },
+ {
+ description: 'Foundry test case with correct Function declaration',
+ code: 'function testFuzz_FuzzyTest() public {}',
+ },
+ ],
+ bad: [
+ {
+ description: 'Foundry test case with incorrect Function declaration',
+ code: 'function numberIs42() public {}',
+ },
+ ],
+ },
+ notes: [
+ {
+ note: 'This rule can be configured to skip certain function names in the SKIP array. In Example Config. ```setUp``` function will be skipped',
+ },
+ { note: 'Supported Regex: ```test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\\w{1,}```' },
+ {
+ note: 'This rule should be executed in a separate folder with a separate .solhint.json => ```solhint --config .solhint.json testFolder/**/*.sol```',
+ },
+ {
+ note: 'This rule applies only to `external` and `public` functions',
+ },
+ {
+ note: 'This rule skips the `setUp()` function',
+ },
+ ],
+ },
+
+ isDefault: false,
+ recommended: false,
+ defaultSetup: [DEFAULT_SEVERITY, DEFAULT_SKIP_FUNCTIONS],
+
+ schema: { type: 'array' },
+}
+
+class FoundryTestFunctionsChecker extends BaseChecker {
+ constructor(reporter, config) {
+ super(reporter, ruleId, meta)
+ this.skippedFunctions = config
+ ? config.getArray(ruleId, DEFAULT_SKIP_FUNCTIONS)
+ : DEFAULT_SKIP_FUNCTIONS
+ }
+
+ FunctionDefinition(node) {
+ // function name should not be in skipped functions array
+ // should be external or public
+ if (
+ !this.searchInArray(this.skippedFunctions, node.name) &&
+ (node.visibility === 'public' || node.visibility === 'external')
+ ) {
+ if (naming.isNotFoundryTestCase(node.name)) {
+ this.error(node, `Function ${node.name}() must match Foundry test naming convention`)
+ }
+ }
+ }
+
+ searchInArray(array, searchString) {
+ return array.indexOf(searchString) !== -1
+ }
+}
+
+module.exports = FoundryTestFunctionsChecker
diff --git a/lib/rules/naming/func-named-parameters.js b/lib/rules/naming/func-named-parameters.js
index 24d71d13..170ea4ba 100644
--- a/lib/rules/naming/func-named-parameters.js
+++ b/lib/rules/naming/func-named-parameters.js
@@ -2,15 +2,14 @@ const BaseChecker = require('../base-checker')
const { severityDescription } = require('../../doc/utils')
const DEFAULT_SEVERITY = 'warn'
-const DEFAULT_MAX_UNNAMED_ARGUMENTS = 2
+const DEFAULT_MIN_UNNAMED_ARGUMENTS = 4
const ruleId = 'func-named-parameters'
const meta = {
type: 'naming',
docs: {
- description:
- 'Enforce function calls with Named Parameters when containing more than the configured qty',
+ description: `Enforce named parameters for function calls with ${DEFAULT_MIN_UNNAMED_ARGUMENTS} or more arguments. This rule may have some false positives`,
category: 'Style Guide Rules',
options: [
{
@@ -18,14 +17,15 @@ const meta = {
default: DEFAULT_SEVERITY,
},
{
- description: 'Maximum qty of unnamed parameters for a function call.',
- default: JSON.stringify(DEFAULT_MAX_UNNAMED_ARGUMENTS),
+ description:
+ 'Minimum qty of unnamed parameters for a function call (to prevent false positives on builtin functions).',
+ default: JSON.stringify(DEFAULT_MIN_UNNAMED_ARGUMENTS),
},
],
examples: {
good: [
{
- description: 'Function call with two UNNAMED parameters (default is max 2)',
+ description: 'Function call with two UNNAMED parameters (default is 4)',
code: "functionName('0xA81705c8C247C413a19A244938ae7f4A0393944e', 1e18)",
},
{
@@ -39,7 +39,7 @@ const meta = {
],
bad: [
{
- description: 'Function call with four UNNAMED parameters (default is max 2)',
+ description: 'Function call with four UNNAMED parameters (default 4)',
code: 'functionName(_senderAddress, 1e18, _tokenAddress, _receiverAddress )',
},
],
@@ -47,8 +47,8 @@ const meta = {
},
isDefault: false,
- recommended: true,
- defaultSetup: [DEFAULT_SEVERITY, DEFAULT_MAX_UNNAMED_ARGUMENTS],
+ recommended: false,
+ defaultSetup: [DEFAULT_SEVERITY, DEFAULT_MIN_UNNAMED_ARGUMENTS],
schema: { type: 'integer' },
}
@@ -56,8 +56,11 @@ const meta = {
class FunctionNamedParametersChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)
- this.maxUnnamedArguments = config && config.getNumber(ruleId, DEFAULT_MAX_UNNAMED_ARGUMENTS)
- if (!(this.maxUnnamedArguments >= 0)) this.maxUnnamedArguments = DEFAULT_MAX_UNNAMED_ARGUMENTS
+ this.maxUnnamedArguments =
+ (config && config.getNumber(ruleId, DEFAULT_MIN_UNNAMED_ARGUMENTS)) ||
+ DEFAULT_MIN_UNNAMED_ARGUMENTS
+ if (this.maxUnnamedArguments < DEFAULT_MIN_UNNAMED_ARGUMENTS)
+ this.maxUnnamedArguments = DEFAULT_MIN_UNNAMED_ARGUMENTS
}
FunctionCall(node) {
@@ -68,7 +71,7 @@ class FunctionNamedParametersChecker extends BaseChecker {
if (qtyNamed === 0 && qtyArgs > this.maxUnnamedArguments) {
this.error(
node,
- `Missing named parameters. Max unnamed parameters value is ${this.maxUnnamedArguments}`
+ `Named parameters missing. MIN unnamed argumenst is ${this.maxUnnamedArguments}`
)
}
}
diff --git a/lib/rules/naming/index.js b/lib/rules/naming/index.js
index 1dffa3f8..efb3901f 100644
--- a/lib/rules/naming/index.js
+++ b/lib/rules/naming/index.js
@@ -10,6 +10,8 @@ const VarNameMixedcaseChecker = require('./var-name-mixedcase')
const NamedParametersMappingChecker = require('./named-parameters-mapping')
const ImmutableVarsNamingChecker = require('./immutable-vars-naming')
const FunctionNamedParametersChecker = require('./func-named-parameters')
+const FoundryTestFunctionsChecker = require('./foundry-test-functions')
+const NamedReturnValuesChecker = require('./named-return-values')
module.exports = function checkers(reporter, config) {
return [
@@ -25,5 +27,7 @@ module.exports = function checkers(reporter, config) {
new NamedParametersMappingChecker(reporter),
new ImmutableVarsNamingChecker(reporter, config),
new FunctionNamedParametersChecker(reporter, config),
+ new FoundryTestFunctionsChecker(reporter, config),
+ new NamedReturnValuesChecker(reporter),
]
}
diff --git a/lib/rules/naming/named-return-values.js b/lib/rules/naming/named-return-values.js
new file mode 100644
index 00000000..492a81ce
--- /dev/null
+++ b/lib/rules/naming/named-return-values.js
@@ -0,0 +1,60 @@
+const BaseChecker = require('../base-checker')
+const { severityDescription } = require('../../doc/utils')
+
+const DEFAULT_SEVERITY = 'warn'
+
+const ruleId = 'named-return-values'
+const meta = {
+ type: 'naming',
+
+ docs: {
+ description: `Enforce the return values of a function to be named`,
+ category: 'Style Guide Rules',
+ options: [
+ {
+ description: severityDescription,
+ default: DEFAULT_SEVERITY,
+ },
+ ],
+ examples: {
+ good: [
+ {
+ description: 'Function definition with named return values',
+ code: 'function checkBalance(address wallet) external view returns(uint256 retBalance) {}',
+ },
+ ],
+ bad: [
+ {
+ description: 'Function definition with UNNAMED return values',
+ code: 'function checkBalance(address wallet) external view returns(uint256) {}',
+ },
+ ],
+ },
+ },
+
+ isDefault: false,
+ recommended: false,
+ defaultSetup: DEFAULT_SEVERITY,
+
+ schema: null,
+}
+
+class NamedReturnValuesChecker extends BaseChecker {
+ constructor(reporter) {
+ super(reporter, ruleId, meta)
+ }
+
+ FunctionDefinition(node) {
+ if (node.returnParameters) {
+ let index = 0
+ for (const returnValue of node.returnParameters) {
+ if (!returnValue.name) {
+ this.error(node, `Named return value is missing - Index ${index}`)
+ }
+ index++
+ }
+ }
+ }
+}
+
+module.exports = NamedReturnValuesChecker
diff --git a/lib/rules/order/ordering.js b/lib/rules/order/ordering.js
index 494f3ad5..6d0e8a07 100644
--- a/lib/rules/order/ordering.js
+++ b/lib/rules/order/ordering.js
@@ -122,6 +122,13 @@ function sourceUnitPartOrder(node) {
throw new Error('Unrecognized source unit part, please report this issue')
}
+function isInitializerModifier(modifiers, targetName, targetArguments) {
+ // search the modifiers array with the name === 'initializer'
+ return modifiers.some(
+ (modifier) => modifier.name === targetName && modifier.arguments === targetArguments
+ )
+}
+
function contractPartOrder(node) {
if (node.type === 'UsingForDeclaration') {
return [0, 'using for declaration']
@@ -154,8 +161,13 @@ function contractPartOrder(node) {
return [40, 'modifier definition']
}
- if (node.isConstructor) {
- return [50, 'constructor']
+ if (
+ node.isConstructor ||
+ (node.type === 'FunctionDefinition' &&
+ node.name === 'initialize' &&
+ isInitializerModifier(node.modifiers, 'initializer', null))
+ ) {
+ return [50, 'constructor/initializer']
}
if (isReceiveFunction(node)) {
diff --git a/lib/rules/security/check-send-result.js b/lib/rules/security/check-send-result.js
index a20ee2ed..e6381f0f 100644
--- a/lib/rules/security/check-send-result.js
+++ b/lib/rules/security/check-send-result.js
@@ -14,16 +14,26 @@ const meta = {
good: [
{
description: 'result of "send" call checked with if statement',
- code: require('../../../test/fixtures/security/send-result-checked'),
+ code: 'if(x.send(55)) {}',
+ },
+ {
+ description: 'result of "send" call checked within a require',
+ code: 'require(payable(walletAddress).send(moneyAmount), "Failed to send moneyAmount");',
},
],
bad: [
{
description: 'result of "send" call ignored',
- code: require('../../../test/fixtures/security/send-result-ignored'),
+ code: 'x.send(55);',
},
],
},
+ notes: [
+ {
+ note: 'Rule will rise false positive on this: `bool success = walletAddress.send(amount); require(success, "Failed to send"); ` ',
+ },
+ { note: 'Rule will skip ERC777 "send" function to prevent false positives' },
+ ],
},
isDefault: false,
@@ -44,17 +54,24 @@ class CheckSendResultChecker extends BaseChecker {
validateSend(node) {
if (node.memberName === 'send') {
- const hasVarDeclaration = traversing.statementNotContains(node, 'VariableDeclaration')
- const hasIfStatement = traversing.statementNotContains(node, 'IfStatement')
- const hasRequire = traversing.someParent(node, this.isRequire)
- const hasAssert = traversing.someParent(node, this.isAssert)
+ if (this.isNotErc777Token(node)) {
+ const hasVarDeclaration = traversing.statementNotContains(node, 'VariableDeclaration')
+ const hasIfStatement = traversing.statementNotContains(node, 'IfStatement')
+ const hasRequire = traversing.someParent(node, this.isRequire)
+ const hasAssert = traversing.someParent(node, this.isAssert)
- if (!hasIfStatement && !hasVarDeclaration && !hasRequire && !hasAssert) {
- this.error(node, 'Check result of "send" call')
+ if (!hasIfStatement && !hasVarDeclaration && !hasRequire && !hasAssert) {
+ this.error(node, 'Check result of "send" call')
+ }
}
}
}
+ isNotErc777Token(node) {
+ const isErc777 = node.parent.type === 'FunctionCall' && node.parent.arguments.length >= 3
+ return !isErc777
+ }
+
isRequire(node) {
return node.type === 'FunctionCall' && node.expression.name === 'require'
}
diff --git a/lib/rules/security/compiler-version.js b/lib/rules/security/compiler-version.js
index 3bb51324..955a53bd 100644
--- a/lib/rules/security/compiler-version.js
+++ b/lib/rules/security/compiler-version.js
@@ -4,7 +4,7 @@ const { severityDescription } = require('../../doc/utils')
const ruleId = 'compiler-version'
const DEFAULT_SEVERITY = 'error'
-const DEFAULT_SEMVER = '^0.5.8'
+const DEFAULT_SEMVER = '^0.8.0'
const meta = {
type: 'security',
@@ -36,7 +36,8 @@ class CompilerVersionChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)
- this.requirement = (config && config.getString(ruleId, DEFAULT_SEMVER)) || DEFAULT_SEMVER
+ this.requirement =
+ (config && config.getStringFromArray(ruleId, DEFAULT_SEMVER)) || DEFAULT_SEMVER
}
SourceUnit(node) {
diff --git a/lib/rules/security/not-rely-on-time.js b/lib/rules/security/not-rely-on-time.js
index f3420c71..00a96098 100644
--- a/lib/rules/security/not-rely-on-time.js
+++ b/lib/rules/security/not-rely-on-time.js
@@ -10,7 +10,7 @@ const meta = {
},
isDefault: false,
- recommended: true,
+ recommended: false,
defaultSetup: 'warn',
schema: null,
diff --git a/package.json b/package.json
index dabbe266..7c53d289 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
{
"name": "solhint",
- "version": "3.5.1",
+ "version": "3.6.1",
"description": "Solidity Code Linter",
"main": "lib/index.js",
"keywords": [
diff --git a/scripts/generate-rule-docs.js b/scripts/generate-rule-docs.js
index 1e7a02e1..c170bb74 100755
--- a/scripts/generate-rule-docs.js
+++ b/scripts/generate-rule-docs.js
@@ -1,75 +1,77 @@
#!env node
const { loadRules } = require('../lib/load-rules')
-const fs = require('fs');
-const { exec, mkdir } = require("shelljs");
-const semver = require('semver');
-const path = require('path');
-const table = require('markdown-table');
-const { ruleSeverityEnum } = require('../lib/doc/utils');
+const fs = require('fs')
+const { exec, mkdir } = require('shelljs')
+const semver = require('semver')
+const path = require('path')
+const table = require('markdown-table')
+const { ruleSeverityEnum } = require('../lib/doc/utils')
/**
* Borrowed from https://github.com/eslint/eslint/blob/master/Makefile.js
*/
class GitHelper {
- /**
- * Gets the tag name where a given file was introduced first.
- * @param {string} filePath The file path to check.
- * @returns {string} The tag name.
- */
- static getFirstVersionOfFile(filePath) {
- const firstCommit = GitHelper.getFirstCommitOfFile(filePath);
- let tags = GitHelper.execSilent(`git tag --contains ${firstCommit}`);
-
- tags = GitHelper.splitCommandResultToLines(tags);
- return tags.reduce((list, version) => {
- const validatedVersion = semver.valid(version.trim());
-
- if (validatedVersion) {
- list.push(validatedVersion);
- }
- return list;
- }, []).sort(semver.compare)[0];
- }
+ /**
+ * Gets the tag name where a given file was introduced first.
+ * @param {string} filePath The file path to check.
+ * @returns {string} The tag name.
+ */
+ static getFirstVersionOfFile(filePath) {
+ const firstCommit = GitHelper.getFirstCommitOfFile(filePath)
+ let tags = GitHelper.execSilent(`git tag --contains ${firstCommit}`)
+
+ tags = GitHelper.splitCommandResultToLines(tags)
+ return tags
+ .reduce((list, version) => {
+ const validatedVersion = semver.valid(version.trim())
+
+ if (validatedVersion) {
+ list.push(validatedVersion)
+ }
+ return list
+ }, [])
+ .sort(semver.compare)[0]
+ }
- /**
- * Gets the first commit sha of the given file.
- * @param {string} filePath The file path which should be checked.
- * @returns {string} The commit sha.
- */
- static getFirstCommitOfFile(filePath) {
- let commits = GitHelper.execSilent(`git rev-list HEAD -- ${filePath}`);
+ /**
+ * Gets the first commit sha of the given file.
+ * @param {string} filePath The file path which should be checked.
+ * @returns {string} The commit sha.
+ */
+ static getFirstCommitOfFile(filePath) {
+ let commits = GitHelper.execSilent(`git rev-list HEAD -- ${filePath}`)
- commits = GitHelper.splitCommandResultToLines(commits);
- return commits[commits.length - 1].trim();
- }
+ commits = GitHelper.splitCommandResultToLines(commits)
+ return commits[commits.length - 1].trim()
+ }
- /**
- * Splits a command result to separate lines.
- * @param {string} result The command result string.
- * @returns {Array} The separated lines.
- */
- static splitCommandResultToLines(result) {
- return result.trim().split("\n");
- }
+ /**
+ * Splits a command result to separate lines.
+ * @param {string} result The command result string.
+ * @returns {Array} The separated lines.
+ */
+ static splitCommandResultToLines(result) {
+ return result.trim().split('\n')
+ }
- /**
- * Executes a command and returns the output instead of printing it to stdout.
- * @param {string} cmd The command string to execute.
- * @returns {string} The result of the executed command.
- */
- static execSilent(cmd) {
- return exec(cmd, {silent: true}).stdout;
- }
+ /**
+ * Executes a command and returns the output instead of printing it to stdout.
+ * @param {string} cmd The command string to execute.
+ * @returns {string} The result of the executed command.
+ */
+ static execSilent(cmd) {
+ return exec(cmd, { silent: true }).stdout
+ }
}
function generateRuleDoc(rule) {
- const isDefault = !rule.meta.deprecated && rule.meta.isDefault;
- const isRecommended = !rule.meta.deprecated && rule.meta.recommended;
- const isDeprecated = rule.meta.deprecated;
- const version = GitHelper.getFirstVersionOfFile(rule.file);
- const defaultSeverity = getDefaultSeverity(rule);
+ const isDefault = !rule.meta.deprecated && rule.meta.isDefault
+ const isRecommended = !rule.meta.deprecated && rule.meta.recommended
+ const isDeprecated = rule.meta.deprecated
+ const version = GitHelper.getFirstVersionOfFile(rule.file)
+ const defaultSeverity = getDefaultSeverity(rule)
- return `---
+ return `---
warning: "This is a dynamically generated file. Do not edit manually."
layout: "default"
title: "${rule.ruleId} | Solhint"
@@ -77,14 +79,20 @@ title: "${rule.ruleId} | Solhint"
# ${rule.ruleId}
${[
- recommendedBadge(isRecommended),
- deprecatedBadge(isDeprecated),
- categoryBadge(rule.meta.docs.category),
- defaultSeverityBadge(defaultSeverity),
- isDefault ? '> The {"extends": "solhint:default"} property in a configuration file enables this rule.\n' : '',
- isRecommended ? '> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.\n' : '',
- isDeprecated ? '> This rule is **deprecated**\n' : ''
-].filter(s => s !== '').join("\n")}
+ recommendedBadge(isRecommended),
+ deprecatedBadge(isDeprecated),
+ categoryBadge(rule.meta.docs.category),
+ defaultSeverityBadge(defaultSeverity),
+ isDefault
+ ? '> The {"extends": "solhint:default"} property in a configuration file enables this rule.\n'
+ : '',
+ isRecommended
+ ? '> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.\n'
+ : '',
+ isDeprecated ? '> This rule is **deprecated**\n' : '',
+]
+ .filter((s) => s !== '')
+ .join('\n')}
## Description
${rule.meta.docs.description}
@@ -94,7 +102,7 @@ ${loadOptions(rule)}
### Example Config
${loadExampleConfig(rule)}
-
+${loadNotes(rule)}
## Examples
${loadExamples(rule)}
@@ -109,44 +117,48 @@ ${linkToVersion(version)}
}
function categoryBadge(category) {
- return `}-informational)`;
+ return `}-informational)`
}
function recommendedBadge(isRecommended) {
- return isRecommended ? `` : '';
+ return isRecommended
+ ? ``
+ : ''
}
function deprecatedBadge(isDeprecated) {
- return isDeprecated ? `` : '';
+ return isDeprecated ? `` : ''
}
function defaultSeverityBadge(severity) {
- const colors = {
- 'warn': 'yellow',
- 'error': 'red'
- };
- return ``;
+ const colors = {
+ warn: 'yellow',
+ error: 'red',
+ }
+ return ``
}
function loadOptions(rule) {
- if (Array.isArray(rule.meta.defaultSetup)) {
- const optionsTable = [['Index', 'Description', 'Default Value']];
- rule.meta.docs.options.forEach((option, index) => {
- optionsTable.push([index, option.description, option.default]);
- });
- return `This rule accepts an array of options:
+ if (Array.isArray(rule.meta.defaultSetup)) {
+ const optionsTable = [['Index', 'Description', 'Default Value']]
+ rule.meta.docs.options.forEach((option, index) => {
+ optionsTable.push([index, option.description, option.default])
+ })
+ return `This rule accepts an array of options:
${table(optionsTable)}
-`;
- } else if (typeof rule.meta.defaultSetup === 'string') {
- return `This rule accepts a string option of rule severity. Must be one of ${ruleSeverityEnum}. Default to ${rule.meta.defaultSetup}.`
- } else {
- throw new Error(`Unhandled type of rule.meta.defaultSetup from rule ${rule.ruleId}`);
- }
+`
+ } else if (typeof rule.meta.defaultSetup === 'string') {
+ return `This rule accepts a string option of rule severity. Must be one of ${ruleSeverityEnum}. Default to ${rule.meta.defaultSetup}.`
+ } else {
+ throw new Error(`Unhandled type of rule.meta.defaultSetup from rule ${rule.ruleId}`)
+ }
}
function loadExampleConfig(rule) {
- return `\`\`\`json
+ return `\`\`\`json
{
"rules": {
"${rule.ruleId}": ${JSON.stringify(rule.meta.defaultSetup)}
@@ -156,77 +168,109 @@ function loadExampleConfig(rule) {
`
}
-function linkToVersion(version) {
- if (version) {
- return `This rule was introduced in [Solhint ${version}](https://github.com/protofire/solhint/tree/v${version})`;
- } else {
- return `This rule is introduced in the latest version.`;
+function loadNotes(rule) {
+ let textToReturn = ''
+ let noteValue = ''
+ if (rule.meta.docs.notes) {
+ textToReturn = `### Notes\n`
+ for (let i = 0; i < rule.meta.docs.notes.length; i++) {
+ noteValue = rule.meta.docs.notes[i].note
+ textToReturn = textToReturn + `- ${noteValue}\n`
}
+ }
+
+ return textToReturn
+}
+
+function linkToVersion(version) {
+ if (version) {
+ return `This rule was introduced in [Solhint ${version}](https://github.com/protofire/solhint/tree/v${version})`
+ } else {
+ return `This rule is introduced in the latest version.`
+ }
}
function linkToSource(rule) {
- const link = rule.file.replace(path.resolve(path.join(__dirname, '..')), '');
- return `https://github.com/protofire/solhint/tree/master${link}`;
+ const link = rule.file.replace(path.resolve(path.join(__dirname, '..')), '')
+ return `https://github.com/protofire/solhint/tree/master${link}`
}
function linkToDocumentSource(rule) {
- const link = rule.file.replace(path.resolve(path.join(__dirname, '..')), '').replace("lib/rules", "docs/rules").replace(/\.js$/, ".md");
- return `https://github.com/protofire/solhint/tree/master${link}`;
+ const link = rule.file
+ .replace(path.resolve(path.join(__dirname, '..')), '')
+ .replace('lib/rules', 'docs/rules')
+ .replace(/\.js$/, '.md')
+ return `https://github.com/protofire/solhint/tree/master${link}`
}
function linkToTestCase(rule) {
- const link = rule.file.replace(path.resolve(path.join(__dirname, '..', 'lib', 'rules')), '');
- return `https://github.com/protofire/solhint/tree/master/test/rules${link}`;
+ const link = rule.file.replace(path.resolve(path.join(__dirname, '..', 'lib', 'rules')), '')
+ return `https://github.com/protofire/solhint/tree/master/test/rules${link}`
}
function loadExamples(rule) {
- if (!rule.meta.docs.examples) {
- return "This rule does not have examples.";
- }
+ if (!rule.meta.docs.examples) {
+ return 'This rule does not have examples.'
+ }
- const examples = [loadCorrectExample(rule), loadIncorrectExample(rule)].filter(s => s !== '').join('\n\n');
- return examples === '' ? 'This rule does not have examples.' : examples;
+ const examples = [loadCorrectExample(rule), loadIncorrectExample(rule)]
+ .filter((s) => s !== '')
+ .join('\n\n')
+ return examples === '' ? 'This rule does not have examples.' : examples
}
function loadIncorrectExample(rule) {
- if (rule.meta.docs.examples.bad && rule.meta.docs.examples.bad.length) {
- return `### 👎 Examples of **incorrect** code for this rule
-
-${rule.meta.docs.examples.bad.map(ex => `#### ${ex.description}\n\n\`\`\`solidity\n${ex.code}\n\`\`\``).join("\n\n")}`;
- } else {
- return '';
- }
+ if (rule.meta.docs.examples.bad && rule.meta.docs.examples.bad.length) {
+ return `### 👎 Examples of **incorrect** code for this rule
+
+${rule.meta.docs.examples.bad
+ .map((ex) => `#### ${ex.description}\n\n\`\`\`solidity\n${ex.code}\n\`\`\``)
+ .join('\n\n')}`
+ } else {
+ return ''
+ }
}
function loadCorrectExample(rule) {
- if (rule.meta.docs.examples.good && rule.meta.docs.examples.good.length) {
- return `### 👍 Examples of **correct** code for this rule
-
-${rule.meta.docs.examples.good.map(ex => `#### ${ex.description}\n\n\`\`\`solidity\n${ex.code}\n\`\`\``).join("\n\n")}`;
- } else {
- return '';
- }
+ if (rule.meta.docs.examples.good && rule.meta.docs.examples.good.length) {
+ return `### 👍 Examples of **correct** code for this rule
+
+${rule.meta.docs.examples.good
+ .map((ex) => `#### ${ex.description}\n\n\`\`\`solidity\n${ex.code}\n\`\`\``)
+ .join('\n\n')}`
+ } else {
+ return ''
+ }
}
function getDefaultSeverity(rule) {
- if (Array.isArray(rule.meta.defaultSetup)) {
- return rule.meta.defaultSetup[0];
- } else {
- return rule.meta.defaultSetup;
- }
+ if (Array.isArray(rule.meta.defaultSetup)) {
+ return rule.meta.defaultSetup[0]
+ } else {
+ return rule.meta.defaultSetup
+ }
}
function generateRuleIndex(rulesIndexed) {
- const contents = Object.keys(rulesIndexed).map(category => {
- const rows = [["Rule Id", "Error", "Recommended", "Deprecated"]];
- rulesIndexed[category].map(rule => [`[${rule.ruleId}](./rules/${rule.meta.type}/${rule.ruleId}.md)`, rule.meta.docs.description, (rule.meta.recommended && !rule.meta.deprecated) ? '$~~~~~~~~$✔️' : '', rule.meta.deprecated ? '$~~~~~~~$✔️' : '']).forEach(row => rows.push(row));
- return `## ${category}
+ const contents = Object.keys(rulesIndexed)
+ .map((category) => {
+ const rows = [['Rule Id', 'Error', 'Recommended', 'Deprecated']]
+ rulesIndexed[category]
+ .map((rule) => [
+ `[${rule.ruleId}](./rules/${rule.meta.type}/${rule.ruleId}.md)`,
+ rule.meta.docs.description,
+ rule.meta.recommended && !rule.meta.deprecated ? '$~~~~~~~~$✔️' : '',
+ rule.meta.deprecated ? '$~~~~~~~$✔️' : '',
+ ])
+ .forEach((row) => rows.push(row))
+ return `## ${category}
${table(rows)}
- `;
- }).join("\n\n");
+ `
+ })
+ .join('\n\n')
- return `---
+ return `---
warning: "This is a dynamically generated file. Do not edit manually."
layout: "default"
title: "Rule Index of Solhint"
@@ -238,61 +282,63 @@ ${contents}
- [ConsenSys Guide for Smart Contracts](https://consensys.github.io/smart-contract-best-practices/recommendations/)
- [Solidity Style Guide](http://solidity.readthedocs.io/en/develop/style-guide.html)
-`;
+`
}
function main() {
- const program = require('commander');
- program.option('--rule-id ', 'rule id');
- program.option('--index-only', 'only generate rule index');
- program.parse(process.argv);
-
- const rules = loadRules();
- if (!program.indexOnly) {
- let ruleIds = [];
- if (program.ruleId) {
- ruleIds = program.ruleId.split(",");
- }
-
- rules.filter(rule => {
- if (ruleIds.length) {
- return ruleIds.includes(rule.ruleId);
- } else {
- return true;
- }
- }).forEach(rule => {
- const ruleDoc = generateRuleDoc(rule);
- const dir = path.resolve(path.join(__dirname, '..', 'docs', 'rules', rule.meta.type));
- const fileName = `${rule.ruleId}.md`;
- const filePath = path.resolve(path.join(dir, fileName));
- mkdir('-p', dir);
- fs.writeFile(filePath, ruleDoc, function (err) {
- if (err) {
- console.error(err);
- } else {
- console.log(`Wrote ${filePath}`)
- }
- });
- });
+ const program = require('commander')
+ program.option('--rule-id ', 'rule id')
+ program.option('--index-only', 'only generate rule index')
+ program.parse(process.argv)
+
+ const rules = loadRules()
+ if (!program.indexOnly) {
+ let ruleIds = []
+ if (program.ruleId) {
+ ruleIds = program.ruleId.split(',')
}
- const rulesIndexed = {};
- rules.forEach(rule => {
- if (!rulesIndexed[rule.meta.docs.category]) {
- rulesIndexed[rule.meta.docs.category] = [rule];
+ rules
+ .filter((rule) => {
+ if (ruleIds.length) {
+ return ruleIds.includes(rule.ruleId)
} else {
- rulesIndexed[rule.meta.docs.category].push(rule);
+ return true
}
- });
- const ruleIndexDoc = generateRuleIndex(rulesIndexed);
- const ruleIndexDocPath = path.resolve(path.join(__dirname, '..', 'docs', 'rules.md'));
- fs.writeFile(ruleIndexDocPath, ruleIndexDoc, function (err) {
- if (err) {
- console.error(err);
- } else {
- console.log(`Wrote ${ruleIndexDocPath}`)
- }
- });
+ })
+ .forEach((rule) => {
+ const ruleDoc = generateRuleDoc(rule)
+ const dir = path.resolve(path.join(__dirname, '..', 'docs', 'rules', rule.meta.type))
+ const fileName = `${rule.ruleId}.md`
+ const filePath = path.resolve(path.join(dir, fileName))
+ mkdir('-p', dir)
+ fs.writeFile(filePath, ruleDoc, function (err) {
+ if (err) {
+ console.error(err)
+ } else {
+ console.log(`Wrote ${filePath}`)
+ }
+ })
+ })
+ }
+
+ const rulesIndexed = {}
+ rules.forEach((rule) => {
+ if (!rulesIndexed[rule.meta.docs.category]) {
+ rulesIndexed[rule.meta.docs.category] = [rule]
+ } else {
+ rulesIndexed[rule.meta.docs.category].push(rule)
+ }
+ })
+ const ruleIndexDoc = generateRuleIndex(rulesIndexed)
+ const ruleIndexDocPath = path.resolve(path.join(__dirname, '..', 'docs', 'rules.md'))
+ fs.writeFile(ruleIndexDocPath, ruleIndexDoc, function (err) {
+ if (err) {
+ console.error(err)
+ } else {
+ console.log(`Wrote ${ruleIndexDocPath}`)
+ }
+ })
}
-main();
+main()
diff --git a/test/fixtures/naming/func-named-parameters.js b/test/fixtures/naming/func-named-parameters.js
index d4ceb7ac..ef0203c6 100644
--- a/test/fixtures/naming/func-named-parameters.js
+++ b/test/fixtures/naming/func-named-parameters.js
@@ -1,53 +1,64 @@
const FUNCTION_CALLS_ERRORS = {
- maxUnnamed_0_1u: {
- code: 'funcName(_sender);',
- maxUnnamed: 0,
+ err1: {
+ code: 'funcName(_sender, amount, receiver, token1, token2, token3);',
+ minUnnamed: 5,
},
- maxUnnamed_1_3u: {
- code: 'funcName(_sender, amount, receiver);',
- maxUnnamed: 1,
+ err2: {
+ code: 'funcName(_sender, amount, receiver, token1, token2);',
+ minUnnamed: 4,
},
- maxUnnamed_2_3u: {
- code: 'funcName(_sender, amount, receiver);',
- maxUnnamed: 2,
- },
-
- maxUnnamed_3_4u: {
- code: 'funcName(_sender, amount, receiver, token);',
- maxUnnamed: 3,
+ err3: {
+ code: 'funcName(_sender, amount, receiver, token1, token2);',
+ minUnnamed: 0,
},
}
const FUNCTION_CALLS_OK = {
- maxUnnamed_0_0u: {
+ ok1: {
code: 'funcName();',
- maxUnnamed: 0,
+ minUnnamed: 0,
},
- maxUnnamed_0_0uB: {
- code: 'funcName();',
- maxUnnamed: 10,
+ ok2: {
+ code: 'address(0);',
+ minUnnamed: 10,
},
- maxUnnamed_1_0u: {
+ ok3: {
code: 'funcName({ sender: _sender, amount: _amount, receiver: _receiver });',
- maxUnnamed: 1,
+ minUnnamed: 1,
},
- maxUnnamed_1_1u: {
- code: 'funcName(sender);',
- maxUnnamed: 1,
+ ok4: {
+ code: 'assert(1 == 3);',
+ minUnnamed: 1,
},
- maxUnnamed_2_2u: {
- code: 'funcName(sender, amount);',
- maxUnnamed: 2,
+
+ ok5: {
+ code: 'bytes foo = abi.encodeWithSelector(hex"0102030405060708", uint16(0xff00));',
+ minUnnamed: 2,
},
- maxUnnamed3_0u: {
- code: 'funcName({ sender: _sender, amount: _amount, receiver: _receiver });',
- maxUnnamed: 3,
+ ok6: {
+ code: 'funcName({ sender: _sender, amount: _amount, receiver: _receiver, token1: _token1, token2: _token2 });',
+ minUnnamed: 5,
+ },
+
+ ok7: {
+ code: 'new BeaconProxy(address(0),abi.encodeWithSelector(bytes4(""),address(0),address(0),address(0)));',
+ minUnnamed: 2,
+ },
+
+ ok8: {
+ code: 'salt = keccak256(abi.encode(msg.sender, block.chainid, salt));',
+ minUnnamed: 0,
+ },
+
+ ok9: {
+ code: 'require(foobar != address(0), "foobar must a valid address");',
+ minUnnamed: 0,
},
}
diff --git a/test/fixtures/security/send-result-checked.js b/test/fixtures/security/send-result-checked.js
deleted file mode 100644
index 6b4f3848..00000000
--- a/test/fixtures/security/send-result-checked.js
+++ /dev/null
@@ -1 +0,0 @@
-module.exports = 'if(x.send(55)) {}'
diff --git a/test/fixtures/security/send-result-ignored.js b/test/fixtures/security/send-result-ignored.js
deleted file mode 100644
index 040959e6..00000000
--- a/test/fixtures/security/send-result-ignored.js
+++ /dev/null
@@ -1 +0,0 @@
-module.exports = 'x.send(55);'
diff --git a/test/rules/best-practises/custom-errors.js b/test/rules/best-practises/custom-errors.js
new file mode 100644
index 00000000..1904a3ad
--- /dev/null
+++ b/test/rules/best-practises/custom-errors.js
@@ -0,0 +1,103 @@
+const assert = require('assert')
+const {
+ assertNoWarnings,
+ assertNoErrors,
+ assertErrorMessage,
+ assertErrorCount,
+ assertWarnsCount,
+} = require('../../common/asserts')
+const linter = require('../../../lib/index')
+const { funcWith } = require('../../common/contract-builder')
+
+describe('Linter - custom-errors', () => {
+ it('should raise error for revert()', () => {
+ const code = funcWith(`revert();`)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+
+ assertErrorCount(report, 1)
+ assertErrorMessage(report, 'Use Custom Errors instead of revert statement')
+ })
+
+ it('should raise error for revert([string])', () => {
+ const code = funcWith(`revert("Insufficent funds");`)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+
+ assertErrorCount(report, 1)
+ assertErrorMessage(report, 'Use Custom Errors instead of revert statement')
+ })
+
+ it('should NOT raise error for revert ErrorFunction()', () => {
+ const code = funcWith(`revert ErrorFunction();`)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+
+ assertNoWarnings(report)
+ assertNoErrors(report)
+ })
+
+ it('should NOT raise error for revert ErrorFunction() with arguments', () => {
+ const code = funcWith(`revert ErrorFunction({ msg: "Insufficent funds msg" });`)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+
+ assertNoWarnings(report)
+ assertNoErrors(report)
+ })
+
+ it('should raise error for require', () => {
+ const code = funcWith(`require(!has(role, account), "Roles: account already has role");
+ role.bearer[account] = true;role.bearer[account] = true;
+ `)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+
+ assertErrorCount(report, 1)
+ assertErrorMessage(report, 'Use Custom Errors instead of require statement')
+ })
+
+ it('should NOT raise error for regular function call', () => {
+ const code = funcWith(`callOtherFunction();
+ role.bearer[account] = true;role.bearer[account] = true;
+ `)
+ const report = linter.processStr(code, {
+ rules: { 'custom-errors': 'error' },
+ })
+ assertNoWarnings(report)
+ assertNoErrors(report)
+ })
+
+ it('should raise error for require, revert message and not for revert CustomError() for [recommended] config', () => {
+ const code = funcWith(`require(!has(role, account), "Roles: account already has role");
+ revert("RevertMessage");
+ revert CustomError();
+ `)
+ const report = linter.processStr(code, {
+ extends: 'solhint:recommended',
+ rules: { 'compiler-version': 'off' },
+ })
+
+ assertWarnsCount(report, 2)
+ assert.equal(report.reports[0].message, 'Use Custom Errors instead of require statements')
+ assert.equal(report.reports[1].message, 'Use Custom Errors instead of revert statements')
+ })
+
+ it('should NOT raise error for default config', () => {
+ const code = funcWith(`require(!has(role, account), "Roles: account already has role");
+ revert("RevertMessage");
+ revert CustomError();
+ `)
+ const report = linter.processStr(code, {
+ extends: 'solhint:default',
+ })
+
+ assertWarnsCount(report, 0)
+ assertErrorCount(report, 0)
+ })
+})
diff --git a/test/rules/best-practises/explicit-types.js b/test/rules/best-practises/explicit-types.js
index 842329c6..2ff4a296 100644
--- a/test/rules/best-practises/explicit-types.js
+++ b/test/rules/best-practises/explicit-types.js
@@ -1,4 +1,3 @@
-const assert = require('assert')
const linter = require('../../../lib/index')
const contractWith = require('../../common/contract-builder').contractWith
const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('../../common/asserts')
@@ -21,30 +20,24 @@ const getZeroErrosObject = () => {
}
describe('Linter - explicit-types rule', () => {
- it('should throw an error with a wrong configuration rule example 1', () => {
+ it('should throw an error with a wrong configuration rule', () => {
const code = contractWith('uint256 public constant SNAKE_CASE = 1;')
- try {
- linter.processStr(code, {
- rules: { 'explicit-types': ['error', 'implicito'] },
- })
- } catch (error) {
- assert.ok(
- error.toString().includes('Error: Config error on [explicit-types]. See explicit-types.md.')
- )
- }
+
+ const report = linter.processStr(code, {
+ rules: { 'explicit-types': ['error', 'implicito'] },
+ })
+
+ assertErrorCount(report, 1)
+ assertErrorMessage(report, `Error: Config error on [explicit-types]. See explicit-types.md.`)
})
- it('should throw an error with a wrong configuration rule example 2', () => {
+ it('should NOT throw error without the config array and default should take place', () => {
const code = contractWith('uint256 public constant SNAKE_CASE = 1;')
- try {
- linter.processStr(code, {
- rules: { 'explicit-types': 'error' },
- })
- } catch (error) {
- assert.ok(
- error.toString().includes('Error: Config error on [explicit-types]. See explicit-types.md.')
- )
- }
+ const report = linter.processStr(code, {
+ rules: { 'explicit-types': 'error' },
+ })
+
+ assertNoErrors(report)
})
for (const key in VAR_DECLARATIONS) {
diff --git a/test/rules/best-practises/no-global-import.js b/test/rules/best-practises/no-global-import.js
index 3ab26a1b..7a357e18 100644
--- a/test/rules/best-practises/no-global-import.js
+++ b/test/rules/best-practises/no-global-import.js
@@ -28,7 +28,7 @@ describe('Linter - no-global-import', () => {
assertErrorMessage(report, 'Specify names to import individually')
})
it('should raise warning when using solhint:recommended', () => {
- const code = `pragma solidity ^0.5.8; import "./A.sol";`
+ const code = `pragma solidity ^0.8.0; import "./A.sol";`
const report = linter.processStr(code, {
extends: 'solhint:recommended',
diff --git a/test/rules/best-practises/no-unused-import.js b/test/rules/best-practises/no-unused-import.js
index 49a2c496..f3666891 100644
--- a/test/rules/best-practises/no-unused-import.js
+++ b/test/rules/best-practises/no-unused-import.js
@@ -27,8 +27,28 @@ describe('Linter - no-unused-import', () => {
assertErrorMessage(report, 'imported name A is not used')
})
+ it('should not crash on, but also not recognize, malformed inheritdoc statements', () => {
+ const code = `
+ import {A} from './A.sol';
+ // inheritdoc A
+ // @inheritdoc A
+ /// inheritdoc A
+ /// @ inherit A
+ /// @ inheritdoc A
+ /// @inheritdoc somethingelse
+ /// @inheritdoc
+ /// @inheritdoc
+ `
+
+ const report = linter.processStr(code, {
+ rules: { 'no-unused-import': 'error' },
+ })
+ assertErrorCount(report, 1)
+ assertErrorMessage(report, 'imported name A is not used')
+ })
+
it('should raise error when using solhint:recommended', () => {
- const code = `pragma solidity ^0.5.8; import {A} from "./A.sol";`
+ const code = `pragma solidity ^0.8.0; import {A} from "./A.sol";`
const report = linter.processStr(code, {
extends: 'solhint:recommended',
@@ -171,10 +191,11 @@ describe('Linter - no-unused-import', () => {
description: 'Name is used in an override',
code: `
pragma solidity >=0.8.19;
+
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
+
contract MyContract {
- /// @inheritdoc ERC721
function tokenURI(uint256 streamId) public view override(IERC721Metadata, ERC721) returns (string memory uri) {
uri = "example.com";
}
@@ -185,7 +206,9 @@ describe('Linter - no-unused-import', () => {
description: 'Type is used in a constructor initializator',
code: `
pragma solidity >=0.8.19;
+
import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
+
contract MyContract {
constructor() ERC721("Sablier V2 Lockup Dynamic NFT", "SAB-V2-LOCKUP-DYN") { }
}
@@ -195,7 +218,9 @@ describe('Linter - no-unused-import', () => {
description: 'Import is used as value in a function parameter',
code: `
pragma solidity >=0.8.19 <0.9.0;
+
import { UD60x18, ZERO } from "@prb/math/UD60x18.sol";
+
contract SetProtocolFee_Integration_Fuzz_Test {
function testFuzz_SetProtocolFee(UD60x18 newProtocolFee) external {
newProtocolFee = _bound(newProtocolFee, 1, MAX_FEE);
@@ -209,6 +234,7 @@ describe('Linter - no-unused-import', () => {
description: 'Import is used as value in a binary expression',
code: `
import { ZERO } from "@prb/math/UD60x18.sol";
+
contract Foo {
function returnFifteen() public returns (uint){
return ZERO + 15;
@@ -220,6 +246,7 @@ describe('Linter - no-unused-import', () => {
description: 'Import is used as value in an assignment',
code: `
import { ZERO } from "@prb/math/UD60x18.sol";
+
contract Foo {
uint256 public howMuch;
constructor () {
@@ -228,6 +255,69 @@ describe('Linter - no-unused-import', () => {
}
`,
},
+ {
+ description: 'Import is used in the left side of a using for statement',
+ code: `
+ pragma solidity >=0.8.19;
+
+ import {EnumerableSetUD60x18, EnumerableSet} from "./libraries/EnumerableSetUD60x18.sol";
+
+ contract SolhintTest {
+ using EnumerableSetUD60x18 for EnumerableSet.Bytes32Set;
+ }
+ `,
+ },
+ {
+ description: 'Import is used as the type value of a mapping',
+ code: `
+ pragma solidity >=0.8.19;
+
+ import {UD60x18} from "@prb/math/UD60x18.sol";
+
+ contract SolhintTest {
+ mapping(address user => UD60x18 amount) internal balance;
+ }
+ `,
+ },
+ {
+ description: 'Import is used as the type value of a nested mapping',
+ code: `
+ pragma solidity >=0.8.19;
+ import {SD59x18} from "@prb/math/SD59x18.sol";
+
+ contract SolhintTest {
+ mapping(address user => mapping(address relayer => SD59x18 amount)) internal balance;
+ }
+ `,
+ },
+ {
+ description: 'Import is used in /// @inheritdoc',
+ code: `
+ import { IPRBProxyPlugin } from "@prb/proxy/interfaces/IPRBProxyPlugin.sol";
+
+ contract SablierV2ProxyPlugin {
+ /// @inheritdoc IPRBProxyPlugin
+ function getMethods() external pure returns (bytes4[] memory methods) {
+ methods = new bytes4[](1);
+ methods[0] = this.onStreamCanceled.selector;
+ }
+ }
+ `,
+ },
+ {
+ description: 'Import is used in /** @inheritdoc',
+ code: `
+ import { IPRBProxyPlugin } from "@prb/proxy/interfaces/IPRBProxyPlugin.sol";
+
+ contract SablierV2ProxyPlugin {
+ /** @inheritdoc IPRBProxyPlugin */
+ function getMethods() external pure returns (bytes4[] memory methods) {
+ methods = new bytes4[](1);
+ methods[0] = this.onStreamCanceled.selector;
+ }
+ }
+ `,
+ },
].forEach(({ description, code }) => {
it(`should not raise when ${description}`, () => {
const report = linter.processStr(code, {
diff --git a/test/rules/naming/foundry-test-functions.js b/test/rules/naming/foundry-test-functions.js
new file mode 100644
index 00000000..9a0feeb9
--- /dev/null
+++ b/test/rules/naming/foundry-test-functions.js
@@ -0,0 +1,224 @@
+const assert = require('assert')
+const linter = require('../../../lib/index')
+const contractWith = require('../../common/contract-builder').contractWith
+const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('../../common/asserts')
+
+const ALLOWED_FUNCTION_NAMES = [
+ 'test_',
+ 'testFork_',
+ 'testFuzz_',
+ 'testFail_',
+ 'test_Revert_',
+ 'test_If_',
+ 'test_When_',
+ 'testFail_Revert_',
+ 'testFail_If_',
+ 'testFail_When_',
+ 'testFork_Revert_',
+ 'testFork_If_',
+ 'testFork_When_',
+ 'testFuzz_Revert_',
+ 'testFuzz_If_',
+ 'testFuzz_When_',
+]
+
+const DISALLOWED_FUNCTION_NAMES = ['Test_', 'Test', 'test', 'testo_', '', 'any', 'setUp', 'other']
+
+const composeFunctionName = (prefix, name, visibility) =>
+ 'function ' + prefix + name + ' ' + visibility + ' { testNumber = 42; }'
+
+describe('Linter - foundry-test-functions', () => {
+ for (const prefix of DISALLOWED_FUNCTION_NAMES) {
+ it(`should raise error for DISALLOWED_FUNCTION_NAMES [${prefix}] when PUBLIC`, () => {
+ const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'public')
+ const code = contractWith(functionDefinition)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertErrorCount(report, 1)
+ assertErrorMessage(
+ report,
+ `Function ${prefix + 'FunctionName()'} must match Foundry test naming convention`
+ )
+ })
+ }
+
+ for (const prefix of DISALLOWED_FUNCTION_NAMES) {
+ it(`should NOT raise error for DISALLOWED_FUNCTION_NAMES [${prefix}] when INTERNAL`, () => {
+ const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'internal')
+ const code = contractWith(functionDefinition)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertNoErrors(report)
+ })
+ }
+
+ for (const prefix of ALLOWED_FUNCTION_NAMES) {
+ it(`should NOT raise error for ALLOWED_FUNCTION_NAMES [${prefix}] when PUBLIC`, () => {
+ const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'public')
+ const code = contractWith(functionDefinition)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertNoErrors(report)
+ })
+ }
+
+ for (const prefix of ALLOWED_FUNCTION_NAMES) {
+ it(`should NOT raise error for ALLOWED_FUNCTION_NAMES [${prefix}] when EXTERNAL`, () => {
+ const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'external')
+ const code = contractWith(functionDefinition)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertNoErrors(report)
+ })
+ }
+
+ for (const prefix of ALLOWED_FUNCTION_NAMES) {
+ it(`should NOT raise error for ALLOWED_FUNCTION_NAMES [${prefix}] when INTERNAL`, () => {
+ const functionDefinition = composeFunctionName(prefix, 'FunctionName()', 'external')
+ const code = contractWith(functionDefinition)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertNoErrors(report)
+ })
+ }
+
+ it(`should NOT raise error for setUp function, since is confired as SKIPPED`, () => {
+ const code = contractWith(
+ 'function setUp() public { testNumber = 42; } function finish() public { testNumber = 42; }'
+ )
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertNoErrors(report)
+ })
+
+ it(`should NOT raise error for setUp and finish functions but RAISE for the other two functions`, () => {
+ const code = contractWith(`
+ function setUp() public { testNumber = 42; }
+ function finish() public { testNumber = 43; }
+ function invalidFunction1() external { testNumber = 44; }
+ function invalidFunction2() external { testNumber = 45; }`)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', ['setUp', 'finish']] },
+ })
+
+ assertErrorCount(report, 2)
+ assert.equal(
+ report.reports[0].message,
+ `Function invalidFunction1() must match Foundry test naming convention`
+ )
+ assert.equal(
+ report.reports[1].message,
+ 'Function invalidFunction2() must match Foundry test naming convention'
+ )
+ })
+
+ it('should NOT raise error when recommended rules are configured', () => {
+ const code = contractWith(`
+ function setUp() public { testNumber = 42; }
+ function finish() public { testNumber = 43; }
+ function invalidFunction1() external { testNumber = 44; }
+ function invalidFunction2() external { testNumber = 45; }`)
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:recommended',
+ rules: { 'compiler-version': 'off' },
+ })
+
+ assertNoErrors(report)
+ })
+
+ it('should raise 2 errors when all rules are configured and setUp is skipped', () => {
+ const code = contractWith(`
+ function setUp() public { testNumber = 42; }
+ function invalidFunction1() external { testNumber = 44; }
+ function invalidFunction2() external { testNumber = 45; }`)
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:recommended',
+ rules: {
+ 'compiler-version': 'off',
+ 'foundry-test-functions': ['error', ['setUp', 'finish']],
+ },
+ })
+
+ assertErrorCount(report, 2)
+ assert.equal(
+ report.reports[0].message,
+ `Function invalidFunction1() must match Foundry test naming convention`
+ )
+ assert.equal(
+ report.reports[1].message,
+ 'Function invalidFunction2() must match Foundry test naming convention'
+ )
+ })
+
+ it(`should NOT raise error only for setUp when rule is just on 'error'`, () => {
+ const code = contractWith(`
+ function setUp() public { testNumber = 42; }
+ function finish() public { testNumber = 43; }
+ function invalidFunction1() external { testNumber = 44; }
+ function invalidFunction2() external { testNumber = 45; }`)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': 'error' },
+ })
+
+ assertErrorCount(report, 3)
+ assert.equal(
+ report.reports[0].message,
+ 'Function finish() must match Foundry test naming convention'
+ )
+ assert.equal(
+ report.reports[1].message,
+ `Function invalidFunction1() must match Foundry test naming convention`
+ )
+ assert.equal(
+ report.reports[2].message,
+ 'Function invalidFunction2() must match Foundry test naming convention'
+ )
+ })
+
+ it(`should raise error for all functions when rule SKIP array is empty`, () => {
+ const code = contractWith(`
+ function setUp() public { testNumber = 42; }
+ function finish() public { testNumber = 43; }
+ function invalidFunction() external { testNumber = 44; }`)
+
+ const report = linter.processStr(code, {
+ rules: { 'foundry-test-functions': ['error', []] },
+ })
+
+ assertErrorCount(report, 3)
+ assert.equal(
+ report.reports[0].message,
+ 'Function setUp() must match Foundry test naming convention'
+ )
+ assert.equal(
+ report.reports[1].message,
+ `Function finish() must match Foundry test naming convention`
+ )
+ assert.equal(
+ report.reports[2].message,
+ 'Function invalidFunction() must match Foundry test naming convention'
+ )
+ })
+})
diff --git a/test/rules/naming/func-named-parameters.js b/test/rules/naming/func-named-parameters.js
index 9fe90566..9591e434 100644
--- a/test/rules/naming/func-named-parameters.js
+++ b/test/rules/naming/func-named-parameters.js
@@ -11,33 +11,37 @@ const {
FUNCTION_CALLS_OK,
} = require('../../fixtures/naming/func-named-parameters')
+const DEFAULT_MIN_UNNAMED_ARGUMENTS = 4
+
describe('Linter - func-named-parameters', () => {
for (const key in FUNCTION_CALLS_ERRORS) {
it(`should raise error for FUNCTION_CALLS_ERRORS [${key}]`, () => {
- const { code, maxUnnamed } = FUNCTION_CALLS_ERRORS[key]
+ const { code, minUnnamed } = FUNCTION_CALLS_ERRORS[key]
const sourceCode = contractWith('function callerFunction() public { ' + code + ' }')
const report = linter.processStr(sourceCode, {
- rules: { 'func-named-parameters': ['error', maxUnnamed] },
+ rules: { 'func-named-parameters': ['error', minUnnamed] },
})
assertErrorCount(report, 1)
assertErrorMessage(
report,
- `Missing named parameters. Max unnamed parameters value is ${maxUnnamed}`
+ `Named parameters missing. MIN unnamed argumenst is ${
+ minUnnamed < DEFAULT_MIN_UNNAMED_ARGUMENTS ? DEFAULT_MIN_UNNAMED_ARGUMENTS : minUnnamed
+ }`
)
})
}
for (const key in FUNCTION_CALLS_OK) {
it(`should NOT raise error for FUNCTION_CALLS_OK [${key}]`, () => {
- const { code, maxUnnamed } = FUNCTION_CALLS_OK[key]
+ const { code, minUnnamed } = FUNCTION_CALLS_OK[key]
const sourceCode = contractWith('function callerFunction() public { ' + code + ' }')
const report = linter.processStr(sourceCode, {
- rules: { 'func-named-parameters': ['error', maxUnnamed] },
+ rules: { 'func-named-parameters': ['error', minUnnamed] },
})
assertNoErrors(report)
@@ -46,7 +50,7 @@ describe('Linter - func-named-parameters', () => {
it('should NOT raise error when default rules are configured', () => {
const code = contractWith(
- `function callerFunction() public { funcName(sender, amount, receiver); }`
+ `function callerFunction() public { funcName(sender, amount, receiver, token1, token2); }`
)
const report = linter.processStr(code, {
extends: 'solhint:default',
@@ -56,22 +60,39 @@ describe('Linter - func-named-parameters', () => {
assertNoErrors(report)
})
- it('should raise error when recommended rules are configured', () => {
+ it('should NOT raise error when recommended rules are configured', () => {
const code = contractWith(
- `function callerFunction() public { funcName(sender, amount, receiver); }`
+ `function callerFunction() public { funcName(sender, amount, receiver, token1, token2); }`
)
const report = linter.processStr(code, {
extends: 'solhint:recommended',
rules: { 'compiler-version': 'off' },
})
+ assertNoErrors(report)
+ })
+
+ it('should raise warning when all rules are configured (no min value)', () => {
+ const code = contractWith(
+ `function callerFunction() public { funcName(sender, amount, receiver, token1, token2, token3); }`
+ )
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:all',
+ rules: { 'compiler-version': 'off', 'comprehensive-interface': 'off' },
+ })
+
assertWarnsCount(report, 1)
- assertErrorMessage(report, `Missing named parameters. Max unnamed parameters value is 2`)
+
+ assertErrorMessage(
+ report,
+ `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}`
+ )
})
- it('should raise error when rule has no maxUnnamed arguments is set and default value takes over', () => {
+ it('should raise error when rule has no minUnnamed arguments is set and default value takes over', () => {
const code = contractWith(
- `function callerFunction() public { funcName(sender, amount, receiver); }`
+ `function callerFunction() public { funcName(sender, amount, receiver, token1, token2, token3); }`
)
const report = linter.processStr(code, {
rules: { 'func-named-parameters': 'error' },
@@ -79,7 +100,9 @@ describe('Linter - func-named-parameters', () => {
assertErrorCount(report, 1)
- // default value is 2 for this rule
- assertErrorMessage(report, `Missing named parameters. Max unnamed parameters value is 2`)
+ assertErrorMessage(
+ report,
+ `Named parameters missing. MIN unnamed argumenst is ${DEFAULT_MIN_UNNAMED_ARGUMENTS}`
+ )
})
})
diff --git a/test/rules/naming/named-return-values.js b/test/rules/naming/named-return-values.js
new file mode 100644
index 00000000..b93d1349
--- /dev/null
+++ b/test/rules/naming/named-return-values.js
@@ -0,0 +1,92 @@
+const assert = require('assert')
+const linter = require('../../../lib/index')
+const contractWith = require('../../common/contract-builder').contractWith
+const { assertErrorCount, assertNoErrors, assertWarnsCount } = require('../../common/asserts')
+
+describe('Linter - named-return-values', () => {
+ it('should NOT raise error for named return values', () => {
+ const code = contractWith(
+ `function getBalanceFromTokens(address wallet) public returns(address token1, address token2, uint256 balance1, uint256 balance2) { balance = 1; }`
+ )
+ const report = linter.processStr(code, {
+ rules: { 'named-return-values': 'error' },
+ })
+ assertNoErrors(report)
+ })
+
+ it('should raise error for unnamed return values', () => {
+ const code = contractWith(
+ `function getBalanceFromTokens(address wallet) public returns(address, address, uint256, uint256) { balance = 1; }`
+ )
+ const report = linter.processStr(code, {
+ rules: { 'named-return-values': 'error' },
+ })
+
+ assertErrorCount(report, 4)
+ for (let index = 0; index < report.reports.length; index++) {
+ assert.equal(report.reports[index].message, `Named return value is missing - Index ${index}`)
+ }
+ })
+
+ it('should NOT raise error for functions without return values', () => {
+ const code = contractWith(`function writeOnStorage(address wallet) public { balance = 1; }`)
+ const report = linter.processStr(code, {
+ rules: { 'named-return-values': 'error' },
+ })
+ assertNoErrors(report)
+ })
+
+ it('should raise error for 2 unnamed return values', () => {
+ const code = contractWith(
+ `function getBalanceFromTokens(address wallet) public returns(address user, address, uint256 amount, uint256) { balance = 1; }`
+ )
+ const report = linter.processStr(code, {
+ rules: { 'named-return-values': 'error' },
+ })
+
+ assertErrorCount(report, 2)
+ assert.equal(report.reports[0].message, `Named return value is missing - Index 1`)
+ assert.equal(report.reports[1].message, `Named return value is missing - Index 3`)
+ })
+
+ it('should NOT raise error for solhint:recommended setup', () => {
+ const code = contractWith(
+ `function getBalanceFromTokens(address wallet) public returns(address, address, uint256, uint256) { balance = 1; }`
+ )
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:recommended',
+ rules: { 'compiler-version': 'off' },
+ })
+
+ assertNoErrors(report)
+ })
+
+ it('should NOT raise error for solhint:default setup', () => {
+ const code = contractWith(
+ `function getBalance(address wallet) public returns(uint256) { balance = 1; }`
+ )
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:default',
+ })
+
+ assertNoErrors(report)
+ })
+
+ it('should raise error for solhint:all setup', () => {
+ const code = contractWith(
+ `function getBalance(uint256 wallet) public override returns(uint256, address) { wallet = 1; }`
+ )
+
+ const report = linter.processStr(code, {
+ extends: 'solhint:all',
+ rules: { 'compiler-version': 'off' },
+ })
+
+ assertWarnsCount(report, 2)
+ for (let index = 0; index < report.reports.length; index++) {
+ assert.equal(report.reports[index].message, `Named return value is missing - Index ${index}`)
+ }
+ })
+})
diff --git a/test/rules/order/ordering.js b/test/rules/order/ordering.js
index d83d73f4..1ff356ef 100644
--- a/test/rules/order/ordering.js
+++ b/test/rules/order/ordering.js
@@ -427,4 +427,63 @@ describe('Linter - ordering', () => {
assert.equal(report.errorCount, 0)
})
+
+ it('should raise error when INITIALIZER is NOT well located', () => {
+ const code = `
+ // SPDX-License-Identifier: MIT
+ pragma solidity ^0.8.0;
+ import "@openzeppelin/contracts/ownership/Ownable.sol";
+ uint256 constant oneNiceConstant = 1;
+ struct OneNiceStruct { uint256 a; uint256 b; }
+ enum MyEnum { A, B }
+ error Unauthorized();
+ function freeFunction() pure returns (uint256) {
+ return 1;
+ }
+ contract OneNiceContract {
+ function initialize() initializer {
+ oneNiceConstant;
+ }
+ struct OneNiceStruct { uint256 a; uint256 b; }
+ }
+ `
+
+ const report = linter.processStr(code, {
+ rules: { ordering: 'error' },
+ })
+
+ assert.equal(report.errorCount, 1)
+ assert.ok(
+ report.messages[0].message.includes(
+ 'Function order is incorrect, struct definition can not go after constructor/initializer'
+ )
+ )
+ })
+
+ it('should NOT raise error when INITIALIZER is well located', () => {
+ const code = `
+ // SPDX-License-Identifier: MIT
+ pragma solidity ^0.8.0;
+ import "@openzeppelin/contracts/ownership/Ownable.sol";
+ uint256 constant oneNiceConstant = 1;
+ enum MyEnum { A, B }
+ struct OneNiceStruct { uint256 a; uint256 b; }
+ error Unauthorized();
+ function freeFunction() pure returns (uint256) {
+ return 1;
+ }
+ contract OneNiceContract {
+ struct OneNiceStruct { uint256 a; uint256 b; }
+ function initialize() initializer {
+ oneNiceConstant;
+ }
+ }
+ `
+
+ const report = linter.processStr(code, {
+ rules: { ordering: 'error' },
+ })
+
+ assert.equal(report.errorCount, 0)
+ })
})
diff --git a/test/rules/security/check-send-result.js b/test/rules/security/check-send-result.js
index c2d75d8b..69f0875d 100644
--- a/test/rules/security/check-send-result.js
+++ b/test/rules/security/check-send-result.js
@@ -4,7 +4,7 @@ const funcWith = require('../../common/contract-builder').funcWith
describe('Linter - check-send-result', () => {
it('should return "send" call verification error', () => {
- const code = funcWith(require('../../fixtures/security/send-result-ignored'))
+ const code = funcWith('x.send(55);')
const report = linter.processStr(code, {
rules: { 'check-send-result': 'error' },
@@ -15,7 +15,7 @@ describe('Linter - check-send-result', () => {
})
it('should not return "send" call verification error', () => {
- const code = funcWith(require('../../fixtures/security/send-result-checked'))
+ const code = funcWith('if(x.send(55)) {}')
const report = linter.processStr(code, {
rules: { 'check-send-result': 'error' },
@@ -73,4 +73,14 @@ describe('Linter - check-send-result', () => {
assert.equal(report.errorCount, 1)
})
+
+ it('should not emit error when the send() is used for an ERC777', () => {
+ const code = funcWith('erc777.send(recipient, amount, "");')
+
+ const report = linter.processStr(code, {
+ rules: { 'check-send-result': 'error' },
+ })
+
+ assert.equal(report.errorCount, 0)
+ })
})
diff --git a/test/rules/security/compiler-version.js b/test/rules/security/compiler-version.js
index 3f18d6fd..b3c51feb 100644
--- a/test/rules/security/compiler-version.js
+++ b/test/rules/security/compiler-version.js
@@ -2,6 +2,8 @@ const assert = require('assert')
const { assertNoErrors, assertErrorCount, assertErrorMessage } = require('../../common/asserts')
const linter = require('../../../lib/index')
+const DEFAULT_SEMVER = '^0.8.0'
+
describe('Linter - compiler-version', () => {
it('should disable only one compiler error on next line', () => {
const report = linter.processStr(
@@ -11,7 +13,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
@@ -26,7 +28,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
@@ -41,7 +43,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
@@ -56,7 +58,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
@@ -72,12 +74,12 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
assertErrorCount(report, 1)
- assertErrorMessage(report, '0.5.2')
+ assertErrorMessage(report, DEFAULT_SEMVER)
})
it('should disable all errors', () => {
@@ -88,7 +90,7 @@ describe('Linter - compiler-version', () => {
pragma solidity 0.3.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
assertNoErrors(report)
@@ -103,58 +105,58 @@ describe('Linter - compiler-version', () => {
pragma solidity ^0.4.4;
`,
{
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
}
)
assertErrorCount(report, 1)
- assertErrorMessage(report, '0.5.2')
+ assertErrorMessage(report, DEFAULT_SEMVER)
})
it('should return compiler version error', () => {
const report = linter.processStr('pragma solidity 0.3.4;', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 1)
- assert.ok(report.reports[0].message.includes('0.5.2'))
+ assert.ok(report.reports[0].message.includes(DEFAULT_SEMVER))
})
it('should not report compiler version error on exact match', () => {
- const report = linter.processStr('pragma solidity 0.5.2;', {
- rules: { 'compiler-version': ['error', '0.5.2'] },
+ const report = linter.processStr('pragma solidity 0.8.0;', {
+ rules: { 'compiler-version': ['error', '0.8.0'] },
})
assert.equal(report.errorCount, 0)
})
it('should not report compiler version error on range match', () => {
- const report = linter.processStr('pragma solidity ^0.5.2;', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ const report = linter.processStr('pragma solidity ^0.8.0;', {
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 0)
})
it('should not report compiler version error on patch bump', () => {
- const report = linter.processStr('pragma solidity 0.5.3;', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ const report = linter.processStr('pragma solidity 0.8.1;', {
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 0)
})
it('should not report compiler version error on range match', () => {
- const report = linter.processStr('pragma solidity ^0.5.3;', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ const report = linter.processStr('pragma solidity ^0.8.2;', {
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 0)
})
it('should report compiler version error on range not matching', () => {
- const report = linter.processStr('pragma solidity ^0.5.2;', {
- rules: { 'compiler-version': ['error', '^0.5.3'] },
+ const report = linter.processStr('pragma solidity ^0.8.1;', {
+ rules: { 'compiler-version': ['error', '^0.8.3'] },
})
assert.equal(report.errorCount, 1)
@@ -162,7 +164,7 @@ describe('Linter - compiler-version', () => {
it('should report compiler version error on minor bump', () => {
const report = linter.processStr('pragma solidity 0.6.0;', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 1)
@@ -170,7 +172,7 @@ describe('Linter - compiler-version', () => {
it(`should report compiler version error if pragma doesn't exist`, () => {
const report = linter.processStr('contract Foo {}', {
- rules: { 'compiler-version': ['error', '^0.5.2'] },
+ rules: { 'compiler-version': ['error', DEFAULT_SEMVER] },
})
assert.equal(report.errorCount, 1)
@@ -178,10 +180,10 @@ describe('Linter - compiler-version', () => {
it(`should not report compiler version error if pragma exist`, () => {
const report = linter.processStr(
- `pragma solidity 0.6.0;
+ `pragma solidity 0.8.2;
contract Foo {}`,
{
- rules: { 'compiler-version': ['error', '^0.6.0'] },
+ rules: { 'compiler-version': ['error', '^0.8.2'] },
}
)
@@ -202,4 +204,62 @@ describe('Linter - compiler-version', () => {
assert.equal(report.errorCount, 0)
})
+
+ it(`should not report compiler version error using default and correct pragma`, () => {
+ const report = linter.processStr(
+ `pragma solidity ^0.8.1;
+ pragma experimental ABIEncoderV2;
+
+ contract Main {
+ }`,
+ {
+ rules: { 'compiler-version': 'error' },
+ }
+ )
+
+ assert.equal(report.errorCount, 0)
+ })
+
+ it(`should report compiler version error using default and lower pragma`, () => {
+ const report = linter.processStr(
+ `pragma solidity ^0.7.4;
+
+ contract Main {
+ }`,
+ {
+ rules: { 'compiler-version': 'error' },
+ }
+ )
+
+ assert.equal(report.errorCount, 1)
+ assertErrorMessage(report, DEFAULT_SEMVER)
+ })
+
+ it(`should not report compiler version error using >= and default and correct pragma`, () => {
+ const report = linter.processStr(
+ `pragma solidity >=0.8.0;
+
+ contract Main {
+ }`,
+ {
+ rules: { 'compiler-version': 'error' },
+ }
+ )
+
+ assert.equal(report.errorCount, 0)
+ })
+
+ it(`should report compiler version error using >= and default and lower pragma`, () => {
+ const report = linter.processStr(
+ `pragma solidity >=0.7.4;
+
+ contract Main {
+ }`,
+ {
+ rules: { 'compiler-version': 'error' },
+ }
+ )
+ assert.equal(report.errorCount, 1)
+ assertErrorMessage(report, DEFAULT_SEMVER)
+ })
})