add error code, aka "source" to the output

[braver]

Example output in this mode:

<?xml version="1.0" encoding="UTF-8"?>
<checkstyle version="3.12.0">
<file name="/peppered/domains/Dev/posthuistheater/tests/Core/Model/Site/Queue/BotsTest.php">
 <error line="15" column="138" severity="warning" message="Line exceeds 120 characters; contains 138 characters" source="Generic.Files.LineLength.TooLong"/>
</file>
</checkstyle>

This is trivial to parse, and also has the "source", which we need to make the quick action actually work.

[kaste]

They have various formatters. You could use csv. Python ships a csv reader.

import csv
from io import StringIO

csv_text = '''File,Line,Column,Type,Message,Source,Severity,Fixable
"/path/to/code/classA.php",2,1,error,"Missing file doc comment",PEAR.Commenting.FileComment.Missing,5,0
"/path/to/code/classA.php",4,12,error,"TRUE, FALSE and NULL must be lowercase; expected \"false\" but found \"FALSE\"",Generic.PHP.LowerCaseConstant.Found,5,1
"/path/to/code/classA.php",6,2,error,"Line indented incorrectly; expected at least 4 spaces, found 1",PEAR.WhiteSpace.ScopeIndent.Incorrect,5,1
"/path/to/code/classA.php",9,1,error,"Missing function doc comment",PEAR.Commenting.FunctionComment.Missing,5,0
"/path/to/code/classA.php",11,5,warning,"Inline control structures are discouraged",Generic.ControlStructures.InlineControlStructure.Discouraged,5,1'''

# Read CSV from string
csv_reader = csv.DictReader(StringIO(csv_text))

You would implement def find_errors(self, output):

    for match in csv_reader:
        yield LintMatch(
            match=match,
            filename=match['File'],
            line=int(match['Line']) - 1,  # apply line_col_base manually ??
            col=_try(lambda: int(match['Column']) - 1), ??
            error_type=match['Type'],
            code=match.get('Source', ''),
            message=match['Message'],
        )

Does that make sense at all? But if you find xml easytrivial to parse then throw the regex on it.

[braver]

But if you find xml easytrivial to parse then throw the regex on it.

Ha, well, "parse" meant loosely here. Of course using a regex here is loaded with assumptions (each error on a line, attributes in a certain order). With these kinds of things, that usually works well though.

I'll look into using the json output, that's more my speed than csv. Might even try to look into the other open issues 🤞🏻

[kaste]

csv is faster 😏

csv_reader = csv.DictReader(StringIO(output))

[braver]

Might be faster, but then I need to do more work to get rid of various escapes in strings (\" etc). Perhaps the CSV lib can do that for me, but couldn't find that info. The JSON lib takes care of that for me, so that ends up being a cleaner implementation IMO.

Edit: add a pretty screenshot :)

[kaste]

Ah, sorry, quotechar and delimiter would have been just options/kwargs to the DictReader.

Is this ready. Looks like it is.

[braver]

quotechar and delimiter would have been just options/kwargs to the DictReader

Delimiter is the default, so if I just specify quotechar like so:

for match in csv.DictReader(StringIO(output), quotechar='"'):

I still get escaped quotes in the output:

Is this ready

Yeah it's ready. I'd prefer to keep it with the json output rather than spend more time with the csv mode. It's not like it's problematically slow (doesn't seem like a problem anyway).

[kaste]

Hm. Actually, what is the change here? The error code was in regex before. You added that in #51

[braver]

Actually, what is the change here

Good point. Guess it stopped working (perhaps for a given version of phpcs the output changed?)...
This is before this PR, it just reports "error" instead of the actual error code:

[kaste]

Have you -s on or off?

[braver]

Oh... guess I didn't read the readme. Granted, it's been 5 years 😅 And nobody reads readmes 😉 .

Why would you have to turn on? Also it makes the quick action not work at all (it will insert something like ignore error which doesn't do anything).

More than 40 chars is excessive

You know, that's a fair point, but without the error codes it leaves the plugin in a half broken state IMO. Maybe aesthetically less pleasing, but error codes are useful even if they're ugly and long, and capturing "error" or "warning" instead is just wrong.

Huh, ok... so I guess this PR disagrees with me from 5 years ago. I like to think I learned and improved in the mean time 😉 If you want to keep it as is that's fine, I'll fork it.

[kaste]

Let's take it. Less options.