Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tested code valid Python code #657

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 24 additions & 26 deletions pycodestyle.py
Original file line number Diff line number Diff line change
Expand Up @@ -871,17 +871,15 @@ def whitespace_around_named_parameter_equals(logical_line, tokens):
Don't use spaces around the '=' sign when used to indicate a
keyword argument or a default parameter value.

Okay: def complex(real, imag=0.0):
Okay: return magic(r=real, i=imag)
Okay: def complex(real, imag=0.0):\n return magic(r=real, i=imag)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were two separate checks for two separate test cases. Collapsing them may introduce bugs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whole thing is still tested, isn't it ? Or I can write:

def complex(real, imag=0.0): pass

and

magic(r=real, i=imag)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather retain two separate tests cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am suggesting 2 separate test cases, each of them corresponding to a piece of valid Python code.

Okay: boolean(a == b)
Okay: boolean(a != b)
Okay: boolean(a <= b)
Okay: boolean(a >= b)
Okay: def foo(arg: int = 42):
Okay: async def foo(arg: int = 42):
Okay: def foo(arg: int = 42):\n pass
Okay: async def foo(arg: int = 42):\n pass

E251: def complex(real, imag = 0.0):
E251: return magic(r = real, i = imag)
E251: def complex(real, imag = 0.0):\n return magic(r = real, i = imag)
"""
parens = 0
no_space = False
Expand Down Expand Up @@ -1148,7 +1146,7 @@ def break_around_binary_operator(logical_line, tokens):

Okay: (width == 0 +\n height == 0)
Okay: foo(\n -x)
Okay: foo(x\n [])
Okay: foo(x,\n [])
Okay: x = '''\n''' + ''
Okay: foo(x,\n -y)
Okay: foo(x, # comment\n -y)
Expand Down Expand Up @@ -1192,11 +1190,11 @@ def comparison_to_singleton(logical_line, noqa):
Comparisons to singletons like None should always be done
with "is" or "is not", never the equality operators.

Okay: if arg is not None:
E711: if arg != None:
E711: if None == arg:
E712: if arg == True:
E712: if False == arg:
Okay: arg is not None
E711: arg != None
E711: None == arg
E712: arg == True
E712: False == arg

Also, beware of writing if x when you really mean if x is not None --
e.g. when testing whether a variable or argument that defaults to None was
Expand Down Expand Up @@ -1248,15 +1246,15 @@ def comparison_type(logical_line, noqa):

Do not compare types directly.

Okay: if isinstance(obj, int):
E721: if type(obj) is type(1):
Okay: isinstance(obj, int)
E721: type(obj) is type(1)

When checking if an object is a string, keep in mind that it might be a
unicode string too! In Python 2.3, str and unicode have a common base
class, basestring, so you can do:

Okay: if isinstance(obj, basestring):
Okay: if type(a1) is type(b1):
Okay: isinstance(obj, basestring)
Okay: type(a1) is type(b1)
"""
match = COMPARE_TYPE_REGEX.search(logical_line)
if match and not noqa:
Expand All @@ -1270,9 +1268,9 @@ def comparison_type(logical_line, noqa):
def bare_except(logical_line, noqa):
r"""When catching exceptions, mention specific exceptions when possible.

Okay: except Exception:
Okay: except BaseException:
E722: except:
Okay: try:\n pass\nexcept Exception:\n pass
Okay: try:\n pass\nexcept BaseException:\n pass
E722: try:\n pass\nexcept:\n pass
"""
if noqa:
return
Expand Down Expand Up @@ -1301,10 +1299,10 @@ def ambiguous_identifier(logical_line, tokens):
function definitions, 'global' and 'nonlocal' statements, exception
handlers, and 'with' statements.

Okay: except AttributeError as o:
Okay: with lock as L:
E741: except AttributeError as O:
E741: with lock as l:
Okay: try:\n pass\nexcept AttributeError as o:\n pass
Okay: with lock as L:\n pass
E741: try:\n pass\nexcept AttributeError as O:\n pass
E741: with lock as l:\n pass
E741: global I
E741: nonlocal l
E742: class I(object):
Expand Down Expand Up @@ -1340,7 +1338,7 @@ def ambiguous_identifier(logical_line, tokens):
def python_3000_has_key(logical_line, noqa):
r"""The {}.has_key() method is removed in Python 3: use the 'in' operator.

Okay: if "alph" in d:\n print d["alph"]
Okay: "alph" in d
W601: assert d.has_key('alph')
"""
pos = logical_line.find('.has_key(')
Expand Down Expand Up @@ -1368,8 +1366,8 @@ def python_3000_not_equal(logical_line):

The older syntax is removed in Python 3.

