diff --git a/CHANGELOG.md b/CHANGELOG.md index 9af36d16..d63bff5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,29 @@ +## [3.4.2] - 2023-08-04 +### Updated +- Support `ignoreConstructors` option for `no-empty-blocks` [#418](https://github.com/protofire/solhint/pull/418) +- Bump json5 from 2.1.3 to 2.2.3 [#376](https://github.com/protofire/solhint/pull/376) +- Bump json-schema and jsprim [#370](https://github.com/protofire/solhint/pull/370) +- Bump semver from 6.3.0 to 7.5.2 [#438](https://github.com/protofire/solhint/pull/438) +- Corrected "Category" of `quotes` rule, added default rules list on readme [#443](https://github.com/protofire/solhint/pull/443) +- 'Deprecated' column on `rules.md`` [#444](https://github.com/protofire/solhint/pull/444) +- Information about maxCharacters allowed on `reason-string` rule [#446](https://github.com/protofire/solhint/pull/446) +- E2E tests for `max-warnings` [#455](https://github.com/protofire/solhint/pull/455) +- Replaced blacklist and whitelist words [#459](https://github.com/protofire/solhint/pull/459) +### 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) + +### Fixed +- `maxWarnings` parameter waiting review [#439](https://github.com/protofire/solhint/pull/439) +- `–fix` option not working in avoid-throw rule [#442](https://github.com/protofire/solhint/pull/442) +- Formatter option fixed for `stdin` command [#450](https://github.com/protofire/solhint/pull/450) + + +

## [3.4.1] - 2023-03-06 ### Updated - Updated solidity parser to 0.16.0 [#420](https://github.com/protofire/solhint/pull/420) diff --git a/docs/rules.md b/docs/rules.md index 9765afd3..1e3d4d12 100644 --- a/docs/rules.md +++ b/docs/rules.md @@ -6,20 +6,21 @@ title: "Rule Index of Solhint" ## Best Practise Rules -| Rule Id | Error | Recommended | -| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ----------- | -| [code-complexity](./rules/best-practises/code-complexity.md) | Function has cyclomatic complexity "current" but allowed no more than maxcompl. | | -| [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. | ✔️ | -| [no-console](./rules/best-practises/no-console.md) | No console.log/logInt/logBytesX/logString/etc & No hardhat and forge-std console.sol import statements. | ✔️ | -| [no-empty-blocks](./rules/best-practises/no-empty-blocks.md) | Code contains empty block. | ✔️ | -| [no-global-import](./rules/best-practises/no-global-import.md) | Import statement includes an entire file instead of selected symbols. | ✔️ | -| [no-unused-import](./rules/best-practises/no-unused-import.md) | Imported name is not used. | ✔️ | -| [no-unused-vars](./rules/best-practises/no-unused-vars.md) | Variable "name" is unused. | ✔️ | -| [payable-fallback](./rules/best-practises/payable-fallback.md) | When fallback is not payable you will not be able to receive ethers. | ✔️ | -| [reason-string](./rules/best-practises/reason-string.md) | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | ✔️ | -| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword. | | +| 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. | $~~~~~~~~$✔️ | | +| [no-console](./rules/best-practises/no-console.md) | No console.log/logInt/logBytesX/logString/etc & No hardhat and forge-std console.sol import statements. | $~~~~~~~~$✔️ | | +| [no-empty-blocks](./rules/best-practises/no-empty-blocks.md) | Code block has zero statements inside. Exceptions apply. | $~~~~~~~~$✔️ | | +| [no-global-import](./rules/best-practises/no-global-import.md) | Import statement includes an entire file instead of selected symbols. | $~~~~~~~~$✔️ | | +| [no-unused-import](./rules/best-practises/no-unused-import.md) | Imported name is not used. | $~~~~~~~~$✔️ | | +| [no-unused-vars](./rules/best-practises/no-unused-vars.md) | Variable "name" is unused. | $~~~~~~~~$✔️ | | +| [payable-fallback](./rules/best-practises/payable-fallback.md) | When fallback is not payable you will not be able to receive ethers. | $~~~~~~~~$✔️ | | +| [reason-string](./rules/best-practises/reason-string.md) | Require or revert statement must have a reason string and check that each reason string is at most N characters long. | $~~~~~~~~$✔️ | | +| [constructor-syntax](./rules/best-practises/constructor-syntax.md) | Constructors should use the new constructor keyword. | | | ## Miscellaneous @@ -32,23 +33,23 @@ title: "Rule Index of Solhint" ## Style Guide Rules -| Rule Id | Error | Recommended | -| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | ----------- | -| [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 name must be in CamelCase. | ✔️ | -| [event-name-camelcase](./rules/naming/event-name-camelcase.md) | Event name must be in CamelCase. | ✔️ | -| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md) | Function name must be in mixedCase. | ✔️ | -| [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. | ✔️ | -| [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-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Private and internal names must 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 | +| ------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------- | ------------ | ----------- | +| [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 name must be in CamelCase. | $~~~~~~~~$✔️ | | +| [event-name-camelcase](./rules/naming/event-name-camelcase.md) | Event name must be in CamelCase. | $~~~~~~~~$✔️ | | +| [func-name-mixedcase](./rules/naming/func-name-mixedcase.md) | Function name must be in mixedCase. | $~~~~~~~~$✔️ | | +| [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. | $~~~~~~~~$✔️ | | +| [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-vars-leading-underscore](./rules/naming/private-vars-leading-underscore.md) | Private and internal names must 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. | $~~~~~~~~$✔️ | | ## Security Rules diff --git a/docs/rules/best-practises/explicit-types.md b/docs/rules/best-practises/explicit-types.md new file mode 100644 index 00000000..96cb06a5 --- /dev/null +++ b/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) diff --git a/docs/rules/best-practises/no-empty-blocks.md b/docs/rules/best-practises/no-empty-blocks.md index d4f3d0b0..cbc7caec 100644 --- a/docs/rules/best-practises/no-empty-blocks.md +++ b/docs/rules/best-practises/no-empty-blocks.md @@ -12,7 +12,7 @@ title: "no-empty-blocks | Solhint" ## Description -Code contains empty block. +Code block has zero statements inside. Exceptions apply. ## Options This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn. @@ -28,7 +28,39 @@ This rule accepts a string option of rule severity. Must be one of "error", "war ## Examples -This rule does not have examples. +### 👍 Examples of **correct** code for this rule + +#### Empty fallback function + +```solidity +fallback () external { } +``` + +#### Empty constructor with member initialization list + +```solidity +constructor(uint param) Foo(param) Bar(param*2) { } +``` + +### 👎 Examples of **incorrect** code for this rule + +#### Empty block on if statement + +```solidity +if (condition) { } +``` + +#### Empty contract + +```solidity +contract Foo { } +``` + +#### Empty block in constructor without parent initialization + +```solidity +constructor () { } +``` ## Version This rule was introduced in [Solhint 1.1.5](https://github.com/protofire/solhint/tree/v1.1.5) diff --git a/lib/common/identifier-naming.js b/lib/common/identifier-naming.js index 2a969a89..1444d398 100644 --- a/lib/common/identifier-naming.js +++ b/lib/common/identifier-naming.js @@ -4,7 +4,7 @@ function match(text, regex) { module.exports = { isMixedCase(text) { - return match(text, /[_]*[a-z]+[a-zA-Z0-9$]*[_]?/) + return match(text, /[_]*[a-z$]+[a-zA-Z0-9$]*[_]?/) }, isNotMixedCase(text) { @@ -12,7 +12,7 @@ module.exports = { }, isCamelCase(text) { - return match(text, /[A-Z]+[a-zA-Z0-9$]*/) + return match(text, /[A-Z$]+[a-zA-Z0-9$]*/) }, isNotCamelCase(text) { @@ -20,7 +20,7 @@ module.exports = { }, isUpperSnakeCase(text) { - return match(text, /_{0,2}[A-Z0-9]+[_A-Z0-9]*/) + return match(text, /_{0,2}[A-Z0-9$]+[_A-Z0-9$]*/) }, isNotUpperSnakeCase(text) { diff --git a/lib/rules/best-practises/explicit-types.js b/lib/rules/best-practises/explicit-types.js new file mode 100644 index 00000000..4f2a6149 --- /dev/null +++ b/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 diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js index fc03f0f9..6b2ac780 100644 --- a/lib/rules/best-practises/index.js +++ b/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), ] } diff --git a/lib/rules/best-practises/no-empty-blocks.js b/lib/rules/best-practises/no-empty-blocks.js index f49bd935..52ae07f0 100644 --- a/lib/rules/best-practises/no-empty-blocks.js +++ b/lib/rules/best-practises/no-empty-blocks.js @@ -6,8 +6,25 @@ const meta = { type: 'best-practises', docs: { - description: 'Code contains empty block.', + description: 'Code block has zero statements inside. Exceptions apply.', category: 'Best Practise Rules', + examples: { + bad: [ + { description: 'Empty block on if statement', code: 'if (condition) { }' }, + { description: 'Empty contract', code: 'contract Foo { }' }, + { + description: 'Empty block in constructor without parent initialization', + code: 'constructor () { }', + }, + ], + good: [ + { description: 'Empty fallback function', code: 'fallback () external { }' }, + { + description: 'Empty constructor with member initialization list', + code: 'constructor(uint param) Foo(param) Bar(param*2) { }', + }, + ], + }, }, isDefault: false, diff --git a/package.json b/package.json index 76bbf50c..9d27df97 100644 --- a/package.json +++ b/package.json @@ -41,8 +41,6 @@ "chalk": "^4.1.2", "commander": "^10.0.0", "cosmiconfig": "^8.0.0", - "cross-spawn": "^7.0.3", - "execa": "^4.1.0", "fast-diff": "^1.2.0", "glob": "^8.0.3", "ignore": "^5.2.4", diff --git a/test/fixtures/best-practises/explicit-types.js b/test/fixtures/best-practises/explicit-types.js new file mode 100644 index 00000000..b63c4838 --- /dev/null +++ b/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 diff --git a/test/rules/best-practises/explicit-types.js b/test/rules/best-practises/explicit-types.js new file mode 100644 index 00000000..842329c6 --- /dev/null +++ b/test/rules/best-practises/explicit-types.js @@ -0,0 +1,95 @@ +const assert = require('assert') +const linter = require('../../../lib/index') +const contractWith = require('../../common/contract-builder').contractWith +const { assertErrorCount, assertNoErrors, assertErrorMessage } = require('../../common/asserts') +const VAR_DECLARATIONS = require('../../fixtures/best-practises/explicit-types') + +const getZeroErrosObject = () => { + const zeroErrorsExplicit = {} + const zeroErrorsImplicit = {} + for (const key in VAR_DECLARATIONS) { + const obj = VAR_DECLARATIONS[key] + if (obj.errorsImplicit === 0) { + zeroErrorsImplicit[key] = obj + } + if (obj.errorsExplicit === 0) { + zeroErrorsExplicit[key] = obj + } + } + + return { zeroErrorsImplicit, zeroErrorsExplicit } +} + +describe('Linter - explicit-types rule', () => { + it('should throw an error with a wrong configuration rule example 1', () => { + const code = contractWith('uint256 public constant SNAKE_CASE = 1;') + try { + linter.processStr(code, { + rules: { 'explicit-types': ['error', 'implicito'] }, + }) + } catch (error) { + assert.ok( + error.toString().includes('Error: Config error on [explicit-types]. See explicit-types.md.') + ) + } + }) + + it('should throw an error with a wrong configuration rule example 2', () => { + const code = contractWith('uint256 public constant SNAKE_CASE = 1;') + try { + linter.processStr(code, { + rules: { 'explicit-types': 'error' }, + }) + } catch (error) { + assert.ok( + error.toString().includes('Error: Config error on [explicit-types]. See explicit-types.md.') + ) + } + }) + + for (const key in VAR_DECLARATIONS) { + it(`should raise error for ${key} on 'implicit' mode`, () => { + const { code, errorsImplicit } = VAR_DECLARATIONS[key] + const report = linter.processStr(contractWith(code), { + rules: { 'explicit-types': ['error', 'implicit'] }, + }) + assertErrorCount(report, errorsImplicit) + if (errorsImplicit > 0) assertErrorMessage(report, `Rule is set with implicit type [var/s:`) + }) + } + + for (const key in VAR_DECLARATIONS) { + it(`should raise error for ${key} on 'explicit' mode`, () => { + const { code, errorsExplicit } = VAR_DECLARATIONS[key] + const report = linter.processStr(contractWith(code), { + rules: { 'explicit-types': ['error', 'explicit'] }, + }) + assertErrorCount(report, errorsExplicit) + if (errorsExplicit > 0) assertErrorMessage(report, `Rule is set with explicit type [var/s:`) + }) + } + + describe('Rule [explicit-types] - should not raise any error', () => { + const { zeroErrorsImplicit, zeroErrorsExplicit } = getZeroErrosObject() + + for (const key in zeroErrorsExplicit) { + it(`should NOT raise error for ${key} on 'implicit' mode`, () => { + const { code } = zeroErrorsExplicit[key] + const report = linter.processStr(contractWith(code), { + rules: { 'explicit-types': ['error', 'explicit'] }, + }) + assertNoErrors(report) + }) + } + + for (const key in zeroErrorsImplicit) { + it(`should NOT raise error for ${key} on 'implicit' mode`, () => { + const { code } = zeroErrorsImplicit[key] + const report = linter.processStr(contractWith(code), { + rules: { 'explicit-types': ['error', 'implicit'] }, + }) + assertNoErrors(report) + }) + } + }) +}) diff --git a/test/rules/naming/const-name-snakecase.js b/test/rules/naming/const-name-snakecase.js index 125c5300..54357517 100644 --- a/test/rules/naming/const-name-snakecase.js +++ b/test/rules/naming/const-name-snakecase.js @@ -74,4 +74,23 @@ describe('Linter - const-name-snakecase', () => { assert.equal(report.errorCount, 0) }) + + describe('constant name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('uint32 private constant $THE_CONSTANT = 10;'), + 'containing a $': contractWith('uint32 private constant THE_$_CONSTANT = 10;'), + 'ending with $': contractWith('uint32 private constant THE_CONSTANT$ = 10;'), + 'only with $': contractWith('uint32 private constant $ = 10;'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise error for ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'const-name-snakecase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/contract-name-camelcase.js b/test/rules/naming/contract-name-camelcase.js index 02f98696..787816c3 100644 --- a/test/rules/naming/contract-name-camelcase.js +++ b/test/rules/naming/contract-name-camelcase.js @@ -35,4 +35,61 @@ describe('Linter - contract-name-camelcase', () => { assert.equal(report.errorCount, 1) assert.ok(report.messages[0].message.includes('CamelCase')) }) + + describe('Struct name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('struct $MyStruct {}'), + 'containing a $': contractWith('struct My$Struct {}'), + 'ending with $': contractWith('struct MyStruct$ {}'), + 'only with $': contractWith('struct $ {}'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise contract name error for Structs ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'contract-name-camelcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) + + describe('Enums name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('enum $MyEnum {}'), + 'containing a $': contractWith('enum My$Enum {}'), + 'ending with $': contractWith('enum MyEnum$ {}'), + 'only with $': contractWith('enum $ {}'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise contract name error for Enums ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'contract-name-camelcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) + + describe('Contract name with $ character', () => { + const WITH_$ = { + 'starting with $': 'contract $MyContract {}', + 'containing a $': 'contract My$Contract {}', + 'ending with $': 'contract MyContract$ {}', + 'only with $': 'contract $ {}', + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise contract name error for Contracts ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'contract-name-camelcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/event-name-camelcase.js b/test/rules/naming/event-name-camelcase.js index 9c0dd2db..27bcd05c 100644 --- a/test/rules/naming/event-name-camelcase.js +++ b/test/rules/naming/event-name-camelcase.js @@ -13,4 +13,23 @@ describe('Linter - event-name-camelcase', () => { assert.equal(report.errorCount, 1) assert.ok(report.messages[0].message.includes('CamelCase')) }) + + describe('Event name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('event $Event1(uint a);'), + 'containing a $': contractWith('event Eve$nt1(uint a);'), + 'ending with $': contractWith('event Event1$(uint a);'), + 'only with $': contractWith('event $(uint a);'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise event name error for Events ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'event-name-camelcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/func-name-mixedcase.js b/test/rules/naming/func-name-mixedcase.js index fe9d62c8..c468b26a 100644 --- a/test/rules/naming/func-name-mixedcase.js +++ b/test/rules/naming/func-name-mixedcase.js @@ -23,4 +23,23 @@ describe('Linter - func-name-mixedcase', () => { assert.equal(report.errorCount, 0) }) + + describe('Function names with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('function $aFunc1Nam23e () public {}'), + 'containing a $': contractWith('function aFunc$1Nam23e () public {}'), + 'ending with $': contractWith('function aFunc1Nam23e$ () public {}'), + 'only with $': contractWith('function $() public {}'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise func name error for Functions ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'func-name-mixedcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/func-param-name-mixedcase.js b/test/rules/naming/func-param-name-mixedcase.js index 5d7169da..72ef9beb 100644 --- a/test/rules/naming/func-param-name-mixedcase.js +++ b/test/rules/naming/func-param-name-mixedcase.js @@ -24,4 +24,41 @@ describe('Linter - func-param-name-mixedcase', () => { assert.equal(report.errorCount, 1) assert.ok(report.messages[0].message.includes('mixedCase')) }) + + describe('Function Paramters name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('function funcName (uint $param) public {}'), + 'containing a $': contractWith('function funcName (uint pa$ram) public {}'), + 'ending with $': contractWith('function funcName (uint param$) public {}'), + 'only with $': contractWith('function funcName(uint $) public {}'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise func param name error for Function Parameters ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'func-param-name-mixedcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) + describe('Event parameters name with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('event Event1(uint $param);'), + 'containing a $': contractWith('event Event1(uint pa$ram);'), + 'ending with $': contractWith('event Event1(uint param$);'), + 'only with $': contractWith('event Event1(uint $);'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise func name error for Event Parameters ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'func-name-mixedcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/immutable-vars-naming.js b/test/rules/naming/immutable-vars-naming.js index 4b9cb7de..b695a7ec 100644 --- a/test/rules/naming/immutable-vars-naming.js +++ b/test/rules/naming/immutable-vars-naming.js @@ -65,53 +65,43 @@ describe('immutable-vars-naming', () => { assert.equal(report.errorCount, 0) }) - describe('immutable-vars-naming ==> warnings (same as above)', () => { - it('should raise warning when immutablesAsConstants = false and variable is in snake case', () => { - const code = contractWith('uint32 private immutable SNAKE_CASE;') - - const report = linter.processStr(code, { - rules: { 'immutable-vars-naming': ['warn', { immutablesAsConstants: false }] }, - }) - - assert.equal(report.warningCount, 1) - assert.ok( - report.messages[0].message.includes('Immutable variables names are set to be in mixedCase') - ) - }) - - it('should NOT raise warning when immutablesAsConstants = false and variable is in snake case', () => { - const code = contractWith('uint32 private immutable SNAKE_CASE;') - - const report = linter.processStr(code, { - rules: { 'immutable-vars-naming': ['warn', { immutablesAsConstants: true }] }, + describe('Immutable variable with $ character as mixedCase', () => { + const WITH_$ = { + 'starting with $': contractWith('uint32 immutable private $D;'), + 'containing a $': contractWith('uint32 immutable private testWith$Contained;'), + 'ending with $': contractWith('uint32 immutable private testWithEnding$;'), + 'only with $': contractWith('uint32 immutable private $;'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise immutable error for variables ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'immutable-vars-naming': ['error', { immutablesAsConstants: false }] }, + }) + + assert.equal(report.errorCount, 0) }) + } + }) - assert.equal(report.warningCount, 0) - }) - - it('should raise warning when immutablesAsConstants = true and variable is in mixedCase', () => { - const code = contractWith('uint32 private immutable mixedCase;') - - const report = linter.processStr(code, { - rules: { 'immutable-vars-naming': ['warn', { immutablesAsConstants: true }] }, - }) - - assert.equal(report.warningCount, 1) - assert.ok( - report.messages[0].message.includes( - 'Immutable variables name are set to be in capitalized SNAKE_CASE' - ) - ) - }) - - it('should NOT raise warning when immutablesAsConstants = false and variable is in mixedCase', () => { - const code = contractWith('uint32 private immutable mixedCase;') - - const report = linter.processStr(code, { - rules: { 'immutable-vars-naming': ['warn', { immutablesAsConstants: false }] }, + describe('Immutable variable with $ character as SNAKE_CASE', () => { + const WITH_$ = { + 'starting with $': contractWith('uint32 immutable private $_D;'), + 'starting with $D': contractWith('uint32 immutable private $D;'), + 'containing a $': contractWith('uint32 immutable private TEST_WITH_$_CONTAINED;'), + 'ending with $': contractWith('uint32 immutable private TEST_WITH_ENDING_$;'), + 'ending with D$': contractWith('uint32 immutable private TEST_WITH_ENDING_D$;'), + 'only with $': contractWith('uint32 immutable private $;'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise immutable error for variables ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'immutable-vars-naming': ['error', { immutablesAsConstants: true }] }, + }) + + assert.equal(report.errorCount, 0) }) - - assert.equal(report.warningCount, 0) - }) + } }) }) diff --git a/test/rules/naming/modifier-name-mixedcase.js b/test/rules/naming/modifier-name-mixedcase.js index a652c156..5dfd1a36 100644 --- a/test/rules/naming/modifier-name-mixedcase.js +++ b/test/rules/naming/modifier-name-mixedcase.js @@ -23,4 +23,23 @@ describe('Linter - modifier-name-mixedcase', () => { assert.equal(report.errorCount, 0) }) + + describe('with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('modifier $ownedBy(address a) { }'), + 'containing a $': contractWith('modifier owned$By(address a) { }'), + 'ending with $': contractWith('modifier ownedBy$(address a) { }'), + 'only with $': contractWith('modifier $(uint a) { }'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise func name error for Modifiers ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'modifier-name-mixedcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) }) diff --git a/test/rules/naming/var-name-mixedcase.js b/test/rules/naming/var-name-mixedcase.js index 635a7924..2e6730e3 100644 --- a/test/rules/naming/var-name-mixedcase.js +++ b/test/rules/naming/var-name-mixedcase.js @@ -66,4 +66,23 @@ describe('Linter - var-name-mixedcase', () => { assert.equal(report.errorCount, 0) }) + + describe('with $ character', () => { + const WITH_$ = { + 'starting with $': contractWith('uint32 private $D = 10;'), + 'containing a $': contractWith('uint32 private testWith$Contained = 10;'), + 'ending with $': contractWith('uint32 private testWithEnding$ = 10;'), + 'only with $': contractWith('uint32 private $;'), + } + + for (const [key, code] of Object.entries(WITH_$)) { + it(`should not raise var name error for variables ${key}`, () => { + const report = linter.processStr(code, { + rules: { 'var-name-mixedcase': 'error' }, + }) + + assert.equal(report.errorCount, 0) + }) + } + }) })