Bug: FlatArrayConfig options cannot be overriden

[magne4000]

Environment

Node version: v16.20.0
npm version: v8.19.4
Local ESLint version: Not found
Global ESLint version: Not found
Operating System: linux 6.5.7-arch1-1

What parser are you using?

Default (Espree)

What did you do?

Configuration

import * as tsParseForESLint from "@typescript-eslint/parser";

const config = [
  {
    languageOptions: {
      parser: tsParseForESLint,
    },
    files: ["**/*.ts", "**/*.js", "**/*.tsx", "**/*.jsx"],
  },
]

const linter = new Linter({
  configType: "flat",
});
const report = linter.verifyAndFix(code, config, filename);

What did you expect to happen?

I want to be able to override FlatConfigArray options

What actually happened?

There is no option to forward options to FlatConfigArray

Link to Minimal Reproducible Example

https://eslint.org/play/#eyJ0ZXh0IjoiaW1wb3J0IHsgTGludGVyIH0gZnJvbSBcImVzbGludFwiO1xuaW1wb3J0ICogYXMgdHNQYXJzZUZvckVTTGludCBmcm9tIFwiQHR5cGVzY3JpcHQtZXNsaW50L3BhcnNlclwiO1xuXG5jb25zdCBjb25maWcgPSBbXG4gIHtcbiAgICBsYW5ndWFnZU9wdGlvbnM6IHtcbiAgICAgIHBhcnNlcjogdHNQYXJzZUZvckVTTGludCxcbiAgICB9LFxuICAgIGZpbGVzOiBbXCIqKi8qLnRzXCIsIFwiKiovKi5qc1wiLCBcIioqLyoudHN4XCIsIFwiKiovKi5qc3hcIl0sXG4gIH0sXG5dXG5cbmV4cG9ydCBmdW5jdGlvbiBnZXRMaW50ZXIoKSB7XG4gIHJldHVybiBuZXcgTGludGVyKHtcbiAgICBjb25maWdUeXBlOiBcImZsYXRcIixcbiAgfSk7XG59XG5cbmNvbnN0IGxpbnRlciA9IGdldExpbnRlcigpO1xubGludGVyLnZlcmlmeUFuZEZpeCgnY29uc3QgYSA9IFwiXCI7JywgY29uZmlnLCAndGVzdC50cycpOyIsIm9wdGlvbnMiOnsiZW52Ijp7ImVzNiI6dHJ1ZX0sInBhcnNlck9wdGlvbnMiOnsiZWNtYUZlYXR1cmVzIjp7fSwiZWNtYVZlcnNpb24iOiJsYXRlc3QiLCJzb3VyY2VUeXBlIjoibW9kdWxlIn0sInJ1bGVzIjp7ImNvbnN0cnVjdG9yLXN1cGVyIjpbImVycm9yIl0sImZvci1kaXJlY3Rpb24iOlsiZXJyb3IiXSwiZ2V0dGVyLXJldHVybiI6WyJlcnJvciJdLCJuby1hc3luYy1wcm9taXNlLWV4ZWN1dG9yIjpbImVycm9yIl0sIm5vLWNhc2UtZGVjbGFyYXRpb25zIjpbImVycm9yIl0sIm5vLWNsYXNzLWFzc2lnbiI6WyJlcnJvciJdLCJuby1jb21wYXJlLW5lZy16ZXJvIjpbImVycm9yIl0sIm5vLWNvbmQtYXNzaWduIjpbImVycm9yIl0sIm5vLWNvbnN0LWFzc2lnbiI6WyJlcnJvciJdLCJuby1jb25zdGFudC1jb25kaXRpb24iOlsiZXJyb3IiXSwibm8tY29udHJvbC1yZWdleCI6WyJlcnJvciJdLCJuby1kZWJ1Z2dlciI6WyJlcnJvciJdLCJuby1kZWxldGUtdmFyIjpbImVycm9yIl0sIm5vLWR1cGUtYXJncyI6WyJlcnJvciJdLCJuby1kdXBlLWNsYXNzLW1lbWJlcnMiOlsiZXJyb3IiXSwibm8tZHVwZS1lbHNlLWlmIjpbImVycm9yIl0sIm5vLWR1cGUta2V5cyI6WyJlcnJvciJdLCJuby1kdXBsaWNhdGUtY2FzZSI6WyJlcnJvciJdLCJuby1lbXB0eSI6WyJlcnJvciJdLCJuby1lbXB0eS1jaGFyYWN0ZXItY2xhc3MiOlsiZXJyb3IiXSwibm8tZW1wdHktcGF0dGVybiI6WyJlcnJvciJdLCJuby1leC1hc3NpZ24iOlsiZXJyb3IiXSwibm8tZXh0cmEtYm9vbGVhbi1jYXN0IjpbImVycm9yIl0sIm5vLWV4dHJhLXNlbWkiOlsiZXJyb3IiXSwibm8tZmFsbHRocm91Z2giOlsiZXJyb3IiXSwibm8tZnVuYy1hc3NpZ24iOlsiZXJyb3IiXSwibm8tZ2xvYmFsLWFzc2lnbiI6WyJlcnJvciJdLCJuby1pbXBvcnQtYXNzaWduIjpbImVycm9yIl0sIm5vLWlubmVyLWRlY2xhcmF0aW9ucyI6WyJlcnJvciJdLCJuby1pbnZhbGlkLXJlZ2V4cCI6WyJlcnJvciJdLCJuby1pcnJlZ3VsYXItd2hpdGVzcGFjZSI6WyJlcnJvciJdLCJuby1sb3NzLW9mLXByZWNpc2lvbiI6WyJlcnJvciJdLCJuby1taXNsZWFkaW5nLWNoYXJhY3Rlci1jbGFzcyI6WyJlcnJvciJdLCJuby1taXhlZC1zcGFjZXMtYW5kLXRhYnMiOlsiZXJyb3IiXSwibm8tbmV3LXN5bWJvbCI6WyJlcnJvciJdLCJuby1ub25vY3RhbC1kZWNpbWFsLWVzY2FwZSI6WyJlcnJvciJdLCJuby1vYmotY2FsbHMiOlsiZXJyb3IiXSwibm8tb2N0YWwiOlsiZXJyb3IiXSwibm8tcHJvdG90eXBlLWJ1aWx0aW5zIjpbImVycm9yIl0sIm5vLXJlZGVjbGFyZSI6WyJlcnJvciJdLCJuby1yZWdleC1zcGFjZXMiOlsiZXJyb3IiXSwibm8tc2VsZi1hc3NpZ24iOlsiZXJyb3IiXSwibm8tc2V0dGVyLXJldHVybiI6WyJlcnJvciJdLCJuby1zaGFkb3ctcmVzdHJpY3RlZC1uYW1lcyI6WyJlcnJvciJdLCJuby1zcGFyc2UtYXJyYXlzIjpbImVycm9yIl0sIm5vLXRoaXMtYmVmb3JlLXN1cGVyIjpbImVycm9yIl0sIm5vLXVuZGVmIjpbImVycm9yIl0sIm5vLXVuZXhwZWN0ZWQtbXVsdGlsaW5lIjpbImVycm9yIl0sIm5vLXVucmVhY2hhYmxlIjpbImVycm9yIl0sIm5vLXVuc2FmZS1maW5hbGx5IjpbImVycm9yIl0sIm5vLXVuc2FmZS1uZWdhdGlvbiI6WyJlcnJvciJdLCJuby11bnNhZmUtb3B0aW9uYWwtY2hhaW5pbmciOlsiZXJyb3IiXSwibm8tdW51c2VkLWxhYmVscyI6WyJlcnJvciJdLCJuby11bnVzZWQtdmFycyI6WyJlcnJvciJdLCJuby11c2VsZXNzLWJhY2tyZWZlcmVuY2UiOlsiZXJyb3IiXSwibm8tdXNlbGVzcy1jYXRjaCI6WyJlcnJvciJdLCJuby11c2VsZXNzLWVzY2FwZSI6WyJlcnJvciJdLCJuby13aXRoIjpbImVycm9yIl0sInJlcXVpcmUteWllbGQiOlsiZXJyb3IiXSwidXNlLWlzbmFuIjpbImVycm9yIl0sInZhbGlkLXR5cGVvZiI6WyJlcnJvciJdfX19

Participation

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

Additional comments

No response

[nzakas]

This is be design. FlatConfigArray is an internal structure to ESLint and not part of the public API. What is it you're trying to do?

[magne4000]

I'm instanciating new Linter in a repo, and running verifyAndFix on dynamically generated files in /tmp folder like so:

const linter = new Linter({
  configType: "flat",
});
const filename = '/tmp/myfile.ts';
const report = linter.verifyAndFix(code, config, filename);

But /tmp/myfile.ts files is flagged as ignored in the process because /tmp is outside my repository folder (/home/magne/workspace). It is not obvious to guess why my files were being ignored and how to opt-out of it. Also setting cwd: '/tmp' while instanciating new Linter did not help.

In my specific case I found out that I can just pass basename(filename) to "fix" issue issue.

You can check full usage here https://github.com/batijs/bati/blob/92b3745621fe21483508b16908136861733945dc/packages/core/src/parse/linters/common.ts#L12

Replacing basename(filename) by filename, and running pnpm -w run build && pnpm -w run test:e2e will generate the error.

[nzakas]

But /tmp/myfile.ts files is flagged as ignored in the process because /tmp is outside my repository folder (/home/magne/workspace). It is not obvious to guess why my files were being ignored and how to opt-out of it. Also setting cwd: '/tmp' while instanciating new Linter did not help.

I don't think that's what's going on here. Linter has no concept of cwd and uses the empty string as the base path for FlatConfigArray by default. I suspect the real problem is just the leading / on the filename. If you remove it, I'm guessing it will work as expected.

[magne4000]

Indeed it does. But the behaviour is unexpected with a leading / don't you think so?
I don't know if this needs a fix per-se, but at least a note in the documentation I guess.

