[cashtab] add arcanist linter support for eslint
AbandonedPublic
Actions

Authored by majcosta on Apr 30 2021, 18:02.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Summary

basic eslint recommended settings configuration and a helpful .arclint from https://secure.phabricator.com/D12198

it does static analysis, catches js errors and is generally well regarded. we already have it as a devDependency for cashtab so we might as well run it and integrate with phab

Test Plan

arc diff this patch touching one file with a warning and a file with errors

~~fail to submit the patch if the errors aren't fixed~~ see all linter warnings and errors in the diff body

Diff Detail

Repository

rABC Bitcoin ABC

Branch

add_arc_lint_eslint

Lint

Lint Errors

Severity	Location	Code	Message
Error	web/cashtab/src/serviceWorker.js:1	no-undef	Lint
Error	web/cashtab/src/serviceWorker.js:5	no-undef	Lint
Error	web/cashtab/src/serviceWorker.js:6	no-undef	Lint
Error	web/cashtab/src/serviceWorker.js:14	no-undef	Lint
Warning	web/cashtab/config/webpack.config.js:1	strict	Lint

Unit

No Test Coverage

Build Status

Buildable 15609
Build 31134: Build Diff	cashtab-tests
Build 31133: arc lint + arc unit

Event Timeline

majcosta created this revision.Apr 30 2021, 18:02

Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 30 2021, 18:02

majcosta requested review of this revision.Apr 30 2021, 18:02

Harbormaster completed remote builds in B15609: Diff 28318.Apr 30 2021, 18:02

revert the linter complaints and green the diff

Harbormaster completed remote builds in B15610: Diff 28319.Apr 30 2021, 18:05

majcosta edited the test plan for this revision. (Show Details)Apr 30 2021, 18:06

majcosta edited the summary of this revision. (Show Details)Apr 30 2021, 18:46

majcosta planned changes to this revision.Apr 30 2021, 19:48

majcosta requested review of this revision.Apr 30 2021, 22:28

Fabien requested changes to this revision.May 3 2021, 06:43

Fabien added a subscriber: Fabien.

Fabien added inline comments.

.arclint
297 ↗	(On Diff #28319)	This is probably not doing what you expect. I suppose you meant `(\\.jsx?$)` ?
298 ↗	(On Diff #28319)	If it's empty you'd better remove it
299 ↗	(On Diff #28319)	What is the `$0` here ? There are a few reasons why I don't like script-and-regex, and here are some: There is no version checking There is no guarantee this will work on all platforms (does it run on macOS for example ?) Performance suffers from calling a shell for each file to process. This has proven to be a problem in the past. In this case it's even worst: PHP is calling a shell, which calls a shell, which calls two shells (see your parenthesis), one of them ending up calling `eslint`. I don't see a reason why this could not be a plain PHP linter that solves all of these issues. Let me know if you need help for writing it.