Python review_generator

Given a Python file containing a Python script written with bad style, this script will output a review adressing its problems.

```
RESERVED_KEYWORDS=['abs','dict','help','min','setattr','all','dir','hex','next','slice',
'any','divmod','id','object','sorted','ascii','enumerate','input','oct',
'staticmethod','bin','eval','int','open','str','bool','exec','isinstance',
'ord','sum','bytearray','filter','issubclass','pow','super','bytes','float',
'iter','print','tuple','callable','format','len','property','type','chr',
'frozenset','list','range','vars','classmethod','getattr','locals','repr','zip',
'compile','globals','map','reversed',
'__import__','complex','hasattr','max','round','delattr','hash','memoryview','set']

FILENAME = "code_with_bad_style.py"

BULTIN_REASSIGNED_ERROR = """You wrote:

{} = "something"

That is not good because {} is a built-in in Python
and you should never re-assign new values to the
built-ins, in case you are wondering wheter a word is a builtin or
not go to https://docs.python.org/3/library/functions.html to read the
complete list"""

NAME_NOT_USED_ERROR="""You should use

if __name__ == "__name__":
main()
So that your file is going to usable as both
a stand-alone programme and an importable programme.
"""

NO_DOCS_ERROR = """You should consider using some docstrings.
Docstrings are multiline comments that explain what a function does,
they are of great help for the reader. They look like the following:

def function(a, b):
\"\"\"Do X and return a list.\"\"\"
"""

USE_LIST_COMPREHENSION_ERROR = """In python there is
a very powerful language feature called [list comprehension][https://docs.python.org/2/tutorial/datastructures.html#list-comprehensions].
The following:

result = []
for i in lst:
if foo(i):
result.append(bar(i))

should be replaced with:

result = [bar(i) for i in lst if foo(i)]
"""

USE_WITH_ERROR = """There is a very convenient way of handling files in python:
the w

This code doesn't pass even basic PEP8 tests.
It makes it sound a bit a hypocritical ;-)

With the target filename hardcoded in the file,
this script is very inconvenient:

FILENAME = "code_with_bad_style.py"

Take a look at argparse, and make this script work with a filename parameter on the command line.

Several methods split the code to multiple lines, for example:

def no_docs_error(code):
    for line in code.splitlines():
        # ...

def print_breaking_3_error(code):
    for line in code.splitlines():
        # ...

With larger source code, this can be very wasteful.
It would be better to split to lines once in the beginning,
and pass the list to the methods that need a list instead of the single string version.

This check is completely wrong:

if line.startswith("import") and "*" not in line:
        return IMPORT_STAR_ERROR

... so this will match this kind of statement:

import re

... and it will NOT match this kind of statement:

from long_long_long_name import *

Dropping the "not" is not enough,
because the rule won't match what's you're trying to prevent.

It would be better to use a regex,
in this rule and many other rules.
For example, define at the top of the file with the other globals:

RE_IMPORT_STAR = re.compile(r'^from .* import \*')

Then do a check like this:

if RE_IMPORT_STAR.match(line):
    return IMPORT_STAR_ERROR

Many other tests could use regexes, for better judgment,
and often better performance too.

The rules you have defined are sometimes way too relaxed, for example:

for built_in in RESERVED_KEYWORDS:
    if built_in + " =" in code or built_in + "=" in code:
        return BULTIN_REASSIGNED_ERROR.format(built_in, built_in)

This won't match things like:

abs     = 'hello'

At the same time, this same rule makes this perfectly valid code fail:

parser.add_argument('-a', '--alphabet',
                    help='override the default alphabet')

There are many more similar examples in this code.

StackExchange Code Review Q#73554, answer score: 22