Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

MERGE Develop into Master #473

Merged
merged 29 commits into from
Aug 11, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
9324cf5
fix: false positives builtin functions
dbale-altoros Aug 7, 2023
3d88e40
Merge pull request #472 from protofire/i471-fix-func-named-parameters
dbale-altoros Aug 7, 2023
a9d25e8
fix: [ordering] initializer as constructor
dbale-altoros Aug 7, 2023
1be1277
Merge pull request #474 from protofire/i404-treat-initializer-as-cons…
dbale-altoros Aug 7, 2023
d4596a5
new: enforce custom errors
dbale-altoros Aug 8, 2023
11b1a09
new: foundry test functions names
dbale-altoros Aug 9, 2023
c6977a8
fix missing code on regex rule doc
dbale-altoros Aug 9, 2023
db1d6ad
fix: check-send-result for erc777
dbale-altoros Aug 10, 2023
3a2b848
new rule: named-return-values
dbale-altoros Aug 10, 2023
c18bbc5
20230810 last changes v1
dbale-altoros Aug 10, 2023
726fba9
Merge pull request #475 from protofire/i349-enforce-custom-errors
dbale-altoros Aug 10, 2023
dd272c0
no-unusued-imports-fix
dbale-altoros Aug 10, 2023
23728d9
Merge branch 'develop' into no-unused-imports-fix
dbale-altoros Aug 10, 2023
6bb75a7
fix-explicit-types default value
dbale-altoros Aug 10, 2023
3822e4a
Merge pull request #481 from protofire/fix-explicit-types
dbale-altoros Aug 10, 2023
02bb01d
Merge pull request #476 from protofire/i402-foundry-test-function-names
dbale-altoros Aug 11, 2023
e04f9fb
Merge pull request #478 from protofire/i340-name-return-values
dbale-altoros Aug 11, 2023
cc1bfa2
Merge branch 'develop' into 20230810-release-3-6-1-v1
dbale-altoros Aug 11, 2023
af509e8
Merge pull request #479 from protofire/20230810-release-3-6-1-v1
dbale-altoros Aug 11, 2023
5bb7937
Merge branch 'develop' into no-unused-imports-fix
dbale-altoros Aug 11, 2023
34562bb
Merge pull request #480 from protofire/no-unused-imports-fix
dbale-altoros Aug 11, 2023
2876615
update-check-send-result-docs
dbale-altoros Aug 11, 2023
038e10d
Merge pull request #482 from protofire/i295-update-check-send-result-…
dbale-altoros Aug 11, 2023
3ea57c5
fix and bump compiler version default
dbale-altoros Aug 11, 2023
14bb853
Merge branch 'develop' into i230-fix-compiler-version-default-rule
dbale-altoros Aug 11, 2023
75db884
fix and bump compiler-version rule default
dbale-altoros Aug 11, 2023
84f366f
Merge branch 'i230-fix-compiler-version-default-rule' of github.com:p…
dbale-altoros Aug 11, 2023
9228379
pre release fixes
dbale-altoros Aug 11, 2023
b67d523
Merge pull request #483 from protofire/i230-fix-compiler-version-defa…
dbale-altoros Aug 11, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion conf/rulesets/solhint-all.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module.exports = Object.freeze({
'contract-name-camelcase': 'warn',
'event-name-camelcase': 'warn',
'func-name-mixedcase': 'warn',
'func-named-parameters': ['warn', 2],
'func-named-parameters': ['warn', 4],
'func-param-name-mixedcase': 'warn',
'immutable-vars-naming': [
'warn',
Expand Down
1 change: 0 additions & 1 deletion conf/rulesets/solhint-recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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',
{
Expand Down
36 changes: 18 additions & 18 deletions docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,24 @@ 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. | $~~~~~~~~$✔️ | |
| [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. | | |
| [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
Expand Down
19 changes: 8 additions & 11 deletions docs/rules/naming/func-named-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,26 @@ title: "func-named-parameters | Solhint"
---

# func-named-parameters
![Recommended Badge](https://img.shields.io/badge/-Recommended-brightgreen)
![Category Badge](https://img.shields.io/badge/-Style%20Guide%20Rules-informational)
![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
> The {"extends": "solhint:recommended"} property in a configuration file enables this rule.


## Description
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]
}
}
```
Expand All @@ -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)
Expand All @@ -56,7 +53,7 @@ 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 )
Expand Down
27 changes: 15 additions & 12 deletions lib/rules/naming/func-named-parameters.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,30 +2,30 @@ 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: [
{
description: severityDescription,
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)",
},
{
Expand All @@ -39,25 +39,28 @@ 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 )',
},
],
},
},

isDefault: false,
recommended: true,
defaultSetup: [DEFAULT_SEVERITY, DEFAULT_MAX_UNNAMED_ARGUMENTS],
recommended: false,
defaultSetup: [DEFAULT_SEVERITY, DEFAULT_MIN_UNNAMED_ARGUMENTS],

schema: { type: 'integer' },
}

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) {
Expand All @@ -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}`
)
}
}
Expand Down
16 changes: 14 additions & 2 deletions lib/rules/order/ordering.js
Original file line number Diff line number Diff line change
Expand Up @@ -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']
Expand Down Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "solhint",
"version": "3.5.1",
"version": "3.5.2",
"description": "Solidity Code Linter",
"main": "lib/index.js",
"keywords": [
Expand Down
71 changes: 41 additions & 30 deletions test/fixtures/naming/func-named-parameters.js
Original file line number Diff line number Diff line change
@@ -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,
},
}

Expand Down
Loading