Skip to content

Commit

Permalink
feat: free functions don't need underscore
Browse files Browse the repository at this point in the history
  • Loading branch information
@LHerskind
LHerskind committed Nov 4, 2024
1 parent 958995d commit 82c456c
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 34 deletions.
48 changes: 24 additions & 24 deletions docs/rules.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,30 +26,30 @@ title: "Rule Index of Solhint"

## Style Guide Rules

| Rule Id | Error | Recommended | Deprecated |
| ------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
| [interface-starts-with-i](./rules/naming/interface-starts-with-i.md) | Solidity Interfaces names should start with an `I` | | |
| [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, Structs and Enums should 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-leading-underscore](./rules/naming/func-param-name-leading-underscore.md) | Function param name must start a single underscore | | |
| [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. | $~~~~~~~~$✔️ | |
| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy (read "Notes section") | | |
| [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-func-leading-underscore](./rules/naming/private-func-leading-underscore.md) | Private and internal function names must start with a single underscore, unless they are in a library. | | |
| [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | |
| [private-vars-no-leading-underscore](./rules/naming/private-vars-no-leading-underscore.md) | Private and internal variable names must NOT 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 |
| ------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------- | ------------ | ----------- |
| [interface-starts-with-i](./rules/naming/interface-starts-with-i.md) | Solidity Interfaces names should start with an `I` | | |
| [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, Structs and Enums should 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-leading-underscore](./rules/naming/func-param-name-leading-underscore.md) | Function param name must start a single underscore | | |
| [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. | $~~~~~~~~$✔️ | |
| [imports-order](./rules/naming/imports-order.md) | Order the imports of the contract to follow a certain hierarchy (read "Notes section") | | |
| [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-func-leading-underscore](./rules/naming/private-func-leading-underscore.md) | Private and internal function names must start with a single underscore, unless they are in a library or free. | | |
| [private-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Non-external functions and state variables should start with a single underscore. Others, shouldn't | | |
| [private-vars-no-leading-underscore](./rules/naming/private-vars-no-leading-underscore.md) | Private and internal variable names must NOT 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. | $~~~~~~~~$✔️ | |

## Best Practice Rules
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/naming/private-func-leading-underscore.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ title: "private-func-leading-underscore | Solhint"
![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)

## Description
Private and internal function names must start with a single underscore, unless they are in a library.
Private and internal function names must start with a single underscore, unless they are in a library or free.

## Options
This rule accepts an array of options:
Expand Down
11 changes: 6 additions & 5 deletions lib/rules/naming/private-func-leading-underscore.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const meta = {

docs: {
description:
'Private and internal function names must start with a single underscore, unless they are in a library.',
'Private and internal function names must start with a single underscore, unless they are in a library or free.',
category: 'Style Guide Rules',
options: [
{
Expand All @@ -30,16 +30,17 @@ const meta = {
class PrivateFuncLeadingUnderscoreChecker extends BaseChecker {
constructor(reporter) {
super(reporter, ruleId, meta)
this.inContract = false
}

ContractDefinition(node) {
if (node.kind === 'library') {
this.inLibrary = true
if (node.kind === 'contract') {
this.inContract = true
}
}

'ContractDefinition:exit'() {
this.inLibrary = false
this.inContract = false
}

FunctionDefinition(node) {
Expand All @@ -49,7 +50,7 @@ class PrivateFuncLeadingUnderscoreChecker extends BaseChecker {

const isPrivate = node.visibility === 'private'
const isInternal = node.visibility === 'internal' || node.visibility === 'default'
const shouldHaveLeadingUnderscore = !this.inLibrary && (isPrivate || isInternal)
const shouldHaveLeadingUnderscore = this.inContract && (isPrivate || isInternal)
this.validateName(node, shouldHaveLeadingUnderscore)
}

Expand Down
9 changes: 9 additions & 0 deletions test/common/contract-builder.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
const { times } = require('lodash')

function freeFunction(code) {
return `
pragma solidity 0.4.4;
${code}
`
}

function contractWith(code) {
return `
pragma solidity 0.4.4;
Expand Down Expand Up @@ -68,6 +76,7 @@ function repeatLines(line, count) {
module.exports = {
contractWith,
libraryWith,
freeFunction,
funcWith,
modifierWith,
multiLine,
Expand Down
1 change: 1 addition & 0 deletions test/rules/gas-consumption/gas-named-return-values.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ describe('Linter - gas-named-return-values', () => {
'comprehensive-interface': 'off',
'foundry-test-functions': 'off',
'func-param-name-leading-underscore': 'off',
'strict-override': 'off',
},
})

Expand Down
5 changes: 2 additions & 3 deletions test/rules/naming/func-param-name-leading-underscore.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const { contractWith, libraryWith } = require('../../common/contract-builder')
describe('Linter - func-param-name-leading-underscore', () => {
const SHOULD_WARN_CASES = [
// warn when param names don't start with _
contractWith('struct foo { uint bar; }\nfunction foo(foo memory bar) {}'),

contractWith('function foo(uint bar) {}'),
contractWith('function foo(uint bar) internal {}'),
Expand All @@ -20,9 +21,7 @@ describe('Linter - func-param-name-leading-underscore', () => {
]

const SHOULD_NOT_WARN_CASES = [
// warn when param names start with _

// don't warn when private/internal/default names start with _
contractWith('struct foo { uint bar; }\nfunction foo(foo memory _bar) {}'),

// don't warn when public/external names don't start with _
contractWith('uint public foo;'),
Expand Down
12 changes: 11 additions & 1 deletion test/rules/naming/private-func-leading-underscore.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const assert = require('assert')
const linter = require('../../../lib/index')
const { contractWith, libraryWith } = require('../../common/contract-builder')
const { contractWith, libraryWith, freeFunction } = require('../../common/contract-builder')

describe('Linter - private-func-leading-underscore', () => {
const SHOULD_WARN_CASES = [
Expand All @@ -20,6 +20,12 @@ describe('Linter - private-func-leading-underscore', () => {
libraryWith('function _foo() private {}'),
libraryWith('function _foo() public {}'),
libraryWith('function _foo() external {}'),

freeFunction('function _foo() {}'),
freeFunction('function _foo() internal {}'),
freeFunction('function _foo() private {}'),
freeFunction('function _foo() public {}'),
freeFunction('function _foo() external {}'),
]

const SHOULD_NOT_WARN_CASES = [
Expand All @@ -38,6 +44,10 @@ describe('Linter - private-func-leading-underscore', () => {
libraryWith('function foo() internal {}'),
libraryWith('function foo() private {}'),

// don't warn when internal/private functions in free functions don't start with _
freeFunction('function foo() internal {}'),
freeFunction('function foo() private {}'),

// don't warn for constructors
contractWith('constructor() public {}'),

Expand Down

0 comments on commit 82c456c

Please sign in to comment.