Page MenuHomePhabricator

Allow linters to run once
ClosedPublic

Authored by Fabien on Jul 22 2019, 16:35.

Details

Summary

This diff replaces the lint engine to override the linter paths to run
on. Only the first path is kept so the linter will effectively run once.

This is achieved by extending the GlobalExternalLinter class
and implementing the parseGlobalLinterOutput method. The check-doc
linter is the first to benefit this feature.

Test Plan

Comment one of the exceptions in test/lint/check-doc.py.

arc lint --everything --trace 2>&1 | grep "check-doc"

Look at the traces for Linter 'check-doc' will run once..
Check the linter return the error from the commented exception only
once.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Jul 22 2019, 18:43

Sounds like a hacky, but workable approach. Please reword the comment as it currently makes no sense. Otherwise, LGTM.

arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
25 ↗(On Diff #10374)

in order to run.

26 ↗(On Diff #10374)

anyone => any

27 ↗(On Diff #10374)

You probably want to reword this whole paragraph.

This revision now requires changes to proceed.Jul 22 2019, 18:43
deadalnix added inline comments.
arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
21 ↗(On Diff #10384)

You probably want to use instanceof to be type safe rather than reflection.

This revision is now accepted and ready to land.Jul 23 2019, 00:12
arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
21 ↗(On Diff #10384)

Doing so will prevent adding a (non external) GlobalLinter that extends ArcanistLinter, or you have to add another case for it.
Getting type safety with PHP is fighting a losing battle.

arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
21 ↗(On Diff #10384)

No, you can do it with an interface.

Define an ILintOnce interface and check for type instead of using reflection.

arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
21 ↗(On Diff #10384)

Nice, I didn't know that instanceof worked with interfaces.

arcanist/engine/ExtendedConfigurationDrivenLintEngine.php
7 ↗(On Diff #10402)
ArcanistConfigurationDrivenLintEngine::class

Address feedbacks, use the interface to define the global linter.

Rebase to account for the new CI configuration.

This revision was automatically updated to reflect the committed changes.