Skip to content

Disable failing ESLint rules in setup wizard #1260

@matejchalk

Description

@matejchalk

User story

The main advantage of integrating @code-pushup/eslint-plugin is that it allows setting a strict aspirational eslint.config.js, even though the current codebase isn't prepared to meet those standards. This can be set up with 2 ESLint configs - 1 strict config for Code PushUp (and IDEs), 1 derived config that disables failing rules for an existing CI job. This facilitates incremental improvements over a longer period of time, with progress being tracked and no blocking CI.

Since this config splitting is both deterministic and cumbersome to do manually, it would be very useful if the setup wizard could automate this process when setting up the ESLint plugin.

Real-world example

eslint.config.js
import typescript from '@code-pushup/eslint-config/typescript.js';
import nx from '@nx/eslint-plugin';
import svelte from 'eslint-plugin-svelte';
import { defineConfig } from 'eslint/config';
import tseslint from 'typescript-eslint';

export default defineConfig(
  ...typescript,
  ...svelte.configs['flat/recommended'],
  {
    files: ['**/*.{ts,svelte}'],
    languageOptions: {
      parserOptions: {
        projectService: {
          allowDefaultProject: [
            'code-pushup.config.ts',
            'playwright.config.ts',
          ],
        },
        extraFileExtensions: ['.svelte'],
        parser: tseslint.parser,
      },
    },
  },
  {
    settings: {
      'import/resolver': {
        typescript: {
          alwaysTryTypes: true,
          project: 'tsconfig.json',
        },
      },
    },
  },
  {
    files: ['*.ts', '*.svelte'],
    plugins: {
      '@nx': nx,
    },
    rules: {
      '@nx/enforce-module-boundaries': 'error',
    },
  },
  {
    ignores: ['**/.svelte-kit', '**/build', '**/static', '**/.code-pushup'],
  },
  {
    files: ['**/*.svelte'],
    rules: {
      'unicorn/filename-case': [
        'warn',
        { case: 'pascalCase', ignore: [/^\+/] },
      ],
      'svelte/valid-compile': 'warn',
      'unicorn/prefer-top-level-await': 'off',
      'import/no-self-import': 'off',
    },
  },
  {
    files: ['**/+page.server.ts', '**/+server.ts', '**/src/lib/server/**/*.ts'],
    rules: {
      '@typescript-eslint/only-throw-error': 'off',
    },
  },
  {
    files: ['**/*.svelte', '**/*.svelte.ts'],
    rules: {
      'prefer-const': 'off',
    },
  },
);
eslint.config.ci.js
import { defineConfig } from 'eslint/config';
import strictConfig from './eslint.config.js';

export default defineConfig(...strictConfig, {
  // temporarily disable failing rules so lint in CI passes
  // number of errors/warnings per rule recorded at Fri Jul 19 2024 12:05:59 GMT+0200 (Central European Summer Time)
  rules: {
    'arrow-body-style': 'off', // 9 warnings
    eqeqeq: 'off', // 57 errors
    'max-lines': 'off', // 8 warnings
    'max-lines-per-function': 'off', // 18 warnings
    'max-nested-callbacks': 'off', // 1 warning
    'no-async-promise-executor': 'off', // 3 errors
    'no-console': 'off', // 156 warnings
    'no-constant-condition': 'off', // 6 errors
    'no-dupe-keys': 'off', // 1 error
    'no-duplicate-imports': 'off', // 1 warning
    'no-global-assign': 'off', // 4 errors
    'no-param-reassign': 'off', // 93 errors
    'no-prototype-builtins': 'off', // 2 errors
    'no-sequences': 'off', // 1 error
    'no-undef': 'off', // 4 errors
    'no-undef-init': 'off', // 5 warnings
    'prefer-const': 'off', // 31 errors
    'prefer-template': 'off', // 36 warnings
    radix: 'off', // 40 warnings
    '@typescript-eslint/class-methods-use-this': 'off', // 4 warnings
    '@typescript-eslint/consistent-type-definitions': 'off', // 4 warnings
    '@typescript-eslint/max-params': 'off', // 11 warnings
    '@typescript-eslint/naming-convention': 'off', // 7 warnings
    '@typescript-eslint/no-confusing-void-expression': 'off', // 3 warnings
    '@typescript-eslint/no-explicit-any': 'off', // 6 errors
    '@typescript-eslint/no-floating-promises': 'off', // 6 errors
    '@typescript-eslint/no-shadow': 'off', // 104 warnings
    '@typescript-eslint/no-this-alias': 'off', // 1 error
    '@typescript-eslint/no-unnecessary-condition': 'off', // 16 warnings
    '@typescript-eslint/no-unsafe-argument': 'off', // 61 errors
    '@typescript-eslint/no-unsafe-assignment': 'off', // 282 errors
    '@typescript-eslint/no-unsafe-call': 'off', // 159 errors
    '@typescript-eslint/no-unsafe-member-access': 'off', // 507 errors
    '@typescript-eslint/no-unsafe-return': 'off', // 42 errors
    '@typescript-eslint/no-unused-expressions': 'off', // 17 warnings
    '@typescript-eslint/no-unused-vars': 'off', // 101 errors
    '@typescript-eslint/only-throw-error': 'off', // 5 errors
    '@typescript-eslint/require-await': 'off', // 3 errors
    '@typescript-eslint/restrict-plus-operands': 'off', // 31 errors
    'deprecation/deprecation': 'off', // 1 error
    'functional/immutable-data': 'off', // 140 errors
    'functional/no-let': 'off', // 78 warnings
    'functional/no-loop-statements': 'off', // 57 warnings
    'functional/prefer-tacit': 'off', // 2 warnings
    'import/default': 'off', // 1 error
    'import/extensions': 'off', // 9 warnings
    'import/namespace': 'off', // 11 errors
    'import/no-duplicates': 'off', // 4 warnings
    'import/no-named-as-default': 'off', // 1 warning
    'import/no-unassigned-import': 'off', // 1 warning
    'import/no-unresolved': 'off', // 2 errors
    'promise/catch-or-return': 'off', // 4 errors
    'sonarjs/cognitive-complexity': 'off', // 2 errors
    'sonarjs/no-collapsible-if': 'off', // 7 errors
    'sonarjs/no-duplicate-string': 'off', // 24 warnings
    'sonarjs/no-extra-arguments': 'off', // 3 errors
    'sonarjs/no-gratuitous-expressions': 'off', // 1 error
    'sonarjs/no-identical-expressions': 'off', // 2 errors
    'sonarjs/no-redundant-boolean': 'off', // 2 errors
    'sonarjs/no-use-of-empty-return-value': 'off', // 1 error
    'sonarjs/prefer-immediate-return': 'off', // 5 warnings
    'svelte/no-unused-svelte-ignore': 'off', // 1 error
    'svelte/valid-compile': 'off', // 11 errors
    'turbo/no-undeclared-env-vars': 'off', // 2 errors
    'unicorn/catch-error-name': 'off', // 13 warnings
    'unicorn/explicit-length-check': 'off', // 1 warning
    'unicorn/filename-case': 'off', // 2 warnings
    'unicorn/new-for-builtins': 'off', // 2 warnings
    'unicorn/no-array-push-push': 'off', // 2 warnings
    'unicorn/no-for-loop': 'off', // 4 warnings
    'unicorn/no-lonely-if': 'off', // 7 warnings
    'unicorn/no-negated-condition': 'off', // 4 warnings
    'unicorn/no-new-array': 'off', // 4 warnings
    'unicorn/no-this-assignment': 'off', // 1 warning
    'unicorn/no-useless-undefined': 'off', // 7 warnings
    'unicorn/no-zero-fractions': 'off', // 72 warnings
    'unicorn/numeric-separators-style': 'off', // 58 warnings
    'unicorn/prefer-add-event-listener': 'off', // 28 warnings
    'unicorn/prefer-array-some': 'off', // 2 warnings
    'unicorn/prefer-at': 'off', // 1 warning
    'unicorn/prefer-blob-reading-methods': 'off', // 3 warnings
    'unicorn/prefer-code-point': 'off', // 4 warnings
    'unicorn/prefer-date-now': 'off', // 4 warnings
    'unicorn/prefer-dom-node-append': 'off', // 1 warning
    'unicorn/prefer-modern-math-apis': 'off', // 4 warnings
    'unicorn/prefer-native-coercion-functions': 'off', // 1 warning
    'unicorn/prefer-node-protocol': 'off', // 4 warnings
    'unicorn/prefer-number-properties': 'off', // 104 warnings
    'unicorn/prefer-optional-catch-binding': 'off', // 7 warnings
    'unicorn/prefer-regexp-test': 'off', // 5 warnings
    'unicorn/prefer-set-has': 'off', // 1 warning
    'unicorn/prefer-spread': 'off', // 3 warnings
    'unicorn/prefer-string-slice': 'off', // 7 warnings
    'unicorn/prefer-switch': 'off', // 2 warnings
    'unicorn/prefer-ternary': 'off', // 5 warnings
    'unicorn/prefer-top-level-await': 'off', // 1 warning
  },
});

