Return to Answer

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 *

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.

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.

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 "*" 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 *

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.

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 *

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.

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 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 "*" 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 *

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.