• Problem 1: cpplint may raise messages at line 0, but arcanist can not render these messages after the severity of runtime-level errors was increased in D21044.

I solved this in D21289 by adding code to raise messages without a line number if cpplint raises them at line 0. This reveals two new problems:

• Problem 2: Arcanist does not check that messages emitted by linters have a valid line number.
• Problem 3: The cpplint unit tests fail, and the failure appears to be pre-existing and significant.

Here's a simplified reproduction case for "Problem 1":

{
  "linters": {
    "cpplint": {
      "type": "cpplint",
      "include": [
        "(\\.cpp$)",
        "(\\.h$)"
      ]
    }
  }
}

// example.h

$ arc lint example.h
 Exception 
Undefined offset: -1
(Run with `--trace` for a full exception trace.)

$ arc lint example.h
>>> Lint for example.h:


   Warning  (legal/copyright) legal/copyright
    No copyright message found.  You should have a line: "Copyright [year]
    <Copyright Owner>"

   Warning  (build/header_guard) build/header_guard
    No #ifndef header guard found, suggested CPP variable is: EXAMPLE_H_

• Problem 4: If a file ends in non-newline whitespace, the text linter may raise a message which removes that line. The renderer then tries to highlight part of it, which fatals.

Here's a reproduction case:

{
  "linters": {
    "text": {
      "type": "text",
      "include": [
        "(\\.txt$)"
      ]
    }
  }
}

<any file ending in spaces with no trailing newline>

$ arc lint --never-apply-patches example.txt --trace
...
[2020-05-27 11:21:31] EXCEPTION: (RuntimeException) Undefined offset: 11 at [<arcanist>/src/error/PhutilErrorHandler.php:258]
arcanist(head=master, ref.master=25ee39b657b8)
  #0 PhutilErrorHandler::handleError(integer, string, string, integer, array) called at [<arcanist>/src/lint/renderer/ArcanistConsoleLintRenderer.php:156]
  #1 ArcanistConsoleLintRenderer::renderContext(ArcanistLintMessage, string, array) called at [<arcanist>/src/lint/renderer/ArcanistConsoleLintRenderer.php:91]
  #2 ArcanistConsoleLintRenderer::renderLintResult(ArcanistLintResult) called at [<arcanist>/src/workflow/ArcanistLintWorkflow.php:296]
  #3 ArcanistLintWorkflow::run() called at [<arcanist>/scripts/arcanist.php:411]

Problem 3: The cpplint unit tests fail, and the failure appears to be pre-existing and significant.

Arcanist was written under the assumption that it could pass data directly to linters. When this assumption is true, it simplifies the code (linters don't need to interact with the filesystem), and plausibly improves performance somewhat (we only have to go to disk once, and could parallelize disk reads and linter dispatch), and makes linters easier to test. It allows lint to be executed against a version of the code not currently present in the working copy. It ensures linters are side-effect free, at least if they don't go crazy.

This assumption holds for first-party linters, which operate directly on data.

However, virtually no third-party linters actually work like this. Many linters must be invoked as linter path/to/example.c and some have behavior which depends on the name of the path.

The current test harness writes unit test data to an actual path on disk (a tempfile) and passes that to the linter. However, it does not rename the file, and doesn't pass the actual on-disk path to the linter anyway.

I'm going to update the test harness to better support linter path/to/example.c linters.