Implementation notes

We used to have a helper script that automated the ESLint configs transformation, including running ESLint to collect all failing rules. The script only works with the legacy config format, but may still serve as inspiration.

eslint-to-code-pushup.mjs
import {
  createProjectGraphAsync,
  readProjectsConfigurationFromProjectGraph,
} from '@nx/devkit';
import { ESLint } from 'eslint';
import minimatch from 'minimatch';
import fs from 'node:fs/promises';
import path from 'node:path';

// replace these patterns as needed
const TEST_FILE_PATTERNS = [
  '*.spec.ts',
  '*.test.ts',
  '**/test/**/*',
  '**/mock/**/*',
  '**/mocks/**/*',
  '*.cy.ts',
  '*.stories.ts',
];

const graph = await createProjectGraphAsync({ exitOnError: true });
const projects = Object.values(
  readProjectsConfigurationFromProjectGraph(graph).projects,
)
  .filter(project => 'lint' in (project.targets ?? {}))
  .sort((a, b) => a.root.localeCompare(b.root));

for (let i = 0; i < projects.length; i++) {
  const project = projects[i];

  /** @type {import('@nx/eslint/src/executors/lint/schema').Schema} */
  const options = project.targets.lint.options;

  const eslintrc = options.eslintConfig ?? `${project.root}/.eslintrc.json`;
  const patterns = options.lintFilePatterns ?? project.root;

  console.info(
    `Processing Nx ${project.projectType ?? 'project'} "${project.name}" (${
      i + 1
    }/${projects.length}) ...`,
  );

  const eslint = new ESLint({
    overrideConfigFile: eslintrc,
    useEslintrc: false,
    errorOnUnmatchedPattern: false,
    resolvePluginsRelativeTo: options.resolvePluginsRelativeTo ?? undefined,
    ignorePath: options.ignorePath ?? undefined,
    rulePaths: options.rulesdir ?? [],
  });

  const results = await eslint.lintFiles(patterns);

  /** @type {Set<string>} */
  const failingRules = new Set();
  /** @type {Set<string>} */
  const failingRulesTestsOnly = new Set();
  /** @type {Map<string, number>} */
  const errorCounts = new Map();
  /** @type {Map<string, number>} */
  const warningCounts = new Map();

  for (const result of results) {
    const isTestFile = TEST_FILE_PATTERNS.some(pattern =>
      minimatch(result.filePath, pattern),
    );
    for (const { ruleId, severity } of result.messages) {
      if (isTestFile) {
        if (!failingRules.has(ruleId)) {
          failingRulesTestsOnly.add(ruleId);
        }
      } else {
        failingRules.add(ruleId);
        failingRulesTestsOnly.delete(ruleId);
      }
      if (severity === 1) {
        warningCounts.set(ruleId, (warningCounts.get(ruleId) ?? 0) + 1);
      } else {
        errorCounts.set(ruleId, (errorCounts.get(ruleId) ?? 0) + 1);
      }
    }
  }

  /** @param {string} ruleId */
  const formatCounts = ruleId =>
    [
      { kind: 'error', count: errorCounts.get(ruleId) },
      { kind: 'warning', count: warningCounts.get(ruleId) },
    ]
      .filter(({ count }) => count > 0)
      .map(({ kind, count }) =>
        count === 1 ? `1 ${kind}` : `${count} ${kind}s`,
      )
      .join(', ');

  if (failingRules.size > 0) {
    console.info(`• ${failingRules.size} rules need to be disabled`);
    failingRules.forEach(ruleId => {
      console.info(`  - ${ruleId} (${formatCounts(ruleId)})`);
    });
  }
  if (failingRulesTestsOnly.size > 0) {
    console.info(
      `• ${failingRulesTestsOnly.size} rules need to be disabled only for test files`,
    );
    failingRulesTestsOnly.forEach(ruleId => {
      console.info(`  - ${ruleId} (${formatCounts(ruleId)})`);
    });
  }

  if (failingRules.size === 0 && failingRulesTestsOnly.size === 0) {
    console.info('• no rules need to be disabled, nothing to do here\n');
    continue;
  }

  const cpEslintrc =
    'code-pushup.' + path.basename(eslintrc).replace(/^\./, '');

  /** @param {Set<string>} rules */
  const formatRules = (rules, indentLevel = 2) =>
    Array.from(rules.values())
      .sort((a, b) => {
        if (a.includes('/') !== b.includes('/')) {
          return a.includes('/') ? 1 : -1;
        }
        return a.localeCompare(b);
      })
      .map(
        (ruleId, i, arr) =>
          '  '.repeat(indentLevel) +
          `"${ruleId}": "off"${
            i === arr.length - 1 ? '' : ','
          } // ${formatCounts(ruleId)}`,
      )
      .join('\n')
      .replace(/,$/, '');

  /** @type {import('eslint').Linter.Config} */
  const config = `{
  "extends": ["./${cpEslintrc}"],
  // temporarily disable failing rules so \`nx lint\` passes
  // number of errors/warnings per rule recorded at ${new Date().toString()}
  "rules": {
${formatRules(failingRules)}
  }
  ${
    !failingRulesTestsOnly.size
      ? ''
      : `,
  "overrides": [
    {
      "files": ${JSON.stringify(TEST_FILE_PATTERNS)},
      "rules": {
${formatRules(failingRulesTestsOnly, 4)}
      }
    }
  ]`
  }
}`;

  const content = /\.c?[jt]s$/.test(eslintrc)
    ? `module.exports = ${config}`
    : config;

  const cpEslintrcPath = path.join(project.root, cpEslintrc);
  await fs.copyFile(eslintrc, cpEslintrcPath);
  console.info(`• copied ${eslintrc} to ${cpEslintrcPath}`);

  await fs.writeFile(eslintrc, content);
  console.info(
    `• replaced ${eslintrc} to extend ${cpEslintrc} and disable failing rules\n`,
  );
}

process.exit(0);

Acceptance criteria

  • When running and configuring the ESLint plugin in the setup wizard, the user (via prompt or CLI) may opt in to having their ESLint config split into strict and CI-passing configs.
    • The strict config has the same contents as the current configuration.
    • The CI config extends the strict config and disables every rule that reports problems.
      • Lint results are collected by running eslint with the provided config.
      • Every rule with errors or warnings is disabled.
      • For every disabled rule, a comment is added with the current number of errors and warnings. A timestamp is added to the config so that it's clear when these stats were accurate.
    • If running ESLint didn't work, then the error is reported to the user, and this setup step is skipped (no config splitting).

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions