Navigation Menu

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

AssertionError [ERR_ASSERTION]: 'newBasePath' should be an absolute path #12850

Closed
drkestel opened this issue Jan 30, 2020 · 9 comments · Fixed by #13078
Closed

AssertionError [ERR_ASSERTION]: 'newBasePath' should be an absolute path #12850

drkestel opened this issue Jan 30, 2020 · 9 comments · Fixed by #13078
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

Comments

@drkestel
Copy link

drkestel commented Jan 30, 2020

Windows
Node v12.13.0
npm v6.13.6
eslint 6.8.0
Parser: @typescript-eslint/parser
Bash terminal

eslint config:

"use strict";
const allExtensions = [".ts", ".tsx", ".d.ts", ".js", ".jsx"];
module.exports = {
    env: {
        "browser": true,
        "es6": true,
    },
    parser: "@typescript-eslint/parser",
    parserOptions: {
        project: "./tsconfig.json",
        sourceType: "module",
    },
    plugins: [
        "import",
        "@typescript-eslint",
        "@typescript-eslint/tslint",
    ],
    settings: {
        "import/extensions": allExtensions,
        "import/parsers": {
            "@typescript-eslint/parser": [".ts", ".tsx", ".d.ts"],
        },
        "import/resolver": {
            "node": {
                "extensions": allExtensions,
            },
        },
    },
    rules: {
     
        "import/no-deprecated": "warn",
        "@typescript-eslint/no-unused-vars": "error",
        "@typescript-eslint/tslint/config": [
            "warn",
            {
                "rules": {
                    "deprecation": true,
                },
            },
        ],
    },
};

I'm currently using a globally installed npm package (call it foo cli) that wraps eslint. I have a workspace in c:\src\myproject. foo cli has an internal reference to eslint 6.8.0. When I run ap lint from c:\src\myproject, foo cli invokes eslint with some default parameters, and I get the error from the bug title. Here's the raw output:

/c/myproject (master)
$ ap lint
Running C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\.bin\eslint.cmd **/*.ts --ignore-path C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintignore -c C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintrc.json --resolve-plugins-relative-to C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint
AssertionError [ERR_ASSERTION]: 'newBasePath' should be an absolute path.
    at IgnorePattern.getPatternsRelativeTo (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:210:9)
    at C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:150:42
    at Array.map (<anonymous>)
    at Function.createIgnore (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:150:31)
    at createConfig (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\config-array.js:282:40)
    at ConfigArray.extractConfig (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\config-array.js:452:33)
    at FileEnumerator._isIgnoredFile (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:479:24)
    at FileEnumerator._iterateFilesRecursive (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:407:38)
    at _iterateFilesRecursive.next (<anonymous>)
    at FileEnumerator.iterateFiles (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:251:49)

I believe the problem exists in ignore-pattern.js and was introduced in this pull request: https://github.com/eslint/eslint/pull/12274/files

If I add some debug logging to IgnorePattern.getCommonAncestorPath:

function getCommonAncestorPath(sourcePaths) {
    let result = sourcePaths[0];

    for (let i = 1; i < sourcePaths.length; ++i) {
        const a = result;
        const b = sourcePaths[i];

        // Set the shorter one (it's the common ancestor if one includes the other).
        result = a.length < b.length ? a : b;

        // Set the common ancestor.
        for (let j = 0, lastSepPos = 0; j < a.length && j < b.length; ++j) {
            if (a[j] !== b[j]) {
                result = a.slice(0, lastSepPos);
                break;
            }
            if (a[j] === path.sep) {
                lastSepPos = j;
            }
        }
    }
    console.log("**: ", { result: result, sep: path.sep, sourcePaths: sourcePaths }); // new line here
    return result || path.sep;
}

I see the following output:

/c/myproject (master)
$ ap lint
Running C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\.bin\eslint.cmd **/*.ts --ignore-path C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintignore -c C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintrc.json --resolve-plugins-relative-to C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint
**:  {
  result: 'C:\\src\\myproject',
  sep: '\\',
  sourcePaths: [ 'C:\\src\\myproject' ]
}
**:  {
  result: 'C:\\src\\myproject',
  sep: '\\',
  sourcePaths: [ 'C:\\src\\myproject' ]
}
**:  {
  result: 'C:',
  sep: '\\',
  sourcePaths: [
    'C:\\src\\myproject',
    'C:\\Users\\drkestel\\AppData\\Roaming\\npm\\node_modules\\@foocli\\lint'
  ]
}
AssertionError [ERR_ASSERTION]: 'newBasePath' should be an absolute path.
    at IgnorePattern.getPatternsRelativeTo (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:210:9)
    at C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:150:42
    at Array.map (<anonymous>)
    at Function.createIgnore (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\ignore-pattern.js:150:31)
    at createConfig (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\config-array.js:282:40)
    at ConfigArray.extractConfig (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\config-array\config-array.js:452:33)
    at FileEnumerator._isIgnoredFile (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:479:24)
    at FileEnumerator._iterateFilesRecursive (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:407:38)
    at _iterateFilesRecursive.next (<anonymous>)
    at FileEnumerator.iterateFiles (C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\eslint\lib\cli-engine\file-enumerator.js:251:49)

/c/myproject (master)
$

Notice the result: 'C:', - that is not considered a valid absolute path.

Interestingly enough, I can mitigate the problem by using windows cmd.exe and setting my current prompt to c:\ (lowercase, as opposed to C:\). Because c:\ != C:\, it works. See the output:

