Add custom linting rules (#4132)

Co-authored-by: Francisco Giordano <fg@frang.io>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>

.solhint.json

@@ -1,15 +0,0 @@
-{
-  "rules": {
-    "no-unused-vars": "error",
-    "const-name-snakecase": "error",
-    "contract-name-camelcase": "error",
-    "event-name-camelcase": "error",
-    "func-name-mixedcase": "error",
-    "func-param-name-mixedcase": "error",
-    "modifier-name-mixedcase": "error",
-    "private-vars-leading-underscore": "error",
-    "var-name-mixedcase": "error",
-    "imports-on-top": "error",
-    "no-global-import": "error"
-  }
-}

package-lock.json

@@ -43,6 +43,7 @@
         "rimraf": "^3.0.2",
         "semver": "^7.3.5",
         "solhint": "^3.3.6",
+        "solhint-plugin-openzeppelin": "file:scripts/solhint-custom",
         "solidity-ast": "^0.4.25",
         "solidity-coverage": "^0.8.0",
         "solidity-docgen": "^0.6.0-beta.29",
@@ -12436,6 +12437,10 @@
         "prettier": "^2.8.3"
       }
     },
+    "node_modules/solhint-plugin-openzeppelin": {
+      "resolved": "scripts/solhint-custom",
+      "link": true
+    },
     "node_modules/solhint/node_modules/@solidity-parser/parser": {
       "version": "0.16.0",
       "resolved": "https://registry.npmjs.org/@solidity-parser/parser/-/parser-0.16.0.tgz",
@@ -15405,6 +15410,16 @@
       "funding": {
         "url": "https://github.com/sponsors/sindresorhus"
       }
+    },
+    "scripts/lints": {
+      "version": "1.0.2",
+      "extraneous": true,
+      "license": "MIT"
+    },
+    "scripts/solhint-custom": {
+      "version": "1.0.2",
+      "dev": true,
+      "license": "MIT"
     }
   },
   "dependencies": {
@@ -25126,6 +25141,9 @@
         }
       }
     },
+    "solhint-plugin-openzeppelin": {
+      "version": "file:scripts/solhint-custom"
+    },
     "solidity-ast": {
       "version": "0.4.49",
       "resolved": "https://registry.npmjs.org/solidity-ast/-/solidity-ast-0.4.49.tgz",

package.json

@@ -84,6 +84,7 @@
     "rimraf": "^3.0.2",
     "semver": "^7.3.5",
     "solhint": "^3.3.6",
+    "solhint-plugin-openzeppelin": "file:scripts/solhint-custom",
     "solidity-ast": "^0.4.25",
     "solidity-coverage": "^0.8.0",
     "solidity-docgen": "^0.6.0-beta.29",

scripts/solhint-custom/index.js

@@ -0,0 +1,84 @@
+const path = require('path');
+const minimatch = require('minimatch');
+
+// Files matching these patterns will be ignored unless a rule has `static global = true`
+const ignore = ['contracts/mocks/**/*', 'test/**/*'];
+
+class Base {
+  constructor(reporter, config, source, fileName) {
+    this.reporter = reporter;
+    this.ignored = this.constructor.global || ignore.some(p => minimatch(path.normalize(fileName), p));
+    this.ruleId = this.constructor.ruleId;
+    if (this.ruleId === undefined) {
+      throw Error('missing ruleId static property');
+    }
+  }
+
+  error(node, message) {
+    if (!this.ignored) {
+      this.reporter.error(node, this.ruleId, message);
+    }
+  }
+}
+
+module.exports = [
+  class extends Base {
+    static ruleId = 'interface-names';
+
+    ContractDefinition(node) {
+      if (node.kind === 'interface' && !/^I[A-Z]/.test(node.name)) {
+        this.error(node, 'Interface names should have a capital I prefix');
+      }
+    }
+  },
+
+  class extends Base {
+    static ruleId = 'private-variables';
+
+    VariableDeclaration(node) {
+      const constantOrImmutable = node.isDeclaredConst || node.isImmutable;
+      if (node.isStateVar && !constantOrImmutable && node.visibility !== 'private') {
+        this.error(node, 'State variables must be private');
+      }
+    }
+  },
+
+  class extends Base {
+    static ruleId = 'leading-underscore';
+
+    VariableDeclaration(node) {
+      if (node.isDeclaredConst) {
+        if (/^_/.test(node.name)) {
+          // TODO: re-enable and fix
+          // this.error(node, 'Constant variables should not have leading underscore');
+        }
+      } else if (node.visibility === 'private' && !/^_/.test(node.name)) {
+        this.error(node, 'Non-constant private variables must have leading underscore');
+      }
+    }
+
+    FunctionDefinition(node) {
+      if (node.visibility === 'private' || (node.visibility === 'internal' && node.parent.kind !== 'library')) {
+        if (!/^_/.test(node.name)) {
+          this.error(node, 'Private and internal functions must have leading underscore');
+        }
+      }
+      if (node.visibility === 'internal' && node.parent.kind === 'library') {
+        if (/^_/.test(node.name)) {
+          this.error(node, 'Library internal functions should not have leading underscore');
+        }
+      }
+    }
+  },
+
+  // TODO: re-enable and fix
+  // class extends Base {
+  //   static ruleId = 'no-external-virtual';
+  //
+  //   FunctionDefinition(node) {
+  //     if (node.visibility == 'external' && node.isVirtual) {
+  //       this.error(node, 'Functions should not be external and virtual');
+  //     }
+  //   }
+  // },
+];

scripts/solhint-custom/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "solhint-plugin-openzeppelin",
+  "private": true
+}

solhint.config.js

@@ -0,0 +1,20 @@
+const customRules = require('./scripts/solhint-custom');
+
+const rules = [
+  'no-unused-vars',
+  'const-name-snakecase',
+  'contract-name-camelcase',
+  'event-name-camelcase',
+  'func-name-mixedcase',
+  'func-param-name-mixedcase',
+  'modifier-name-mixedcase',
+  'var-name-mixedcase',
+  'imports-on-top',
+  'no-global-import',
+  ...customRules.map(r => `openzeppelin/${r.ruleId}`),
+];
+
+module.exports = {
+  plugins: ['openzeppelin'],
+  rules: Object.fromEntries(rules.map(r => [r, 'error'])),
+};