Merge pull request #396 from protofire/fix/disable-func-visibility-fo…

…r-free-functions

fix: skip free functions on func-visibility

lib/rules/security/func-visibility.js

@@ -61,18 +61,28 @@ class FuncVisibilityChecker extends BaseChecker {
       config && config.getObjectPropertyBoolean(ruleId, 'ignoreConstructors', false)
   }
 
+  ContractDefinition() {
+    this.isContract = true
+  }
+
+  'ContractDefinition:exit'() {
+    this.isContract = false
+  }
+
   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'
-      )
+    if (this.isContract) {
+      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'
+        )
+      }
     }
   }
 }

test/fixtures/security/contracts-with-free-functions.js

@@ -0,0 +1,69 @@
+const CONTRACTS_FREE_FUNCTIONS_ERRORS_2 = `
+function freeAa() returns(bool) {
+    return true;
+}
+contract A {
+    // error here
+    function functionPublicViewA() returns (uint256) {
+        return 1;
+    }
+}
+
+function freeBb() returns(bool) {
+    return true;
+}
+contract B {
+    // error here
+    function functionPublicViewA() returns (uint256) {
+        return 1;
+    }
+
+    function functionPublicPureB() internal pure returns (bool) {
+        return true;
+    }
+}
+`
+
+const CONTRACT_FREE_FUNCTIONS_ERRORS_1 = `
+contract A {
+    constructor() {}
+
+    // error here
+    function functionPublicViewA() returns (uint256) {
+        return 1;
+    }
+}
+
+function freeBb() returns(bool) {
+    return true;
+}
+`
+
+const NOCONTRACT_FREE_FUNCTION_ERRORS_0 = `
+    // NO error here
+    function functionPublicViewA() returns (uint256) {
+        return 1;
+    }
+`
+
+const CONTRACT_FREE_FUNCTIONS_ERRORS_0 = `
+function freeBb() returns(bool) {
+    return true;
+}
+
+contract A {
+    constructor() {}
+
+    // NO error here
+    function functionPublicViewA() external returns (uint256) {
+        return 1;
+    }
+}
+`
+
+module.exports = {
+  CONTRACTS_FREE_FUNCTIONS_ERRORS_2,
+  CONTRACT_FREE_FUNCTIONS_ERRORS_1,
+  NOCONTRACT_FREE_FUNCTION_ERRORS_0,
+  CONTRACT_FREE_FUNCTIONS_ERRORS_0,
+}

test/rules/security/func-visibility.js

@@ -1,6 +1,48 @@
 const assert = require('assert')
 const linter = require('../../../lib/index')
 const contractWith = require('../../common/contract-builder').contractWith
+const CONTRACTS = require('../../fixtures/security/contracts-with-free-functions')
+
+describe('Linter - func-visibility with free functions', () => {
+  it('should return two warnings and skip free functions', () => {
+    const code = CONTRACTS.CONTRACTS_FREE_FUNCTIONS_ERRORS_2
+    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'))
+  })
+
+  it('should return one warning and skip free functions', () => {
+    const code = CONTRACTS.CONTRACT_FREE_FUNCTIONS_ERRORS_1
+    const report = linter.processStr(code, {
+      rules: { 'func-visibility': ['warn', { ignoreConstructors: true }] },
+    })
+
+    assert.equal(report.warningCount, 1)
+    assert.ok(report.reports[0].message.includes('visibility'))
+  })
+
+  it('should not return any warning for a free function only', () => {
+    const code = CONTRACTS.NOCONTRACT_FREE_FUNCTION_ERRORS_0
+    const report = linter.processStr(code, {
+      rules: { 'func-visibility': ['warn', { ignoreConstructors: true }] },
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+
+  it('should not return any warning for a correct contract with a free function', () => {
+    const code = CONTRACTS.CONTRACT_FREE_FUNCTIONS_ERRORS_0
+    const report = linter.processStr(code, {
+      rules: { 'func-visibility': ['warn', { ignoreConstructors: true }] },
+    })
+
+    assert.equal(report.warningCount, 0)
+  })
+})
 
 describe('Linter - func-visibility', () => {
   it('should return required visibility error', () => {