Page MenuHomePhabricator

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

Authored by Fabien on Mon, Feb 10, 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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Mon, Feb 10, 15:50
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Feb 10, 15:50
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.

Fabien added inline comments.Tue, Feb 11, 09:17
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.Tue, Feb 11, 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.Tue, Feb 11, 16:42
Fabien updated this revision to Diff 16350.Thu, Feb 13, 10:23

Use the Filesystem facility and explode the string.

deadalnix requested changes to this revision.Thu, Feb 13, 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.Thu, Feb 13, 16:38
Fabien updated this revision to Diff 16363.Thu, Feb 13, 19:58

Use Unix EOL.

deadalnix accepted this revision.Fri, Feb 14, 16:04
This revision is now accepted and ready to land.Fri, Feb 14, 16:04