Merge pull request #719 from protofire/feat-foundry-no-block-rule

Feat foundry no block rule

Author: dbale-altoros

conf/rulesets/solhint-all.js

@@ -67,6 +67,7 @@ module.exports = Object.freeze({
     'gas-struct-packing': 'warn',
     'comprehensive-interface': 'warn',
     'duplicated-imports': 'warn',
+    'foundry-no-block-time-number': ['warn', ['test', 'tests']],
     'import-path-check': ['warn', ['[~dependenciesPath]']],
     quotes: ['error', 'double'],
     'const-name-snakecase': 'warn',

docs/rules.md

@@ -69,11 +69,12 @@ title:       "Rule Index of Solhint"
 
 ## Miscellaneous
 
-| Rule Id                                                                     | Error                                                                                                                                    | Recommended  | Deprecated |
-| --------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
-| [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md) | Check that all public or external functions are overridden. This is useful to make sure that the whole API is extracted in an interface. |              |            |
-| [import-path-check](./rules/miscellaneous/import-path-check.md)             | Check if an import file exits in target path                                                                                             | $~~~~~~~~$✔️ |            |
-| [quotes](./rules/miscellaneous/quotes.md)                                   | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.                      | $~~~~~~~~$✔️ |            |
+| Rule Id                                                                               | Error                                                                                                                                    | Recommended  | Deprecated |
+| ------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------- | ------------ | ---------- |
+| [comprehensive-interface](./rules/miscellaneous/comprehensive-interface.md)           | Check that all public or external functions are overridden. This is useful to make sure that the whole API is extracted in an interface. |              |            |
+| [foundry-no-block-time-number](./rules/miscellaneous/foundry-no-block-time-number.md) | Warn on the use of block.timestamp / block.number inside Foundry test files; recommend vm.getBlockTimestamp() / vm.getBlockNumber().     |              |            |
+| [import-path-check](./rules/miscellaneous/import-path-check.md)                       | Check if an import file exits in target path                                                                                             | $~~~~~~~~$✔️ |            |
+| [quotes](./rules/miscellaneous/quotes.md)                                             | Enforces the use of double or simple quotes as configured for string literals. Values must be 'single' or 'double'.                      | $~~~~~~~~$✔️ |            |
         
 
 ## Security Rules

docs/rules/miscellaneous/foundry-no-block-time-number.md

@@ -0,0 +1,50 @@
+---
+warning:     "This is a dynamically generated file. Do not edit manually."
+layout:      "default"
+title:       "foundry-no-block-time-number | Solhint"
+---
+
+# foundry-no-block-time-number
+![Category Badge](https://img.shields.io/badge/-Miscellaneous-informational)
+![Default Severity Badge warn](https://img.shields.io/badge/Default%20Severity-warn-yellow)
+
+## Description
+Warn on the use of block.timestamp / block.number inside Foundry test files; recommend vm.getBlockTimestamp() / vm.getBlockNumber().
+
+## Options
+This rule accepts an array of options:
+
+| Index | Description                                                                                    | Default Value    |
+| ----- | ---------------------------------------------------------------------------------------------- | ---------------- |
+| 0     | Rule severity. Must be one of "error", "warn", "off".                                          | warn             |
+| 1     | Array of folder names for solhint to execute (defaults to ["test","tests"], case-insensitive). | ["test","tests"] |
+
+
+### Example Config
+```json
+{
+  "rules": {
+    "foundry-no-block-time-number": [
+      "warn",
+      [
+        "test",
+        "tests"
+      ]
+    ]
+  }
+}
+```
+
+### Notes
+- This rule only runs for files located under the configured test directories (e.g., test/** or tests/**).
+
+## Examples
+This rule does not have examples.
+
+## Version
+This rule was introduced in the latest version.
+
+## Resources
+- [Rule source](https://github.com/protofire/solhint/blob/master/lib/rules/miscellaneous/foundry-no-block-time-number.js)
+- [Document source](https://github.com/protofire/solhint/blob/master/docs/rules/miscellaneous/foundry-no-block-time-number.md)
+- [Test cases](https://github.com/protofire/solhint/blob/master/test/rules/miscellaneous/foundry-no-block-time-number.js)

lib/rules/miscellaneous/foundry-no-block-time-number.js

@@ -0,0 +1,102 @@
+const path = require('path')
+const BaseChecker = require('../base-checker')
+const { severityDescription } = require('../../doc/utils')
+
+const ruleId = 'foundry-no-block-time-number'
+
+const DEFAULT_SEVERITY = 'warn'
+const DEFAULT_TEST_DIRS = ['test', 'tests'] // default folders considered as Foundry test roots
+
+const meta = {
+  type: 'miscellaneous',
+
+  docs: {
+    description:
+      'Warn on the use of block.timestamp / block.number inside Foundry test files; recommend vm.getBlockTimestamp() / vm.getBlockNumber().',
+    category: 'Miscellaneous',
+    options: [
+      {
+        description: severityDescription,
+        default: DEFAULT_SEVERITY,
+      },
+      {
+        description:
+          'Array of folder names for solhint to execute (defaults to ["test","tests"], case-insensitive).',
+        default: JSON.stringify(DEFAULT_TEST_DIRS),
+      },
+    ],
+    notes: [
+      {
+        note: 'This rule only runs for files located under the configured test directories (e.g., test/** or tests/**).',
+      },
+    ],
+  },
+
+  fixable: false,
+  recommended: false,
+  // defaultSetup: [severity, testDirs[]]
+  defaultSetup: [DEFAULT_SEVERITY, DEFAULT_TEST_DIRS],
+
+  schema: {
+    type: 'array',
+    description: 'Array of folder names for solhint to execute the rule (case-insensitive).',
+    items: { type: 'string', errorMessage: 'Each item must be a string' },
+  },
+}
+
+class FoundryNoBlockTimeNumberChecker extends BaseChecker {
+  constructor(reporter, config, fileName) {
+    super(reporter, ruleId, meta)
+
+    // Read array of folders from config. If invalid/empty, fallback to defaults.
+    const arr = config ? config.getArray(ruleId) : []
+    this.testDirs = Array.isArray(arr) && arr.length > 0 ? arr.slice() : DEFAULT_TEST_DIRS.slice()
+
+    this.fileName = fileName
+    this.enabledForThisFile = this.isInAnyTestDir(fileName, this.testDirs)
+  }
+
+  // Only evaluate the AST if the current file is inside a configured test directory.
+  MemberAccess(node) {
+    if (!this.enabledForThisFile) return
+
+    // Detect `block.timestamp` / `block.number`
+    if (node && node.type === 'MemberAccess') {
+      const expr = node.expression
+      const member = node.memberName
+
+      if (expr && expr.type === 'Identifier' && expr.name === 'block') {
+        if (member === 'timestamp') {
+          this.error(
+            node,
+            'Avoid `block.timestamp` in Foundry tests. Use `vm.getBlockTimestamp()` instead.'
+          )
+        } else if (member === 'number') {
+          this.error(
+            node,
+            'Avoid `block.number` in Foundry tests. Use `vm.getBlockNumber()` instead.'
+          )
+        }
+      }
+    }
+  }
+
+  // ---------- helpers ----------
+  isInAnyTestDir(fileName, testDirs) {
+    try {
+      // Make the path relative to the project root to compare path segments
+      const rel = path.relative(process.cwd(), fileName)
+      const norm = path.normalize(rel) // normalize separators for Win/Linux
+      const segments = norm.split(path.sep).map((s) => s.toLowerCase())
+
+      // Match by exact directory segment (case-insensitive)
+      const wanted = new Set(testDirs.map((d) => String(d).toLowerCase()))
+      return segments.some((seg) => wanted.has(seg))
+    } catch (_) {
+      // Fail-safe: if anything goes wrong, do not enable the rule for this file.
+      return false
+    }
+  }
+}
+
+module.exports = FoundryNoBlockTimeNumberChecker

lib/rules/miscellaneous/index.js

@@ -2,12 +2,14 @@ const QuotesChecker = require('./quotes')
 const ComprehensiveInterfaceChecker = require('./comprehensive-interface')
 const DuplicatedImportsChecker = require('./duplicated-imports')
 const ImportPathChecker = require('./import-path-check')
+const FoundryNoBlockTimeNumberChecker = require('./foundry-no-block-time-number')
 
 module.exports = function checkers(reporter, config, tokens, fileName) {
   return [
     new QuotesChecker(reporter, config, tokens),
     new ComprehensiveInterfaceChecker(reporter, config, tokens),
     new DuplicatedImportsChecker(reporter),
     new ImportPathChecker(reporter, config, fileName),
+    new FoundryNoBlockTimeNumberChecker(reporter, config, fileName),
   ]
 }

lib/rules/naming/foundry-test-function-naming.js

@@ -71,7 +71,7 @@ const meta = {
   },
 }
 
-class FoundryTestFunctionNaming extends BaseChecker {
+class FoundryTestFunctionNamingChecker extends BaseChecker {
   constructor(reporter, config) {
     super(reporter, ruleId, meta)
     this.skippedFunctions = config
@@ -97,4 +97,4 @@ class FoundryTestFunctionNaming extends BaseChecker {
   }
 }
 
-module.exports = FoundryTestFunctionNaming
+module.exports = FoundryTestFunctionNamingChecker

lib/rules/naming/index.js

@@ -14,7 +14,7 @@ const FunctionNamedParametersChecker = require('./func-named-parameters')
 // 👇 old (alias with deprecation)
 const FoundryTestFunctionsChecker = require('./foundry-test-functions')
 // 👇 new name
-const FoundryTestFunctionNaming = require('./foundry-test-function-naming')
+const FoundryTestFunctionNamingChecker = require('./foundry-test-function-naming')
 
 module.exports = function checkers(reporter, config) {
   return [
@@ -33,6 +33,6 @@ module.exports = function checkers(reporter, config) {
 
     // 👇 call both
     new FoundryTestFunctionsChecker(reporter, config),
-    new FoundryTestFunctionNaming(reporter, config),
+    new FoundryTestFunctionNamingChecker(reporter, config),
   ]
 }

test/common/config-validator.js

@@ -580,4 +580,141 @@ describe('Better errors addition + rule disable on error', () => {
     assert.ok(logged.includes("invalid configuration for rule 'use-natspec'"))
     assert.ok(warnSpy.called)
   })
+
+  //
+  // ---- foundry-no-block-time-number: CONFIG VALIDATOR TESTS ----
+  //
+
+  it('Valid CFG - accept: foundry-no-block-time-number with only severity (uses DEFAULT test dirs)', () => {
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': 'warn' },
+    })
+
+    // Not asserting execution here (rule is directory-gated and dummy file path may not match),
+    // we only assert the config is accepted and no reporter was used.
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    // No config warning should be printed
+    sinon.assert.notCalled(warnSpy)
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
+
+  it('Valid CFG - accept: foundry-no-block-time-number with custom test dirs array', () => {
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': ['warn', ['tests', 'e2e', 'it']] },
+    })
+
+    // Only checking config acceptance (no execution guarantees here).
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    // No schema warning expected
+    sinon.assert.notCalled(warnSpy)
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
+
+  it('Invalid CFG - not execute: foundry-no-block-time-number when wrong value type in array is provided', () => {
+    // Second arg must be an array of strings; here it's [1] (invalid element type)
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': ['error', [1]] },
+    })
+
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    const logged = warnSpy
+      .getCalls()
+      .map((c) => c.args[0])
+      .join('\n')
+
+    assert.ok(
+      logged.includes("invalid configuration for rule 'foundry-no-block-time-number'"),
+      `Expected a warning for foundry-no-block-time-number but got:\n${logged}`
+    )
+
+    assert.ok(warnSpy.called, 'console.warn should have been called')
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
+
+  it('Invalid CFG - not execute: foundry-no-block-time-number when wrong option type is provided (string)', () => {
+    // Second arg must be an array; here it's a string (invalid)
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': ['error', 'wrong'] },
+    })
+
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    const logged = warnSpy
+      .getCalls()
+      .map((c) => c.args[0])
+      .join('\n')
+
+    assert.ok(
+      logged.includes("invalid configuration for rule 'foundry-no-block-time-number'"),
+      `Expected a warning for foundry-no-block-time-number but got:\n${logged}`
+    )
+
+    assert.ok(warnSpy.called, 'console.warn should have been called')
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
+
+  it('Invalid CFG - not execute: foundry-no-block-time-number when empty object is provided', () => {
+    // Second arg must be an array; here it's an object (invalid)
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': ['error', {}] },
+    })
+
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    const logged = warnSpy
+      .getCalls()
+      .map((c) => c.args[0])
+      .join('\n')
+
+    assert.ok(
+      logged.includes("invalid configuration for rule 'foundry-no-block-time-number'"),
+      `Expected a warning for foundry-no-block-time-number but got:\n${logged}`
+    )
+
+    assert.ok(warnSpy.called, 'console.warn should have been called')
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
+
+  it('Invalid CFG - not execute: foundry-no-block-time-number when array items are not strings (mixed types)', () => {
+    // Mixed invalid item types
+    const report = linter.processStr(dummyCode, {
+      rules: { 'foundry-no-block-time-number': ['warn', ['tests', 123, null]] },
+    })
+
+    assert.equal(report.errorCount, 0)
+    assert.equal(report.warningCount, 0)
+    assert.deepEqual(report.messages, [])
+
+    const logged = warnSpy
+      .getCalls()
+      .map((c) => c.args[0])
+      .join('\n')
+
+    assert.ok(
+      logged.includes("invalid configuration for rule 'foundry-no-block-time-number'"),
+      `Expected a warning for foundry-no-block-time-number but got:\n${logged}`
+    )
+
+    assert.ok(warnSpy.called, 'console.warn should have been called')
+    sinon.assert.notCalled(reportErrorSpy)
+    sinon.assert.notCalled(reportWarnSpy)
+  })
 })