Commit b5af851c authored by Grant Paton-Simpson's avatar Grant Paton-Simpson

Warn about for-else; bump version; fix typo

parent d8a14a4a
Pipeline #579 failed with stages
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.17
version number: 0.9.18
author: Grant Paton-Simpson
## Overview
......@@ -65,7 +65,7 @@ example provided.
strings in his functions. He learns a standard approach and starts using it
more often.
* Moana wants to check the quality of some code before including it in her project. She learns about some issues and makes improvements before integratin it.
* Moana wants to check the quality of some code before including it in her project. She learns about some issues and makes improvements before integrating it.
# Example Usage
......
......@@ -2,7 +2,7 @@ from setuptools import setup, find_packages # @UnresolvedImport
from codecs import open
from os import path
__version__ = '0.9.17'
__version__ = '0.9.18'
here = path.abspath(path.dirname(__file__))
......
......@@ -182,6 +182,84 @@ def for_index_iteration(block_dets, *, repeat=False):
}
return message
@filt_block_advisor(xpath=FOR_XPATH, warning=True)
def for_else(block_dets, *, repeat=False):
"""
Look for the for-else construct and warn about its safe usage.
"""
for_els = block_dets.element.xpath(FOR_XPATH)
has_for_else = False
for for_el in for_els:
for_else_els = for_el.xpath('orelse')
non_empty_for_else_els = [el for el in for_else_els if el.getchildren()]
if non_empty_for_else_els:
has_for_else = True
break
if not has_for_else:
return None
title = layout("""\
### Ambiguous `for-else` language feature used
""")
if not repeat:
problem = (
layout("""\
`for`-`else` is a useful language feature but it is so widely
misunderstood it is dangerous to use without special comment. For
example, in the snippet below do we pass through `else` because we
finished the looping or because we didn't?
""")
+
layout("""\
for i in range(size):
if i > 100:
break
else:
pass
""", is_code=True)
)
solution = (
layout("""\
The correct answer, which as many as a third of people will guess
correctly ;-), is that we only follow the `else` path if we finished
the for-looping without breaking. The thinking is that we loop
through something to find something and exit the loop. `else`
applies when that search fails. But it is very easy to get the logic
confused. And given that programs are made to be read by humans we
obviously need to do something. We can either avoid the feature
altogether or we can add a brief comment each time (my preference).
For example:
""")
+
layout("""\
for i in range(size):
if i > 100:
break
else: # didn't break for-loop
pass
""", is_code=True)
)
extra_msg = layout("""\
#### Interesting discussion on `for`-`else`
<https://nedbatchelder.com/blog/201110/forelse.html>
One comment noted that the construct violates the principle of Least
Astonishment.
""")
else:
problem = ''
solution = ''
extra_msg = ''
message = {
conf.BRIEF: title + problem,
conf.MAIN: title + problem + solution,
conf.EXTRA: extra_msg,
}
return message
@filt_block_advisor(xpath=FOR_XPATH)
def nested_fors(block_dets, *, repeat=False):
"""
......
......@@ -30,31 +30,13 @@ else:
## When testing user-supplied snippets watch out for the BOM MS inserts via Notepad. AST chokes on it.
## All snippets here should be raw strings (see https://stackoverflow.com/questions/53636723/python-parsing-code-with-new-line-character-in-them-using-ast)
TEST_SNIPPET = r"""
from random import choice
import asyncio
async def run_job(num):
await asyncio.sleep(choice([0.1, 3, 1, 10, 0.5]))
return f"I slept for {num:,} seconds!"
async def run_jobs():
jobs = []
for num in range(1, 11):
job = run_job(num) ## not await because we don't want the result but the future itself ready to be submitted
jobs.append(job)
done_jobs, _pending = await asyncio.wait(jobs,
return_when=asyncio.FIRST_COMPLETED)
results = [done_job.result() for done_job in done_jobs]
return results
loop = asyncio.get_event_loop()
try:
results = loop.run_until_complete(run_jobs())
for result in results:
print(result)
finally:
loop.close()
## https://www.mnn.com/earth-matters/animals/stories/21-animals-with-completely-ridiculous-names
words = ['Spiny lumpsucker', 'Wunderpus photogenicus', 'Pleasing fungus beetle']
for word in words:
if len(word) > 16:
break
else:
print("Never found any long words")
"""
PY3_6 = '3.6'
......
......@@ -14,6 +14,7 @@ def test_misc():
ROOT + 'comprehension_option': 0,
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 0,
ROOT + 'for_else': 0,
}
),
(
......@@ -26,6 +27,7 @@ def test_misc():
ROOT + 'comprehension_option': 1,
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 0,
ROOT + 'for_else': 0,
}
),
(
......@@ -39,6 +41,7 @@ def test_misc():
ROOT + 'comprehension_option': 1, ## the inner one not too long to get advice on a possible list comprehension etc
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 1,
ROOT + 'for_else': 0,
}
),
(
......@@ -60,6 +63,7 @@ def test_misc():
ROOT + 'comprehension_option': 3, ## three of four are not too long to get advice on a possible list comprehension etc
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 1,
ROOT + 'for_else': 0,
}
),
(
......@@ -72,6 +76,7 @@ def test_misc():
ROOT + 'comprehension_option': 1,
ROOT + 'for_index_iteration': 1,
ROOT + 'nested_fors': 0,
ROOT + 'for_else': 0,
}
),
(
......@@ -85,6 +90,42 @@ def test_misc():
ROOT + 'comprehension_option': 1, ## one block
ROOT + 'for_index_iteration': 1,
ROOT + 'nested_fors': 1,
ROOT + 'for_else': 0,
}
),
(
dedent("""\
## https://www.mnn.com/earth-matters/animals/stories/21-animals-with-completely-ridiculous-names
words = ['Spiny lumpsucker', 'Wunderpus photogenicus', 'Pleasing fungus beetle']
for word in words:
if len(word) > 16:
break
else:
print("Never found any long words")
"""),
{
ROOT + 'comprehension_option': 0, ## one block
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 0,
ROOT + 'for_else': 1,
}
),
(
dedent("""\
## https://www.mnn.com/earth-matters/animals/stories/21-animals-with-completely-ridiculous-names
words = ['Spiny lumpsucker', 'Wunderpus photogenicus', 'Pleasing fungus beetle']
for i in range(2):
for word in words:
if len(word) > 16:
break
else:
print("Never found any long words")
"""),
{
ROOT + 'comprehension_option': 0, ## one block
ROOT + 'for_index_iteration': 0,
ROOT + 'nested_fors': 1,
ROOT + 'for_else': 1,
}
),
]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment