[PATCH] tests: add a "check-flake8" test for validating python code style

[Daniel P . Berrangé]

The flake8 program is a standard tool used by Python projects for
validating many commonly recommended style rules. It would be desirable
for QEMU to come into alignment with normal Python coding style best
practices.

QEMU currently violates a huge number of the style rules, so we can't
blindly turn it on. Instead this patch introduces use of flake8 with
a huge ignore list to turn off everything that is currently violated.

The following descriptions are mostly taken from:

  https://www.flake8rules.com/

Indentation:

 E111   Indentation is not a multiple of four
 E114   Indentation is not a multiple of four (comment)
 E115   Expected an indented block (comment)
 E116   Unexpected indentation (comment)
 E117   Over-indented
 E121   Continuation line under-indented for hanging indent
 E122   Continuation line missing indentation or outdented
 E123   Closing bracket does not match indentation of opening bracket's line
 E124   Closing bracket does not match visual indentation
 E125   Continuation line with same indent as next logical line
 E126   Continuation line over-indented for hanging indent
 E127   Continuation line over-indented for visual indent
 E128   Continuation line under-indented for visual indent
 E129   Visually indented line with same indent as next logical line
 E131   Continuation line unaligned for hanging indent

Whitespace:

 E201   Whitespace after '('
 E202   Whitespace before ')'
 E203   Whitespace before ':'
 E211   Whitespace before '('
 E221   Multiple spaces before operator
 E222   Multiple spaces after operator
 E225   Missing whitespace around operator
 E226   Missing whitespace around arithmetic operator
 E228   Missing whitespace around modulo operator
 E231   Missing whitespace after ',', ';', or ':'
 E241   Multiple spaces after ','
 E251   Unexpected spaces around keyword / parameter equals
 E261   At least two spaces before inline comment
 E265   Block comment should start with '# '
 E266   Too many leading '#' for block comment

Blank lines:

 E301   Expected 1 blank line, found 0
 E302   Expected 2 blank lines, found 0
 E303   Too many blank lines (3)
 E305   Expected 2 blank lines after end of function or class
 E306   Expected 1 blank line before a nested definition

Imports:

 E401   Multiple imports on one line
 E402   Module level import not at top of file

Line length:

 E501   Line too long (82 > 79 characters)
 E502   The backslash is redundant between brackets

Statements:

 E701   Multiple statements on one line (colon)
 E702   Multiple statements on one line (semicolon)
 E703   Statement ends with a semicolon
 E711   Comparison to none should be 'if cond is none:'
 E712   Comparison to true should be 'if cond is true:' or 'if cond:'
 E713   Test for membership should be 'not in'
 E714   Test for object identity should be 'is not'
 E722   Do not use bare 'except'
 E731   Do not assign a lambda expression, use a def
 E741   Do not use variables named 'I', 'O', or 'l'

Errors:

 F401   Module imported but unused
 F403   'from module import *' used; unable to detect undefined names
 F405   Name may be undefined, or defined from star imports: module
 F811   Redefinition of unused name from line n
 F821   Undefined name name
 F841   Local variable name is assigned to but never used

Warnings:

 W391   Blank line at end of file
 W503   Line break occurred before a binary operator
 W504   Line break occurred after a binary operator
 W605   Invalid escape sequence 'x'

Over time code should be fixed and rules removed from the ignore list.
A handful of style rules may not warrant fixing as the cure is arguably
worse and very subjective. e.g.

 E501: Force breaking lines at < 80 characters results in
       some really unnatural code formatting which harms
       readability.

 W504: Knuth code style requires the operators "or" and "and" etc
       to be at the start of line in a multi-line conditional.

Signed-off-by: Daniel P. Berrangé <address@hidden>
---

On its own this patch doesn't really do much of use except try to stop the
situation getting worse. To be valuable some motivated contributor(s)
would need to go through fixing the code, and re-enabling each excluded
warning category one at a time.

I'm mostly proposing this patch as a starting point for discussion, to
see if anyone is indeed motivated to take on the code cleanup challenge,
and feed the fixes in through the various maintainers trees.

 tests/Makefile.include | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 51de676298..f08e99b09c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -4,7 +4,7 @@
 check-help:
        @echo "Regression testing targets:"
        @echo
-       @echo " $(MAKE) check                Run unit, qapi-schema, qtest and 
decodetree"
+       @echo " $(MAKE) check                Run unit, qapi-schema, qtest and 
decodetree flake8"
        @echo
        @echo " $(MAKE) check-qtest-TARGET   Run qtest tests for given target"
        @echo " $(MAKE) check-qtest          Run qtest tests"
@@ -19,6 +19,7 @@ check-help:
        @echo " $(MAKE) check-report.tap     Generates an aggregated TAP test 
report"
        @echo " $(MAKE) check-venv           Creates a Python venv for tests"
        @echo " $(MAKE) check-clean          Clean the tests and related data"
+       @echo " $(MAKE) check-flake8         Clean the tests and related data"
        @echo
        @echo " $(MAKE) get-vm-images        Downloads all images used by 
acceptance tests, according to configured targets (~350 MB each, 1.5 GB max)"
        @echo
@@ -923,7 +924,40 @@ check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 ifeq ($(CONFIG_TOOLS),y)
 check-block: $(patsubst %,check-%, $(check-block-y))
 endif
-check: check-block check-qapi-schema check-unit check-softfloat check-qtest 
check-decodetree
+
+is_git := $(shell test -d $(SRC_PATH)/.git && echo 1 || echo 0)
+
+ifeq (1, $(is_git))
+PYTHON_FILES = $(shell git ls-tree -r HEAD: --name-only | grep '.py$$')
+PYTHON_FILES += $(shell git ls-tree -r HEAD: --name-only 
tests/qemu-iotests/??? | xargs grep -l '/usr/bin/env python')
+
+# Validate many python style rules
+FLAKE8_INDENTATION = 
E111,E114,E115,E116,E117,E121,E122,E123,E124,E125,E126,E127,E128,E129,E131
+FLAKE8_WHITESPACE = 
E201,E202,E203,E211,E221,E222,E225,E226,E228,E231,E241,E251,E261,E265,E266
+FLAKE8_BLANK_LINES = E301,E302,E303,E305,E306
+FLAKE8_IMPORTS = E401,E402
+FLAKE8_LINE_LENGTH = E501,E502
+FLAKE8_STATEMENTS = E701,E702,E703,E711,E712,E713,E714,E722,E731,E741
+FLAKE8_ERRORS = F401,F403,F405,F811,F821,F841
+FLAKE8_WARNINGS = W391,W503,W504,W605
+
+FLAKE8_IGNORE = $(FLAKE8_INDENTATION),$\
+               $(FLAKE8_WHITESPACE),$\
+               $(FLAKE8_BLANK_LINES),$\
+               $(FLAKE8_IMPORTS),$\
+               $(FLAKE8_LINE_LENGTH),$\
+               $(FLAKE8_STATEMENTS),$\
+               $(FLAKE8_ERRORS),$\
+               $(FLAKE8_WARNINGS) \
+               $(NULL)
+
+check-flake8:
+       $(call quiet-command,flake8 --ignore=$(FLAKE8_IGNORE) $(PYTHON_FILES))
+else
+check-flake8:
+endif
+
+check: check-block check-qapi-schema check-unit check-softfloat check-qtest 
check-decodetree check-flake8
 check-clean:
        rm -rf $(check-unit-y) tests/*.o tests/*/*.o $(QEMU_IOTESTS_HELPERS-y)
        rm -rf $(sort $(foreach target,$(SYSEMU_TARGET_LIST), 
$(check-qtest-$(target)-y:%=tests/qtest/%$(EXESUF))) 
$(check-qtest-generic-y:%=tests/qtest/%$(EXESUF)))
-- 
2.25.4