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

Symbolic links are ignored #13551

Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features
Projects

Comments

@vjekoart
Copy link

vjekoart commented Aug 7, 2020

  • ESLint Version: 7.6.0
  • Node Version: 12.18.0
  • npm Version: 6.14.4

What parser (default, Babel-ESLint, etc.) are you using?
@typescript-eslint/parser

Please show your full configuration:

Configuration
{
    "env": {
        "browser": true
    },
    "extends": [
        "eslint:recommended",
        "plugin:@typescript-eslint/recommended",
        "plugin:@typescript-eslint/recommended-requiring-type-checking"
    ],
    "parser": "@typescript-eslint/parser",
    "parserOptions": {
        "ecmaVersion": 6,
        "sourceType": "module",
        "project": ["./tsconfig.json"]
    },
    "plugins": [ "@typescript-eslint" ],
    "rules": {
        "brace-style": ["error", "allman", { "allowSingleLine": true }],
        "eol-last": ["error", "always"],
        "eqeqeq": ["error", "always"],
        "indent": ["error", 4, { "SwitchCase": 1 }],
        "linebreak-style": ["error", "unix"],
        "lines-between-class-members": ["error", "always"],
        "max-len": ["error", { "code": 120 }],
        "new-cap": "error",
        "no-irregular-whitespace": ["off"],
        "no-multiple-empty-lines": "error",
        "no-nested-ternary": "error",
        "no-trailing-spaces": "error",
        "no-unexpected-multiline": ["off"],
        "object-curly-spacing": ["error", "always"],
        "object-property-newline": "error",
        "one-var": ["error", "never"],
        "quotes": ["error", "double"],
        "semi": ["error", "always"],
        "space-before-blocks": ["error", "always"],
        "space-in-parens": ["error", "always"],
        "spaced-comment": ["error", "always", {
            "block": { "balanced": true },
            "exceptions": []
        }],
        "@typescript-eslint/no-empty-interface": ["error", { "allowSingleExtends": true }],
        "@typescript-eslint/no-explicit-any": ["error"],
        "@typescript-eslint/no-floating-promises": ["error"],
        "@typescript-eslint/no-non-null-assertion": ["error"],
        "@typescript-eslint/no-unsafe-assignment": ["error"],
        "@typescript-eslint/no-unsafe-call": ["error"],
        "@typescript-eslint/no-unsafe-member-access": ["error"],
        "@typescript-eslint/no-unsafe-return": ["error"],
        "@typescript-eslint/no-unused-vars": ["error"],
        "@typescript-eslint/restrict-template-expressions": ["error"]
    }
}

What did you do? Please include the actual source code causing the issue, as well as the command that you used to run ESLint.

// Not relevant, since files are analysed correctly, but not all files are checked.
# This command is executed from `package.json`, i.e. this code is under the `scripts.lint` property.
eslint --ext ts -c .eslintrc.json src

What did you expect to happen?
I'm working on a project which has the following structure of src directory:

src/
    code.ts
    utils/  # Folder with files and subfolders.
    shared/ # Symbolic link to another folder which is not within the `src/` folder.

I've some errors in TS files inside src/shared/ directory. I expected errors and warnings from ESLint when running a lint action.

What actually happened? Please include the actual, raw output from ESLint.
All files, except those in src/shared/ directory have been checked for errors.

After some research, I've identified method _iterateFilesRecursive as problematic in lib/cli-engine/file-enumerator.js file.

Inside that function all entries inside a specific directory are checked if they are a file or a folder. But if entry is a symbolic link, it's completely ignored.

Currently, I've made a workaround to patch aforementioned file with modified version of _iterateFilesRecursive method. Of course, that solution is not good since I'm locked to a specific version of ESLint package.

