Page MenuHomePhabricator

Improve the flake8 line-matching regex
ClosedPublic

Authored by jparise on Mar 24 2017, 3:36 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Dec 10, 2:26 AM
Unknown Object (File)
Sun, Dec 8, 2:09 AM
Unknown Object (File)
Thu, Dec 5, 12:38 PM
Unknown Object (File)
Thu, Dec 5, 11:24 AM
Unknown Object (File)
Tue, Nov 26, 6:27 AM
Unknown Object (File)
Thu, Nov 21, 7:08 PM
Unknown Object (File)
Nov 10 2024, 12:45 PM
Unknown Object (File)
Oct 30 2024, 8:42 PM
Subscribers
Tokens
"Cup of Joe" token, awarded by epriestley.

Details

Summary

Because the character offset group is optional, the logic for dealing
with the previous index-based match object was more complex than it
needs to be. Switching to named groups makes this clearer.

As an example of how this was causing a problem, when the character
group *was* present (at index 3), it was being appending to the linter's
name in the call to ->setName(), resulting in confusing linter output.
We may have also been setting the character offset to the error code's
string, too, which is further nonsense.

Because we reliably capture the flake8 error code, I don't think there's
a need to append it to the linter's name at all now, so I removed that
part (so it's always just flake8), but it's not a problem to restore.

Lastly, I hoisted $regex out of the loop because it's a constant and
de-indenting it gave me enough room to continuing writing it all on one
line.

Test Plan

Tested primarily with flake8 3.3.0

Diff Detail

Repository
rARC Arcanist
Branch
flake8-regex (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 16131
Build 21409: Run Core Tests
Build 21408: arc lint + arc unit

Event Timeline

This can sneak through all the red tape in T10038 since it's mostly an unambiguous improvement.

Ideally, the "name" would be a short human-readable title for the error that's as specific as possible ("Undefined Variable") while the "description" provides more details ("The variable $x is not defined in this context. Define variables prior to use.") but they're both human-readable text so any reasonable behavior should be fine.

This revision is now accepted and ready to land.Mar 24 2017, 4:02 PM

Ideally, the "name" would be a short human-readable title for the error that's as specific as possible ("Undefined Variable") while the "description" provides more details ("The variable $x is not defined in this context. Define variables prior to use.") but they're both human-readable text so any reasonable behavior should be fine.

In that case, I'm going to restore the error code suffix because that more closely matches your ideal description of the field.

Restore the flake8 error code suffx

This revision was automatically updated to reflect the committed changes.