Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report deprecated rules that are in use #188

Closed
randycoulman opened this issue Oct 7, 2016 · 6 comments
Closed

Report deprecated rules that are in use #188

randycoulman opened this issue Oct 7, 2016 · 6 comments

Comments

@randycoulman
Copy link
Contributor

In addition to #172, which would keep eslint-find-rules from reporting errors when deprecated rules are unused, it might also be nice to have a command-line option that shows deprecated rules that are still being used in a configuration.

randycoulman added a commit to randycoulman/eslint-find-rules that referenced this issue Mar 11, 2017
Breaking change: Requires eslint >= 3.12.0

Starting with version 3.12.0, eslint provides a getRules()
API that allows us to directly access the rules rather than
trying to load them from the file system.  This is cleaner,
eliminates a dependency on the internal structure of the eslint
codebase, and may even be faster.

This change paves the way for further features involving
deprecated rules, such as sarbbottam#172 and sarbbottam#188.

Closes sarbbottam#211.
randycoulman added a commit to randycoulman/eslint-find-rules that referenced this issue May 2, 2017
Breaking change: Requires eslint >= 3.12.0

Starting with version 3.12.0, eslint provides a getRules()
API that allows us to directly access the rules rather than
trying to load them from the file system.  This is cleaner,
eliminates a dependency on the internal structure of the eslint
codebase, and may even be faster.

This change paves the way for further features involving
deprecated rules, such as sarbbottam#172 and sarbbottam#188.

Closes sarbbottam#211.
ta2edchimp pushed a commit that referenced this issue May 5, 2017
Breaking change: Requires eslint >= 3.12.0

Starting with version 3.12.0, eslint provides a getRules()
API that allows us to directly access the rules rather than
trying to load them from the file system.  This is cleaner,
eliminates a dependency on the internal structure of the eslint
codebase, and may even be faster.

This change paves the way for further features involving
deprecated rules, such as #172 and #188.

Closes #211.
@randycoulman
Copy link
Contributor Author

I've been thinking more about this as I think about implementing it, and I realize that eslint already warns when deprecated rules are in use. Is this feature useful enough to implement here anyway? Or should I just close the issue?

@ljharb
Copy link
Collaborator

ljharb commented May 19, 2017

I think it's still useful to get a zero/nonzero exit code based on the config files, without having to actually run eslint.

@randycoulman
Copy link
Contributor Author

Yeah, good point. I'll keep it open and work on it when I get a chance.

@alexilyaev
Copy link

alexilyaev commented Sep 4, 2017

ESLint doesn't warn when using deprecated rules in the config, only when using rules that don't exist anymore.

I've got 2 deprecated rules here: https://github.com/alexilyaev/react-es6-starter
newline-after-var and newline-before-return
Running eslint -c .eslintrc --ignore-path .gitignore . doesn't show me any warning.

I think an option to show currently configured deprecated rules is very useful.
We actually have no way of checking it right now

I'm doing something similar with stylelint-find-rules.
Can show configured deprecated rules.

Just tried filtering the currentRules:

  this.getCurrentDeprecatedRules = function () {
    var rules = eslint.linter.getRules();

    return getSortedRules(currentRules).filter(function (rule) {
      return _isDeprecated(rules.get(rule) || '')
    });
  };

It worked, though if I have extends, it counts rules from them as well.
Perhaps ignore extends somehow.

@ta2edchimp
Copy link
Collaborator

A flag to ignore a config's "extends" would be nice.
But atm, only manipulating the input (removing the "extends" property of the config to be loaded) before actually loading the configuration via ESLint's CLIEngine comes to my mind.

@alexilyaev
Copy link

alexilyaev commented Oct 8, 2019

@ta2edchimp @randycoulman So, after 2 years, I find myself back here :-)

I'm trying to validate user configured rules to not have deprecated rules.
This is useful when upgrading configs/plugins which deprecate certain rules.

Right now running eslint-find-rules -d .eslintrc will show deprecated rules not only from user configured rules, but also from extends.

I dug into the code and found it's not so simple to mitigate.
At _getConfigs we use cliEngine.getConfigForFile which loads the configFile and merges all extends on top of it.
Which means that currentRuleNames will already include rules from extends and not only the user defined rules.
And then deprecatedRuleNames can show rules that were not defined by the user.

