1387448 - [mozlint] Print results one file at a time

Status: NEW

(Developer Infrastructure :: Lint and Formatting, enhancement, P3)

Description

[Andrew Halberstadt [:ahal]]

The formatters currently store the lint errors in a single string that gets returned and printed at the end.

If there are a lot of errors, this can run into problems with python's buffering of sys.stdout and the output can be truncated. Instead, we should format + print the errors from one file at a time.

We can turn the formatters into generators to accomplish this.

Comment 1

[Mark Banner (:standard8)]

Would this help bug 1314453?

Comment 2

[Andrew Halberstadt [:ahal]]

No, that looks like a unicode error, probably because of the little 'x' that gets printed in the summary (we can test this by trying --formatter=treeherder).

Someone ran all the linters on thunderbird (with 1000s of errors), and the output was so large it gets truncated. This would fix that problem.

Comment 3

[Aditya Addepalli]

I think this file https://dxr.mozilla.org/mozilla-central/source/python/mozlint/mozlint/formatters/treeherder.py needs modifications to fix this error.
If there are too many errors, the message[] list object will be very large and cannot display all the errors in sys.stdout.
Should I change the call method in the Treeherder Formatter Class to yield a generator object? The user can maybe even input a value to see how many errors he wishes to see in std.stdout if the errors are too many.

Comment 4

[Andrew Halberstadt [:ahal]]

Yes, you got it! Make the __call__ method yield output, one error at a time.

You'll need to modify all the formatters though, not just treeherder.py. You'll also need to modify cli.run so that it still prints everything, something like:

for output in formatter(results, ...):
    print(output)

Let's not worry about providing an option to limit the number of errors displayed for now.

Comment 5

[Aditya Addepalli]

How do I test any changes in the code I wrote? I don't have anything specific to trigger the bug.

Comment 6

[Andrew Halberstadt [:ahal]]

Good question! I think we can replace sys.stdout with a new file object with an artificially small buffer. I haven't tested this, but maybe something like this:

old_stdout = sys.stdout
sys.stdout = os.fdopen(sys.stdout.fileno(), 'w', 100)
# do test
sys.stdout = old_stdout


You'd have to tweak bufsize to be big enough for 1 test result but too small for 2. Let me know if you have any trouble making progress with this and I'll take a deeper look. I'm not 100% sure this will work.

p.s thanks for thinking of tests!

Comment 7

[Aditya Addepalli]

I was thinking of this approach:
Currently the formatters return string objects, if I change them to return the list object instead, then I can run through this list and print the values out using a generator.

For example in treeherder.py,
I'll replace 
" return "\n".join(messages) "
with 
" return messages " which is a list object.

Then in cli.py run:
" 
complete_list = []
for output in formatter(results, failed=lint.failed).encode(sys.stdout.encoding or 'ascii', 'replace'):
    complete_list.append(output)
flat_list = [item for sublist in complete_list for item in sublist]

def generate(l):
    for item in l:
        yield item
"

and then print them using:

"
for output in generate(flat_list):
    print(output, end = '\n')
"

Essentially I am creating a list by appending the smaller lists retrieved from the formatters and then trying to print them using a generator.

This however seems very messy. Any ideas to where I can improve?

Comment 8

[Andrew Halberstadt [:ahal]]

That would work, but I think it would be better if the formatters were generators themselves. I think it would be reasonable if they yielded all the errors for a given file at once.

For example:

for path, errors in result.iteritems():
    message = []

    for err in errors:
        ...
        message.append(err)

    yield '\n'.join(message)


Then in cli.py:

    for output in formatter(...):
        print(output)


This is a bit of extra work as you'll need to update all the formatters, though it shouldn't be too bad.