Merge pull request #488 from protofire/develop

Merge Develop into Master v3.6.2 (a)
protofire · Aug 17, 2023 · 229db23 · 229db23
2 parents 48258ca + 400248d
commit 229db23
Show file tree

Hide file tree

Showing 20 changed files with 324 additions and 37 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,15 @@
+## [3.6.2] - 2023-08-17
+### Added
+- New Rule: `one-contract-per-file` - Enforces the use of ONE contract per file [#487](https://github.com/protofire/solhint/pull/487)
+
+
+### Fixed
+- `foundry-test-functions` - Modified regex to include invariant and statefulFuzz tests [#484](https://github.com/protofire/solhint/pull/484)
+- `quotes` - To allow quotes inside double quotes and viceversa [#485](https://github.com/protofire/solhint/pull/485)
+- `JSON` - Formatter returning JS object instead of standard json [#490](https://github.com/protofire/solhint/pull/490) 
+
+
+
 ## [3.6.1] - 2023-08-11
 
 ### BREAKING CHANGE

diff --git a/conf/rulesets/solhint-all.js b/conf/rulesets/solhint-all.js
@@ -16,6 +16,7 @@ module.exports = Object.freeze({
     'no-global-import': 'warn',
     'no-unused-import': 'warn',
     'no-unused-vars': 'warn',
+    'one-contract-per-file': 'warn',
     'payable-fallback': 'warn',
     'reason-string': [
       'warn',

diff --git a/conf/rulesets/solhint-recommended.js b/conf/rulesets/solhint-recommended.js
@@ -13,6 +13,7 @@ module.exports = Object.freeze({
     'no-global-import': 'warn',
     'no-unused-import': 'warn',
     'no-unused-vars': 'warn',
+    'one-contract-per-file': 'warn',
     'payable-fallback': 'warn',
     'reason-string': [
       'warn',

diff --git a/docs/rules.md b/docs/rules.md
@@ -6,30 +6,31 @@ title:       "Rule Index of Solhint"
 
 ## Best Practise Rules
 
-| Rule Id                                                            | Error                                                                                                                 | Recommended  | Deprecated |
-| ------------------------------------------------------------------ | --------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
-| [code-complexity](./rules/best-practises/code-complexity.md)       | Function has cyclomatic complexity "current" but allowed no more than maxcompl.                                       |              |            |
-| [custom-errors](./rules/best-practises/custom-errors.md)           | Enforces the use of Custom Errors over Require and Revert statements                                                  | $~~~~~~~~$✔️ |            |
-| [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 object name is not being used by the contract.                                                               | $~~~~~~~~$✔️ |            |
-| [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.                                                            |              |            |
+| [custom-errors](./rules/best-practises/custom-errors.md)                 | Enforces the use of Custom Errors over Require and Revert statements                                                                       | $~~~~~~~~$✔️ |            |
+| [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 object name is not being used by the contract.                                                                                    | $~~~~~~~~$✔️ |            |
+| [no-unused-vars](./rules/best-practises/no-unused-vars.md)               | Variable "name" is unused.                                                                                                                 | $~~~~~~~~$✔️ |            |
+| [one-contract-per-file](./rules/best-practises/one-contract-per-file.md) | Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names) | $~~~~~~~~$✔️ |            |
+| [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
 
 | Rule Id                                                                     | Error                                                                                                                                  | Recommended  | Deprecated |
 | --------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
 | [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md) | Check that all public or external functions are override. This is iseful to make sure that the whole API is extracted in an interface. |              |            |
-| [quotes](./rules/miscellaneous/quotes.md)                                   | Use double quotes for string literals. Values must be 'single' or 'double'.                                                            | $~~~~~~~~$✔️ |            |
+| [quotes](./rules/miscellaneous/quotes.md)                                   | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.                    | $~~~~~~~~$✔️ |            |
         
 
 ## Style Guide Rules

diff --git a/docs/rules/best-practises/custom-errors.md b/docs/rules/best-practises/custom-errors.md
@@ -63,7 +63,7 @@ revert("Insufficient Balance");
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.6.1](https://github.com/protofire/solhint/tree/v3.6.1)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/custom-errors.js)

diff --git a/docs/rules/best-practises/one-contract-per-file.md b/docs/rules/best-practises/one-contract-per-file.md
@@ -0,0 +1,39 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "one-contract-per-file | Solhint"
+---
+
+# one-contract-per-file
+![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
+Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names)
+
+## Options
+This rule accepts a string option of rule severity. Must be one of "error", "warn", "off". Default to warn.
+
+### Example Config
+```json
+{
+  "rules": {
+    "one-contract-per-file": "warn"
+  }
+}
+```
+
+
+## Examples
+This rule does not have examples.
+
+## Version
+This rule is introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/best-practises/one-contract-per-file.js)
+- [Document source](https://github.com/protofire/solhint/tree/master/docs/rules/best-practises/one-contract-per-file.md)
+- [Test cases](https://github.com/protofire/solhint/tree/master/test/rules/best-practises/one-contract-per-file.js)
diff --git a/docs/rules/miscellaneous/quotes.md b/docs/rules/miscellaneous/quotes.md
@@ -12,7 +12,7 @@ title:       "quotes | Solhint"
 
 
 ## Description
-Use double quotes for string literals. Values must be 'single' or 'double'.
+Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.
 
 ## Options
 This rule accepts an array of options:
@@ -32,11 +32,13 @@ This rule accepts an array of options:
 }
 ```
 
+### Notes
+- This rule allows to put a double quote inside single quote string and viceversa
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule
 
-#### String with double quotes
+#### Configured with double quotes
 
 ```solidity
 
@@ -49,9 +51,47 @@ This rule accepts an array of options:
     
 ```
 
+#### Configured with single quotes
+
+```solidity
+
+      pragma solidity 0.4.4;
+        
+        
+      contract A {
+        string private a = 'test';
+      }
+    
+```
+
+#### Configured with double quotes
+
+```solidity
+string private constant STR = "You shall 'pass' !";
+```
+
+#### Configured with single quotes
+
+```solidity
+string private constant STR = 'You shall "pass" !';
+```
+
 ### 👎 Examples of **incorrect** code for this rule
 
-#### String with single quotes
+#### Configured with single quotes
+
+```solidity
+
+      pragma solidity 0.4.4;
+        
+        
+      contract A {
+        string private a = "test";
+      }
+    
+```
+
+#### Configured with double quotes
 
 ```solidity
 

diff --git a/docs/rules/naming/foundry-test-functions.md b/docs/rules/naming/foundry-test-functions.md
@@ -33,7 +33,7 @@ This rule accepts an array of options:
 - Supported Regex: ```test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}```
 - This rule should be executed in a separate folder with a separate .solhint.json => ```solhint --config .solhint.json testFolder/**/*.sol```
 - This rule applies only to `external` and `public` functions
-- This rule skips the `setUp()` function
+- This rule skips the `setUp()` function by default
 
 ## Examples
 ### 👍 Examples of **correct** code for this rule
@@ -65,7 +65,7 @@ function numberIs42() public {}
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.6.1](https://github.com/protofire/solhint/tree/v3.6.1)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/foundry-test-functions.js)

diff --git a/docs/rules/naming/named-return-values.md b/docs/rules/naming/named-return-values.md
@@ -42,7 +42,7 @@ function checkBalance(address wallet) external view returns(uint256) {}
 ```
 
 ## Version
-This rule is introduced in the latest version.
+This rule was introduced in [Solhint 3.6.1](https://github.com/protofire/solhint/tree/v3.6.1)
 
 ## Resources
 - [Rule source](https://github.com/protofire/solhint/tree/master/lib/rules/naming/named-return-values.js)

diff --git a/lib/common/identifier-naming.js b/lib/common/identifier-naming.js
@@ -31,8 +31,18 @@ module.exports = {
     return text && text[0] === '_'
   },
 
-  isNotFoundryTestCase(text) {
-    const regex = /^test(Fork)?(Fuzz)?(Fail)?_(Revert(If_|When_){1})?\w{1,}$/
-    return !match(text, regex)
+  isFoundryTestCase(text) {
+    // this one checks CamelCase after test keyword
+    // const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?[A-Z](Revert(If_|When_){1})?\w{1,}$/
+
+    const regexTest = /^test(Fork)?(Fuzz)?(Fail)?(_)?(Revert(If_|When_){1})?\w{1,}$/
+    const matchRegexTest = match(text, regexTest)
+
+    // this one checks CamelCase after test keyword
+    // const regexInvariant = /^(invariant|statefulFuzz)(_)?[A-Z]\w{1,}$/
+    const regexInvariant = /^(invariant|statefulFuzz)(_)?\w{1,}$/
+    const matchRegexInvariant = match(text, regexInvariant)
+
+    return matchRegexTest || matchRegexInvariant
   },
 }
diff --git a/lib/formatters/json.js b/lib/formatters/json.js
@@ -58,5 +58,5 @@ module.exports = function (results) {
 
   if (finalMessage) allMessages.push(finalMessage)
 
-  return allMessages.length > 0 ? JSON.parse(JSON.stringify(allMessages)) : ''
+  return allMessages.length > 0 ? JSON.stringify(allMessages) : ''
 }
diff --git a/lib/rules/best-practises/index.js b/lib/rules/best-practises/index.js
@@ -11,6 +11,7 @@ const NoGlobalImportsChecker = require('./no-global-import')
 const NoUnusedImportsChecker = require('./no-unused-import')
 const ExplicitTypesChecker = require('./explicit-types')
 const CustomErrorsChecker = require('./custom-errors')
+const OneContractPerFileChecker = require('./one-contract-per-file')
 
 module.exports = function checkers(reporter, config, inputSrc, tokens) {
   return [
@@ -27,5 +28,6 @@ module.exports = function checkers(reporter, config, inputSrc, tokens) {
     new NoUnusedImportsChecker(reporter, tokens),
     new ExplicitTypesChecker(reporter, config),
     new CustomErrorsChecker(reporter),
+    new OneContractPerFileChecker(reporter),
   ]
 }
diff --git a/lib/rules/best-practises/one-contract-per-file.js b/lib/rules/best-practises/one-contract-per-file.js
@@ -0,0 +1,51 @@
+const BaseChecker = require('../base-checker')
+const { severityDescription } = require('../../doc/utils')
+
+const DEFAULT_SEVERITY = 'warn'
+
+const ruleId = 'one-contract-per-file'
+const meta = {
+  type: 'best-practises',
+
+  docs: {
+    description:
+      'Enforces the use of ONE Contract per file see [here](https://docs.soliditylang.org/en/v0.8.21/style-guide.html#contract-and-library-names)',
+    category: 'Best Practise Rules',
+    options: [
+      {
+        description: severityDescription,
+        default: DEFAULT_SEVERITY,
+      },
+    ],
+  },
+
+  isDefault: false,
+  recommended: true,
+  defaultSetup: DEFAULT_SEVERITY,
+
+  schema: null,
+}
+
+class OneContractPerFileChecker extends BaseChecker {
+  constructor(reporter) {
+    super(reporter, ruleId, meta)
+  }
+
+  SourceUnit(node) {
+    const contractDefinitionCount = node.children.reduce((count, child) => {
+      if (child.type === 'ContractDefinition') {
+        return count + 1
+      }
+      return count
+    }, 0)
+
+    if (contractDefinitionCount > 1) {
+      this.error(
+        node,
+        `Found more than One contract per file. ${contractDefinitionCount} contracts found!`
+      )
+    }
+  }
+}
+
+module.exports = OneContractPerFileChecker
diff --git a/lib/rules/miscellaneous/quotes.js b/lib/rules/miscellaneous/quotes.js
@@ -9,7 +9,7 @@ const meta = {
   type: 'miscellaneous',
 
   docs: {
-    description: `Use double quotes for string literals. Values must be 'single' or 'double'.`,
+    description: `Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.`,
     category: 'Miscellaneous',
     options: [
       {
@@ -24,17 +24,38 @@ const meta = {
     examples: {
       good: [
         {
-          description: 'String with double quotes',
+          description: 'Configured with double quotes',
           code: require('../../../test/fixtures/miscellaneous/string-with-double-quotes'),
         },
+        {
+          description: 'Configured with single quotes',
+          code: require('../../../test/fixtures/miscellaneous/string-with-single-quotes'),
+        },
+        {
+          description: 'Configured with double quotes',
+          code: 'string private constant STR = "You shall \'pass\' !";',
+        },
+        {
+          description: 'Configured with single quotes',
+          code: 'string private constant STR = \'You shall "pass" !\';',
+        },
       ],
       bad: [
         {
-          description: 'String with single quotes',
+          description: 'Configured with single quotes',
+          code: require('../../../test/fixtures/miscellaneous/string-with-double-quotes'),
+        },
+        {
+          description: 'Configured with double quotes',
           code: require('../../../test/fixtures/miscellaneous/string-with-single-quotes'),
         },
       ],
     },
+    notes: [
+      {
+        note: 'This rule allows to put a double quote inside single quote string and viceversa',
+      },
+    ],
   },
 
   isDefault: false,
@@ -64,7 +85,6 @@ class QuotesChecker extends BaseChecker {
         token.loc.start.line === node.loc.start.line &&
         token.loc.start.column === node.loc.start.column
     )
-
     if (token && !this.alreadyVisited(token)) {
       this.addVisitedNode(token)
       this.validateQuotes(token)