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

Add msgs on Python names & values; extend assignment detection; misc

* Add messages on Python names and values when name to name assignment occurs
* Detect assignment to object attributes and dictionary fields, not just standard names
* Shift all version-dependent AST functions into ast_funcs
* Improve descriptions of pairs
* Fix minor bug referring to method as function
* Bump version
parent f020ec62
Pipeline #582 failed with stages
0# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.20
version number: 0.9.21
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.9.20'
__version__ = '0.9.21'
here = path.abspath(path.dirname(__file__))
......
......@@ -314,18 +314,18 @@ def one_method_classes(block_dets, *, repeat=False):
func_plural = 's' if multi_sole else ''
summary = layout(f"""\
### Possible option of converting class{class_plural} to single function{func_plural}
### Possible option of converting class{class_plural} to single
function{func_plural}
The following class{class_plural} only {class_have_has} one main function at
The following class{class_plural} only {class_have_has} one main method at
most (excluding `__init__`):
""")
n_methods_msg_bits = []
for class_name, method_name in classes_sole_methods:
method2use = (
f"`{method_name}`" if method_name else 'nothing but `__init__`')
method2use = (f"`{method_name}`" if method_name
else 'nothing but `__init__` (if even that)')
n_methods_msg_bits.append(layout(f"""\
- {class_name}: {method2use}
- `{class_name}`: {method2use}
"""))
n_methods_msg = ''.join(n_methods_msg_bits)
if not repeat:
......
import logging
from ..advisors import all_blocks_advisor
from ..ast_funcs import num_str_from_val, _get_var_equal_plussed, _get_var_plus_equalled
from .. import conf
from ..utils import get_python_version, layout_comment as layout
def _get_num_3_7(value_el):
positive_num_els = value_el.xpath('Num')
if len(positive_num_els) == 1:
num = positive_num_els[0].get('n')
if num not in ['0', '1']:
return None
else:
sub_els = value_el.xpath('UnaryOp/op/USub')
if not sub_els:
return None
negative_num_els = value_el.xpath('UnaryOp/operand/Num')
if len(negative_num_els) != 1:
return None
pos_num = negative_num_els[0].get('n')
if pos_num != '1':
return None
num = f'-{pos_num}'
return num
def _get_num_3_8(value_el):
positive_num_els = value_el.xpath('Constant')
if len(positive_num_els) == 1:
positive_num_el = positive_num_els[0]
val = positive_num_el.get('value')
if positive_num_el.get('type') not in ('int', 'float'):
return None
if val not in ['0', '1']:
return None
num = val
else:
sub_els = value_el.xpath('UnaryOp/op/USub')
if not sub_els:
return None
negative_num_els = value_el.xpath('UnaryOp/operand/Constant')
if len(negative_num_els) != 1:
return None
negative_num_el = negative_num_els[0]
val = negative_num_el.get('value')
if negative_num_el.get('type') not in ('int', 'float'):
return None
if val != '1':
return None
num = f'-{val}'
return num
def _get_var_plus_equalled_all(el):
plus_equalled_els = el.xpath('descendant-or-self::AugAssign')
if len(plus_equalled_els) != 1:
return None
plus_equalled_el = plus_equalled_els[0]
target_els = plus_equalled_el.xpath('target/Name')
if len(target_els) != 1:
return None
target = target_els[0].get('id') ## e.g. 'i' or 'counter' etc
add_sub_els = plus_equalled_el.xpath('op/Add | op/Sub') ## only interested in Add / Sub
if len(add_sub_els) != 1:
return None
return target, plus_equalled_el
def _get_var_plus_equalled_3_7(el):
ok_num = '1'
res = _get_var_plus_equalled_all(el)
if res is None:
return None
target, plus_equalled_el = res
num_els = plus_equalled_el.xpath('value/Num')
if len(num_els) != 1:
return None
num = num_els[0].get('n')
if num != ok_num:
return None
var_plus_equalled = target
return var_plus_equalled
def _get_var_plus_equalled_3_8(el):
ok_num = '1'
res = _get_var_plus_equalled_all(el)
if res is None:
return None
target, plus_equalled_el = res
num_els = plus_equalled_el.xpath('value/Constant')
if len(num_els) != 1:
return None
num_el = num_els[0]
val = num_el.get('value')
if num_el.get('type') not in ('int', 'float'):
return None
if val != ok_num:
return None
var_plus_equalled = target
return var_plus_equalled
def _get_var_equal_plussed_all(el):
assign_els = el.xpath('descendant-or-self::Assign')
if len(assign_els) != 1:
return None
assign_el = assign_els[0]
target_name_els = assign_el.xpath('targets/Name')
if len(target_name_els) != 1:
return None
var_name = target_name_els[0].get('id')
binop_els = assign_el.xpath('value/BinOp')
if len(binop_els) != 1:
return None
bin_op_el = binop_els[0]
left_val_els = bin_op_el.xpath('left/Name')
if len(left_val_els) != 1:
return None
left_val = left_val_els[0].get('id')
if left_val != var_name: ## we want i = i ... + 1
return None
add_els = bin_op_el.xpath('op/Add')
if len(add_els) != 1:
return None
return var_name, bin_op_el
def _get_var_equal_plussed_3_7(el):
ok_val = '1'
res = _get_var_equal_plussed_all(el)
if res is None:
return None
var_name, bin_op_el = res
right_els = bin_op_el.xpath('right/Num')
if len(right_els) != 1:
return None
right_val = right_els[0].get('n') ## ... + 1
if right_val != ok_val:
return None
var_equal_plussed = var_name
return var_equal_plussed
def _get_var_equal_plussed_3_8(el):
ok_val = '1'
res = _get_var_equal_plussed_all(el)
if res is None:
return None
var_name, bin_op_el = res
right_els = bin_op_el.xpath('right/Constant')
if len(right_els) != 1:
return None
right_el = right_els[0]
right_val = right_el.get('value') ## ... + 1
if right_el.get('type') not in ('int', 'float'):
return None
if right_val != ok_val:
return None
var_equal_plussed = var_name
return var_equal_plussed
python_version = get_python_version()
if python_version in (conf.PY3_6, conf.PY3_7):
_get_num = _get_num_3_7
_get_var_plus_equalled = _get_var_plus_equalled_3_7
_get_var_equal_plussed = _get_var_equal_plussed_3_7
elif python_version == conf.PY3_8:
_get_num = _get_num_3_8
_get_var_plus_equalled = _get_var_plus_equalled_3_8
_get_var_equal_plussed = _get_var_equal_plussed_3_8
else:
raise Exception(f"Unexpected Python version {python_version}")
from ..utils import layout_comment as layout
def get_num(value_el):
"""
......@@ -190,7 +30,7 @@ def get_num(value_el):
</value>
</Assign>
"""
return _get_num(value_el)
return num_str_from_val(value_el)
def get_var_initialised(el):
"""
......
......@@ -2,61 +2,10 @@
Covers functions and methods.
"""
from ..advisors import filt_block_advisor
from ..ast_funcs import get_el_lines_dets
from ..ast_funcs import get_danger_status, get_docstring_from_value, \
get_el_lines_dets
from .. import conf, utils
from ..utils import get_nice_pairs, get_python_version, layout_comment as layout
def get_danger_status_3_7(child_el):
if (child_el.tag == 'NameConstant'
and child_el.get('value') in ['True', 'False']):
danger_status = 'Boolean'
elif child_el.tag == 'Num' and child_el.get('n'):
danger_status = 'Number'
else:
danger_status = None
return danger_status
def get_danger_status_3_8(child_el):
if child_el.tag == 'Constant':
val = child_el.get('value')
if val in ['True', 'False']:
danger_status = 'Boolean'
else:
try:
float(val)
except TypeError:
danger_status = None
else:
danger_status = 'Number'
else:
danger_status = None
return danger_status
def get_docstring_from_value_3_7(first_value_el):
if first_value_el.tag != 'Str':
docstring = None
else:
docstring = first_value_el.get('s')
return docstring
def get_docstring_from_value_3_8(first_value_el):
if first_value_el.tag != 'Constant':
docstring = None
elif first_value_el.get('type') != 'str':
docstring = None
else:
docstring = first_value_el.get('value')
return docstring
python_version = get_python_version()
if python_version in (conf.PY3_6, conf.PY3_7):
get_danger_status = get_danger_status_3_7
get_docstring_from_value = get_docstring_from_value_3_7
elif python_version == conf.PY3_8:
get_danger_status = get_danger_status_3_8
get_docstring_from_value = get_docstring_from_value_3_8
else:
raise Exception(f"Unexpected Python version {python_version}")
from ..utils import get_nice_pairs, layout_comment as layout
FUNC_DEFN_XPATH = 'descendant-or-self::FunctionDef'
......@@ -424,10 +373,12 @@ def mutable_default(block_dets, *, repeat=False):
for i, mutable_default_dets in enumerate(mutable_defaults_dets):
name, func_type_lbl, mutable_default_args = mutable_default_dets
first = (i == 0)
nice_pairs = get_nice_pairs(
mutable_default_args, pair_glue=' defaults to a ')
issue = layout(f"""\
`{name}` has the following parameters with mutable defaults:
{get_nice_pairs(mutable_default_args, left_quoter='')}.
{nice_pairs}.
""")
brief_summary_bits.append(issue)
main_summary_bits.append(issue)
......
from collections import defaultdict, namedtuple, Counter
from ..advisors import filt_block_advisor
from .. import conf
from ..utils import get_python_version, int2nice, layout_comment as layout
def get_string_3_7(comparison_el):
string = comparison_el.get('s')
if not string:
return None
return string
def get_string_3_8(comparison_el):
val = comparison_el.get('value')
if comparison_el.get('type') == 'str':
string = val
else:
string = None
return string
def get_num_3_7(comparison_el):
num = comparison_el.get('n')
if not num:
return None
return num
def get_num_3_8(comparison_el):
val = comparison_el.get('value')
if comparison_el.get('type') in ('int', 'float'):
num = val
else:
num = None
return num
python_version = get_python_version()
if python_version in (conf.PY3_6, conf.PY3_7):
get_string = get_string_3_7
get_num = get_num_3_7
elif python_version == conf.PY3_8:
get_string = get_string_3_8
get_num = get_num_3_8
else:
raise Exception(f"Unexpected Python version {python_version}")
from .. import ast_funcs, conf
from ..utils import int2nice, layout_comment as layout
IfDets = namedtuple('IfDetails',
'multiple_conditions, missing_else, if_clauses')
......@@ -331,13 +293,13 @@ def get_split_membership_dets(if_el):
if not comparison_els:
continue
comparison_el = comparison_els[0] ## Str or Num etc
comp_val = get_string(comparison_el)
comp_val = ast_funcs.str_from_el(comparison_el)
if comp_val is not None:
basic_types.add(STRING)
if len(basic_types) > 1:
return None
else:
comp_val = get_num(comparison_el)
comp_val = ast_funcs.num_str_from_el(comparison_el)
if comp_val is not None:
basic_types.add(NUMBER)
if len(basic_types) > 1:
......@@ -450,7 +412,7 @@ def get_has_explicit_count(if_el):
if not comparison_els:
return False
comparison_el = comparison_els[0]
n = get_num(comparison_el)
n = ast_funcs.num_str_from_el(comparison_el)
if n is None:
return False
explicit_booleans = [
......
from collections import defaultdict
from ..advisors import any_block_advisor, is_reserved_name
from .. import conf, utils
from ..utils import get_nice_str_list, int2nice, layout_comment as layout
from ..advisors import all_blocks_advisor, any_block_advisor, is_reserved_name
from .. import ast_funcs, conf, utils
from ..utils import get_nice_pairs, get_nice_str_list, \
int2nice, layout_comment as layout
ASSIGN_SUBSCRIPT_XPATH = 'descendant-or-self::Assign/value/Subscript'
ASSIGN_NAME_XPATH = 'descendant-or-self::Assign/value/Name'
def get_subscript_name_value(assign_subscript_el):
"""
Looking for:
people[0] (list)
capitals['NZ'] (dict)
"""
name_value_el = assign_subscript_el.xpath('value/Name')[0]
name_value_name = name_value_el.get('id')
slice_dets = ast_funcs.get_slice_dets(assign_subscript_el)
name_value = f"{name_value_name}{slice_dets}"
return name_value
def get_name2name_pair(name2name_el):
"""
Get details of a name to name assignment.
:return: tuple of name, name_value. name_value might be a standard Python
name e.g. person, or a subscript of some sort e.g. people[0] or
capitals['NZ']
"""
ancestor_assign_els = name2name_el.xpath('ancestor::Assign')
try:
ancestor_assign_el = ancestor_assign_els[-1]
except IndexError:
raise IndexError( ## in theory, guaranteed to be one given how we got the name2name_els
"Unable to identify ancestor Assign for name-to-name assignment")
try:
_name_type, _name_details, name_str = ast_funcs.get_assigned_name(
name2name_el)
except Exception as e:
raise Exception("Unable to identify name for name-value assignment. "
f"Orig error: {e}")
assign_subscript_els = ancestor_assign_el.xpath(ASSIGN_SUBSCRIPT_XPATH)
if assign_subscript_els:
assign_subscript_el = assign_subscript_els[0]
name_value = get_subscript_name_value(assign_subscript_el)
else:
assign_name_els = ancestor_assign_el.xpath(ASSIGN_NAME_XPATH)
direct_assign_el = assign_name_els[0]
name_value = direct_assign_el.get('id')
if not name_value:
raise Exception(
"Unable to identify name_value even though there should be one")
return name_str, name_value
def get_name2name_pairs(block_el):
"""
Get details of name-to-name assignments.
Examples of name to name assignment:
animal = pet
first_person = people[0]
capital_nz = capitals['NZ']
Obviously, being Python, the assignment is to the value under the name but
it is a name nonetheless.
"""
name2name_els = block_el.xpath(
f"{ASSIGN_SUBSCRIPT_XPATH}|{ASSIGN_NAME_XPATH}")
if not name2name_els:
return None
name2name_pairs = [get_name2name_pair(el) for el in name2name_els]
return name2name_pairs
@all_blocks_advisor()
def names_and_values(blocks_dets):
"""
Look for names assigned to other names and explain names and values in
Python.
"""
name2name_pairs = []
for block_dets in blocks_dets:
block_el = block_dets.element
block_name2name_pairs = get_name2name_pairs(block_el)
if block_name2name_pairs:
name2name_pairs.extend(block_name2name_pairs)
if not name2name_pairs:
return None
nice_name2name_pairs = get_nice_pairs(name2name_pairs,
pair_glue=' is assigned to ', left_quoter='`', right_quoter='`')
first_name, first_name_value = name2name_pairs[0]
title = layout("""
### Names assigned to names
""")
assignments = layout(f"""\
Your code assigned names to other names as values: {nice_name2name_pairs}.
""")
brief_summary = layout("""\
To get a better understanding of what that means in Python see / read
<https://nedbatchelder.com/text/names1.html>. Or look at this advice at a
higher level of detail e.g. 'Main'.
""")
main_summary = (
layout(f"""\
So what does that mean in Python? Having a firm grasp of how names and
values work in Python means you can confidently reason about your code
and pre-emptively avoid numerous bugs.
#### How variables work in Python - names and values
Assignment is linking names and values e.g. `fname = "Maggie"`
Another way of expressing the same idea:
> "Assignment: make this name refer to that value" Ned Batchelder
Names are something you can assign to a value.
Multiple names can refer to the same value. No name is the "real" name
in Python - they are all just labels to a value.
Names can take a variety of forms. For example:
""")
+
layout("""\
fname = value
mydict['fruit'] = value
family.pet = value
""", is_code=True)
+
layout("""
are all names.
A wide variety of data structures can be values to which names are
assigned. For example:
""")
+
layout("""\
name = 1066
name = 'cheerful'
name = [1, 2, 3, 4]
name = {'width': 12.5, 'height': 23.75}
etc
""", is_code=True)
+
layout(f"""
The key idea is that we are always linking names to values - NEVER names
to other names. Python has references (names pointing to objects) but
not pointers (names pointing to other names). When we assign one name to
another we are actually linking to the underlying value of the second
name.
And be very clear - assignment never **copies** data. It just links
names to values. There are no exceptions to this rule.
So if `a = 'cat'` and `b = a` then both `a` and `b` are labels assigned
to the string 'cat'. If we change what label `a` is attached to we don't
affect `b`. It remains linked to 'cat'.
So when your code assigned name `{first_name}` to another name
`{first_name_value}`, for example, it was actually linking
`{first_name}` to the **value** `{first_name_value}` was linked to. If
you reassigned `{first_name_value}` to something else it would not
affect `{first_name}`.
The names aren't necessarily independent though. If a mutable object
like a list has multiple labels then any changes to the value, i.e. the
list, are shared by all labels because they are all linked to the same
value.
For example:
""")
+
layout("""\
fruit = ['apple', 'banana']
ingredients = fruit
fruit.append('cherry')
## What is ingredients now? Try this code and find out. If you
## understand the ideas above you should be able to reason it out with
## confidence.
""", is_code=True)
)
ned_talk = layout("""\
The talk to watch / read on the topic is:
<https://nedbatchelder.com/text/names1.html>
<https://lerner.co.il/2019/06/18/understanding-python-assignment/> is also
helpful.
A note about using the verb 'assign'? I recommend saying that we assign
names to values rather than the other way around because it better fits how
Python actually works. The central reality in Python is the value not the
name. The name is nothing substantial - it's not a container for a value
or anything like that - it is merely a label.
""")
message = {
conf.BRIEF: title + assignments + brief_summary,
conf.MAIN: title + assignments + main_summary,
conf.EXTRA: ned_talk,
}
return message
def _get_shamed_names_title(reserved_names, bad_names, dubious_names):
if not (reserved_names or bad_names or dubious_names):
......
from collections import namedtuple
from ..advisors import all_blocks_advisor
from .. import conf
from ..utils import get_python_version, layout_comment as layout
from .. import ast_funcs, conf
from ..utils import layout_comment as layout
NTDets = namedtuple('NamedTupleDetails', 'name, label, fields_str, fields_list')
def get_lbl_flds_3_7(assign_block_el):
label_el, fields_el = assign_block_el.xpath('value/Call/args/Str')
label = label_el.get('s')
fields_str = fields_el.get('s')
return label, fields_str
def get_lbl_flds_3_8(assign_block_el):
label_el, fields_el = assign_block_el.xpath('value/Call/args/Constant')
label = label_el.get('value')
fields_str = fields_el.get('value')
return label, fields_str
python_version = get_python_version()
if python_version in (conf.PY3_6, conf.PY3_7):
get_lbl_flds = get_lbl_flds_3_7
elif python_version == conf.PY3_8:
get_lbl_flds = get_lbl_flds_3_8
else:
raise Exception(f"Unexpected Python version {python_version}")
def get_named_tuple_dets(named_tuple_el):
"""
Get name, label, fields.
......@@ -33,7 +13,7 @@ def get_named_tuple_dets(named_tuple_el):
ancestor_elements = named_tuple_el.xpath('ancestor-or-self::*')
assign_block_el = ancestor_elements[-5] ## Assign value Call func Name
name = assign_block_el.xpath('targets/Name')[0].get('id')
label, fields_str = get_lbl_flds(assign_block_el)
label, fields_str = ast_funcs.get_lbl_flds(assign_block_el)
fields_list = [field.strip() for field in fields_str.split(',')]
named_tuple_dets = NTDets(name, label, fields_str, fields_list)
return named_tuple_dets
......
from collections import defaultdict
from ..advisors import filt_block_advisor
from ..ast_funcs import get_assign_name
from ..ast_funcs import get_assigned_name, assigned_num_els_from_block
from .. import code_execution, conf
from ..utils import get_nice_str_list, get_python_version,\
layout_comment as layout
from ..utils import get_nice_str_list, layout_comment as layout
ASSIGN_VAL_XPATH = 'descendant-or-self::Assign/value'
def get_num_els_3_7(block_el):
num_els = block_el.xpath('descendant-or-self::Assign/value/Num')
return num_els
def get_num_els_3_8(block_el):
val_els = block_el.xpath(ASSIGN_VAL_XPATH)
num_els = []
for val_el in val_els:
constant_els = val_el.xpath('Constant')
if len(constant_els) != 1:
continue
constant_el = constant_els[0]
if constant_el.get('type') in ('int', 'float'):
num_els.append(constant_el)
return num_els
python_version = get_python_version()