[nzakas]

Happy to add something to the docs. Where would you expect to find that note?

[magne4000]

I guess in https://eslint.org/docs/latest/integrate/nodejs-api#linterverify on options.filename and add probably also https://eslint.org/docs/latest/integrate/nodejs-api#linterverifyandfix which does not yet document its parameters.

[nzakas]

Would you be interested in sending a PR for that?

[magne4000]

Yes I could help with that, but I would first need to know how to properly describe the current behaviour, because it still seems odd after testing a little more.

Take a look at https://stackblitz.com/edit/stackblitz-starters-byefgh?file=index.js
The behaviour is in fact related to cwd, but where? Can it be overriden? And if this is normal behaviour, how to justify it?

[nzakas]

Linter has no concept of a filesystem, so it doesn't know anything about how a filename relates to cwd. The cwd option for the Linter constructor is simply passed through to context.cwd so rules have some idea about the context in which they're executed.

The FlatConfigArray understands its basePath and filenames are resolved relative to that, but as I already explained, in your example basePath is set to "", which is the default (basePath isn't passed explicitly to the constructor here). The cwd is not used.

That said, it does seem strange that /tmp/test.ts yields a different result than /home/projects/stackblitz-starters-73twqj/test.ts when there is **/*.ts in the files array. Both begin with a /, which should cause them to behave the same way. I'll take a look at that.

[nzakas]

Okay, so I figured out what is going on. This is a behavior inside of Node.js, specifically, inside of path.relative(). In order to determine whether we should lint a file, we first check to see if the directory is inside the basePath here. It turns out:

console.log(path.relative("", process.cwd()));       // outputs ""

console.log(path.relative("", "/foo/bar"));          // outputs "..\..\..\..\..\foo\bar"

From looking at the Node.js code, it looks like path.relative() is called posix.resolve() on both parameters, and posix.resolve("") always resolves to process.cwd(). That's why cwd ends up being a special case.

So what we probably want to do is just skip the path.relative() check if basePath is an empty string. @mdjermanovic does that make sense to you?

[mdjermanovic]

@magne4000 did you try with ESLint/FlatESLint class instead of Linter? As noted in the Linter docs:

Unless you are working in the browser, you probably want to use the ESLint class instead.

For example:

import api from 'eslint/use-at-your-own-risk';
import { join } from 'node:path';
import * as tsParseForESLint from '@typescript-eslint/parser';

const { FlatESLint } = api;

const config = [
  {
    languageOptions: {
      parser: tsParseForESLint,
    },
    files: ['**/*.ts', '**/*.js', '**/*.tsx', '**/*.jsx'],
  },
];

const eslint = new FlatESLint({
    cwd: "/tmp",
    overrideConfigFile: true,
    overrideConfig: config
});

const [result] = await eslint.lintText(
    'const a = "";',
    {
        filePath: join('/tmp', 'test.ts')
    }
);

console.log(result);

[magne4000]

@mdjermanovic yes I did, it is indeed clearer to understand what happens here.

As noted in the Linter docs:

Unless you are working in the browser, you probably want to use the ESLint class instead.

Reading the documentation I was not detered by this particular message it seems, as documented Linter API seemed to be just what I needed, without involving filesystem stuffs.

I'm not sure how you want to progress with this issue. I think that what @nzakas discovered with path.relative() is still unexpected behaviour.

[nzakas]

Yes, this is still a bug we need to fix.

[nzakas]

After giving this some thought, I think there are actually two problems with the current implementation.

First, basePath is always set to "" inside of Linter. I think, ideally, if cwd is passed as an argument to create Linter, then it should be used as basePath as well. That would at least get us consistent behavior between cwd and basePath.

Second, treating "" as equivalent to process.cwd(). This is the root problem on this issue that we need to figure out how to address.

[mdjermanovic]

I think, ideally, if cwd is passed as an argument to create Linter, then it should be used as basePath as well.

I'm working on this.

[nzakas]

Note from the TSC meeting: we are still trying to decide whether or not to address the second problem (treating "" as process.cwd()).

[nzakas]

@mdjermanovic It seems like we solved the most pressing part of this in #17705. I think we can close this now?

[mdjermanovic]

Yes, I also think we can close this now.

Starting from ESLint v8.54.0, cwd passed to the Linter constructor is used as config's basePath. So, for example, this works:

const { Linter } = require("eslint");

const linter = new Linter({
    configType: "flat",
    cwd: "/tmp" // <-
});

const code = "foo";
const config = [{
    files: ["**/*.ts"],
    rules: {
        "no-undef": "error"
    }
}];
const filename = "/tmp/myfile.ts";

const report = linter.verifyAndFix(code, config, filename);

console.log(report);

/*

    {
        fixed: false,
        messages: [
            {
                ruleId: 'no-undef',
                severity: 2,
                message: "'foo' is not defined.",
                line: 1,
                column: 1,
                nodeType: 'Identifier',
                messageId: 'undef',
                endLine: 1,
                endColumn: 4
            }
        ],
        output: 'foo'
    }

*/