Page MenuHomePhabricator
Create Task
Maniphest T13007

Integrating `arc lint` and auto-fixers
Closed, InvalidPublic
Actions

Description

We have a linter that calls out to eslint to lint our javascript code. eslint has a --fix flag that can be used to auto-fix errors. arc lint also has the capacity to suggest fixes for errors, using a particular API call (or output format in our case, since we're using the pattern-and-file lint engine).

My quetsion is: is there a way to integrate eslint in a way that arc can take its fixing suggestions from eslint --fix? I guess the idea is that we'd run eslint --fix and have it emit the fixed code somewhere else, and do a diff, and then arc would suggest that diff.

Right now when we emit the control codes for arc auto-fixing, we do it on a lint-error-by-lint-error basis. I don't know if it's possible, or kosher, to do whole-file fixing suggestions like eslint wants to do.

I know that arcanist has a built-in jshint linter, but looking at the source it doesn't look like it does anything with the --fix flag either.

Related Objects

Event Timeline

csilvers created this task.Oct 10 2017, 5:11 PM
Herald added a subscriber: sophiebits. · View Herald TranscriptOct 10 2017, 5:11 PM
sophiebits added a comment.EditedOct 10 2017, 5:44 PM

At Facebook we have a custom arcanist lint engine that wraps eslint with a custom (JS) runner that extracts more info about each fix:

js
  var engine = new eslint.CLIEngine({baseConfig: config, useEslintrc: false});
  var results = engine.executeOnText(json.contents, json.path).results;

  // Pull out the original and replacement to pass to arcanist. This is
  // necessary since the futures system we're using doesn't track the original
  // file content. Ranges provided in the fix data from ESLint are for the whole
  // file and are not relative to the location of the reported error so we must
  // get the original content here and send it back to PHP.
  var messages = results[0].messages;
  messages.forEach((message) => {
    if (message.fix) {
      message.original =
        json.contents.slice(message.fix.range[0], message.fix.range[1]);
      message.replacement = message.fix.text;
    }
  });

and then stitch that back so the ArcanistLintMessage is created with line, char, original, and replacement.

csilvers added a comment.Oct 10 2017, 6:04 PM

Doh!, I was looking at the jshint engine before, not the eslint engine. It looks like phab doesn't have built-in support for eslint yet. But this looks promising: maybe the autofixing eslint reports *is* on an error-by-error basis.

epriestley closed this task as Invalid.Oct 10 2017, 6:23 PM
epriestley added a subscriber: epriestley.

Please file feature requests through the community forum (https://discourse.phabricator-community.org) or a support pact (https://admin.phacility.com/book/phacility/article/support_pacts/).

See T12134 for context on why we no longer use Maniphest as a catch-all channel for bug reports / feature requests / discussion.

csilvers added a comment.Oct 10 2017, 6:39 PM

Oops, I filed it in the wrong place, my apologies!

I think @sophiebits may have pointed us in the right direction though. I'll re-file if we can't figure this out ourselves.

joshuaspence added a subscriber: joshuaspence.Oct 16 2017, 11:59 AM
Phabricator · Built by Phacility