Page MenuHomePhabricator

[LINTER] Fix the doxygen linter when inline comments are multilined
ClosedPublic

Authored by Fabien on Feb 10 2020, 15:50.

Details

Summary
Test Plan

Edit a source file to create a situation similar to
https://reviews.bitcoinabc.org/D5239?id=16218#inline-32123 and run:

arc lint -- <your_file>

Check the linter don't report any autofix.

Edit again to add a new line like:

//!< This should not occur

Check the linter autofixes this comment.

arc lint --everything

Diff Detail

Repository
rABC Bitcoin ABC
Branch
linter_doxygen_multiline
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9484
Build 16881: Default Diff Build & Tests
Build 16880: arc lint + arc unit

Event Timeline

deadalnix added inline comments.
arcanist/linter/DoxygenLinter.php
47 ↗(On Diff #16221)

Why the change? It's probably better to use the facilities of the framework you are working in.

arcanist/linter/DoxygenLinter.php
47 ↗(On Diff #16221)

They don't do the same, Filesystem::readFile() stores the content into a single string, while file() stores an array of lines. There is no equivalent in the Filesystem library.

deadalnix requested changes to this revision.Feb 11 2020, 16:42
deadalnix added inline comments.
arcanist/linter/DoxygenLinter.php
47 ↗(On Diff #16221)

Just explode the string.

BTW, this introduce an error condition in the case file returns false, which is exactly why you don't want to do this and just use readFile.

This revision now requires changes to proceed.Feb 11 2020, 16:42

Use the Filesystem facility and explode the string.

deadalnix requested changes to this revision.Feb 13 2020, 16:38
deadalnix added inline comments.
arcanist/linter/DoxygenLinter.php
47 ↗(On Diff #16350)

We are not using PHP_EOL, we are using unix style end of line.

This revision now requires changes to proceed.Feb 13 2020, 16:38
This revision is now accepted and ready to land.Feb 14 2020, 16:04