Page MenuHomePhabricator
Create Task
Maniphest T6311

'arc linters' no longer reports linters as 'Configured', all show as 'Available'
Closed, ResolvedPublic
Actions

Description

The output of 'arc linters' no longer correctly differentiates configured linters from available ones.

Having traced this through a little, it looks like the 'built_linters' array at https://secure.phabricator.com/diffusion/ARC/browse/master/src/lint/engine/ArcanistConfigurationDrivenLintEngine.php$46 does not get any entries as '$this->getPaths()' is empty in the Linters workflow.

It feels like the presence or otherwise of a linter should probably not depend on the paths at all.

Steps to reproduce:
Create a .arclint file with the following contents:

{
  "linters": {
    "generated": {
      "type": "generated"
    }
  }
}

Expected output from 'arc linters':

CONFIGURED  generated (Generated Code)

Actual output:

AVAILABLE   generated (Generated Code)

Revisions and Commits

rARC Arcanist
D10773rARC6f7bedaceb68 Fix linter config check

Related Objects
Search...

Event Timeline

agenticarus created this task.Oct 15 2014, 1:03 AM
agenticarus raised the priority of this task from to Needs Triage.
agenticarus updated the task description. (Show Details)
agenticarus added a project: Arcanist.
agenticarus added a subscriber: agenticarus.
• stokersara moved this task from Backlog to In Progress on the Arcanist board.Oct 25 2014, 7:14 AM
• stokersara moved this task from In Progress to Backlog on the Arcanist board.
joshuaspence added a subscriber: joshuaspence.Oct 26 2014, 3:44 AM
chad added subscribers: avivey, chad.Nov 1 2014, 5:00 PM

I've traced this back to D9843

avivey claimed this task.Nov 2 2014, 1:15 AM

buildLinters() in ConfigurationDrivenLintEngine is a derivative of LintEngine, and was used to be used to get all the linters that should run. LintEngine itself is somewhat legacy, and almost all of its use-cases are now covered with ConfigurationDrivenLintEngine.

D9843 was meant to fix T5124, and more specifically, fixing running lint on arcanist/phabricator repos.

A cleaner change would be in https://secure.phabricator.com/diffusion/ARC/browse/master/src/lint/engine/ArcanistLintEngine.php;52947cfd92b331916adffa1ff3233546446c881b$219-226, remove linters that have no paths set, instead of D9843.

avivey added a comment.Nov 2 2014, 1:57 AM

An even cleaner one would be to fix ExternalLinter to check if there's something to execute before trying to get a version...

avivey added a revision: D10773: Fix linter config check.Nov 2 2014, 2:25 AM
chad triaged this task as Normal priority.Nov 2 2014, 8:00 PM
snoyl8 moved this task from Backlog to In Progress on the Arcanist board.Nov 4 2014, 3:45 PM
snoyl8 moved this task from In Progress to Completed on the Arcanist board.
snoyl8 moved this task from Completed to In Progress on the Arcanist board.
snoyl8 moved this task from In Progress to Backlog on the Arcanist board.
chad added a project: Lint.Dec 15 2014, 4:24 AM
joshuaspence mentioned this in D10773: Fix linter config check.Jan 4 2015, 9:26 PM
joshuaspence added a parent task: T6061: `arc linters` should do basic environment checking.Jan 9 2015, 10:44 PM
epriestley closed this task as Resolved by committing rARC6f7bedaceb68: Fix linter config check.May 5 2015, 9:03 PM
epriestley added a commit: rARC6f7bedaceb68: Fix linter config check.
Phabricator · Built by Phacility