Skip to content

[PEP08] Project PEPin! #1277

@guedou

Description

@guedou

Goals

During yesterday Scapy meetup, we discussed how we can improve the current state of Scapy source style with the following constraints:

  • limit the effects on git history
  • perform incremental changes
  • ease reviewing
  • aim to fix a single type of error per pull-request, and explain why some are not fixed
  • enforce PEP08 checks
  • credit original authors

Methodology

We performed a flake8 run on commit f89055dac106f79f8892a33eca2e8b0ac29475ff that shows 21733 errors made of 55 different errors/warnings. Scapy LoC is currently 38k.

We decided to classify the errors type into three categories:

  1. immediate fixes: errors that not related to the style and can be fixed by regular commits by anyone;
  2. to be discussed: these errors need to be fixed carefully. Applying a linter might lead to bugs or hard to review changes;
  3. style related: we wish to do a single commit per error. This commit will fix all lines concerned by this error, and will be authored by 'Phil phil@secdev.org'. The error message will take advantage of the 'Co-authored-by' to credit original authors. For example, the following message will be used for E231:
    E231 - missing whitespace after ','
    
    Co-authored-by: gpotter2 <10530980+gpotter2@users.noreply.github.com>
    Co-authored-by: Guillaume Valadon <guillaume@valadon.net
    Co-authored-by: Philippe ROSE <shadawan@gmail.com>
    Co-authored-by: Francois Contat <francois.contat@ssi.gouv.fr>
    [..]
    

Automatic tests will be added and enforced as errors are fixed. In the mean time, the following command can be used to check for already fixed errors:
tox -e flake8

Stats

Here are the errors that need to be fixed. Comments, help and feedback are welcome!

  • immediate fixes
% Count Type & example
0.234666 51 F821 undefined name 'long'
0.138039 30 F601 dictionary key 'ldapUrl' repeated with different values
0.10583 23 F841 local variable 'name' is assigned to but never used
0.0782221 17 F811 redefinition of unused 'PcapTimeoutElapsed' from line 68
0.0046013 1 F402 import 'conf' from line 30 shadowed by loop variable
  • style related
% Count Type & example
18.1521 3945 E231 missing whitespace after ','
12.4971 2716 E501 line too long (80 > 79 characters)
9.53849 2073 E302 expected 2 blank lines, found 1
7.40349 1609 E201 whitespace after '{'
6.57065 1428 E203 whitespace before ';'
5.28229 1148 E128 continuation line under-indented for visual indent
5.19947 1130 E202 whitespace before '}'
4.85897 1056 E266 too many leading '#' for block comment
4.81296 1046 E301 expected 1 blank line, found 0
2.72397 592 E225 missing whitespace around operator
2.4709 537 E251 unexpected spaces around keyword / parameter equals
2.45709 534 W293 blank line contains whitespace
1.89113 411 E261 at least two spaces before inline comment
1.73929 378 E111 indentation is not a multiple of four
1.73929 378 E265 block comment should start with '# '
1.72549 375 W291 trailing whitespace
0.980076 213 E305 expected 2 blank lines after class or function definition, found 1
0.92026 200 E127 continuation line over-indented for visual indent
0.81443 177 E303 too many blank lines (2)
0.561358 122 E221 multiple spaces before operator
0.478535 104 E262 inline comment should start with '# '
0.414117 90 E227 missing whitespace around bitwise or shift operator
0.39111 85 E124 closing bracket does not match visual indentation
0.28528 62 W391 blank line at end of file
0.276078 60 E131 continuation line unaligned for hanging indent
0.271477 59 E228 missing whitespace around modulo operator
0.266875 58 W191 indentation contains tabs
0.230065 50 E114 indentation is not a multiple of four (comment)
0.207058 45 E129 visually indented line with same indent as next logical line
0.174849 38 E401 multiple imports on one line
0.174849 38 E701 multiple statements on one line (colon)
0.133438 29 E115 expected an indented block (comment)
0.128836 28 E122 continuation line missing indentation or outdented
0.119634 26 E711 comparison to None should be 'if cond is None:'
0.115032 25 E306 expected 1 blank line before a nested definition, found 0
0.115032 25 E702 multiple statements on one line (semicolon)
0.0966272 21 E222 multiple spaces after operator
0.092026 20 E713 test for membership should be 'not in'
0.0874247 19 E116 unexpected indentation (comment)
0.0736208 16 E271 multiple spaces after keyword
0.0322091 7 E272 multiple spaces before keyword
0.0322091 7 E703 statement ends with a semicolon
0.0092026 2 E101 indentation contains mixed spaces and tabs
0.0092026 2 E211 whitespace before '('
0.0046013 1 E712 comparison to True should be 'if cond is True:' or 'if cond:'
  • to be discussed
% Count Type & example
1.70708 371 E741 ambiguous variable name 'l'
0.634979 138 E722 do not use bare except
0.363503 79 E402 module level import not at top of file
0.138039 30 E731 do not assign a lambda expression, use a def
0.0368104 8 E502 the backslash is redundant between brackets

Metadata

Metadata

Assignees

No one assigned

    Labels

    PEPinApply PEP08 rulesmajorMajor changes

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions