eslint icon indicating copy to clipboard operation
eslint copied to clipboard

Bug: `context.parserOptions` becomes empty with flat config and custom parser

Open ryym opened this issue 2 years ago • 10 comments

Environment

Environment Info:

Node version: v18.10.0 npm version: v8.19.2 Local ESLint version: v8.27.0 (Currently used) Global ESLint version: Not found Operating System: darwin 21.6.0

What parser are you using?

@typescript-eslint/parser

What did you do?

I used new ESLint configuration file eslint.config.js with some plugins, and noticed that some plugin rules do not work correctly when used with a non-default parser. For example, the no-default-export rule in eslint-plugin-import does not report errors at all only when used with eslint.config.js and non-default parser such as @typescript-eslint/parser .

I found that this is because parserOptions in context object passed to rule creator functions becomes empty in that case. The no-default-export rule checks parserOptions.sourceType on rule creation, and skips running if the sourceType is not module. https://github.com/import-js/eslint-plugin-import/blob/48e8130a9f33afd0f9d06635c25d4f1df4d63340/src/rules/no-default-export.js#L15-L18

I created a reproduction repository: https://github.com/ryym/eslint-flat-config-ts-parser-issue

In this repo I created a local ESLint rule that just prints context.parserOptions and used it in four patterns:

  1. npm run old-config ... .eslintrc.js with default parser
  2. npm run old-config-ts ... .eslintrc.js with @typescript-plugin/parser parser
  3. npm run new-config ... eslint.config.js with default parser
  4. npm run new-config-ts ... eslint.config.js with @typescript-plugin/parser parser

You can see the output is an empty object only in new-config-ts.

reproduction flow and output
  1. clone the repo
  2. npm install
  3. npm run check
% npm run check

> check
> npm run old-config && npm run old-config-ts && npm run new-config && npm run new-config-ts


> old-config
> ESLINT_USE_FLAT_CONFIG=false npx eslint a.js

{
  ecmaVersion: 11,
  sourceType: 'module',
  ecmaFeatures: { globalReturn: false }
}

> old-config-ts
> ESLINT_USE_FLAT_CONFIG=false TS_PARSER=true npx eslint a.js

{
  ecmaVersion: 11,
  sourceType: 'module',
  ecmaFeatures: { globalReturn: false }
}

> new-config
> ESLINT_USE_FLAT_CONFIG=true npx eslint a.js

{ sourceType: 'module' }

> new-config-ts
> ESLINT_USE_FLAT_CONFIG=true TS_PARSER=true npx eslint a.js

{}

I also confirmed that this occurs with @babel/eslint-parser as well.

What did you expect to happen?

ESLint rules using context.parserOptions internally work in any combination of config file format and parser.

What actually happened?

As I wrote before, a rule using context.parserOptions do not work with new flat config eslint.config.js with non-default parser such-as @typescript-eslint/parser, @babel-eslint/parser . This is because parserOptions becomes an empty object only in that case.

Participation

  • [X] I am willing to submit a pull request for this issue.

Additional comments

No response

ryym avatar Nov 18 '22 00:11 ryym

Hi @ryym, thanks for the issue!

This is intentional. Some rules will need to be changed to be compatible with flat config.

In particular:

  • context.languageOptions.sourceType should be used instead of context.parserOptions.sourceType.
  • context.languageOptions.ecmaVersion should be used instead of context.parserOptions.ecmaVersion.
  • context.languageOptions.parser should be used instead of context.parserPath.

I believe this will be included in https://github.com/eslint/eslint/issues/13481 Phase 3: Compatibility testing, so I'm not sure if we should keep this open as a separate issue. @nzakas what do you think?

mdjermanovic avatar Nov 18 '22 16:11 mdjermanovic

You’re correct, this is intentional. When using flat config, all of the options are enclosed in languageOptions, so you need to use context.languageOptions to access them. This will also work with eslintrc configs because that mapping is easy. It’s not as easy to go in the opposite direction, so changing to always use context.languageOptions is the preferred choice.

We can keep this open for phase 3, as I’m sure this question will come up in the future again, and maybe we will figure out a better way to map these two approaches.

nzakas avatar Nov 18 '22 18:11 nzakas

Thank you for your responses! Let me check if I understood correctly. I understood the rule should access languageOptions instead of parserOptions , but what I wondered was why parserOptions is empty only when using a non-default parser. Without specifying a parser, parserOptions.sourceType still be set even if a flat config is used. Is this also intentional?

As an example, if you run ESLint with the following rule, parserOptions will be empty only when parser is specified.

// rule js
module.exports = {
  create(context) {
    console.log(context.parserOptions);
    return {};
  },
};
// eslint.config.js
const tsParser = require("@typescript-eslint/parser");
module.exports = [
  {
    // console output: { sourceType: 'module' }
    languageOptions: { sourceType: 'module' },

    // console output: {}
    languageOptions: { sourceType: 'module', parser: tsParser },
  },
];

ryym avatar Nov 19 '22 06:11 ryym

Because context.parserOptions is filled by a top-level parserOptions in the config. When a top-level parserOptions doesn't exist, then there is no value to fill in there. Additionally, some of the previous properties in parserOptions are now in languageOptions (such as ecmaVersion and sourceType), so there's no direct mapping. Hope this help.

nzakas avatar Nov 23 '22 20:11 nzakas

Given this file:

top-level-await.js

import { glob } from 'glob'

await glob('*.js', { ignore: 'node_modules/**' })

With this config:

export default [
  // This very well could be an extended plugin config, such as one from eslint-plugin-n
  {
    languageOptions: {
      parserOptions: {
        ecmaVersion: 2021,
        sourceType: 'module'
      }
    }
  },
  // Say this is my own custom config
  {
    languageOptions: {
      ecmaVersion: 2023,
      sourceType: 'module'
    },
    files: ['top-level-await.js'],
    rules: {
      'no-console': 'error'
    }
  }
]

Running eslint top-level-await.js will result in:

/path/to/top-level-await.js
  3:1  error  Parsing error: Cannot use keyword 'await' outside an async function

✖ 1 problem (1 error, 0 warnings)

I would have though that this statement

In particular: context.languageOptions.sourceType should be used instead of context.parserOptions.sourceType. context.languageOptions.ecmaVersion should be used instead of context.parserOptions.ecmaVersion. context.languageOptions.parser should be used instead of context.parserPath.

means I do not need to include an additional parserOptions, with it's own nested ecmaVersion to make the expected override work. However, the nested parserOptions is necessary:

export default [
  {
    languageOptions: {
      parserOptions: {
        ecmaVersion: 2021,
        sourceType: 'module'
      }
    }
  },
  {
    languageOptions: {
      ecmaVersion: 2023,
      sourceType: 'module',
      parserOptions: {
        ecmaVersion: 2023
      }
    },
    files: ['top-level-await.js'],
    rules: {
      'no-console': 'error'
    }
  }
]

So, is this expected behavior? Can parserOptions be defined outside of languageOptions? The example doesn’t indicate that.

morganney avatar Jul 19 '23 14:07 morganney

If languageOptions.parserOptions is supplied, then it will be passed to the parser and will overrides anything set in languageOptions.

In your first example, you'll end up with a config that looks like this:

  {
    languageOptions: {
      ecmaVersion: 2023,
      sourceType: 'module',
      parserOptions: {
        ecmaVersion: 2021,
        sourceType: 'module'
      }
    },
    files: ['top-level-await.js'],
    rules: {
      'no-console': 'error'
    }
  }

So even though you have languageOptions.ecmaVersion: 2023 the languageOptions.parserOptions.ecmaVersion: 2021 is what is passed to the parser. (Again: parserOptions will override languageOptions when the key name is the same.)

In your second example, the nested parserOptions is necessary to reset languageOptions.parserOptions.ecmaVersion.

Note: If you have further questions about this, please do so in a discussion; this is not related to this issue.

nzakas avatar Jul 19 '23 15:07 nzakas

Ah, there are Discussions 👌 , noted.

Thanks for your prompt feedback. This helped confirm what I was experiencing:

If languageOptions.parserOptions is supplied, then it will be passed to the parser and will overrides anything set in languageOptions.

However, I didn't see any documentation related to that behavior in the docs I linked to in my previous comment (unless I overlooked something), leaving me to learn about it only through trial and error.

morganney avatar Jul 19 '23 16:07 morganney

@morganney we are still working on the documentation for flat config. That's a big task for v9.0.0.

nzakas avatar Jul 20 '23 15:07 nzakas

@nzakas It seems like it ought to be possible to allow users to specify a parserPath as a transitional strategy. In the long term, of course what we want is for plugins to stop using parserPath. But it doesn't seem problematic (unless I'm missing something) to allow users to opt in to compatibility with context.parserPath (by explicitly specifying one).

For reference, I hit this issue with the no-commented-out-code rule from eslint-plugin-etc, not one of the more banner cases like eslint-plugin-import.

wycats avatar Oct 12 '23 18:10 wycats

@wycats we don't have the data to provide parserPath when flat config is used...we don't have the path to the parser. context.languageOptions.parser should work in both eslintrc and flat config modes.

nzakas avatar Oct 13 '23 14:10 nzakas

Revisiting this now that flat config has shipped. Unfortunately, there just isn't an easy way to ensure that context.parserOptions will have the correct values regardless of the config file format used.

The recommended approach is to always use context.languageOptions and context.languageOptions.parserOptions, as these are present and correct in both flat config and eslintrc formats for both ESLint v8.x and v9.x. If you need to support versions older than that, you'll probably want to do something like this:

const parserOptions = context.languageOptions?.parserOptions ?? context.parserOptions;

nzakas avatar Jun 17 '24 15:06 nzakas