c:\src\myproject>ap lint
Running C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\node_modules\.bin\eslint.cmd **/*.ts --ignore-path C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintignore -c C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint\.eslintrc.json --resolve-plugins-relative-to C:\Users\drkestel\AppData\Roaming\npm\node_modules\@foocli\lint
**:  {
  result: 'c:\\src\\myproject',
  sep: '\\',
  sourcePaths: [ 'c:\\src\\myproject' ]
}
**:  {
  result: 'c:\\src\\myproject',
  sep: '\\',
  sourcePaths: [ 'c:\\src\\myproject' ]
}
**:  {
  result: '',
  sep: '\\',
  sourcePaths: [
    'c:\\src\\myproject',
    'C:\\Users\\drkestel\\AppData\\Roaming\\npm\\node_modules\\@foocli\\lint'
  ]
}
**:  {
  result: '',
  sep: '\\',
  sourcePaths: [
    'c:\\src\\myproject',
    'C:\\Users\\drkestel\\AppData\\Roaming\\npm\\node_modules\\@foocli\\lint'
  ]
}

c:\src\myproject\Client\Index.ts
  66:5   warning  Missing return type on function                 @typescript-eslint/explicit-function-return-type
  67:17  error    'container' is assigned a value but never used  @typescript-eslint/no-unused-vars
  68:15  error    'id' is assigned a value but never used         @typescript-eslint/no-unused-vars

... more valid linting

I can also mitigate the problem by making the following code change to ignore-pattern.js:

function getCommonAncestorPath(sourcePaths) {
    let result = sourcePaths[0];

    for (let i = 1; i < sourcePaths.length; ++i) {
        const a = result;
        const b = sourcePaths[i];

        // Set the shorter one (it's the common ancestor if one includes the other).
        result = a.length < b.length ? a : b;

        // Set the common ancestor.
        for (let j = 0, lastSepPos = 0; j < a.length && j < b.length; ++j) {
            if (a[j] !== b[j]) {
                result = a.slice(0, lastSepPos);
                break;
            }
            if (a[j] === path.sep) {
                lastSepPos = j;
            }
        }
    }
    // this was the previous return statement
    // return result || path.sep;

    // new return statement
    let resolvedResult = result || path.sep;
    if (resolvedResult && resolvedResult.endsWith(":")) {
        resolvedResult = resolvedResult + path.sep;
    }
    return resolvedResult;
}
@drkestel drkestel added bug ESLint is working incorrectly triage An ESLint team member will look at this issue soon labels Jan 30, 2020
@kaicataldo kaicataldo 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 Feb 4, 2020
@kaicataldo
Copy link
Member

Thanks for the detailed bug report. This does seem like a bug to me. Looking at how paths work in WIndows in Microsoft's documentation, it does look like we need to fix this algorithm to not remove the separator from C:, since "C:" is considered relative while "C:\\" is considered absolute.

@kaicataldo kaicataldo added accepted There is consensus among the team that this change meets the criteria for inclusion help wanted The team would welcome a contribution from the community for this issue good first issue Good for people who haven't worked on ESLint before and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Feb 4, 2020
@kaicataldo kaicataldo removed the good first issue Good for people who haven't worked on ESLint before label Feb 17, 2020
@nickharris
Copy link
Contributor

nickharris commented Mar 23, 2020

@kaicataldo are you happy with drkestels //new return statement from the snippet above or would you prefer just update result = a.slice(0, lastSepPos); to result = a.slice(0, lastSepPos + 1);

@kaicataldo
Copy link
Member

This makes sense to me:

    let resolvedResult = result || path.sep;
    if (resolvedResult && resolvedResult.endsWith(":")) {
        resolvedResult = resolvedResult + path.sep;
    }
    return resolvedResult;

I'd be curious to hear from other team members who have more experience with Windows think. I'm no expert on Windows path gotchas.

nickharris added a commit to nickharris/eslint that referenced this issue Mar 24, 2020
@nickharris
Copy link
Contributor

nickharris commented Mar 24, 2020

@kaicataldo how about something like this https://github.com/eslint/eslint/pull/13078/files

nickharris added a commit to nickharris/eslint that referenced this issue Mar 24, 2020
@kaicataldo kaicataldo removed the help wanted The team would welcome a contribution from the community for this issue label Mar 25, 2020
nzakas pushed a commit that referenced this issue Apr 8, 2020
* Fix: newBasePath should be an absolute path (fixes #12850)

* review feedback

* PR feeedback, add comments to test
@nickharris
Copy link
Contributor

@nzakas @kaicataldo do you have docs on your release pipeline? I am trying to understand rough timelines around when this will be available to consume from npmjs in an alpha build and then an official release?

@nzakas
Copy link
Member

nzakas commented Apr 14, 2020

The timeline for major releases is a bit ad-hoc, and given the current worldwide situation and team members dealing with that and other personal issues, I can’t really give you any specify guidance. We wanted to get 7.0.0 out by May, and the next alpha was supposed to be last week but we couldn’t get stuff done due to low availability.

@nickharris
Copy link
Contributor

no probs, completely understand the current situation was just curious so i can plan around it. take care, nick.

@kaicataldo
Copy link
Member

@nickharris v7.0.0 was released last Friday!

@nickharris
Copy link
Contributor

yup already using it :) thank you!

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Oct 6, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Oct 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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
None yet
Development

Successfully merging a pull request may close this issue.

4 participants