Merge pull request #467 from protofire/i387-explicit-types-rule

New Rule: explicit-types

CHANGELOG.md

@@ -12,6 +12,7 @@
 ### Added
 - New Rule: No unused imports [#417](https://github.com/protofire/solhint/pull/417)
 - New Rule: To treat immutable as constants [#458](https://github.com/protofire/solhint/pull/458)
+- New Rule: Explicit-types. To forbid/enforce full type or alias for variables declaration [#467](https://github.com/protofire/solhint/pull/467)
 - JSON formatter support [#440](https://github.com/protofire/solhint/pull/440)
 - Rules List with `list-rules` command [#449](https://github.com/protofire/solhint/pull/449)
 - E2E tests for formatters and new `Compact formatter` [#457](https://github.com/protofire/solhint/pull/457)

docs/rules.md

@@ -9,6 +9,7 @@ title:       "Rule Index of Solhint"
 | Rule Id                                                            | Error                                                                                                                 | Recommended  | Deprecated |
 | ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
 | [code-complexity](./rules/best-practises/code-complexity.md)       | Function has cyclomatic complexity "current" but allowed no more than maxcompl.                                       |              |            |
+| [explicit-types](./rules/best-practises/explicit-types.md)         | Forbid or enforce explicit types (like uint256) that have an alias (like uint).                                       | $~~~~~~~~$✔️ |            |
 | [function-max-lines](./rules/best-practises/function-max-lines.md) | Function body contains "count" lines but allowed no more than maxlines.                                               |              |            |
 | [max-line-length](./rules/best-practises/max-line-length.md)       | Line length must be no more than maxlen.                                                                              |              |            |
 | [max-states-count](./rules/best-practises/max-states-count.md)     | Contract has "some count" states declarations but allowed no more than maxstates.                                     | $~~~~~~~~$✔️ |            |

docs/rules/best-practises/explicit-types.md

@@ -0,0 +1,71 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "explicit-types | Solhint"
+---
+
+# explicit-types
+![Recommended Badge](https://img.shields.io/badge/-Recommended-brightgreen)
+![Category Badge](https://img.shields.io/badge/-Best%20Practise%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
+Forbid or enforce explicit types (like uint256) that have an alias (like uint).
+
+## Options
+This rule accepts an array of options:
+
+| Index | Description                                           | Default Value |
+| ----- | ----------------------------------------------------- | ------------- |
+| 0     | Rule severity. Must be one of "error", "warn", "off". | warn          |
+| 1     | Options need to be one of "explicit", "implicit"      | explicit      |
+
+
+### Example Config
+```json
+{
+  "rules": {
+    "explicit-types": ["warn","explicit"]
+  }
+}
+```
+
+
+## Examples
+### 👍 Examples of **correct** code for this rule
+
+#### If explicit is selected
+
+```solidity
+uint256 public variableName
+```
+
+#### If implicit is selected
+
+```solidity
+uint public variableName
+```
+
+### 👎 Examples of **incorrect** code for this rule
+
+#### If explicit is selected
+
+```solidity
+uint public variableName
+```
+
+#### If implicit is selected
+
+```solidity
+uint256 public variableName
+```
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/explicit-types.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/explicit-types.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/explicit-types.js)

lib/rules/best-practises/explicit-types.js

@@ -0,0 +1,128 @@
+const BaseChecker = require('../base-checker')
+const { severityDescription, formatEnum } = require('../../doc/utils')
+
+const EXPLICIT_TYPES = ['uint256', 'int256', 'ufixed128x18', 'fixed128x18']
+const IMPLICIT_TYPES = ['uint', 'int', 'ufixed', 'fixed']
+const ALL_TYPES = EXPLICIT_TYPES.concat(IMPLICIT_TYPES)
+const VALID_CONFIGURATION_OPTIONS = ['explicit', 'implicit']
+const DEFAULT_OPTION = 'explicit'
+const DEFAULT_SEVERITY = 'warn'
+
+const ruleId = 'explicit-types'
+const meta = {
+  type: 'best-practises',
+
+  docs: {
+    description: 'Forbid or enforce explicit types (like uint256) that have an alias (like uint).',
+    category: 'Best Practise Rules',
+    options: [
+      {
+        description: severityDescription,
+        default: DEFAULT_SEVERITY,
+      },
+      {
+        description: `Options need to be one of ${formatEnum(VALID_CONFIGURATION_OPTIONS)}`,
+        default: DEFAULT_OPTION,
+      },
+    ],
+    examples: {
+      good: [
+        {
+          description: 'If explicit is selected',
+          code: 'uint256 public variableName',
+        },
+        {
+          description: 'If implicit is selected',
+          code: 'uint public variableName',
+        },
+      ],
+      bad: [
+        {
+          description: 'If explicit is selected',
+          code: 'uint public variableName',
+        },
+        {
+          description: 'If implicit is selected',
+          code: 'uint256 public variableName',
+        },
+      ],
+    },
+  },
+
+  isDefault: false,
+  recommended: true,
+  defaultSetup: [DEFAULT_SEVERITY, DEFAULT_OPTION],
+
+  schema: {
+    type: 'string',
+    enum: VALID_CONFIGURATION_OPTIONS,
+  },
+}
+
+class ExplicitTypesChecker extends BaseChecker {
+  constructor(reporter, config) {
+    super(reporter, ruleId, meta)
+    this.configOption = (config && config.getString(ruleId, DEFAULT_OPTION)) || DEFAULT_OPTION
+    this.isExplicit = this.configOption === 'explicit'
+
+    this.validateConfigOption(this.configOption)
+  }
+
+  VariableDeclaration(node) {
+    const typeToFind = 'ElementaryTypeName'
+    const onlyTypes = this.findNamesOfElementaryTypeName(node, typeToFind)
+
+    // if no variables are defined, return
+    if (onlyTypes && onlyTypes.length === 0) return
+
+    // this tells if the variable needs to be checked
+    const varsToBeChecked = onlyTypes.filter((type) => ALL_TYPES.includes(type))
+
+    // if defined variables are not to be checked (example address type), return
+    if (varsToBeChecked && varsToBeChecked.length === 0) return
+
+    // if explicit, it will search for implicit and viceversa
+    if (this.isExplicit) {
+      this.validateVariables(IMPLICIT_TYPES, node, varsToBeChecked)
+    } else {
+      this.validateVariables(EXPLICIT_TYPES, node, varsToBeChecked)
+    }
+  }
+
+  validateConfigOption(configOption) {
+    const configOk = VALID_CONFIGURATION_OPTIONS.includes(configOption)
+    if (!configOk)
+      throw new Error('Error: Config error on [explicit-types]. See explicit-types.md.')
+  }
+
+  validateVariables(configType, node, varsToBeChecked) {
+    const errorVars = varsToBeChecked.filter((type) => configType.includes(type))
+
+    if (errorVars && errorVars.length > 0) {
+      for (const errorVar of errorVars) {
+        this.error(node, `Rule is set with ${this.configOption} type [var/s: ${errorVar}]`)
+      }
+    }
+  }
+
+  findNamesOfElementaryTypeName(jsonObject, typeToFind) {
+    const names = []
+
+    const searchInObject = (obj) => {
+      if (obj.type === typeToFind && obj.name) {
+        names.push(obj.name)
+      }
+
+      for (const key in obj) {
+        if (typeof obj[key] === 'object' && obj[key] !== null) {
+          searchInObject(obj[key])
+        }
+      }
+    }
+
+    searchInObject(jsonObject)
+    return names
+  }
+}
+
+module.exports = ExplicitTypesChecker

lib/rules/best-practises/index.js

@@ -9,6 +9,7 @@ const ReasonStringChecker = require('./reason-string')
 const NoConsoleLogChecker = require('./no-console')
 const NoGlobalImportsChecker = require('./no-global-import')
 const NoUnusedImportsChecker = require('./no-unused-import')
+const ExplicitTypesChecker = require('./explicit-types')
 
 module.exports = function checkers(reporter, config, inputSrc) {
   return [
@@ -23,5 +24,6 @@ module.exports = function checkers(reporter, config, inputSrc) {
     new NoConsoleLogChecker(reporter),
     new NoGlobalImportsChecker(reporter),
     new NoUnusedImportsChecker(reporter),
+    new ExplicitTypesChecker(reporter, config),
   ]
 }

test/fixtures/best-practises/explicit-types.js

@@ -0,0 +1,135 @@
+const VAR_DECLARATIONS = {
+  uint256: {
+    code: 'uint256 public varUint256;',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  uint: {
+    code: 'uint public varUint;',
+    errorsImplicit: 0,
+    errorsExplicit: 1,
+  },
+
+  int256: {
+    code: 'int256 public varInt256;',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  int: {
+    code: 'int public varInt;',
+    errorsImplicit: 0,
+    errorsExplicit: 1,
+  },
+
+  ufixed128x18: {
+    code: 'ufixed128x18 public varUfixed128x18;',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  ufixed: {
+    code: 'ufixed public varUfixed;',
+    errorsImplicit: 0,
+    errorsExplicit: 1,
+  },
+
+  fixed128x18: {
+    code: 'fixed128x18 public varFixed128x18;',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  fixed: {
+    code: 'fixed public varFixed;',
+    errorsImplicit: 0,
+    errorsExplicit: 1,
+  },
+
+  functionParameterAndReturns1: {
+    code: 'function withUint256(uint256 varUint256, uint varUint) public returns(int256 returnInt256) {}',
+    errorsImplicit: 2,
+    errorsExplicit: 1,
+  },
+
+  functionParameterAndReturns2: {
+    code: 'function withUint256(uint varUint, uint varUint) public returns(int returnInt) {}',
+    errorsImplicit: 0,
+    errorsExplicit: 3,
+  },
+
+  eventParameter: {
+    code: 'event EventWithInt256(int256 varWithInt256, address addressVar, bool boolVat, int varWithInt);',
+    errorsImplicit: 1,
+    errorsExplicit: 1,
+  },
+
+  regularMap1: {
+    code: 'mapping(uint256 => fixed128x18) public mapWithFixed128x18;',
+    errorsImplicit: 2,
+    errorsExplicit: 0,
+  },
+
+  regularMap2: {
+    code: 'mapping(uint => fixed128x18) public mapWithFixed128x18;',
+    errorsImplicit: 1,
+    errorsExplicit: 1,
+  },
+
+  mapOfMapping: {
+    code: 'mapping(uint => mapping(fixed128x18 => int256)) mapOfMap;',
+    errorsImplicit: 2,
+    errorsExplicit: 1,
+  },
+
+  struct: {
+    code: 'struct Estructura { ufixed128x18 varWithUfixed128x18; uint varUint; }',
+    errorsImplicit: 1,
+    errorsExplicit: 1,
+  },
+
+  mapOfStruct: {
+    code: 'struct Estructura { ufixed128x18 varWithUfixed128x18; uint varUint; mapping(uint256 => Estructura) mapOfStruct; }',
+    errorsImplicit: 2,
+    errorsExplicit: 1,
+  },
+
+  structWithMap: {
+    code: 'struct Estructura { ufixed varWithUfixed; uint varUint; mapping(address => uint256) structWithMap; } ',
+    errorsImplicit: 1,
+    errorsExplicit: 2,
+  },
+
+  mapOfArrayStructWithMap: {
+    code: 'struct Estructura { ufixed varWithUfixed; uint varUint; mapping(address => uint256) structWithMap; } mapping(uint256 => Estructura[]) mapOfArrayStructWithMap;',
+    errorsImplicit: 2,
+    errorsExplicit: 2,
+  },
+
+  regularArray: {
+    code: 'uint256[] public arr;',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  fixedArray: {
+    code: 'uint[] public arr = [1,2,3];',
+    errorsImplicit: 0,
+    errorsExplicit: 1,
+  },
+
+  fixedArrayOfArrays: {
+    code: 'uint256[] public arr = [[1,2,3]];',
+    errorsImplicit: 1,
+    errorsExplicit: 0,
+  },
+
+  mapOfFixedArray: {
+    code: 'uint[] public arr = [1,2,3]; mapping(uint256 => arr) mapOfFixedArray;',
+    errorsImplicit: 1,
+    errorsExplicit: 1,
+  },
+}
+
+module.exports = VAR_DECLARATIONS