Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Flake8 3.0+ multiple-letter violation codes are not displayed #2163

Closed
myii opened this issue Apr 1, 2018 · 5 comments
Closed

Flake8 3.0+ multiple-letter violation codes are not displayed #2163

myii opened this issue Apr 1, 2018 · 5 comments

Comments

@myii
Copy link

myii commented Apr 1, 2018

Standard format of Flake8 violation codes

Until recently, the standard format was a letter followed by 3 digits, e.g. E501.

Flake8 3.0+ doesn't enforce 4 character long violation codes

https://gitlab.com/pycqa/flake8/issues/337

I believe the R prefix is used by radon if memory serves me well. Perhaps use RST as the prefix. Flake8 3.0+ doesn't enforce 4 character long violation codes.

  • That's a quote from the author of Flake8 (emphasis above is mine)

Some of the Flake8 plugins using multiple-letter violation codes

Violation code range Plugin
CNLxxx flake8-class-newline
FIxx flake8-future-import
IESxxx flake8-invalid-escape-sequences
RSTxxx flake8-rst-docstrings

Syntastic not displaying multiple-letter violations

Based upon the output of running flake8 from the command line:

$ flake8
./.../urls.py:1:1: FI17 __future__ import "generators" missing
./.../urls.py:1:1: D205 1 blank line required between summary line and description
./.../urls.py:4:1: RST301 Unexpected indentation.
./.../urls.py:5:1: RST201 Block quote ends without a blank line; unexpected unindent.
./.../urls.py:7:1: RST301 Unexpected indentation.
./.../urls.py:9:1: RST201 Block quote ends without a blank line; unexpected unindent.

Syntastic only displays the following:

urls.py|1 col 1 warning| 1 blank line required between summary line and description [D205]

Attempts to fix syntax_checkers/python/flake8.vim

Realised the problem was with errorformat:

@@ -23,6 +23,10 @@
 
     let errorformat =
         \ '%E%f:%l: could not compile,%-Z%p^,' .
+        \ '%A%f:%l:%c: %t%*[A-Z]%n: %m,' .
+        \ '%A%f:%l:%c: %t%*[A-Z]%n %m,' .
+        \ '%A%f:%l: %t%*[A-Z]%n: %m,' .
+        \ '%A%f:%l: %t%*[A-Z]%n %m,' .
         \ '%A%f:%l:%c: %t%n: %m,' .
         \ '%A%f:%l:%c: %t%n %m,' .
         \ '%A%f:%l: %t%n: %m,' .

But while all of the errors were now being displayed:

urls.py|1 col 1 error| __future__ import "generators" missing [F017]            
urls.py|1 col 1 warning| 1 blank line required between summary line and description [D205]
urls.py|4 col 1 warning| Unexpected indentation. [R301]                         
urls.py|5 col 1 warning| Block quote ends without a blank line; unexpected unindent. [R201]
urls.py|7 col 1 warning| Unexpected indentation. [R301]                         
urls.py|9 col 1 warning| Block quote ends without a blank line; unexpected unindent. [R201]

The results were unsatisfactory since the codes were incorrect:

  1. F017 was supposed to be FI17
  2. R301 was supposed to be RST301
  3. R201 was supposed to be RST201

Wasn't able to figure out how to pass these through. I had to resort to disabling errorformat entirely (no idea what the repercussions of this are):

@@ -33,8 +37,8 @@
 
     let loclist = SyntasticMake({
         \ 'makeprg': makeprg,
-        \ 'errorformat': errorformat,
         \ 'env': env })
+        " \ 'errorformat': errorformat,
 
     for e in loclist
         " E*** and W*** are pep8 errors

The output isn't perfect but at least it contains all of the required information:

urls.py|1 col 1 warning| FI17 __future__ import "generators" missing [-01]      
urls.py|1 col 1 warning| D205 1 blank line required between summary line and description [-01]
urls.py|4 col 1 warning| RST301 Unexpected indentation. [-01]                   
urls.py|5 col 1 warning| RST201 Block quote ends without a blank line; unexpected unindent. [-01]
urls.py|7 col 1 warning| RST301 Unexpected indentation. [-01]                   
urls.py|9 col 1 warning| RST201 Block quote ends without a blank line; unexpected unindent. [-01]

Is it possible to use errorformat and output the correct codes?

@peterjc
Copy link

peterjc commented Apr 1, 2018

See also https://gitlab.com/pycqa/flake8/issues/339 for getting the prefixes guidelines described in the flake8 plugin development guide.

@lcd047
Copy link
Collaborator

lcd047 commented Apr 2, 2018

Please try b01520f. Keep reading you care about the rationale.

Vim error types (matched by %t) are single letter. Multiple letter types in quickfix lists are errors. Three of these types have a special meaning: E, W, and I (and their lower case versions): they tell Vim to add error, warning, and info when displaying quickfix lists in error windows.

Vim error numbers (matched by %n) are essentially useless. They waste space in error windows, and can be abused to provide additional information to the caller when matching errors, but Vim itself doesn't have any particular use for them.

Syntastic recognizes two error types, E and W. It doesn't recognize I, for historic reasons. Any other type is an error. Also syntastic can flag messages as "style" messages. So basically the entire list of flake8 errors has to be mapped to 2 * 2 = 4 types of messages. Thus the patch above. Mapping messages to errors and warnings, and to style and syntax for all possible values probably still needs some work.

@myii
Copy link
Author

myii commented Apr 2, 2018

@lcd047 Thanks for getting back. That's definitely a lot better:

urls.py|1 col 1 error| __future__ import "generators" missing [FI017]                                                                                                                                            
urls.py|1 col 1 warning| 1 blank line required between summary line and description [D205]
urls.py|4 col 1 warning| Unexpected indentation. [RST301]                       
urls.py|5 col 1 warning| Block quote ends without a blank line; unexpected unindent. [RST201]
urls.py|7 col 1 warning| Unexpected indentation. [RST301]                       
urls.py|9 col 1 warning| Block quote ends without a blank line; unexpected unindent. [RST201]
  • Note, still have an issue with FI017, which is actually FI17
    • The codes from that plugin are always 4 characters long: 2 letters followed by 2 digits
    • Due to %03d? Can that be made to work with any number of digits?
  • Also, the same line is being defined as an error
    • With my trivial understanding, F is supposed to be an error
    • Whereas FI would be considered to be a warning

To test that, I modified the plugin to use SI instead of FI and the violation was displayed as a warning instead:

urls.py|1 col 1 warning| __future__ import "generators" missing [SI017]

As I understand it, this error / warning determination is being performed by this line:

        let e['type'] = e['type'] =~? '\m^[EFHC]' ? 'E' : 'W'

Potentially, numerous plugins could use codes commencing with one of these letters (i.e. EFHC), so could the check be performed on the whole of the (1, 2 or 3-lettered) prefix instead? As in, up to the first digit?

Otherwise, I've also tested this with faked 3-letter codes and all seems well:

urls.py|36 col 1 warning| Try, Except, Pass detected. [BAN110]

@lcd047
Copy link
Collaborator

lcd047 commented Apr 2, 2018

Due to %03d?

This was needed when parsing the number with %n, but I suppose the original format is fine anyway. Please try 2c02edd. It may introduce minor backward incompatibilities for people that use quiet_messages though.

Can that be made to work with any number of digits?

Yes, %03d can print any number of digits.

Whereas FI would be considered to be a warning

As I said: mapping messages to errors and warnings still needs some work. Trying to handle all possible combinations though is unmaintainable. Thus the decision is based only on the first letter.

@myii
Copy link
Author

myii commented Apr 2, 2018

@lcd047 Brilliant, that's working:

urls.py|1 col 1 error| __future__ import "generators" missing [FI17]

Yes, %03d can print any number of digits.

  • I was unclear, I meant without padding zeros -- but that's irrelevant now, with your new fix

As I said: mapping messages to errors and warnings still needs some work. Trying to handle all possible combinations though is unmaintainable. Thus the decision is based only on the first letter.

  • Fair enough, it isn't as if Flake8 differentiates between the types of error anyway

With that, I believe this issue has been resolved so I'll go ahead and close it.

Thanks again.

@myii myii closed this as completed Apr 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants