Skip to content

Conversation

mssabr01
Copy link
Contributor

@mssabr01 mssabr01 commented Aug 19, 2025

PR Type

Enhancement


Description

  • Migrated ESLint configuration to flat config with eslint.config.mjs

  • Added custom ESLint plugin for unused pure function calls

  • Updated and expanded ESLint and related dependencies

  • Improved lint script and dependency versions in package.json


Changes walkthrough 📝

Relevant files
Configuration changes
eslint.config.mjs
Add flat ESLint config with custom rules and ignores         

eslint.config.mjs

  • Introduced new flat ESLint config using modern API
  • Integrated TypeScript, security, and custom pure-functions plugin
  • Replicated and enhanced ignore patterns from previous config
  • Applied Prettier and security rules, and custom pure function rule
  • +166/-0 
    .eslintrc.json
    Remove legacy ESLint configuration file                                   

    .eslintrc.json

    • Removed legacy ESLint config in favor of flat config
    +0/-74   
    Enhancement
    index.js
    Add custom ESLint plugin entry for pure functions               

    eslint-plugin-pure-functions/index.js

  • Added entry point for custom ESLint plugin
  • Registered new rule: no-unused-pure-calls
  • Provided recommended config for plugin usage
  • +15/-0   
    no-unused-pure-calls.js
    Add rule to detect unused pure function calls                       

    eslint-plugin-pure-functions/rules/no-unused-pure-calls.js

  • Implemented ESLint rule to detect unused pure function calls
  • Supports auto-fix and suggestions for assignment
  • Configurable list of pure methods, with sensible defaults
  • Provides detailed context-aware detection of unused results
  • +173/-0 
    Dependencies
    package.json
    Update ESLint dependencies and scripts for new config       

    package.json

  • Updated ESLint and related dependencies to latest versions
  • Added new dev dependencies for flat config and plugin support
  • Improved lint script for quieter output
  • Reordered and cleaned up dependency sections
  • +14/-10 

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🏅 Score: 88
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Version Bump of @shardeum-foundation Packages

    The PR updates dependencies and devDependencies, but also includes unrelated changes to @shardeum-foundation packages. These should be in a separate PR or commit to avoid merge conflicts between branch lines with different versions.

      "@hapi/sntp": "3.1.1",
      "@mapbox/node-pre-gyp": "1.0.10",
      "@shardeum-foundation/lib-crypto-utils": "4.3.1",
      "@shardeum-foundation/lib-net": "1.6.1",
      "@shardeum-foundation/lib-types": "1.4.1",
      "@types/better-sqlite3": "7.6.3",
      "body-parser": "1.18.3",
      "bytenode": "1.3.4",
      "cors": "2.8.5",
      "deepmerge": "4.2.2",
      "express": "4.19.2",
      "got": "11.8.6",
      "log4js": "6.9.1",
      "log4js-extend": "0.2.1",
      "nat-api": "timadinorth/nat-api#appsec-dep-fix",
      "neverthrow": "6.0.0",
      "rfdc": "1.4.1",
      "socket.io": "2.5.0",
      "sqlite3": "5.1.6",
      "streamroller": "3.1.3",
      "tar-fs": "2.1.0",
      "trie-prefix-tree": "1.5.1"
    },
    Rule Coverage and False Positives

    The custom ESLint rule for unused pure function calls uses a hardcoded list of method names and context checks. Reviewers should validate that the rule does not produce false positives or negatives, especially for edge cases or custom pure functions not in the list.

    const defaultPureMethods = [
      // Array methods that return new arrays
      'concat',
      'slice',
      'map',
      'filter',
      'reduce',
      'reduceRight',
      'find',
      'findIndex',
      'some',
      'every',
      'includes',
      'indexOf',
      'lastIndexOf',
      'join',
      'toString',
      'toLocaleString',
      'flatMap',
      'flat',
      'with',
      'toReversed',
      'toSorted',
      'toSpliced',
    
      // String methods that return new strings
      'substring',
      'substr',
      'toLowerCase',
      'toUpperCase',
      'trim',
      'trimStart',
      'trimEnd',
      'replace',
      'replaceAll',
      'split',
      'padStart',
      'padEnd',
      'repeat',
      'charAt',
      'charCodeAt',
      'slice',
      'substr',
      'substring',
    
      // Object methods that return new objects/values
      'assign',
      'keys',
      'values',
      'entries',
      'freeze',
      'seal',
      'getOwnPropertyNames',
      'getOwnPropertyDescriptors',
      'sign',
    ]
    
    const options = context.options[0] || {}
    const pureMethods = [...defaultPureMethods, ...(options.pureMethods || [])]
    
    function isPureMethodCall(node) {
      if (node.type !== 'CallExpression') return false
      if (node.callee.type !== 'MemberExpression') return false
      if (node.callee.property.type !== 'Identifier') return false
    
      return pureMethods.includes(node.callee.property.name)
    }
    Lint Ignore Patterns

    The ignore patterns in the ESLint config are extensive and may unintentionally exclude files that should be linted. Reviewers should confirm that all necessary files are still covered by linting.

    {
      ignores: [
        '**/dist/**',
        '**/build/**',
        '**/node_modules/**',
        '**/*.js',
        '**/*.cjs',
        '**/*.mjs',
        // Specific files to ignore from the old config
        '**/shardus-types.ts',
        '**/Wrapper.ts',
        '**/Utils.ts',
        '**/Template.ts',
        '**/Sync.ts',
        '**/SafetyMode.ts',
        '**/Refresh.ts',
        '**/NodeList.ts',
        '**/Lost.ts',
        '**/GlobalAccounts.ts',
        '**/CycleCreator.ts',
        '**/CycleChain.ts',
        '**/CycleAutoScale.ts',
        '**/Context.ts',
        '**/Comms.ts',
        '**/Archivers.ts',
        '**/Apoptosis.ts',
        '**/Active.ts',
        '**/debugMiddleware.ts',
        '**/debug.ts',
        '**/config.test.ts',
        '**/computePowGenerator.ts',
        '**/storage/index.ts',
        '**/statistics/index.ts',
        '**/snapshot/index.ts',
        '**/shardus/index.ts',
        '**/reporter/index.ts',
        '**/rate-limiting/index.ts',
        '**/random/index.ts',
        '**/network/index.ts',
        '**/logger/index.ts',
        '**/load-detection/index.ts',
        '**/src/index.ts',
        '**/http/index.ts',
        '**/exit-handler/index.ts',
        '**/AccountPatcher.ts',
        '**/CachedAppDataManager.ts',
        '**/iterables.ts',
        '**/rotation.ts',
        '**/src/state-manager/PartitionStats.ts',
        '**/src/state-manager/index.ts',
      ],
    },

    ]

    const options = context.options[0] || {}
    const pureMethods = [...defaultPureMethods, ...(options.pureMethods || [])]

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: The pureMethods array may contain duplicate method names, especially since some methods like 'slice', 'substr', and 'substring' appear multiple times in defaultPureMethods. This can cause unnecessary repeated checks and potential confusion. Remove duplicates from the pureMethods array to ensure each method is only checked once. [general, importance: 6]

    Suggested change
    const pureMethods = [...defaultPureMethods, ...(options.pureMethods || [])]
    const pureMethods = Array.from(new Set([...defaultPureMethods, ...(options.pureMethods || [])]))

    Comment on lines +77 to +86
    'assign',
    'keys',
    'values',
    'entries',
    'freeze',
    'seal',
    'getOwnPropertyNames',
    'getOwnPropertyDescriptors',
    'sign',
    ]

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Suggestion: The defaultPureMethods array includes 'assign', which is not a pure function (Object.assign mutates the first argument). Remove 'assign' to avoid false positives where mutating methods are flagged as pure. [possible issue, importance: 9]

    Suggested change
    'assign',
    'keys',
    'values',
    'entries',
    'freeze',
    'seal',
    'getOwnPropertyNames',
    'getOwnPropertyDescriptors',
    'sign',
    ]
    'keys',
    'values',
    'entries',
    'freeze',
    'seal',
    'getOwnPropertyNames',
    'getOwnPropertyDescriptors',
    'sign',
    ]

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    1 participant