Skip to content

Commit

Permalink
fix: skip free functions on func-visibility
Browse files Browse the repository at this point in the history
  • Loading branch information
dbale-altoros committed Feb 1, 2023
1 parent a2b8ea3 commit 1292516
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 10 deletions.
2 changes: 1 addition & 1 deletion docs/rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ title: "Rule Index of Solhint"
| [avoid-tx-origin](./rules/security/avoid-tx-origin.md) | Avoid to use tx.origin. | ✔️ |
| [check-send-result](./rules/security/check-send-result.md) | Check result of "send" call. | ✔️ |
| [compiler-version](./rules/security/compiler-version.md) | Compiler version must satisfy a semver requirement. | ✔️ |
| [func-visibility](./rules/security/func-visibility.md) | Explicitly mark visibility in function. | ✔️ |
| [func-visibility](./rules/security/func-visibility.md) | Explicitly mark visibility in function (free functions are skipped). | ✔️ |
| [mark-callable-contracts](./rules/security/mark-callable-contracts.md) | Explicitly mark all external contracts as trusted or untrusted. | |
| [multiple-sends](./rules/security/multiple-sends.md) | Avoid multiple calls of "send" method in single transaction. | ✔️ |
| [no-complex-fallback](./rules/security/no-complex-fallback.md) | Fallback function must be simple. | ✔️ |
Expand Down
2 changes: 1 addition & 1 deletion docs/rules/security/func-visibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ title: "func-visibility | Solhint"

## Description
Explicitly mark visibility in function.
Explicitly mark visibility in function (free functions are skipped).

## Options
This rule accepts an array of options:
Expand Down
44 changes: 36 additions & 8 deletions lib/rules/security/func-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const meta = {
type: 'security',

docs: {
description: `Explicitly mark visibility in function.`,
description: `Explicitly mark visibility in function (free functions are skipped).`,
category: 'Security Rules',
options: [
{
Expand Down Expand Up @@ -53,6 +53,8 @@ const meta = {
},
}

let contractsInfo

class FuncVisibilityChecker extends BaseChecker {
constructor(reporter, config) {
super(reporter, ruleId, meta)
Expand All @@ -61,18 +63,44 @@ class FuncVisibilityChecker extends BaseChecker {
config && config.getObjectPropertyBoolean(ruleId, 'ignoreConstructors', false)
}

SourceUnit(node) {
if (!node.children) return

contractsInfo = []

let contractInfo = {}
for (const child of node.children) {
if (child.type === 'ContractDefinition' && child.kind === 'contract') {
contractInfo = {
name: child.name,
start: child.loc.start.line,
end: child.loc.end.line,
}
contractsInfo.push(contractInfo)
}
}
}

FunctionDefinition(node) {
if (node.isConstructor && this.ignoreConstructors) {
return
}

if (node.visibility === 'default') {
this.warn(
node,
node.isConstructor
? 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)'
: 'Explicitly mark visibility in function'
)
const functionStart = node.loc.start.line
const functionEnd = node.loc.end.line

for (const contract of contractsInfo) {
// this checks if the function is inside any contract, if not is a free function
if (functionStart > contract.start && functionEnd < contract.end) {
if (node.visibility === 'default') {
this.warn(
node,
node.isConstructor
? 'Explicitly mark visibility in function (Set ignoreConstructors to true if using solidity >=0.7.0)'
: 'Explicitly mark visibility in function'
)
}
}
}
}
}
Expand Down
176 changes: 176 additions & 0 deletions test/fixtures/security/contracts-with-free-functions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
// all contracts are made with two func-visibility errors

module.exports = [
`
// SPDX-License-Identifier: MIT
pragma solidity 0.4.4;
import "hardhat/console.sol";
interface fufu {
function intFunction() returns(bool);
function intFunction() returns(uint256);
}
function freeAa() returns(bool) {
return true;
}
contract A {
uint256 public a;
constructor() {
a = 2;
}
function functionPublicViewA() returns (uint256) {
if (freeAa()) return a;
else return a+1;
}
function functionPublicPureA() public pure returns (bool) {
return freeAa();
}
}
function freeBb() returns(bool) {
return true;
}
contract B {
uint256 public a;
constructor() {
a = 2;
}
function functionPublicViewB() returns (uint256) {
if (freeBb()) return a;
else return a+1;
}
function functionPublicPureB() public pure returns (bool) {
return freeCc();
}
}
function freeCc() returns(bool) {
return true;
}
`,
`
// SPDX-License-Identifier: MIT
pragma solidity 0.8.4;
import "hardhat/console.sol";
function freeAa() returns(bool) {
return true;
}
function freeBb() returns(bool) {
return true;
}
interface fufu {
function intFunction() returns(bool);
function intFunction() returns(uint256);
}
contract A {
uint256 public a;
constructor() {
a = 2;
}
function functionPublicViewA() returns (uint256) {
if (freeAa()) return a;
else return a+1;
}
function functionPublicPureA() public pure returns (bool) {
return freeBb();
}
}
function freeCc() returns(bool) {
return true;
}
contract B {
uint256 public a;
constructor() {
a = 2;
}
function functionPublicViewB() returns (uint256) {
if (freeAa()) return a;
else return a+1;
}
function functionPublicPureB() public pure returns (bool) {
return freeCc();
}
}
`,
`
// SPDX-License-Identifier: MIT
pragma solidity 0.8.4;
function freeCc() returns(bool) {
return true;
}
contract A {
uint256 public a;
constructor() {
a = 2;
}
function functionExternal() returns (uint256) {
if (freeCc()) return a;
else return a+1;
}
function functionPublicViewA() returns (uint256) {
if (freeCc()) return a;
else return a+1;
}
function functionPublicPureA() public pure returns (bool) {
return freeCc();
}
}
`,
`
// SPDX-License-Identifier: MIT
pragma solidity 0.8.4;
contract A {
uint256 public a;
constructor() {
a = 2;
}
function functionExternal() returns (uint256) {
if (freeCc()) return a;
else return a+1;
}
function functionPublicViewA() returns (uint256) {
if (freeCc()) return a;
else return a+1;
}
function functionPublicPureA() public pure returns (bool) {
return freeCc();
}
}
function freeCc() returns(bool) {
return true;
}
`,
]
16 changes: 16 additions & 0 deletions test/rules/security/func-visibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@ const assert = require('assert')
const linter = require('../../../lib/index')
const contractWith = require('../../common/contract-builder').contractWith

describe('Linter - func-visibility with free functions', () => {
const contractWithFreeFunction = require('../../fixtures/security/contracts-with-free-functions')

contractWithFreeFunction.forEach((code) =>
it('should return required visibility error skiping free functions', () => {
const report = linter.processStr(code, {
rules: { 'func-visibility': ['warn', { ignoreConstructors: true }] },
})

assert.equal(report.warningCount, 2)
assert.ok(report.reports[0].message.includes('visibility'))
assert.ok(report.reports[1].message.includes('visibility'))
})
)
})

describe('Linter - func-visibility', () => {
it('should return required visibility error', () => {
require('../../fixtures/security/functions-without-visibility').forEach((func) => {
Expand Down

0 comments on commit 1292516

Please sign in to comment.