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

crash on malformed package.json #10364

Closed
jacekkopecky opened this issue May 17, 2018 · 7 comments · Fixed by #10376
Closed

crash on malformed package.json #10364

jacekkopecky opened this issue May 17, 2018 · 7 comments · Fixed by #10376
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

@jacekkopecky
Copy link

If package.json is malformed, eslint fails by crashing. This is relevant because then in Atom, every file that is to be checked by eslint is marked as invalid.

Tell us about your environment

  • ESLint Version: 4.19.1
  • Node Version: 9.8.0
  • npm Version: 6.0.1

What parser (default, Babel-ESLint, etc.) are you using?

default

Please show your full configuration:

Configuration

original package.json so I can npm i eslint:

{
  "name": "test",
  "version": "0.1.0",
  "description": "test eslint",
  "main": "app.js",
  "scripts": {
    "lint": "eslint ."
  },
  "author": "Jacek Kopecky <jacek@jacek.cz>",
  "engines": {
    "node": ">=8.7.0"
  },
  "devDependencies": {
    "eslint": "^4.19.1"
  }
}

No .eslintrc or anything like that.

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

I was editing package.json and introduced a syntax error - the usual extra comma (near the end of the file below).

{
  "name": "test",
  "version": "0.1.0",
  "description": "test eslint",
  "main": "app.js",
  "scripts": {
    "lint": "eslint ."
  },
  "author": "Jacek Kopecky <jacek@jacek.cz>",
  "engines": {
    "node": ">=8.7.0"
  },
  "devDependencies": {
    "eslint": "^4.19.1",
  }
}

Then I want to lint the project:

eslint .

What did you expect to happen?

Eslint should either fail gracefully with a warning because it can't parse package.json, or it assumes there's nothing in package.json of interest to eslint (which is correct) and proceeds to check the directory, which is otherwise empty so eslint succeeds quietly. If eslint fails, its output should clearly blame package.json in the normal »this file has a linting error« format.

What actually happened? Please include the actual, raw output from ESLint.

Crash.

Unexpected token } in JSON at position 275
SyntaxError: Unexpected token } in JSON at position 275
    at JSON.parse (<anonymous>)
    at new IgnoredPaths (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/ignored-paths.js:176:55)
    at lodash.memoize.optionsObj (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/util/glob-util.js:115:13)
    at memoized (/Users/kopeckyj/tmp/eslint/node_modules/lodash/lodash.js:10551:27)
    at globPatterns.forEach.pattern (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/util/glob-util.js:175:34)
    at Array.forEach (<anonymous>)
    at Object.listFilesToProcess (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/util/glob-util.js:158:18)
    at CLIEngine.executeOnFiles (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/cli-engine.js:518:35)
    at Object.execute (/Users/kopeckyj/tmp/eslint/node_modules/eslint/lib/cli.js:189:111)
    at Object.<anonymous> (/Users/kopeckyj/tmp/eslint/node_modules/eslint/bin/eslint.js:74:28)
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label May 17, 2018
@platinumazure platinumazure added bug ESLint is working incorrectly core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels May 17, 2018
@platinumazure
Copy link
Member

Hi @jacekkopecky, thanks for the issue. I agree this is a bug, and at the very least we should have a more sensible error message.

Feel free to submit a PR if you wish 😄

@jacekkopecky
Copy link
Author

Hi, I'd love to have a stab at it but I'm too busy with the day job until July easily.

@g-plane
Copy link
Member

g-plane commented May 18, 2018

Should the error stack be outputed?

@not-an-aardvark
Copy link
Member

I think it would be nice to output the error message, but outputting the stack is probably not necessary.

So the output could look something like this:

Failed to parse config file at /foo/bar/.eslintrc.json:

SyntaxError: Unexpected token } in JSON at position 275

@g-plane
Copy link
Member

g-plane commented May 18, 2018

How about removing the stack info before throwing error?

@not-an-aardvark
Copy link
Member

not-an-aardvark commented May 18, 2018

We already have code in place for pretty-printing error messages by adding a messageTemplate property to the thrown error -- if an error has a messageTemplate, then a corresponding message from the messages/ folder is printed rather than a stack trace.

For example, see:

eslint/lib/config.js

Lines 255 to 263 in a812845

const noConfigError = new Error("No ESLint configuration found.");
noConfigError.messageTemplate = "no-config-found";
noConfigError.messageData = {
directory,
filesExamined: localConfigFiles
};
throw noConfigError;

edit: the message templates are in the messages/ folder, not the templates/ folder.

@g-plane
Copy link
Member

g-plane commented May 18, 2018

Thanks! I'll check it.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Nov 26, 2018
@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 Nov 26, 2018
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