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

Add lint checking; misc

* Add flake8 lint advisor
* Skip lint check when testing
* Add new type of advisor (snippet_str_advisor)
* Rename snippet advisor to all_blocks_advisor
* Clarify need to close temp file to see what is written
* Avoid splitting title lines even if long to maintain consistent styling
* Prevent unnecessary runs of layout function by "hiding" large comments inside functions
* Bump version
parent 49419d5e
Pipeline #571 canceled with stages
# https://git.nzoss.org.nz/pyGrant/superhelp
version number: 0.9.9
version number: 0.9.10
author: Grant Paton-Simpson
## Overview
......@@ -97,10 +97,6 @@ The notebook has more detailed instructions at the top.
$ shelp ## to see advice on an example snippet displayed (level Extra)
## TODO Options
## Stretch Ideas
1) Extend advice further to encourage sound practice
2) Perhaps add style linting as an option
3) Extend beyond standard library into popular libraries like requests, bottle, flask etc.
* Extend beyond standard library into popular libraries like requests, bottle, flask etc.
astpath>=0.9.0
cssselect>=1.1.0
lxml>=4.5.0
Markdown>=3.2.1
flake8>=3.5
nose
Pygments>=2.6.1
PyYAML>=5.3.1
tabulate>=0.8.7
## numerous others because I installed cookiecutter in same env - not needed for superhelp itself
## numerous others because I installed cookiecutter and flake8 in same env - not needed for superhelp itself
## Dev/Deployment
#sphinx
#sphinx_rtd_theme
#nose
#coverage
#pypi-publisher
......@@ -2,7 +2,7 @@ from setuptools import setup, find_packages # @UnresolvedImport
from codecs import open
from os import path
__version__ = '0.9.9'
__version__ = '0.9.10'
here = path.abspath(path.dirname(__file__))
......
This diff is collapsed.
from ..advisors import filt_block_advisor
from .. import conf
from ..advisors import AOP_COMMENT
from ..advisors import get_aop_msg
from ..utils import layout_comment as layout
LONG_EXAMPLE_OPEN_CM = (
layout("""\
For example, every time we open a file we need to make sure it is closed
when we are finished. It is not that hard to handle that yourself in
simple cases but can become much harder - perhaps the code exits in
multiple places (perhaps as a result of an `exception`). You have to
remember to handle the tidy-up in every place an exit might occur.
Either you will forget one of them or you'll be repeating code all over
the place violating the DRY principle - namely:
def get_open_cm_msg():
return (
layout("""\
For example, every time we open a file we need to make sure it is
closed when we are finished. It is not that hard to handle that
yourself in simple cases but can become much harder - perhaps the
code exits in multiple places (perhaps as a result of an
`exception`). You have to remember to handle the tidy-up in every
place an exit might occur. Either you will forget one of them or
you'll be repeating code all over the place violating the DRY
principle - namely:
> "Don't Repeat Yourself" (The Pragmatic Programmer)
> "Don't Repeat Yourself" (The Pragmatic Programmer)
WET presumably stands for "Write Every Time", "Write Everything Twice",
"We Enjoy Typing" or "Waste Everyone's Time" ;-)
WET presumably stands for "Write Every Time", "Write Everything
Twice", "We Enjoy Typing" or "Waste Everyone's Time" ;-)
If you don't use a context manager, problems can emerge as a snippet
gradually evolves. And it usually all starts so innocently :-). Here's
one example of the problem unfolding:
If you don't use a context manager, problems can emerge as a snippet
gradually evolves. And it usually all starts so innocently :-).
Here's one example of the problem unfolding:
""")
+
layout("""\
## 1) Not using context manager (with) not really a problem
f = open(fname)
text = f.read()
f.close() # <------------------ First one (I hope this is the only one)
## 2) Maybe we need to handle empty files
f = open(fname)
text = f.read()
if not text:
raise Exception(f"{fname} is empty") # <------ errr ... is the file
f.close() # handle going to get closed?
## 3) We remember to close the file handle if an exception
f = open(fname)
text = f.read()
if not text:
f.close() # <---------------------------- Rats! I need a second one
raise Exception(f"{fname} is empty")
f.close()
## 4) We want the index of TAG for some reason
f = open(fname)
text = f.read()
if not text:
f.close()
raise Exception(f"{fname} is empty")
idx = text.index(TAG) # <--------------- Hmmm - will raise ValueError if
f.close() # TAG not in text? File left open?
## 5) We remember to handle the exception
f = open(fname)
text = f.read()
if not text:
""")
+
layout("""\
## 1) Not using context manager (with) not really a problem
f = open(fname)
text = f.read()
f.close() # <------------------ First one (I hope this is the only one)
## 2) Maybe we need to handle empty files
f = open(fname)
text = f.read()
if not text:
raise Exception(f"{fname} is empty") # <------ errr ... is the file
f.close() # handle going to get closed?
## 3) We remember to close the file handle if an exception
f = open(fname)
text = f.read()
if not text:
f.close() # <---------------------------- Rats! I need a second one
raise Exception(f"{fname} is empty")
f.close()
raise Exception(f"{fname} is empty")
try:
idx = text.index(TAG)
except ValueError:
f.close() # <----------------- Three already! I'm lucky it's only a
raise # one-liner each time.
f.close()
""", is_code=True)
+
layout("""\
The code is much cleaner and robust using a simple context manager :-)
""")
+
layout("""\
with open(fname) as f:
## 4) We want the index of TAG for some reason
f = open(fname)
text = f.read()
if not text:
f.close()
raise Exception(f"{fname} is empty")
idx = text.index(TAG)
## If my code gets here, i.e. past the indented block inside the context
## manager, we are guaranteed to have freed up the file - nice!
""", is_code=True)
)
idx = text.index(TAG) # <--------------- Hmmm - will raise ValueError if
f.close() # TAG not in text? File left open?
## 5) We remember to handle the exception
f = open(fname)
text = f.read()
if not text:
f.close()
raise Exception(f"{fname} is empty")
try:
idx = text.index(TAG)
except ValueError:
f.close() # <----------------- Three already! I'm lucky it's only a
raise # one-liner each time.
f.close()
""", is_code=True)
+
layout("""\
The code is much cleaner and robust using a simple context manager
:-)
""")
+
layout("""\
with open(fname) as f:
text = f.read()
if not text:
raise Exception(f"{fname} is empty")
idx = text.index(TAG)
## If my code gets here, i.e. past the indented block inside the context
## manager, we are guaranteed to have freed up the file - nice!
""", is_code=True)
)
WITH_XPATH = 'descendant-or-self::With'
......@@ -139,8 +142,8 @@ def content_manager_overview(block_dets, *, repeat=False):
connections and cursors we need to clean up after ourselves. There
may be some things we need to do when we start a code block as well.
""")
long_example = LONG_EXAMPLE_OPEN_CM
aop = AOP_COMMENT
long_example = get_open_cm_msg()
aop = get_aop_msg()
else:
brief_example = ''
long_example = ''
......@@ -182,8 +185,8 @@ def file_cm_needed(block_dets, *, repeat=False):
that hard) but using the standard ones has big advantages.
""")
long_example = LONG_EXAMPLE_OPEN_CM
aop = AOP_COMMENT
long_example = get_open_cm_msg()
aop = get_aop_msg()
else:
reasons = ''
long_example = ''
......
from ..advisors import filt_block_advisor
from .. import conf
from ..advisors import AOP_COMMENT
from ..advisors import get_aop_msg
from ..utils import get_nice_str_list, layout_comment as layout
DECORATOR_XPATH = (
......@@ -96,7 +96,7 @@ def decorator_overview(block_dets, *, repeat=False):
say("sausage!")
''', is_code=True)
)
aop = AOP_COMMENT
aop = get_aop_msg()
else:
dec_dets = ''
aop = ''
......
import logging
from ..advisors import snippet_advisor
from ..advisors import all_blocks_advisor
from .. import conf
from ..utils import get_python_version, layout_comment as layout
......@@ -328,7 +328,7 @@ def get_incrementing_var(block_dets, vars_initialised, vars_incremented):
incrementing_var = vars_incremented.pop()
return incrementing_var
@snippet_advisor()
@all_blocks_advisor()
def manual_incrementing(blocks_dets):
"""
Look for manual handling of incrementing inside loops.
......
from ..advisors import snippet_advisor
from ..advisors import all_blocks_advisor
from .. import conf
from ..utils import get_nice_str_list, int2nice, layout_comment as layout
......@@ -25,7 +25,7 @@ def get_exception_blocks(blocks_dets):
exception_blocks.append(block_exception_types)
return exception_blocks
@snippet_advisor()
@all_blocks_advisor()
def exception_overview(blocks_dets):
"""
Provide overview of exception handling.
......@@ -56,7 +56,7 @@ def exception_overview(blocks_dets):
}
return message
@snippet_advisor(warning=True)
@all_blocks_advisor(warning=True)
def unspecific_exception(blocks_dets):
"""
Look for unspecific exceptions.
......
from ..advisors import DICT_COMPREHENSION_COMMENT, \
GENERAL_COMPREHENSION_COMMENT, LIST_COMPREHENSION_COMMENT, \
SET_COMPREHENSION_COMMENT, filt_block_advisor
from ..advisors import get_dict_comprehension_msg, \
get_general_comprehension_msg, get_set_comprehension_msg, filt_block_advisor
from ..ast_funcs import get_el_lines_dets
from .. import conf
from ..utils import layout_comment as layout
......@@ -33,13 +32,13 @@ def comprehension_option(block_dets, *, repeat=False):
comp_comment = ''
if 'append' in block_dets.block_code_str:
comp_type = 'List Comprehension'
comp_comment = LIST_COMPREHENSION_COMMENT
comp_comment = get_dict_comprehension_msg()
elif len(block_dets.element.cssselect('Subscript')): ## Seems a reasonable indicator
comp_type = 'Dictionary Comprehension'
comp_comment = DICT_COMPREHENSION_COMMENT
comp_comment = get_dict_comprehension_msg()
elif 'set' in block_dets.block_code_str:
comp_type = 'Set Comprehension'
comp_comment = SET_COMPREHENSION_COMMENT
comp_comment = get_set_comprehension_msg()
else:
return None
......@@ -60,7 +59,7 @@ def comprehension_option(block_dets, *, repeat=False):
message = {
conf.BRIEF: title + option,
conf.MAIN: (title + option + GENERAL_COMPREHENSION_COMMENT
conf.MAIN: (title + option + get_general_comprehension_msg()
+ '\n\n' + comp_comment),
}
return message
......
from ..advisors import DICT_COMPREHENSION_COMMENT, \
GENERAL_COMPREHENSION_COMMENT, SET_COMPREHENSION_COMMENT, \
from ..advisors import get_dict_comprehension_msg, \
get_general_comprehension_msg, get_set_comprehension_msg, \
filt_block_advisor
from ..ast_funcs import get_assign_name
from .. import code_execution, conf, utils
......@@ -43,7 +43,7 @@ def listcomp_overview(block_dets, *, repeat=False):
### Other "comprehensions"
""")
+ GENERAL_COMPREHENSION_COMMENT
+ get_general_comprehension_msg()
+ layout("""\
List comprehensions aren't the only type of comprehension you
......@@ -51,9 +51,9 @@ def listcomp_overview(block_dets, *, repeat=False):
Comprehensions:
""")
+ DICT_COMPREHENSION_COMMENT
+ get_dict_comprehension_msg()
+ '\n\n'
+ SET_COMPREHENSION_COMMENT
+ get_set_comprehension_msg()
+ '\n\n'
+ layout("""\
Pro tip: don't make comprehension *in*comprehensions ;-). If it
......
from collections import namedtuple
from ..advisors import snippet_advisor
from ..advisors import all_blocks_advisor
from .. import conf
from ..utils import get_python_version, layout_comment as layout
......@@ -51,7 +51,7 @@ def get_named_tuples_dets(blocks_dets):
all_named_tuples_dets.extend(named_tuples_dets)
return all_named_tuples_dets
@snippet_advisor()
@all_blocks_advisor()
def named_tuple_overview(blocks_dets):
"""
Look for named tuples and explain how they can be enhanced.
......
from collections import defaultdict
from ..advisors import UNPACKING_COMMENT, snippet_advisor, filt_block_advisor
from ..advisors import get_unpacking_msg, all_blocks_advisor, \
filt_block_advisor
from .. import conf, utils
from ..utils import get_python_version, layout_comment as layout
......@@ -54,7 +55,7 @@ def unpacking(block_dets, *, repeat=False):
"""))
summary = ''.join(summary_bits)
if not repeat:
unpacking_msg = UNPACKING_COMMENT
unpacking_msg = get_unpacking_msg()
else:
unpacking_msg = ''
......@@ -64,7 +65,7 @@ def unpacking(block_dets, *, repeat=False):
}
return message
@snippet_advisor()
@all_blocks_advisor()
def unpacking_opportunity(blocks_dets):
"""
Look for opportunities to unpack values into multiple names instead of
......@@ -116,6 +117,6 @@ def unpacking_opportunity(blocks_dets):
message = {
conf.BRIEF: title + unpackable,
conf.EXTRA: UNPACKING_COMMENT,
conf.EXTRA: get_unpacking_msg(),
}
return message
from ..advisors import snippet_advisor
from ..advisors import all_blocks_advisor
from .. import conf
from ..utils import get_python_version, layout_comment as layout
......@@ -92,7 +92,7 @@ def used_verbose(block_el):
def used_compile(block_el):
pass
@snippet_advisor()
@all_blocks_advisor()
def verbose_option(blocks_dets):
"""
Check for use of regex without verbose mode and introduce the idea.
......
......@@ -13,6 +13,7 @@ DEV_MODE = f ## (f)
DO_TEST = t ## set test snippet as default rather than the larger demo snippet (t)
DO_HTML = t ## set html displayer as default (t)
DO_DISPLAYER = t ## f is only ever used when testing pre-display (t)
INCLUDE_LINTING = t ## f when running unit tests to massively speed them up (otherwise every snippet in tests is linted each time) (t)
## =============================================================================
......@@ -26,9 +27,7 @@ else:
## When testing user-supplied snippets watch out for the BOM MS inserts via Notepad. AST chokes on it.
TEST_SNIPPET = """\
pets = 'seagull'
home = 'super volcano'
appliance = 'garbage disposal'
home = 'super volcano'
"""
DEMO_SNIPPET = """\
......@@ -228,11 +227,20 @@ MISSING_ADVICE_MESSAGE = ("If there was some advice you think should have "
"'Advice' and explain. Please include a snippet to test as well.")
SYSTEM_MESSAGE = 'System message'
## input types
BLOCKS_DETS = 'blocks_dets'
SNIPPET_STR = 'snippet_str'
XKCD_WARNING_WORDS = ['supervolcano', 'seagull', 'garbage disposal']
VERBOSE_FLAG = 'VERBOSE'
INLINE_RE_VERBOSE_FLAG = '(?x)'
SNIPPET_FNAME = 'snippet.py'
LINT_MSG_TYPE = 'msg_type'
LINT_MSG = 'msg'
LINT_LINE_NO = 'line_no'
LINE_FEED = '&#10;'
## scraped from https://docs.python.org/3/py-modindex.html 2020-04-02
......
......@@ -3,7 +3,7 @@ from textwrap import dedent, indent
import webbrowser
from .. import conf
from ..utils import make_tmp_file
from ..utils import make_open_tmp_file
from markdown import markdown ## https://coderbook.com/@marcus/how-to-render-markdown-syntax-as-html-using-python/ @UnresolvedImport
......@@ -678,7 +678,8 @@ def display(snippet, messages_dets, *,
missing_advice_message=conf.MISSING_ADVICE_MESSAGE,
body_inner=body_inner,
visibility_script=VISIBILITY_SCRIPT)
tmp_fh, fpath = make_tmp_file('superhelp_output.html', mode='w')
tmp_fh, fpath = make_open_tmp_file('superhelp_output.html', mode='w')
tmp_fh.write(html2write)
tmp_fh.close()
url = fpath.as_uri()
webbrowser.open_new_tab(url)
......@@ -86,14 +86,14 @@ astpath.asts.convert_to_xml = convert_to_xml
try:
from . import advisors, ast_funcs, conf # @UnresolvedImport @UnusedImport
from ..utils import get_docstring_start, layout_comment as layout, make_tmp_file # @UnresolvedImport @UnusedImport
from ..utils import get_docstring_start, layout_comment as layout, make_open_tmp_file # @UnresolvedImport @UnusedImport
except (ImportError, ValueError):
from pathlib import Path
import sys
parent = str(Path.cwd().parent)
sys.path.insert(0, parent)
from superhelp import advisors, ast_funcs, conf # @Reimport
from superhelp.utils import get_docstring_start, layout_comment as layout, make_tmp_file # @Reimport
from superhelp.utils import get_docstring_start, layout_comment as layout, make_open_tmp_file # @Reimport
BlockDets = namedtuple(
'BlockDets', 'element, pre_block_code_str, block_code_str, first_line_no')
......@@ -181,13 +181,14 @@ def get_message_dets_from_input(advisor_dets, *,
else:
raise
except Exception as e:
brief_name = '.'.join(name.split('.')[-2:]) ## last two parts only
message = {
conf.BRIEF: (
layout(f"""\
### Advisor "`{name}`" unable to run
### Advisor "`{brief_name}`" unable to run
Advisor {name} unable to run. Advisor description:
Advisor {brief_name} unable to run. Advisor description:
""")
+ ## show first line of docstring (subsequent lines might have more technical, internally-oriented comments)
layout(get_docstring_start(docstring) + '\n')
......@@ -276,12 +277,22 @@ def get_block_level_messages_dets(blocks_dets, xml):
def get_overall_snippet_messages_dets(snippet, blocks_dets):
"""
Returns messages which apply to snippet as a whole, not just specific
blocks. E.g. looking at every block to look for opportunities to unpack.
blocks. E.g. looking at every block to look for opportunities to unpack. Or
reporting on linting results.
"""
messages_dets = []
for advisor_dets in advisors.SNIPPET_ADVISORS:
all_advisors_dets = (
advisors.ALL_BLOCKS_ADVISORS + advisors.SNIPPET_STR_ADVISORS)
for advisor_dets in all_advisors_dets:
if advisor_dets.input_type == conf.BLOCKS_DETS:
advisor_input = blocks_dets
elif advisor_dets.input_type == conf.SNIPPET_STR:
advisor_input = snippet
else:
raise Exception(
f"Unexpected input_type: '{advisor_dets.input_type}'")
message_dets = get_message_dets_from_input(advisor_dets,
advisor_input=blocks_dets, code_str=snippet, first_line_no=None,
advisor_input=advisor_input, code_str=snippet, first_line_no=None,
repeat=False)
if message_dets:
messages_dets.append(message_dets)
......@@ -321,7 +332,7 @@ def get_separated_messages_dets(snippet, snippet_block_els, xml):
return overall_snippet_messages_dets, block_level_messages_dets
def store_ast_output(xml):
_tmp_ast_fh, tmp_ast_output_xml_fpath = make_tmp_file(
_tmp_ast_fh, tmp_ast_output_xml_fpath = make_open_tmp_file(
conf.AST_OUTPUT_XML_FNAME, mode='w')
xml.getroottree().write(str(tmp_ast_output_xml_fpath), pretty_print=True)
logging.info("\n\n\n\n\nUpdating AST\n\n\n\n\n")
......
......@@ -7,7 +7,12 @@ from textwrap import dedent, wrap
from . import conf
def make_tmp_file(fname, mode='w'):
def make_open_tmp_file(fname, mode='w'):
"""
Note - file needs to be closed for changes to be saved to the file -
otherwise it will be 0 bytes. Up to client code to ensure it is closed and
properly available for subsequent steps.
"""
tmp_fh = tempfile.NamedTemporaryFile(mode=mode, delete=False)
randomly_named_fpath = Path(tmp_fh.name)
fpath = Path(randomly_named_fpath.parent) / fname
......@@ -58,6 +63,10 @@ def int2nice(num):
return nice.get(num, num)
def layout_comment(raw_comment, *, is_code=False):
"""
Don't break up long lines which are titles otherwise the subsequent parts
won't carry appropriate heading-level formatting.
"""
if '`' in raw_comment and is_code:
logging.debug("Backtick detected in code which is probably a mistake")
if is_code:
......@@ -80,7 +89,10 @@ def layout_comment(raw_comment, *, is_code=False):
len(raw_paragraph) - len(raw_paragraph.rstrip('\n')))
paragraph = raw_paragraph.strip()
one_line_paragraph = paragraph.replace('\n', ' ') ## actually continuations of same line so no need to put on separate lines
wrapped_paragraph_lines = wrap(one_line_paragraph)
if one_line_paragraph.startswith('#'):
wrapped_paragraph_lines = [one_line_paragraph, ]
else:
wrapped_paragraph_lines = wrap(one_line_paragraph)
new_paragraph = (
(n_start_new_lines * '\n\n')
+ '\n'.join(wrapped_paragraph_lines)
......
......@@ -12,6 +12,8 @@ except (ImportError, ValueError):
from superhelp import conf # @Reimport
from superhelp.messages import _get_tree, get_separated_messages_dets, store_ast_output # @Reimport
conf.INCLUDE_LINTING = False
def get_actual_source_freqs(messages_dets, expected_source_freqs):
"""
Check the message sources are as expected. Note - we don't have to know what
......
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