Page MenuHomePhabricator

[LINTER] Run selected linters last
AbandonedPublic

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

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

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
behavior.

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

Repository
rABC Bitcoin ABC
Branch
arcanist_run_last
Lint
No Lint Coverage
Unit
No Test Coverage
Build Status
Buildable 6281
Build 10609: Bitcoin ABC Buildbot (legacy)
Build 10608: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Jun 12 2019, 13:00
deadalnix added inline comments.
.runlast.arclint
18

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.
.runlast.arclint
18

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.
.runlast.arclint
18

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