Okay: if a != 'no':
W603: if a <> 'no':
Okay: a != 'no'
W603: a <> 'no'
"""
pos = logical_line.find('<>')
if pos > -1:
Expand Down
4 changes: 3 additions & 1 deletion testsuite/E11.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#: E111
if x > 2:
print x
print(x)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means we'll loose coverage for the print statement then, yes?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I fully understand what you mean. The E11 errors does not seem to be related to print and/or parentheses in any way.

#: E111
if True:
print
#: E112
# Potential E901
if False:
print
#: E113
# Potential E901
print
print
#: E114 E116
Expand Down
52 changes: 26 additions & 26 deletions testsuite/E12.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
#: E121
print "E121", (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same concern as above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If parentheses are important for these tests (which is likely), maybe I could replace the

print expression

with

raise expression

or

assert expression

"dent")
print("E121", (
"dent"))
#: E122
print "E122", (
"dent")
print("E122", (
"dent"))
#: E123
my_list = [
1, 2, 3,
4, 5, 6,
]
#: E124
print "E124", ("visual",
print("E124", ("visual",
"indent_two"
)
))
#: E124
print "E124", ("visual",
print("E124", ("visual",
"indent_five"
)
))
#: E124
a = (123,
)
Expand All @@ -25,20 +25,20 @@
col < 0 or self.moduleCount <= col):
raise Exception("%s,%s - %s" % (row, col, self.moduleCount))
#: E126
print "E126", (
"dent")
print("E126", (
"dent"))
#: E126
print "E126", (
"dent")
print("E126", (
"dent"))
#: E127
print "E127", ("over-",
"over-indent")
print("E127", ("over-",
"over-indent"))
#: E128
print "E128", ("visual",
"hanging")
print("E128", ("visual",
"hanging"))
#: E128
print "E128", ("under-",
"under-indent")
print("E128", ("under-",
"under-indent"))
#:


Expand All @@ -63,11 +63,11 @@
4 + \
5 + 6
#: E131
print "hello", (
print("hello", (

"there",
# "john",
"dude")
"dude"))
#: E126
part = set_mimetype((
a.get('mime_type', 'text')),
Expand Down Expand Up @@ -100,11 +100,11 @@
or another_very_long_variable_name:
raise Exception()
#: E122
dictionary = [
dictionary = {
"is": {
"nested": yes(),
},
]
}
#: E122
setup('',
scripts=[''],
Expand All @@ -117,9 +117,9 @@


#: E123 W291
print "E123", (
print("E123", (
"bad", "hanging", "close"
)
))
#
#: E123 E123 E123
result = {
Expand Down Expand Up @@ -358,15 +358,15 @@ def example_issue254():
""".strip().split():
print(foo)
#: E122:6:5 E122:7:5 E122:8:1
print dedent(
print(dedent(
'''
mkdir -p ./{build}/
mv ./build/ ./{build}/%(revision)s/
'''.format(
build='build',
# more stuff
)
)
))
#: E701:1:8 E122:2:1 E203:4:8 E128:5:1
if True:\
print(True)
Expand Down
38 changes: 20 additions & 18 deletions testsuite/E12not.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,34 @@
"indented for visual indent")


print "OK", ("visual",
"indent")
print("OK", ("visual",
"indent"))

print "Okay", ("visual",
print("Okay", ("visual",
"indent_three"
)
))

print "a-ok", (
print("a-ok", (
"there",
"dude",
)
))

print "hello", (
print("hello", (
"there",
"dude")
"dude"))

print "hello", (
print("hello", (

"there",
# "john",
"dude")
"dude"))

print "hello", (
"there", "dude")
print("hello", (
"there", "dude"))

print "hello", (
print("hello", (
"there", "dude",
)
))

# Aligned with opening delimiter
foo = long_function_name(var_one, var_two,
Expand Down Expand Up @@ -189,12 +189,12 @@ def long_function_name(
if ((foo.bar("baz") and
foo.bar("frop")
)):
print "yes"
print("yes")

# also ok, but starting to look like LISP
if ((foo.bar("baz") and
foo.bar("frop"))):
print "yes"
print("yes")

if (a == 2 or
b == "abc def ghi"
Expand All @@ -214,12 +214,12 @@ def long_function_name(
#


print 'l.{line}\t{pos}\t{name}\t{text}'.format(
print('l.{line}\t{pos}\t{name}\t{text}'.format(
line=token[2][0],
pos=pos,
name=tokenize.tok_name[token[0]],
text=repr(token[1]),
)
))

print('%-7d %s per second (%d total)' % (
options.counters[key] / elapsed, key,
Expand Down Expand Up @@ -532,6 +532,8 @@ def f():
If you would like to see debugging output,
try: %s -d5
''' % sys.argv[0])
except IndexError:
pass


d = { # comment
Expand Down
10 changes: 5 additions & 5 deletions testsuite/E20.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@

#: E203:1:10
if x == 4 :
print x, y
print(x, y)
x, y = y, x
#: E203:2:15 E702:2:16
#: E203:2:16 E702:2:17
if x == 4:
print x, y ; x, y = y, x
print(x, y) ; x, y = y, x
#: E203:3:13
if x == 4:
print x, y
print(x, y)
x, y = y , x
#: Okay
if x == 4:
print x, y
print(x, y)
x, y = y, x
a[b1, :] == a[b1, ...]
b = a[:, b1]
Expand Down
1 change: 1 addition & 0 deletions testsuite/E27.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
True and False
#: E271
if 1:
pass
#: E273
True and False
#: E273 E274
Expand Down
4 changes: 4 additions & 0 deletions testsuite/E90.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,18 @@
#: E901
# Potential second E901
}
#: E901
# Potential second E901
= [x
#: E901 E101 W191
# Potential second E901
while True:
try:
pass
except:
print 'Whoops'
#: E122 E225 E251 E251
# Potential E901

# Do not crash if code is invalid
if msg:
Expand Down
Loading