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

Add short-circuiting advisor; bump version

parent d5860f1c
Pipeline #559 failed with stages
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.1.16
version number: 0.9.0
author: Grant Paton-Simpson
## Overview
......
......@@ -2,7 +2,7 @@ from setuptools import setup, find_packages # @UnresolvedImport
from codecs import open
from os import path
__version__ = '0.1.16'
__version__ = '0.9.0'
here = path.abspath(path.dirname(__file__))
......
......@@ -51,7 +51,7 @@ def getters_setters(block_dets, *, repeated_message=False):
""")
if not repeated_message:
brief_msg += layout("""\
Python doesn't need getters and setters. Instead, you can use
properties. These are easily added using decorators e.g.
`@property`.
......
......@@ -521,3 +521,108 @@ def implicit_boolean_enough(block_dets, *, repeated_message=False):
conf.BRIEF: brief_msg,
}
return message
def could_short_circuit(if_el):
"""
Is there the potential to take advantage of Python's ability to short-
circuit and collapse a nested IF into its parent as a single expression?
The if_el must have an IF as its only child AND that nested IF must have an
ORELSE as a child. But that ORELSE must have no children.
:param element if_el: If element
:return: True if the If has the potential to be short-circuited
:rtype: bool
"""
body_els = if_el.xpath('body')
has_one_body = len(body_els) == 1
if not has_one_body:
return False
body_el = body_els[0]
body_children_els = body_el.getchildren()
body_one_child = len(body_children_els) == 1
if not body_one_child:
return False
body_child_el = body_children_els[0]
sole_child_is_if = body_child_el.tag == 'If'
if not sole_child_is_if:
return False
nested_if_el = body_child_el
nested_if_orelse_els = nested_if_el.xpath('orelse')
one_orelse = len(nested_if_orelse_els) == 1
if not one_orelse:
return False
nested_if_orelse_el = nested_if_orelse_els[0]
orelse_has_children = bool(nested_if_orelse_el.getchildren())
if orelse_has_children:
return False
## we have an IF with one child which is an IF and the nested IF's ORELSE has no children
return True
@filt_block_advisor(xpath=IF_XPATH)
def short_circuit(block_dets, *, repeated_message=False):
"""
Look for cases where short-circuiting is possible.
"""
if_els = block_dets.element.xpath(IF_XPATH)
could_short_circuit_something = False
for if_el in if_els:
if could_short_circuit(if_el):
could_short_circuit_something = True
break
if not could_short_circuit_something:
return None
brief_msg = layout("""\
### Potential to collapse `if`s (possibly relying on short-circuiting)
This code contains nested `if`s that could potentially be collapsed into
one single conditional expression.
""")
if not repeated_message:
brief_msg += layout("""\
If the second `if` can only be run once the first `if` has been
evaluated as True it is still possible to combine the two
expressions - as long as the two expressions are combined with `and`
AND the test to see if the nested expression can be run comes first.
We can do this because Python supports "short-circuiting" :-).
""")
main_msg = brief_msg
if not repeated_message:
main_msg += (
layout("""\
For example, we can rewrite the following:
""")
+
layout("""\
if word is not None:
## the next expression would raise TypeError: object of type
## 'NoneType' has no len() if word was None and it got this far
if len(word) > 20:
print(f"'{word}' is a long word")
""", is_code=True)
+
layout("""\
as:
""")
+
layout("""\
## the second clause is only evaluated if the first evaluates as True
## i.e. we have relied on short-circuiting
if word is not None and len(word) > 20:
print(f"'{word}' is a long word")
""", is_code=True)
)
message = {
conf.BRIEF: brief_msg,
conf.MAIN: main_msg,
}
return message
......@@ -27,13 +27,20 @@ else:
## When testing user-supplied snippets watch out for the BOM MS inserts via Notepad. AST chokes on it.
TEST_SNIPPET = """\
class Demo:
def __init__(self, x):
self.__x = x
def get_x(self):
return self.__x
def set_x(self, x):
self.__x == x
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
else:
pass
if word is not None:
if len(word) > 20:
pass
pass
"""
DEMO_SNIPPET = """\
......
......@@ -15,6 +15,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -27,6 +28,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -41,6 +43,7 @@ def test_misc():
ROOT + 'missing_else': 1,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -57,6 +60,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -72,6 +76,7 @@ def test_misc():
ROOT + 'missing_else': 1,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -89,6 +94,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -113,6 +119,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -135,6 +142,7 @@ def test_misc():
ROOT + 'missing_else': 1,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -147,6 +155,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 1,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -159,6 +168,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0, ## only dealing with simple cases of all same type and either str or num
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -172,6 +182,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 1,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -192,6 +203,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 1, ## per block but no repeats
ROOT + 'short_circuit': 0,
}
),
(
......@@ -212,6 +224,7 @@ def test_misc():
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0, ## explicit counts not used as empty/non-empty boolean
ROOT + 'short_circuit': 0,
}
),
(
......@@ -229,6 +242,7 @@ def test_misc():
ROOT + 'missing_else': 0, ## saved from being an elif by having a return inside the else:
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
......@@ -246,6 +260,114 @@ def test_misc():
ROOT + 'missing_else': 1, ## the else: if is effectively an elif because nothing else under it
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0,
}
),
(
dedent("""\
if word is not None:
if len(word) > 20:
pass
pass
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0, ## has other code relying on first expression
}
),
(
dedent("""\
if word is not None:
if len(word) > 20:
pass
else:
pass
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 0, ## has other code relying on first expression
}
),
(
dedent("""\
if word is not None:
if len(word) > 20:
pass
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
}
),
(
dedent("""\
for i in range(2):
if word is not None:
if len(word) > 20:
pass
"""),
{
ROOT + 'if_else_overview': 1,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
}
),
(
dedent("""\
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
else:
pass
if word is not None:
if len(word) > 20:
pass
pass
"""),
{
ROOT + 'if_else_overview': 3,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 1,
}
),
(
dedent("""\
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
if word is not None:
if len(word) > 20:
pass
"""),
{
ROOT + 'if_else_overview': 3,
ROOT + 'missing_else': 0,
ROOT + 'split_group_membership': 0,
ROOT + 'implicit_boolean_enough': 0,
ROOT + 'short_circuit': 3,
}
),
]
......
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