Modified version of `_iterateFilesRecursive`
    *_iterateFilesRecursive(directoryPath, options) {
        debug(`Enter the directory: ${directoryPath}`);
        const { configArrayFactory } = internalSlotsMap.get(this);

        /** @type {ConfigArray|null} */
        let config = null;

        // Enumerate the files of this directory.
        for (const entry of readdirSafeSync(directoryPath)) {
            const filePath = path.join(directoryPath, entry.name);

            // --- CUSTOM CODE STARTS HERE --- //

            // A decision will be made differently if the path represents a symbolic link.
            let isFile = false;
            let isDirectory = false;

            if (entry.isSymbolicLink()) {
                const fileStat = statSafeSync(filePath);

                if (!fileStat) {
                    continue;
                }

                isFile = fileStat.isFile();
                isDirectory = fileStat.isDirectory();

                if (isFile) {
                    console.log('\nCarefully use symbolic links for files since imports may not be resolved correctly!\n');
                }
            } else {
                isFile = entry.isFile();
                isDirectory = entry.isDirectory();
            }

            // --- CUSTOM CODE ENDS HERE --- //

            // Check if the file is matched.
            if (isFile) { // -- CHANGED FROM `entry.isFile()`
                if (!config) {
                    config = configArrayFactory.getConfigArrayForFile(
                        filePath,

                        /*
                         * We must ignore `ConfigurationNotFoundError` at this
                         * point because we don't know if target files exist in
                         * this directory.
                         */
                        { ignoreNotFoundError: true }
                    );
                }
                const matched = options.selector

                    // Started with a glob pattern; choose by the pattern.
                    ? options.selector.match(filePath)

                    // Started with a directory path; choose by file extensions.
                    : this.isTargetPath(filePath, config);

                if (matched) {
                    const ignored = this._isIgnoredFile(filePath, { ...options, config });
                    const flag = ignored ? IGNORED_SILENTLY : NONE;

                    debug(`Yield: ${entry.name}${ignored ? " but ignored" : ""}`);
                    yield {
                        config: configArrayFactory.getConfigArrayForFile(filePath),
                        filePath,
                        flag
                    };
                } else {
                    debug(`Didn't match: ${entry.name}`);
                }

            // Dive into the sub directory.
            } else if (options.recursive && isDirectory) {  // -- CHANGED FROM `entry.isDirectory()`
                if (!config) {
                    config = configArrayFactory.getConfigArrayForFile(
                        filePath,
                        { ignoreNotFoundError: true }
                    );
                }
                const ignored = this._isIgnoredFile(
                    filePath + path.sep,
                    { ...options, config }
                );

                if (!ignored) {
                    yield* this._iterateFilesRecursive(filePath, options);
                }
            }
        }

        debug(`Leave the directory: ${directoryPath}`);
    }

Are you willing to submit a pull request to fix this bug?
Yes.

@vjekoart vjekoart added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Aug 7, 2020
@mdjermanovic mdjermanovic added core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Aug 7, 2020
@mdjermanovic
Copy link
Member

Hi @vjekoart, thanks for the bug report!

Looks like a regression from 2c28fbb

We were previously using fs.statSync which returns the stat of the link's target, but fs.readdirSync apparently doesn't work like that.

@mdjermanovic
Copy link
Member

@vjekoart

As a temporary solution, does this work as expected (without the patch you made):

eslint --ext ts -c .eslintrc.json src src/shared

@mdjermanovic
Copy link
Member

Tried to run npx eslint . -f tap in a directory with linked dirs (links created with ln -s; mklink /D or mklink /J on windows).

eslint v6.8.0 enters target dirs and lints files inside.

eslint v7.0.0 doesn't. The latest eslint v7.6.0 also doesn't.

@mysticatea I think this was changed in #12700 where some calls of fs.statSync were removed in favor of fs.readdirSync with withFileTypes: true, but I'm not sure if dropping support for subdir symlinks was intentional? If that's really the PR that made this change in behavior, I'm guessing it wasn't the intention but asking just to be sure.

@vjekoart
Copy link
Author

@vjekoart

As a temporary solution, does this work as expected (without the patch you made):

eslint --ext ts -c .eslintrc.json src src/shared

This approach works and it's more simple than the current workaround I have. Thanks for the hint @mdjermanovic !

@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@mdjermanovic
Copy link
Member

Reopening, as this is most likely a bug.

@mdjermanovic mdjermanovic reopened this Sep 11, 2020
@mdjermanovic mdjermanovic self-assigned this Sep 11, 2020
@mdjermanovic mdjermanovic removed the auto closed The bot closed this issue label Sep 11, 2020
@blindspeed90
Copy link

Our directory structure has symlinks for all files that aren't checked out in our source control, thus giving us no results now. Is there any workaround for this, short of scanning for the files myself and passing every one in?

@mdjermanovic
Copy link
Member

I think we should fix this in ESLint.

Is there any workaround for this, short of scanning for the files myself and passing every one in?

A (far from ideal) workaround could be to let the shell do that, by using globs that will be expanded to individual file paths instead of passed to ESLint, if that's possible.

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Dec 15, 2020
@mdjermanovic
Copy link
Member

@vjekoart apologies for the delay! This issue is now accepted, and PR is welcome.

Are you still willing to submit a PR to fix this bug? :)

(and #13615, I think your solution covers both)

@vjekoart
Copy link
Author

@mdjermanovic Delay is not a problem, I'm glad the issue is accepted :)

I'm still willing to submit a PR to fix this bug, I'll try to open one as soon as possible. I'm also going to keep in mind that this problem is related to #13615.

@mistic
Copy link

mistic commented Jan 27, 2021

@vjekoart @mdjermanovic do you have any updates on this one?

@mrl5
Copy link

mrl5 commented Jan 28, 2021

I'm also interested if there are any plans to fix this

@mdjermanovic mdjermanovic added this to Needs Triage in Triage via automation Feb 21, 2021
@mdjermanovic mdjermanovic moved this from Needs Triage to Pull Request Opened in Triage Feb 21, 2021
@mdjermanovic
Copy link
Member

This will be fixed by #14126

This was referenced Mar 17, 2021
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 26, 2021
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.