This happened to me with eslint-config-prettier:
prettier/eslint-config-prettier#112

I tried to dig into eslint's CLIEngine code as well.
Looks like there's a cascade of configs that are being loaded and merged under CascadingConfigArrayFactory.
But eventually our configFile is loaded with configArrayFactory.loadFile, which runs _normalizeConfigData, which then loads and merges the extends and other things.

The only thing I came up with is this:

const { ConfigArrayFactory } = require('eslint/lib/cli-engine/config-array-factory.js');
const configArrayFactory = new ConfigArrayFactory();
configArrayFactory.loadFile(configFile, { name: "UserConfig" });

Which gives us an Array of configs... the user config, extends, overrides, etc.
From here it might be possible to filter out the extends and get a merged config only from user defined rules.

Here's how it looks like
[
  { name:
     'UserConfig » eslint-config-ai/react » eslint-config-prettier/react',
    filePath:
     '/Users/alex/www/Node/Personal/eslint-config-ai/node_modules/eslint-config-prettier/react.js',
    criteria: null,
    env: undefined,
    globals: undefined,
    noInlineConfig: undefined,
    parser: undefined,
    parserOptions: undefined,
    plugins: undefined,
    processor: undefined,
    reportUnusedDisableDirectives: undefined,
    root: undefined,
    rules:
     { 'react/jsx-child-element-spacing': 'off',
       'react/jsx-closing-bracket-location': 'off',
       'react/jsx-closing-tag-location': 'off',
       'react/jsx-curly-newline': 'off',
       'react/jsx-curly-spacing': 'off',
       'react/jsx-equals-spacing': 'off',
       'react/jsx-first-prop-new-line': 'off',
       'react/jsx-indent': 'off',
       'react/jsx-indent-props': 'off',
       'react/jsx-max-props-per-line': 'off',
       'react/jsx-one-expression-per-line': 'off',
       'react/jsx-props-no-multi-spaces': 'off',
       'react/jsx-space-before-closing': 'off',
       'react/jsx-tag-spacing': 'off',
       'react/jsx-wrap-multilines': 'off' },
    settings: undefined },
  { name: 'UserConfig » eslint-config-ai/react',
    filePath: '/Users/alex/www/Node/Personal/eslint-config-ai/react.js',
    criteria: null,
    env: { es6: true },
    globals: undefined,
    noInlineConfig: undefined,
    parser:
     { error: null,
       filePath:
        '/Users/alex/www/Node/Personal/eslint-config-ai/node_modules/babel-eslint/lib/index.js',
       id: 'babel-eslint',
       importerName: 'UserConfig » eslint-config-ai/react',
       importerPath: '/Users/alex/www/Node/Personal/eslint-config-ai/react.js' },
    parserOptions:
     { ecmaVersion: 6, ecmaFeatures: [Object], sourceType: 'script' },
    plugins: { react: [Object], compat: [Object] },
    processor: undefined,
    reportUnusedDisableDirectives: undefined,
    root: undefined,
    rules: { 'compat/compat': 1, 'react/jsx-pascal-case': 1 },
    settings: { react: [Object] } },
  { name: 'UserConfig » eslint-config-ai/react#overrides[0]',
    filePath: '/Users/alex/www/Node/Personal/eslint-config-ai/react.js',
    criteria:
     { includes: [Array],
       excludes: null,
       basePath: '/Users/alex/www/Node/Personal/eslint-config-ai' },
    env: { browser: true, node: false },
    globals: { process: true, module: true },
    noInlineConfig: undefined,
    parser: undefined,
    parserOptions: { sourceType: 'module' },
    plugins: undefined,
    processor: undefined,
    reportUnusedDisableDirectives: undefined,
    root: undefined,
    rules: { strict: [Array] },
    settings: undefined },
  { name: 'UserConfig',
    filePath: '/Users/alex/www/Node/Personal/eslint-config-ai/.eslintrc',
    criteria: null,
    env: undefined,
    globals: undefined,
    noInlineConfig: undefined,
    parser: undefined,
    parserOptions: undefined,
    plugins: undefined,
    processor: undefined,
    reportUnusedDisableDirectives: undefined,
    root: true,
    rules: {},
    settings: undefined }
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants