Page MenuHomePhabricator

[LINTER] Run selected linters last

Authored by Fabien on Jun 12 2019, 10:35.


Group Reviewers
Restricted Project

Now that the lint engine can be extended, it becomes possible to run the
lint workflow mutliple times with a different configuration in order to
have some linters run in the desired order.

To do so the lint engine is changed between each workflow run, and a
separated configuration is read which contains the linters for the run.

This diffs introduce a new configuration file, .runlast.arclint. The
linters to be run last (sequentially after the other linters) are moved
from the .arclint configuration file to this one, currently
clang-format and autopep8. These linters are autofix linters which
apply to the whole file, thus leading to a 100% conflict rate with other
autofix linters.

Calling arc lint is now equivalent to runnint it twice, the first time
with all the linters excepted clang-format and autopep8, the second
time only with clang-format and autopep8.

The call to arc diff is also modified to match the arc lint

Depends on D3298

Test Plan

Modify the src/init.cpp file. Add the line:

#include <stddef.h>

after the line:

#include <cstdint>

and before the line:

#include <cstdio>

Linting src/init.cpp now will result in a conflict, as clang-format
want to move the inclusion a line above to sort the includes, and the
cheader linter wants to replace stddeh.h with cstddef.
Before the patch, only the clang-format modification would have been
applied, after this patch, both are accounted:

arc lint --apply-patches -- src/init.cpp

arc diff --preview --trace

Check in the traces that the linters are run as expected.

arc diff --preview --trace --nolint

Check in the traces that no linter is run.

Diff Detail

rABC Bitcoin ABC
No Linters Available
No Unit Test Coverage
Build Status
Buildable 6281
Build 10609: Bitcoin ABC Teamcity Staging
Build 10608: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 12 2019, 10:35
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 12 2019, 10:35
deadalnix requested changes to this revision.Jun 12 2019, 13:00
deadalnix added inline comments.

The linter know what they are doing, so why does this need configuration? When there is only one possible configuration, then it's not a meaningful configuration, it's just bloat.

This revision now requires changes to proceed.Jun 12 2019, 13:00
Fabien requested review of this revision.Jun 12 2019, 13:14
Fabien added inline comments.

I don't get it.
This is not a configuration for the linters, but for the engine to know what linter to run.
It is not a fixed ,other linters may be added later.

deadalnix requested changes to this revision.Jun 12 2019, 17:03
deadalnix added inline comments.

It doesn't make any sense to run these linters in another manner, so it's clearly a property of the linter that dictate some behavior from the engine, not a configuration.

This revision now requires changes to proceed.Jun 12 2019, 17:03
Fabien abandoned this revision.Jul 24 2019, 06:41