pinterest / arcanist-linters

A collection of custom Arcanist linters
Apache License 2.0
62 stars 45 forks source link

ESLint linter assumes line and column are always present #62

Closed Timmmm closed 3 years ago

Timmmm commented 3 years ago

ESLint does not always set the line field, for example:

$ ./node_modules/.bin/eslint '--format=json' --no-color '--env ' 'browser,node' ci/.eslintrc.js
[{"filePath":"/Users/me/workspace/project/ci/.eslintrc.js","messages":[{"ruleId":null,"fatal":true,"severity":2,"message":"Parsing error: \"parserOptions.project\" has been set for @typescript-eslint/parser.\nThe file does not match your project config: ci/.eslintrc.js.\nThe file must be included in at least one of the projects provided."}],"errorCount":1,"warningCount":0,"fixableErrorCount":0,"fixableWarningCount":0,"source":"module.exports = {\n  rules: {\n    // Turn this rule so we don't need a tsconfig.json in this directory.\n    \"@typescript-eslint/strict-boolean-expressions\": \"off\",\n  }\n};\n","usedDeprecatedRules":[]}]

This is due to a sort-of configuration issue in our project but in any case, it's clear that ESLint can output messages with no line or column, however the code assumes that they are always present:

        $message->setLine($offense['line']);
        $message->setChar($offense['column']);

Which leads to this error:

[2021-02-16 14:53:04] EXCEPTION: (PhutilAggregateException) Some linters failed:
    - RuntimeException: Undefined index: line at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:264]
arcanist(head=master, ref.master=7378e2baad69), pinterest-linters(head=master, ref.master=63a85abc7049), rustfmt-linter(head=cmake_linter, ref.master=d9528d176f0b, ref.cmake_linter=d9528d176f0b)
  #0 <#2> PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<pinterest-linters>/src/ESLintLinter.php:140]
  #1 <#2> ESLintLinter::parseLinterOutput(string, integer, string, string) called at [<arcanist>/src/lint/linter/ArcanistExternalLinter.php:437]
  #2 <#2> ArcanistExternalLinter::resolveFuture(string, ExecFuture) called at [<arcanist>/src/lint/linter/ArcanistFutureLinter.php:35]
  #3 <#2> ArcanistFutureLinter::didLintPaths(array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:595]
  #4 <#2> ArcanistLintEngine::executeDidLintOnPaths(ESLintLinter, array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:546]
  #5 <#2> ArcanistLintEngine::executeLintersOnChunk(array, array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:474]
  #6 <#2> ArcanistLintEngine::executeLinters(array) called at [<arcanist>/src/lint/engine/ArcanistLintEngine.php:206]
  #7 ArcanistLintEngine::run() called at [<arcanist>/src/workflow/ArcanistLintWorkflow.php:219]
  #8 ArcanistLintWorkflow::run() called at [<arcanist>/scripts/arcanist.php:411]
jparise commented 3 years ago

@Timmmm give f0efe0a a spin. It should handle these cases much more safely now.