1. Prettier linter now doesn't show a warning for files that don't require any formatting changes (are already formatted)
2. ESLintLinter and PrettierLinter now don't throw an error because of return status code 1
3. Fix ESLinter failing because of missing source parameter in some of messages in eslint output

Yelp released an open source repo (https://github.com/Yelp/detect-secrets) which is used to detect potential secrets in code base. Adding this open source repo as part of pinterest linters will allow us to prevent users from committing secrets in Phabricator.

ruleId comes out to be None for babel-eslint. I fixed by changing to $offense['ruleId'] || 'unknown'

https://github.com/pinterest/arcanist-linters/blob/24c6667187f941dbe39c38efd5961f0a09b17f1b/src/ESLintLinter.php#L166

• Tweak the reported names to match the existing conventions
• Use --version to query the version string (and trim() it)
• Request --verbose JSON output to also report the rule names
• Use the rule name as the linter message's "code"
• Document that we now support v0.34.1 and later

When there are a lot of lint errors raised by graphql-schema-linter, the output JSON it produces can easily exceed the 2^16 character limit on the $stdout contents provided by Arcanist. The resulting truncated string will fail to JSON parse and the script will raise an error.

To correct for this, we check if there were any errors encountered in JSON decoding and raise these explicitly so the user can take action.

For this specific case, we see if the stdout buffer is at its maximum length and, if so, advise the author to run the linter outside of Arcanist and correct issues before running again.

Adds config and eslintConfig options to prettier-eslint linter.

This will allow us to set the path of the configuration files for prettier and eslint when using prettier-eslint linter.

First of all, thanks for publishing your linters!

I've been into an issue with the eslint linter: My .arclint enables the linter for "include": ["(\\.(j|t)sx?$)"], while excludes are defined in .eslintignore (so that other tools also respect them).

Some version of ESLint (IIRC last year) started emitting a warning so that if one invokes eslint some-ignored-file.js, it communicates to the user that it ignored it.

The warning doesn't have most fields set, so PHP emits a bunch of warnings when building ArcanistLintMessage. This change makes the linter just ignore that warning.

Fixes a bug introduced in #22.

$offense['ruleId'] || 'unknown' was returning a bool (true) which caused the following exception when arc tried to post the lint results via the harbormaster.sendmessage API:

[2019-10-15 01:34:16] EXCEPTION: (ConduitClientException) ERR-CONDUIT-CORE: Parameter 'name' has invalid type. Expected type 'string', got type 'bool'. at [<phutil>/src/conduit/ConduitFuture.php:62]
arcanist(head=master, ref.master=96fde137a1dc), linters(head=test, ref.master=81e8d886d5c8, ref.test=f20a8b42a357), phutil(head=master, ref.master=f434f57578dd)
  #0 <#2> ConduitFuture::didReceiveResult(array) called at [<phutil>/src/future/FutureProxy.php:58]
  #1 <#2> FutureProxy::getResult() called at [<phutil>/src/future/FutureProxy.php:35]
  #2 <#2> FutureProxy::resolve() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:2999]
  #3 phlog(ConduitClientException) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:3005]
  #4 ArcanistDiffWorkflow::updateAutotargets(string, NULL) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:537]
  #5 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:394]

In these cases, arc lint and arc diff would successfully run, but the diff created on Phabricator would not show any details about what the lint errors/warnings were, or what line numbers they were on.

With this fix, you can once again see the details of the lint errors and warnings in the Differential UI on Phabricator.

I also changed the default name (when $offense['ruleId'] is not defined) from unknown to ESLintLinter, and tested both cases: https://www.dropbox.com/s/0mb54bf5xfe34hi/Screenshot%202019-10-14%2019.20.51.png?dl=0

1. PrettierLinter: Don't warn for already formatted files
2. ESLintLinter: Don't throw on exit code 1
3. ESLintLinter: Fix error on occasional missing source parameter
4. ESLintLinter: Add --fix & --cwd config options
5. ESLintLinter, PrettierLinter, PrettierESLintLinter: Cleanup & normalize getDefaultBinary & getInstallInstructions
6. ESLintLinter, PrettierLinter, PrettierESLintLinter: Improve global fallback

Thanks @kuba-orlik (https://github.com/pinterest/arcanist-linters/pull/7/)

When there are autofixable lint errors detected by eslint, the order of operations is:

1. arc diff
2. if there are uncommited changes, amend or commit
3. arc lint, which autofixes and returns successful
4. autofixed changes are not committed nor staged
5. diff is pushed to phabricator without changes

This means that arc diff fails to properly apply lint changes before creating a diff.

See messages for a file provided by [email protected]

        "messages": [
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Replace `(flow.component·&&·flow.component.archived)` with `flow.component·&&·flow.component.archived`",
                "line": 61,
                "column": 10,
                "nodeType": null,
                "messageId": "replace",
                "endLine": 61,
                "endColumn": 53,
                "fix": {
                    "range": [
                        1462,
                        1505
                    ],
                    "text": "flow.component && flow.component.archived"
                }
            },
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Insert `(⏎··········`",
                "line": 65,
                "column": 13,
                "nodeType": null,
                "messageId": "insert",
                "endLine": 65,
                "endColumn": 13,
                "fix": {
                    "range": [
                        1675,
                        1675
                    ],
                    "text": "(\n          "
                }
            },
            {
                "ruleId": "prettier/prettier",
                "severity": 2,
                "message": "Replace `⏎⏎⏎········` with `········)`",
                "line": 66,
                "column": 1,
                "nodeType": null,
                "messageId": "replace",
                "endLine": 69,
                "endColumn": 9,
                "fix": {
                    "range": [
                        1685,
                        1696
                    ],
                    "text": "        )"
                }
            }
        ],

Directory structure:

/Users/mmark/code/my-repo
|-- .arclint
|-- client
    |-- node_modules
    |-- .eslintrc
|-- server
    |-- node_modules
    |-- .eslintrc

Before the change:

$ './client/node_modules/.bin/eslint' '--format=json' '--no-color' '--config' './client/.eslintrc' '--fix ' '/Users/mmark/code/my-repo/client/src/index.js'

Oops! Something went wrong! :(

ESLint: 6.8.0.

ESLint couldn't find the plugin "eslint-plugin-react".

(The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "/Users/mmark/code/my-repo".)

After the change:

$ './client/node_modules/.bin/eslint' '--format=json' '--no-color' '--config' './client/.eslintrc' '--fix ' '/Users/mmark/code/metro/client/src/index.js' '--resolve-plugins-relative-to' './client'

[successful output]

Using .arclint entry:

    "client-eslint": {
      "type": "eslint",
      "include": "(client/.*\\.js$)",
      "bin": "./client/node_modules/.bin/eslint",
      "eslint.config": "./client/.eslintrc",
      "eslint.fix": true,
      "eslint.cwd": "./client"
    }

This took me a while to figure out but the ESLintLinter uses the column from ESLint to tell Arc what to fix:

        $message->setChar(idx($offense, 'column'));

However this is the position of the error. It can actually be different to the position of the suggestion. For example with this code:

import { addTab, addIframeTab } from "./tabs.js";

ESLint will output:

      {
        "ruleId": "sort-imports",
        "severity": 2,
        "message": "Member 'addIframeTab' of the import declaration should be sorted alphabetically.",
        "line": 1,
        "column": 18,
        "nodeType": "ImportSpecifier",
        "messageId": "sortMembersAlphabetically",
        "endLine": 1,
        "endColumn": 30,
        "fix": {
          "range": [
            9,
            29
          ],
          "text": "addIframeTab, addTab"
        }
      },

It should use the 9 offset to calculate the column, but it actually uses 18, resulting in this silly patch:

  Auto-Fix  (ESLINT) sort-imports
    Member 'addIframeTab' of the import declaration should be sorted
    alphabetically.

    >>> -      1 import { addTab, addTab, addIframeom "./tabs.js";
        +        import { addTab, addIframeTab, addom "./tabs.js";

Unfortunately it doesn't look trivial to fix. My suggestion would be to apply all the fixes from the JSON in ESLintLinter and then just give the whole file as a single fix in one message.

I did want to just use the --fix flag but unfortunately it can add or remove lines and then the positions of the other errors are wrong (and Arc freaks out if they are beyond the end of the fixed file).

This is more of a question than a bug or feature request:

Since Phacility is winding down operations since June 1st, and therefore Phabricator is no longer actively maintained, how will this impact further development of this project?

Btw, there are early community discussions and efforts for setting up an Open Source fork project:

• Initial chat with Evan Priestley about future of Phabricator
• Phorge: Phabricator Fork Discussion Doc

Would arcanist-linters try to stay compatible with the fork?

If you use the prettier linter, it works fine with normal auto fixes, but if the yaml is invalid and prettier throws to stderr it causes the entire arc lint run to fail. I'll be looking into fixing this, but wanted to post here in case anyone comes looking.

EDIT: I noticed we extend a different base class internally, will follow up on if that is the root cause or not

I'm running arc lint with this config:

{
	"linters": {
		"prettier-eslint": {
			"type": "prettier-eslint",
			"bin": "./node_modules/.bin/prettier-eslint",
			"include": ["(\\.ts$)", "(\\.css$)"]
		}
	}
}

arc lint shows this error:

 Exception 
Error in parsing '.arclint' file, in key 'bin' for linter 'prettier-eslint'.
None of the configured binaries can be located.
(Run with `--trace` for a full exception trace.)

I've already installed prettier-eslint inside the repo

Hi there,

We (Dropbox) also use ESLint with arc and I happened to stumble upon this repo. We just open-sourced https://github.com/dropbox/eslint-formatter-arcanist as well that deals w/ autofix and arc byte char issue so if you guys are interested feel free to take a look.