From 3e29babc5cf807de489b680cd09524e8417126fe Mon Sep 17 00:00:00 2001 From: Richard Lobb Date: Wed, 11 Dec 2024 11:27:52 +1300 Subject: [PATCH] On-going tweaks to lots of question types --- python3_files_function/__pystylechecker.py | 36 ++ python3_files_function/__resulttable.py | 2 +- python3_files_program/__pystylechecker.py | 154 +++++++-- python3_files_program/__resulttable.py | 9 +- python3_files_program/__tester.py | 1 + python3_files_program/marksheet2.csv | 31 -- python3_files_program/pyproject.toml | 4 +- python3_files_program/pytester.py | 2 - python3_scratchpad/__resulttable.py | 2 +- python3_scratchpad/template.py | 34 +- python3_sst/prototypeextra.html | 14 +- python3_sst/template.py | 138 ++++++++ python3_stage1_gapfiller/__pystylechecker.py | 227 +++++++++++-- python3_stage1_gapfiller/__resulttable.py | 145 ++++++-- python3_stage1_gapfiller/__tester.py | 1 + python3_stage1_gapfiller/pyproject.toml | 4 +- python3_stage1_gapfiller/pytester.py | 104 ++++-- python3_stage1_gapfiller/template2.py | 332 +++++++++++++++++++ 18 files changed, 1074 insertions(+), 166 deletions(-) delete mode 100644 python3_files_program/marksheet2.csv create mode 100644 python3_sst/template.py create mode 100644 python3_stage1_gapfiller/template2.py diff --git a/python3_files_function/__pystylechecker.py b/python3_files_function/__pystylechecker.py index 44ec53a..6c52e95 100644 --- a/python3_files_function/__pystylechecker.py +++ b/python3_files_function/__pystylechecker.py @@ -180,6 +180,9 @@ def local_errors(self): if num_constants > self.params['maxnumconstants']: errors.append("You may not use more than " + str(self.params['maxnumconstants']) + " constants.") + if self.params.get('banfunctionredefinitions', True): + errors += self.find_redefinitions() + # (mct63) Check if anything restricted is being imported. if 'restrictedmodules' in self.params: restricted = self.params['restrictedmodules'] @@ -533,6 +536,39 @@ def visit_If(self, node): return global_errors + def find_redefinitions(self): + """Check the code for any cases where a variable has the same name as the + function in which it is being used. + """ + redefinitions = [] + class RedefinitionChecker(ast.NodeVisitor): + def __init__(self): + self.scopes = [] + self.function_names = {} # Map from name to line number of def + + def visit_FunctionDef(self, node): + self.function_names[node.name] = node.lineno # Record the function name and line no. + self.scopes.append(set()) + self.generic_visit(node) # Visit the body + self.scopes.pop() + + def visit_Assign(self, node): + if self.scopes: + current_scope = self.scopes[-1] + for target in node.targets: + if isinstance(target, ast.Name): + if target.id in self.function_names and target.id not in current_scope: + def_linenum = self.function_names[target.id] + redefinitions.append(f"SourceFile:{target.lineno}:0 FUNC_REDEF: Variable '{target.id}' is the name of a function defined at line {def_linenum}.") + current_scope.add(target.id) # Prevent repetitions of the error. + self.generic_visit(node) + + visitor = RedefinitionChecker() + visitor.visit(self.tree) + return redefinitions + + + def is_main_check(self, node): """Return True iff the given node is a check if the current module is '__main__'. Just checks if both the strings '__name__' and '__main__' are present in the line. diff --git a/python3_files_function/__resulttable.py b/python3_files_function/__resulttable.py index fcea668..b9d8070 100644 --- a/python3_files_function/__resulttable.py +++ b/python3_files_function/__resulttable.py @@ -59,7 +59,7 @@ def set_header(self, testcases): if field == 'extra' and self.is_file_question: format = '%h' # See format_extra function. else: - format = format if format else '%s' + format = format[0] if format else '%s' self.column_formats[field] = format self.column_formats_by_hdr[hdr] = format diff --git a/python3_files_program/__pystylechecker.py b/python3_files_program/__pystylechecker.py index 6f18785..6c52e95 100644 --- a/python3_files_program/__pystylechecker.py +++ b/python3_files_program/__pystylechecker.py @@ -37,34 +37,45 @@ def style_errors(self): env = os.environ.copy() env['HOME'] = os.getcwd() pylint_opts = self.params.get('pylintoptions',[]) + ruff_opts = self.params.get('ruffoptions', []) + precheckers = self.params.get('precheckers', ['pylint']) result = '' - if 'pylint' in precheckers: - try: # Run pylint - cmd = f'{sys.executable} -m pylint ' + ' '.join(pylint_opts) + ' __source.py' - result = subprocess.check_output(cmd, - stderr=subprocess.STDOUT, - universal_newlines=True, - env=env, - shell=True) - except Exception as e: - result = e.output - - else: - # (mct63) Abort if there are any comments containing 'pylint:'. - try: - tokenizer = tokenize.tokenize(BytesIO(self.student_answer.encode('utf-8')).readline) - for token_type, token_text, *_ in tokenizer: - if token_type == tokenize.COMMENT and 'pylint:' in token_text: - errors.append("Comments can not include 'pylint:'") - break - - except Exception: - errors.append("Something went wrong while parsing comments. Report this.") - - if "Using config file" in result: - result = '\n'.join(result.splitlines()[1:]).split() + # First try checking with pylint and/or ruff. + # Ruff treats filenames starting with underscore as private, changing its behaviour. + # So we create a temporary file source.py then delete it again. + linters = [ + ('pylint', f'{sys.executable} -m pylint ' + ' '.join(pylint_opts) + ' __source.py', 'pylint:'), + ('ruff', f'cp __source.py source.py;/usr/local/bin/ruff check --quiet {" ".join(ruff_opts)} source.py; rm source.py', 'noqa'), + ] + + for linter, cmd, disable_keyword in linters: + if linter in precheckers: + try: # Run pylint or ruff + result = subprocess.check_output(cmd, + stderr=subprocess.STDOUT, + universal_newlines=True, + env=env, + shell=True) + except Exception as e: + result = e.output + + else: + # (mct63) Abort if there are any comments disabling the linter'. + try: + tokenizer = tokenize.tokenize(BytesIO(self.student_answer.encode('utf-8')).readline) + for token_type, token_text, *_ in tokenizer: + if token_type == tokenize.COMMENT and disable_keyword in token_text: + errors.append(f"Comments can not include '{disable_keyword}'") + break + + except Exception: + errors.append("Something went wrong while parsing comments. Report this.") + + if "Using config file" in result: + result = '\n'.join(result.splitlines()[1:]).split() + if result == '' and 'mypy' in precheckers: code_to_check = 'from typing import List as list, Dict as dict, Tuple as tuple, Set as set, Any\n' + code_to_check @@ -169,6 +180,9 @@ def local_errors(self): if num_constants > self.params['maxnumconstants']: errors.append("You may not use more than " + str(self.params['maxnumconstants']) + " constants.") + if self.params.get('banfunctionredefinitions', True): + errors += self.find_redefinitions() + # (mct63) Check if anything restricted is being imported. if 'restrictedmodules' in self.params: restricted = self.params['restrictedmodules'] @@ -182,6 +196,13 @@ def local_errors(self): name in restricted[import_name].get('disallow', [])): errors.append("Your program should not import '{}' from '{}'.".format(name, import_name)) + if 'maxreturndepth' in self.params and (max_depth := self.params['maxreturndepth']) is not None: + bad_returns = self.find_nested_returns(max_depth) + if bad_returns: + if max_depth == 1: + errors.append("This question does not allow return statements within loops, if statements etc") + else: + errors.append(f"This question does not allow return statements to be indented more than '{max_depth}' levels") return errors def find_all_imports(self): @@ -273,6 +294,48 @@ def visit_AsyncFunctionDef(self, node): visitor = FuncFinder() visitor.visit(self.tree) return defined + + + def nested_returns(self): + """Return a dictionary in which the keys are nesting depth (0, 1, 2, ..9) and the + values are counts of the number of return statements at that level. Nesting + level is deemed to increase with def, if, for, while, try, except and with + statements. + """ + counts = {i: 0 for i in range(10)} + depth = 0 + class ReturnFinder(ast.NodeVisitor): + def visit_body(self, node): + nonlocal depth + depth += 1 + self.generic_visit(node) + depth -= 1 + def visit_For(self, node): + self.visit_body(node) + def visit_While(self, node): + self.visit_body(node) + def visit_FunctionDef(self, node): + self.visit_body(node) + def visit_If(self, node): + self.visit_body(node) + def visit_Try(self, node): + self.visit_body(node) + def visit_TryExcept(self, node): + self.visit_body(node) + def visit_TryFinally(self, node): + self.visit_body(node) + def visit_ExceptHandler(self, node): + self.visit_body(node) + def visit_With(self, node): + self.visit_body(node) + def visit_Return(self, node): + nonlocal depth + counts[depth] += 1 + self.generic_visit(node) + + visitor = ReturnFinder() + visitor.visit(self.tree) + return counts def constructs_used(self): @@ -473,6 +536,39 @@ def visit_If(self, node): return global_errors + def find_redefinitions(self): + """Check the code for any cases where a variable has the same name as the + function in which it is being used. + """ + redefinitions = [] + class RedefinitionChecker(ast.NodeVisitor): + def __init__(self): + self.scopes = [] + self.function_names = {} # Map from name to line number of def + + def visit_FunctionDef(self, node): + self.function_names[node.name] = node.lineno # Record the function name and line no. + self.scopes.append(set()) + self.generic_visit(node) # Visit the body + self.scopes.pop() + + def visit_Assign(self, node): + if self.scopes: + current_scope = self.scopes[-1] + for target in node.targets: + if isinstance(target, ast.Name): + if target.id in self.function_names and target.id not in current_scope: + def_linenum = self.function_names[target.id] + redefinitions.append(f"SourceFile:{target.lineno}:0 FUNC_REDEF: Variable '{target.id}' is the name of a function defined at line {def_linenum}.") + current_scope.add(target.id) # Prevent repetitions of the error. + self.generic_visit(node) + + visitor = RedefinitionChecker() + visitor.visit(self.tree) + return redefinitions + + + def is_main_check(self, node): """Return True iff the given node is a check if the current module is '__main__'. Just checks if both the strings '__name__' and '__main__' are present in the line. @@ -501,3 +597,11 @@ def visit_AsyncFunctionDef(self, node): visitor = MyVisitor() visitor.visit(self.tree) return bad_funcs + + + def find_nested_returns(self, max_depth): + """Return a count of the number of return statements + at a nesting depth in excess of max_depth. + """ + returns = self.nested_returns() + return sum(returns[depth] for depth in range(max_depth + 1, 10)) diff --git a/python3_files_program/__resulttable.py b/python3_files_program/__resulttable.py index ebbb307..b9d8070 100644 --- a/python3_files_program/__resulttable.py +++ b/python3_files_program/__resulttable.py @@ -59,7 +59,7 @@ def set_header(self, testcases): if field == 'extra' and self.is_file_question: format = '%h' # See format_extra function. else: - format = format if format else '%s' + format = format[0] if format else '%s' self.column_formats[field] = format self.column_formats_by_hdr[hdr] = format @@ -255,8 +255,11 @@ def add_image(self, image_html, column_name, row_num): It should be either Expected or Got (all we can handle in this code). row_num is the row number (0 origin, not including the header row). """ - column_num = self.table[0].index(column_name) - self.images[column_num, row_num + 1].append(image_html) + try: + column_num = self.table[0].index(column_name) + self.images[column_num, row_num + 1].append(image_html) + except (IndexError, ValueError): + raise Exception(f"Can't insert '{column_name}' image into result table as the column does not exist.") def equal_strings(self, s1, s2): """ Compare the two strings s1 and s2 (expected and got respectively) diff --git a/python3_files_program/__tester.py b/python3_files_program/__tester.py index 78a1dd2..0996548 100644 --- a/python3_files_program/__tester.py +++ b/python3_files_program/__tester.py @@ -42,6 +42,7 @@ class Tester: def __init__(self, params, testcases): """Initialise the instance, given the test of template and global parameters plus all the testcases. Parameters required by this base class and all subclasses are: + 'IS_PRECHECK': True if this run is a precheck only 'STUDENT_ANSWER': code submitted by the student 'SEPARATOR': the string to be used to separate tests in the output 'ALL_OR_NOTHING: true if grading is all-or-nothing diff --git a/python3_files_program/marksheet2.csv b/python3_files_program/marksheet2.csv deleted file mode 100644 index 0210b44..0000000 --- a/python3_files_program/marksheet2.csv +++ /dev/null @@ -1,31 +0,0 @@ -Username,Asst1,Asst2,Test -xjt373,67.4,72.4,68.5 -opg408,39.1,36.5,54.5 -buj951,78.9,82.9,71.5 -cjb659,55.8,63.2,62.3 -ffo562,45.8,49.3,36.7 -sdz913,49.7,42.4,53.0 -zbl691,95.5,85.5,91.7 -maq792,49.2,35.9,48.2 -jmv820,93.5,81.0,84.8 -rlc056,39.6,48.1,48.5 -aqh584,84.1,98.5,86.5 -fxo656,89.7,87.1,95.1 -nuv914,45.9,39.5,48.0 -uqq114,44.0,46.1,40.1 -onj197,94.5,89.8,88.7 -grw409,81.3,98.9,98.8 -xcf621,90.2,80.9,88.3 -rfx266,45.0,59.7,64.6 -jvd250,47.5,41.3,48.9 -njl581,81.1,72.4,87.0 -lts885,62.8,77.1,80.4 -dql916,99.8,95.4,90.0 -nof586,93.7,91.7,95.2 -zcq092,68.3,72.6,72.7 -tvp451,82.3,77.8,89.1 -lls701,100.0,91.0,100.0 -zhx525,62.9,70.8,60.3 -cwn365,44.3,43.4,47.7 -qii439,100.0,91.6,87.5 -rkq415,52.8,46.2,55.4 diff --git a/python3_files_program/pyproject.toml b/python3_files_program/pyproject.toml index 56a9cc8..8642df5 100644 --- a/python3_files_program/pyproject.toml +++ b/python3_files_program/pyproject.toml @@ -9,7 +9,7 @@ max-branches = 10 [tool.ruff.lint] select = ["A00", "ARG", - "B015", "B018", "B020", + "B015", "B018", "B020", "BLE001", "D1", "E1", "E5", "E701", "E702", "E703", "E711", "E722", "F", @@ -19,5 +19,5 @@ select = ["A00", "S307", "RET503", ] -ignore = ["D105", "D107", "E117", "F401", "F841"] +ignore = ["D105", "D107", "E115", "E116", "E117", "F401", "F841"] preview = true diff --git a/python3_files_program/pytester.py b/python3_files_program/pytester.py index ae6fe81..ea4d833 100644 --- a/python3_files_program/pytester.py +++ b/python3_files_program/pytester.py @@ -169,8 +169,6 @@ def passive_output(self): ' print(f"{_mpl.pyplot.get_figlabels()}")' ]) + '\n' task = pytask.PyTask(self.params, code) - with open(f"WTF{randint(0,100)}.py", 'w') as outfile: - outfile.write(code) task.compile() captured_output, captured_error = task.run_code() return (captured_output + '\n' + captured_error).strip() diff --git a/python3_scratchpad/__resulttable.py b/python3_scratchpad/__resulttable.py index fcea668..b9d8070 100644 --- a/python3_scratchpad/__resulttable.py +++ b/python3_scratchpad/__resulttable.py @@ -59,7 +59,7 @@ def set_header(self, testcases): if field == 'extra' and self.is_file_question: format = '%h' # See format_extra function. else: - format = format if format else '%s' + format = format[0] if format else '%s' self.column_formats[field] = format self.column_formats_by_hdr[hdr] = format diff --git a/python3_scratchpad/template.py b/python3_scratchpad/template.py index 9fa6653..958c62e 100644 --- a/python3_scratchpad/template.py +++ b/python3_scratchpad/template.py @@ -8,7 +8,7 @@ from pytester import PyTester STANDARD_PYLINT_OPTIONS = ['--disable=trailing-whitespace,superfluous-parens,' + - 'too-few-public-methods,consider-using-f-string,' + + 'too-few-public-methods,consider-using-f-string,useless-return,' + 'unbalanced-tuple-unpacking,too-many-statements,' + 'consider-using-enumerate,simplifiable-if-statement,' + 'consider-iterating-dictionary,trailing-newlines,no-else-return,' + @@ -18,7 +18,7 @@ 'consider-using-in,useless-object-inheritance,unnecessary-pass,' + 'reimported,wrong-import-order,wrong-import-position,ungrouped-imports,' + 'consider-using-set-comprehension,no-else-raise,unnecessary-lambda-assignment,' + - 'unspecified-encoding,use-dict-literal,,consider-using-with,' + + 'unspecified-encoding,use-dict-literal,consider-using-with,consider-using-min-builtin,' + 'duplicate-string-formatting-argument,consider-using-dict-items,' + 'consider-using-max-builtin,use-a-generator,unidiomatic-typecheck', '--good-names=i,j,k,n,s,c,_' @@ -30,12 +30,14 @@ KNOWN_PARAMS = { 'abortonerror': True, 'allowglobals': False, - 'banglobalcode': True, 'allownestedfunctions': False, + 'banfunctionredefinitions': True, + 'banglobalcode': True, 'checktemplateparams': True, 'dpi': 65, 'echostandardinput': True, 'extra': 'None', + 'failhiddenonlyfract': 0, 'floattolerance': None, 'forcepylint': False, 'globalextra': 'None', @@ -44,20 +46,22 @@ 'isfunction': True, 'localprechecks': True, 'maxfunctionlength': 30, + 'maxreturndepth': None, + 'maxprechecks': None, 'maxnumconstants': 4, 'maxoutputbytes': 10000, - 'maxprechecks': None, 'maxstringlength': 2000, 'norun': False, 'nostylechecks': False, 'notest': False, 'parsonsproblemthreshold': None, # The number of checks before parsons' problem displayed - 'precheckers': ['pylint'], + 'precheckers': ['ruff'], 'prelude': '', 'proscribedbuiltins': ['exec', 'eval'], 'proscribedfunctions': [], - 'proscribedconstructs': ["goto"], + 'proscribedconstructs': ["goto", "while_with_else"], 'proscribedsubstrings': [], + 'protectedfiles': [], 'pylintoptions': [], 'pylintmatplotlib': False, 'requiredconstructs': [], @@ -91,6 +95,8 @@ 'disallow': ['_.*'] }, }, + 'resultcolumns': [], # If not specified, use question's resultcolumns value. See below. + 'ruffoptions': [], 'runextra': False, 'showfeedbackwhenright': False, 'stdinfromextra': False, @@ -146,12 +152,6 @@ def process_template_params(): else: PARAMS[param_name] = default; - result_columns = """{{QUESTION.resultcolumns}}""".strip(); - if result_columns == '': - PARAMS['resultcolumns'] = [['Test', 'testcode'], ['Input', 'stdin'], ['Expected', 'expected'], ['Got', 'got']] - else: - PARAMS['resultcolumns'] = json.loads(result_columns); - if PARAMS['extra'] == 'stdin': PARAMS['stdinfromextra'] = True if PARAMS['runextra']: @@ -168,6 +168,15 @@ def process_template_params(): PARAMS['precheckers'] = [] if PARAMS['testisbash']: print("testisbash is not implemented for Python") + + # We use the template parameter for resultcolumns if non-empty. + # Otherwise use the value from the question, or an equivalent default if that's empty too. + q_result_columns = """{{QUESTION.resultcolumns}}""".strip(); + if PARAMS['resultcolumns'] == []: + if q_result_columns: + PARAMS['resultcolumns'] = json.loads(q_result_columns); + else: + PARAMS['resultcolumns'] = [['Test', 'testcode'], ['Input', 'stdin'], ['Expected', 'expected'], ['Got', 'got']] def get_test_cases(): @@ -254,6 +263,7 @@ def get_expecteds_from_answer(params, test_cases): new_params['IS_PRECHECK'] = False new_params['nostylechecks'] = True new_params['STUDENT_ANSWER'] = get_answer() + new_params['resultcolumns'] = [['Test', 'testcode'], ['Got', 'got']] # Ensure we have a Got column. new_params['running_sample_answer'] = True tester = PyTester(new_params, test_cases) outcome = tester.test_code() diff --git a/python3_sst/prototypeextra.html b/python3_sst/prototypeextra.html index ba7114c..61862d2 100644 --- a/python3_sst/prototypeextra.html +++ b/python3_sst/prototypeextra.html @@ -39,7 +39,7 @@ table.sst th { font-weight: bold; - border: 2px solid black; + border: 3px solid black; } table.sst th.cr-variables-header { @@ -54,23 +54,23 @@ } table.sst td.cr-bottom { - border-bottom: 2px solid black; + border-bottom: 3px solid black; } table.sst .cr-output-col, table.sst .cr-linenumber-col, table.sst .cr-return-row, table.sst td.cr-separator { - border-right: 2px solid black; - border-left: 2px solid black; + border-right: 3px solid black; + border-left: 3px solid black; } table.sst td.cr-last-col { - border-right: 2px solid black; + border-right: 3px solid black; } table.sst td.cr-variable-name { - border-bottom: 2px solid black; + border-bottom: 3px solid black; } /* Input styles */ @@ -210,7 +210,7 @@

SST parameters

`; } if (has_output) { - bottom = (params.return_width && i == params.num_rows - 1) ? ' cr-bottom' : ''; + bottom = (params.output_width && i == params.num_rows) ? ' cr-bottom' : ''; value = getValue(data, 'output', i - 1) table_html += ` diff --git a/python3_sst/template.py b/python3_sst/template.py new file mode 100644 index 0000000..55957aa --- /dev/null +++ b/python3_sst/template.py @@ -0,0 +1,138 @@ +import json +import ast + +class BadLiteral(Exception): pass + +def partition(values, num_cols): + """Return the given list of values partitioned into a 2D array with given number of columns""" + num_rows = len(values) // num_cols + table = [[values[row * num_cols + col] for col in range(num_cols)] for row in range(num_rows)] + return table + +def first_error(got_cells, expected_cells): + """Return the index of the first difference""" + for i, (got, expect) in enumerate(zip(got_cells, expected_cells)): + if got != expect: + return i + return None # Shouldn't happen + + +def parse_literals(lits): + """Use ast.eval_literal to convert a list of string values into + a list of Python literal values, which is returned. + Any parse errors raise BadLiteral(bad_string). + """ + values = [] + for lit in lits: + if lit == '': + values.append(lit) + else: + try: + values.append(ast.literal_eval(lit)) + except: + raise BadLiteral(lit) + return values + + +def check_return(got_return, expected_return): + """Return true if return value is correct, print and + error and return false otherwise + """ + try: + expected_return_value = ast.literal_eval(expected_return) + except: + print("Naughty question author: invalid literal for expected return value") + return False + + try: + got_return_value = ast.literal_eval(got_return) + except: + print("Your return value is not valid. Missing quotes perhaps?") + + if type(got_return_value) != type(expected_return_value): + print("Your return value is of the wrong type.") + print(f"Expected {type(expected_return_value)}, got {type(got_return_value)}") + return False + + if got_return_value != expected_return_value: + print("Your return value is wrong") + return False + + return True + +got = json.loads("""{{ STUDENT_ANSWER | e('py') }}""") +expected = json.loads("""{{ QUESTION.answer | e('py') }}""") + +ok = True +got_vars = [s.strip() for s in got['variable_name']] +got_vars_lc = [s.lower() for s in got_vars] +expected_vars = [s.strip() for s in expected['variable_name']] +num_vars = len(expected_vars) +expected_vars_lc = [s.lower() for s in expected_vars] +has_output = 'output' in expected +has_return = 'return_value' in expected + +if has_output: + expected_output = [s.strip() for s in expected['output']] + got_output = [s.strip() for s in got['output']] + +if has_return: + expected_return = expected['return_value'][0].strip() + got_return = got['return_value'][0].strip() + +if got_vars != expected_vars: + # Deal with wrong list of variable names + if set(got_vars) == set(expected_vars): + print("Your variable names are in the wrong order") + elif got_vars_lc == expected_vars_lc: + print('Variable names must be lower case') + else: + print("The variable names in the top row are not right.") + #print(expected_vars, got_vars) + ok = False + +if ok: + # Process the list of line numbers + got_line_nums = [s.strip().lower() for s in got['line_number']] + expected_line_nums = [s.strip().lower() for s in expected['line_number']] + if got_line_nums != expected_line_nums: + print("The sequence of line numbers is wrong.") + ok = False + +if ok: + # Process the list of variables + got_values = partition([s.strip() for s in got['variable_value']], num_vars) + expected_values = partition([s.strip() for s in expected['variable_value']], num_vars) + for i, var in enumerate(got_vars): + try: + got_var_values = parse_literals([got_values[row][i] for row in range(len(got_values))]) + expected_var_values = parse_literals([expected_values[row][i] for row in range(len(expected_values))]) + if got_var_values != expected_var_values: + print(f"Your values for the variable '{var}' aren't right.") + line = first_error(got_var_values, expected_var_values) + print(f"The first error is at step {line + 1} (line number {got_line_nums[line]}).") + got = got_var_values[line] + expected = expected_var_values[line] + if type(got) != type(expected): + print("Your value is of the wrong type.") + print(f"Expected {type(expected)}, got {type(got)}") + ok = False + except BadLiteral as bl: + print(f"'{bl}' is an invalid value. Might be missing quotes?") + ok = False + + +if ok and has_output: + # Process the output + if got_output != expected_output: + print("The output table is wrong") + step = first_error(got_output, expected_output) + line_number = got_line_nums[step] + print(f"The first error is at step {step + 1} (line number {line_number}).") + ok = False + +if ok and has_return: + ok = check_return(got_return, expected_return) + +if ok: + print("OK") diff --git a/python3_stage1_gapfiller/__pystylechecker.py b/python3_stage1_gapfiller/__pystylechecker.py index 2118dcc..6c52e95 100644 --- a/python3_stage1_gapfiller/__pystylechecker.py +++ b/python3_stage1_gapfiller/__pystylechecker.py @@ -2,6 +2,7 @@ from io import BytesIO import os +import sys import subprocess import ast import tokenize @@ -36,40 +37,51 @@ def style_errors(self): env = os.environ.copy() env['HOME'] = os.getcwd() pylint_opts = self.params.get('pylintoptions',[]) + ruff_opts = self.params.get('ruffoptions', []) + precheckers = self.params.get('precheckers', ['pylint']) result = '' - if 'pylint' in precheckers: - try: # Run pylint - cmd = 'python3.9 -m pylint ' + ' '.join(pylint_opts) + ' __source.py' - result = subprocess.check_output(cmd, - stderr=subprocess.STDOUT, - universal_newlines=True, - env=env, - shell=True) - except Exception as e: - result = e.output - - else: - # (mct63) Abort if there are any comments containing 'pylint:'. - try: - tokenizer = tokenize.tokenize(BytesIO(self.student_answer.encode('utf-8')).readline) - for token_type, token_text, *_ in tokenizer: - if token_type == tokenize.COMMENT and 'pylint:' in token_text: - errors.append("Comments can not include 'pylint:'") - break - - except Exception: - errors.append("Something went wrong while parsing comments. Report this.") - - if "Using config file" in result: - result = '\n'.join(result.splitlines()[1:]).split() + # First try checking with pylint and/or ruff. + # Ruff treats filenames starting with underscore as private, changing its behaviour. + # So we create a temporary file source.py then delete it again. + linters = [ + ('pylint', f'{sys.executable} -m pylint ' + ' '.join(pylint_opts) + ' __source.py', 'pylint:'), + ('ruff', f'cp __source.py source.py;/usr/local/bin/ruff check --quiet {" ".join(ruff_opts)} source.py; rm source.py', 'noqa'), + ] + + for linter, cmd, disable_keyword in linters: + if linter in precheckers: + try: # Run pylint or ruff + result = subprocess.check_output(cmd, + stderr=subprocess.STDOUT, + universal_newlines=True, + env=env, + shell=True) + except Exception as e: + result = e.output + + else: + # (mct63) Abort if there are any comments disabling the linter'. + try: + tokenizer = tokenize.tokenize(BytesIO(self.student_answer.encode('utf-8')).readline) + for token_type, token_text, *_ in tokenizer: + if token_type == tokenize.COMMENT and disable_keyword in token_text: + errors.append(f"Comments can not include '{disable_keyword}'") + break + + except Exception: + errors.append("Something went wrong while parsing comments. Report this.") + + if "Using config file" in result: + result = '\n'.join(result.splitlines()[1:]).split() + if result == '' and 'mypy' in precheckers: code_to_check = 'from typing import List as list, Dict as dict, Tuple as tuple, Set as set, Any\n' + code_to_check with open('__source2.py', 'w', encoding='utf-8') as outfile: outfile.write(code_to_check) - cmd = 'python3.8 -m mypy --no-error-summary --no-strict-optional __source2.py' + cmd = f'{sys.executable} -m mypy --no-error-summary --no-strict-optional __source2.py' try: # Run mypy subprocess.check_output(cmd, # Raises an exception if there are errors stderr=subprocess.STDOUT, @@ -88,9 +100,25 @@ def style_errors(self): if result: errors = result.strip().splitlines() - errors.append("Sorry, but your code doesn't pass the style checks.") return errors + + def prettied(self, construct): + """Expand, if possible, the name of the given Python construct to a more + user friendly version, e.g. 'listcomprehension' -> 'list comprehension' + """ + expanded = { + 'listcomprehension': 'list comprehension', + 'while': 'while loop', + 'for': 'for loop', + 'try': 'try ... except statement', + 'dictcomprehension': 'dictionary comprehension', + 'slice': 'slice' + } + if construct in expanded: + return expanded[construct] + else: + return f"{construct} statement" def local_errors(self): """Perform various local checks as specified by the current set of @@ -111,6 +139,9 @@ def local_errors(self): elif 'string' in required and required['string'] not in self.student_answer: errors.append(required['errormessage']) + if self.params.get('banglobalcode', True): + errors += self.find_global_code() + if not self.params.get('allownestedfunctions', True): # Except for legacy questions or where explicitly allowed, nested functions are banned nested_funcs = self.find_nested_functions() @@ -137,16 +168,21 @@ def local_errors(self): missing_constructs = self.find_missing_required_constructs() for reqd in missing_constructs: - errors.append("Your program must include at least one " + reqd + " statement.") + expanded = self.prettied(reqd) + errors.append(f"Your program must include at least one {expanded}.") bad_constructs = self.find_illegal_constructs() for notallowed in bad_constructs: - errors.append("Your program must not include any " + notallowed + "s.") + expanded = self.prettied(notallowed) + errors.append(f"Your program must not include any {expanded}s.") num_constants = len([line for line in self.student_answer.split('\n') if re.match(' *[A-Z_][A-Z_0-9]* *=', line)]) if num_constants > self.params['maxnumconstants']: errors.append("You may not use more than " + str(self.params['maxnumconstants']) + " constants.") + if self.params.get('banfunctionredefinitions', True): + errors += self.find_redefinitions() + # (mct63) Check if anything restricted is being imported. if 'restrictedmodules' in self.params: restricted = self.params['restrictedmodules'] @@ -160,6 +196,13 @@ def local_errors(self): name in restricted[import_name].get('disallow', [])): errors.append("Your program should not import '{}' from '{}'.".format(name, import_name)) + if 'maxreturndepth' in self.params and (max_depth := self.params['maxreturndepth']) is not None: + bad_returns = self.find_nested_returns(max_depth) + if bad_returns: + if max_depth == 1: + errors.append("This question does not allow return statements within loops, if statements etc") + else: + errors.append(f"This question does not allow return statements to be indented more than '{max_depth}' levels") return errors def find_all_imports(self): @@ -251,6 +294,48 @@ def visit_AsyncFunctionDef(self, node): visitor = FuncFinder() visitor.visit(self.tree) return defined + + + def nested_returns(self): + """Return a dictionary in which the keys are nesting depth (0, 1, 2, ..9) and the + values are counts of the number of return statements at that level. Nesting + level is deemed to increase with def, if, for, while, try, except and with + statements. + """ + counts = {i: 0 for i in range(10)} + depth = 0 + class ReturnFinder(ast.NodeVisitor): + def visit_body(self, node): + nonlocal depth + depth += 1 + self.generic_visit(node) + depth -= 1 + def visit_For(self, node): + self.visit_body(node) + def visit_While(self, node): + self.visit_body(node) + def visit_FunctionDef(self, node): + self.visit_body(node) + def visit_If(self, node): + self.visit_body(node) + def visit_Try(self, node): + self.visit_body(node) + def visit_TryExcept(self, node): + self.visit_body(node) + def visit_TryFinally(self, node): + self.visit_body(node) + def visit_ExceptHandler(self, node): + self.visit_body(node) + def visit_With(self, node): + self.visit_body(node) + def visit_Return(self, node): + nonlocal depth + counts[depth] += 1 + self.generic_visit(node) + + visitor = ReturnFinder() + visitor.visit(self.tree) + return counts def constructs_used(self): @@ -277,6 +362,8 @@ def visit_For(self, node): self.generic_visit(node) def visit_While(self, node): constructs_seen.add('while') + if node.orelse: + constructs_seen.add('while_with_else') self.generic_visit(node) def visit_Comprehension(self, node): constructs_seen.add('comprehension') @@ -416,6 +503,80 @@ def visit_AsyncFunctionDef(self, node): return bad_funcs + def find_global_code(self): + """Return a list of error messages relating to the existence of + any global assignment, for, while and if nodes. Ignores + global assignment statements with an ALL_CAPS target and + if __name__ == "__main__" + """ + style_checker = self + global_errors = [] + class MyVisitor(ast.NodeVisitor): + def visit_Assign(self, node): + if node.col_offset == 0: + if len(node.targets) > 1 or isinstance(node.targets[0], ast.Tuple): + global_errors.append(f"Multiple targets in global assignment statement at line {node.lineno}") + elif not (node.targets[0].id.isupper() or isinstance(node.value, ast.Lambda)): + global_errors.append(f"Global assignment statement at line {node.lineno}") + + def visit_For(self, node): + if node.col_offset == 0: + global_errors.append(f"Global for loop at line {node.lineno}") + + def visit_While(self, node): + if node.col_offset == 0: + global_errors.append(f"Global while loop at line {node.lineno}") + + def visit_If(self, node): + if node.col_offset == 0 and not style_checker.is_main_check(node): + global_errors.append(f"Global if statement at line {node.lineno}") + + visitor = MyVisitor() + visitor.visit(self.tree) + return global_errors + + + def find_redefinitions(self): + """Check the code for any cases where a variable has the same name as the + function in which it is being used. + """ + redefinitions = [] + class RedefinitionChecker(ast.NodeVisitor): + def __init__(self): + self.scopes = [] + self.function_names = {} # Map from name to line number of def + + def visit_FunctionDef(self, node): + self.function_names[node.name] = node.lineno # Record the function name and line no. + self.scopes.append(set()) + self.generic_visit(node) # Visit the body + self.scopes.pop() + + def visit_Assign(self, node): + if self.scopes: + current_scope = self.scopes[-1] + for target in node.targets: + if isinstance(target, ast.Name): + if target.id in self.function_names and target.id not in current_scope: + def_linenum = self.function_names[target.id] + redefinitions.append(f"SourceFile:{target.lineno}:0 FUNC_REDEF: Variable '{target.id}' is the name of a function defined at line {def_linenum}.") + current_scope.add(target.id) # Prevent repetitions of the error. + self.generic_visit(node) + + visitor = RedefinitionChecker() + visitor.visit(self.tree) + return redefinitions + + + + def is_main_check(self, node): + """Return True iff the given node is a check if the current module is '__main__'. + Just checks if both the strings '__name__' and '__main__' are present in the line. + """ + line = self.student_answer.splitlines()[node.lineno - 1] + return '__main__' in line and '__name__' in line + + def find_nested_functions(self): """Return a list of functions that are declared with non-global scope""" bad_funcs = [] @@ -436,3 +597,11 @@ def visit_AsyncFunctionDef(self, node): visitor = MyVisitor() visitor.visit(self.tree) return bad_funcs + + + def find_nested_returns(self, max_depth): + """Return a count of the number of return statements + at a nesting depth in excess of max_depth. + """ + returns = self.nested_returns() + return sum(returns[depth] for depth in range(max_depth + 1, 10)) diff --git a/python3_stage1_gapfiller/__resulttable.py b/python3_stage1_gapfiller/__resulttable.py index 06232bc..b9d8070 100644 --- a/python3_stage1_gapfiller/__resulttable.py +++ b/python3_stage1_gapfiller/__resulttable.py @@ -5,7 +5,9 @@ """ import html import re +import base64 from collections import defaultdict +from urllib.parse import quote MAX_STRING_LENGTH = 4000 # 4k is default maximum string length @@ -14,27 +16,36 @@ class ResultTable: def __init__(self, params): self.params = params self.mark = 0 + self.max_mark = 0 self.table = None self.failed_hidden = False self.aborted = False self.has_stdins = False self.has_tests = False + self.has_extra = False + self.has_expected = False + self.has_got = False self.hiding = False self.num_failed_tests = 0 + self.num_failed_hidden_tests = 0 self.missing_tests = 0 self.global_error = '' - self.column_formats = None + self.column_formats = {} # Map from raw column name to format + self.column_formats_by_hdr = {} # Map from header name to format self.images = defaultdict(list) default_params = { 'stdinfromextra': False, 'strictwhitespace': True, 'floattolerance': None, + 'resultcolumns': [['Test', 'testcode'], ['Input', 'stdin'], ['Expected', 'expected'], ['Got', 'got']], 'ALL_OR_NOTHING': True } for param, value in default_params.items(): if param not in params: self.params[param] = value + self.is_file_question = self.params['extra'] == 'files' + def set_header(self, testcases): """Given the set of testcases, set the header as the first row of the result table @@ -42,23 +53,50 @@ def set_header(self, testcases): of various table columns. """ header = ['iscorrect'] - self.column_formats = ['%s'] - if any(test.testcode.strip() != '' for test in testcases): - header.append("Test") + required_columns = {} + for hdr, field, *format in self.params['resultcolumns']: + required_columns[field] = hdr + if field == 'extra' and self.is_file_question: + format = '%h' # See format_extra function. + else: + format = format[0] if format else '%s' + self.column_formats[field] = format + self.column_formats_by_hdr[hdr] = format + + if 'testcode' in required_columns and any(test.testcode.strip() != '' for test in testcases): + header.append(required_columns['testcode']) self.has_tests = True + + # *** WHAT WAS THIS ABOUT??? *** # If the test code should be rendered in html then set that as column format. - if any(getattr(test, 'test_code_html', None) for test in testcases): - self.column_formats.append('%h') - else: - self.column_formats.append('%s') - - stdins = [test.extra if self.params['stdinfromextra'] else test.stdin for test in testcases] - if any(stdin.rstrip() != '' for stdin in stdins): - header.append('Input') - self.column_formats.append('%s') - self.has_stdins = True - header += ['Expected', 'Got', 'iscorrect', 'ishidden'] - self.column_formats += ['%s', '%s', '%s', '%s'] + #if any(getattr(test, 'test_code_html', None) for test in testcases): + # self.column_formats.append('%h') + #else: + # self.column_formats.append('%s') + + # If this is a write-a-function file question, the stdin field is hidden regardless. + # TODO: are there exceptions to this? + hide_stdin = self.is_file_question and self.params['isfunction'] + if not hide_stdin: + stdins = [test.extra if self.params['stdinfromextra'] else test.stdin for test in testcases] + if 'stdin' in required_columns and any(stdin.rstrip() != '' for stdin in stdins): + header.append(required_columns['stdin']) + self.has_stdins = True + + if 'extra' in required_columns: + header.append(required_columns['extra']) + self.has_extra = True + + if 'expected' in required_columns: + header.append(required_columns['expected']) + self.has_expected = True + + # *** CONSIDER USE OF GOT IN FILES questions (and others). Can contain error messages. Should always include?? + if 'got' in required_columns: + header.append(required_columns['got']) + self.has_got = True + + header += ['iscorrect', 'ishidden'] self.table = [header] def image_column_nums(self): @@ -70,7 +108,12 @@ def get_column_formats(self): Don't have formats for iscorrect and ishidden columns. """ image_columns = self.image_column_nums() - formats = [self.column_formats[i] if i not in image_columns else '%h' for i in range(len(self.column_formats))] + formats = [] + for i, column_hdr in enumerate(self.table[0]): + if i in image_columns: + formats.append('%h') + else: + formats.append(self.column_formats_by_hdr.get(column_hdr, '%s')) return formats[1:-2] def get_table(self): @@ -109,6 +152,21 @@ def tests_missed(self, num): def record_global_error(self, error_message): """Record some sort of global failure""" self.global_error = error_message + + def format_extra_for_files(self, extra, filename): + """Format the extra field (which should be the contents of a file). If + the longest line exceeds filedownloadwidth or the number of lines + exceeds filedownloadlines the file is converted into a data URI. + Otherwise it's returned as is. + """ + lines = extra.splitlines() + too_wide = len(lines) > 0 and (max(len(line) for line in lines) > self.params['filedownloadwidth']) + too_high = len(lines) > self.params['filedownloadlines'] + if too_wide or too_high: + quoted = quote(extra) + link = f'Download' + return link + return '
' + extra + '
' def add_row(self, testcase, result, error=''): """Add a result row to the table for the given test and result""" @@ -119,11 +177,22 @@ def add_row(self, testcase, result, error=''): row.append(testcase.test_code_html) else: row.append(testcase.testcode) + if self.has_stdins: row.append(testcase.extra if self.params['stdinfromextra'] else testcase.stdin) - row.append(testcase.expected.rstrip()) + + if self.has_extra: + if self.params['extra'] == 'files': + filename = testcase.stdin.splitlines()[0] + row.append(self.format_extra_for_files(testcase.extra, filename)) + else: + row.append(testcase.extra) + + if self.has_expected: + row.append(testcase.expected.rstrip()) + max_len = self.params.get('maxstringlength', MAX_STRING_LENGTH) - result = sanitise(result.strip('\n'), max_len) + result = sanitise(result.rstrip('\n'), max_len) if error: error_message = '*** RUN TIME ERROR(S) ***\n' + sanitise(error, max_len) @@ -131,14 +200,20 @@ def add_row(self, testcase, result, error=''): result = result + '\n' + error_message else: result = error_message - row.append(result) + if self.has_got or error: + row.append(result) + + display = testcase.display.upper() + self.max_mark += testcase.mark if is_correct: self.mark += testcase.mark else: self.num_failed_tests += 1 + if display == 'HIDE': + self.num_failed_hidden_tests += 1 row.append(is_correct) - display = testcase.display.upper() + is_hidden = ( self.hiding or display == 'HIDE' or @@ -155,7 +230,16 @@ def add_row(self, testcase, result, error=''): self.aborted = True def get_mark(self): - return self.mark if self.num_failed_tests == 0 or not self.params['ALL_OR_NOTHING'] else 0 + if self.num_failed_tests == 0: + return self.mark + # Failed one or more tests + elif (self.num_failed_tests == self.num_failed_hidden_tests) and self.params['failhiddenonlyfract'] > 0: + return self.max_mark * self.params['failhiddenonlyfract'] + elif not self.params['ALL_OR_NOTHING']: + return self.mark + else: + return 0 + @staticmethod def htmlise(s): @@ -168,10 +252,14 @@ def htmlise(s): def add_image(self, image_html, column_name, row_num): """Store the given html_image for later inclusion in the cell at the given row and given column. column_name is the name used for the column in the first (header) row. + It should be either Expected or Got (all we can handle in this code). row_num is the row number (0 origin, not including the header row). """ - column_num = self.table[0].index(column_name) - self.images[column_num, row_num + 1].append(image_html) + try: + column_num = self.table[0].index(column_name) + self.images[column_num, row_num + 1].append(image_html) + except (IndexError, ValueError): + raise Exception(f"Can't insert '{column_name}' image into result table as the column does not exist.") def equal_strings(self, s1, s2): """ Compare the two strings s1 and s2 (expected and got respectively) @@ -190,8 +278,9 @@ def equal_strings(self, s1, s2): # Matching with a floating point tolerance. # Use float pattern from Markus Schmassmann at # https://stackoverflow.com/questions/12643009/regular-expression-for-floating-point-numbers + # except we don't match inf or nan which can be embedded in text strings. tol = float(self.params['floattolerance']) - float_pat = r'([-+]?(?:(?:(?:[0-9]+[.]?[0-9]*|[.][0-9]+)(?:[ed][-+]?[0-9]+)?)|(?:inf)|(?:nan)))' + float_pat = r'([-+]?(?:(?:(?:[0-9]+[.]?[0-9]*|[.][0-9]+)(?:[ed][-+]?[0-9]+)?)))' s1_bits = re.split(float_pat, s1) s2_bits = re.split(float_pat, s2) if len(s1_bits) != len(s2_bits): @@ -203,7 +292,7 @@ def equal_strings(self, s1, s2): try: f1 = float(bit1) f2 = float(bit2) - if not (abs(f1 - f2) <= tol or (f1 != 0.0 and abs((f1 - f2) / f1) <= tol)): + if abs(f1 - f2) > tol * 1.001: # Allow tolerance on the float tolerance! match = False except ValueError: if bit1 != bit2: @@ -217,8 +306,8 @@ def check_correctness(self, expected, got): pattern and the floating-point bits will be matched to within the given absolute tolerance. """ - expected_lines = expected.strip().splitlines() - got_lines = got.strip().splitlines() + expected_lines = expected.rstrip().splitlines() + got_lines = got.rstrip().splitlines() if len(got_lines) != len(expected_lines): return False else: diff --git a/python3_stage1_gapfiller/__tester.py b/python3_stage1_gapfiller/__tester.py index 78a1dd2..0996548 100644 --- a/python3_stage1_gapfiller/__tester.py +++ b/python3_stage1_gapfiller/__tester.py @@ -42,6 +42,7 @@ class Tester: def __init__(self, params, testcases): """Initialise the instance, given the test of template and global parameters plus all the testcases. Parameters required by this base class and all subclasses are: + 'IS_PRECHECK': True if this run is a precheck only 'STUDENT_ANSWER': code submitted by the student 'SEPARATOR': the string to be used to separate tests in the output 'ALL_OR_NOTHING: true if grading is all-or-nothing diff --git a/python3_stage1_gapfiller/pyproject.toml b/python3_stage1_gapfiller/pyproject.toml index 56a9cc8..8642df5 100644 --- a/python3_stage1_gapfiller/pyproject.toml +++ b/python3_stage1_gapfiller/pyproject.toml @@ -9,7 +9,7 @@ max-branches = 10 [tool.ruff.lint] select = ["A00", "ARG", - "B015", "B018", "B020", + "B015", "B018", "B020", "BLE001", "D1", "E1", "E5", "E701", "E702", "E703", "E711", "E722", "F", @@ -19,5 +19,5 @@ select = ["A00", "S307", "RET503", ] -ignore = ["D105", "D107", "E117", "F401", "F841"] +ignore = ["D105", "D107", "E115", "E116", "E117", "F401", "F841"] preview = true diff --git a/python3_stage1_gapfiller/pytester.py b/python3_stage1_gapfiller/pytester.py index 12ff2bc..ea4d833 100644 --- a/python3_stage1_gapfiller/pytester.py +++ b/python3_stage1_gapfiller/pytester.py @@ -8,6 +8,7 @@ import re from __tester import Tester from __pystylechecker import StyleChecker +from random import randint class PyTester(Tester): @@ -20,6 +21,19 @@ def __init__(self, params, testcases): own params - q.v. """ super().__init__(params, testcases) # Most of the task is handed by the generic tester + + # If the extra field is set to files, the first line of stdin must be filenames. + # Create all required files. + if params['extra'] == 'files': + if not params['IS_PRECHECK']: + for test in testcases: + stdin_lines = test.stdin.splitlines() + filename = stdin_lines[0].strip() if stdin_lines else '' + if filename == '': + raise Exception('The first line of stdin must be the filename') + if filename not in params['protectedfiles']: + with open(filename, 'w') as outfile: + outfile.write(test.extra.rstrip() + '\n') # Py-dependent attributes self.task = pytask.PyTask(params) @@ -77,39 +91,66 @@ def has_docstring(self): """ prog = self.params['STUDENT_ANSWER'].lstrip() return prog.startswith('"') or prog.startswith("'") + + + def tweaked_warning(self, message): + """Improve the warning message by updating line numbers and replacing : with Line + """ + return self.adjust_error_line_nums(message).replace(':', 'Line ') + def style_errors(self): - """Return a list of all the style errors.""" - try: - # Style-check the program without any test cases or other postlude added - errors = self.style_checker.style_errors() - except Exception as e: - error_text = '*** Unexpected error while runner precheckers. Please report ***\n' + str(e) - errors = [error_text] - errors = [self.adjust_error_line_nums(error) for error in errors] - - if len(errors) == 0: + """Return a list of all the style errors. Start with local tests and continue with pylint + only if there are no local errors. + """ + errors = [] + if self.params.get('localprechecks', True): try: - errors = self.style_checker.local_errors() # Note: prelude not included so don't adjust line nums + errors += self.style_checker.local_errors() # Note: prelude not included so don't adjust line nums except Exception as e: - error_text = '*** Unexpected error while doing local style checks. Please report ***\n' + str(e) - errors = [error_text] + errors += [str(e)] + else: + check_for_passive = (self.params['warnifpassiveoutput'] and self.params['isfunction']) + if check_for_passive: + passive = self.passive_output() + warning_messages = [line for line in passive.splitlines() if 'Warning:' in line] + if warning_messages: + errors += [self.tweaked_warning(message) for message in warning_messages] + elif passive: + errors.append("Your code was not expected to generate any output " + + "when executed stand-alone.\nDid you accidentally include " + + "your test code?\nOr you might have a wrong import statement - have you tested in Wing?") + errors.append(passive) - check_for_passive = (self.params['warnifpassiveoutput'] and self.params['isfunction']) - if len(errors) == 0 and check_for_passive and self.passive_output(): - errors.append("Your code was not expected to generate any output " + - "when executed stand-alone.\nDid you accidentally include " + - "your test code?") + if len(errors) == 0 or self.params.get('forcepylint', False): + # Run precheckers (pylint, mypy) + try: + # Style-check the program without any test cases or other postlude added + errors += self.style_checker.style_errors() + except Exception as e: + error_text = '*** Unexpected error while running precheckers. Please report ***\n' + str(e) + errors += [error_text] + errors = [self.simplify_error(self.adjust_error_line_nums(error)) for error in errors] + errors = [error for error in errors if not error.startswith('************* Module')] + errors = [error.replace(', ', '') for error in errors] # Another error tidying operation + if errors: + errors.append("\nSorry, but your code doesn't pass the style checks.") return errors def prerun_hook(self): """A hook for subclasses to do initial setup or code hacks etc Returns a list of errors, to which other errors are appended. - In this class we use it to perform required hacks to disable - calls to main. If the call to main_hacks fails, assume the code - is bad and will get flagged by pylint in due course. + In this class we use it firstly to check that the number of Prechecks + allowed has not been exceeded and then, of not, to perform + required hacks to disable calls to main. If the call to main_hacks + fails, assume the code is bad and will get flagged by pylint in due course. """ + step_info = self.params['STEP_INFO'] + max_prechecks = self.params.get('maxprechecks', None) + if max_prechecks and step_info['numprechecks'] >= max_prechecks: + return [f"Sorry, you have reached the limit on allowed prechecks ({max_prechecks}) for this question."] + try: return self.main_hacks() except: @@ -124,7 +165,8 @@ def passive_output(self): code += '\n'.join([ 'figs = _mpl.pyplot.get_fignums()', 'if figs:', - ' print(f"{len(figs)} figures found")' + ' print(f"{len(figs)} figures found")', + ' print(f"{_mpl.pyplot.get_figlabels()}")' ]) + '\n' task = pytask.PyTask(self.params, code) task.compile() @@ -154,6 +196,10 @@ def make_test_postlude(self, testcases): tester += test.extra + '\n' if self.params['usesmatplotlib']: + if 'dpi' in self.params and self.params['dpi']: + extra = f", dpi={self.params['dpi']}" + else: + extra = '' if self.params.get('running_sample_answer', False): column = 'Expected' else: @@ -165,7 +211,7 @@ def make_test_postlude(self, testcases): ' _mpl.pyplot.figure(fig)', ' row = {}'.format(test_num), ' column = "{}"'.format(column), - ' _mpl.pyplot.savefig("_image{}.{}.{}.png".format(fig, column, row), bbox_inches="tight")', + ' _mpl.pyplot.savefig("_image{}.{}.{}.png".format(fig, column, row), bbox_inches="tight"' + '{})'.format(extra), ' _mpl.pyplot.close(fig)' ]) + '\n' return tester @@ -186,6 +232,7 @@ def adjust_error_line_nums(self, error): (r'(.*: *)(\d+)(, *\d+:.*\(.*\).*)', [2]), (r'(.*:)(\d+)(:\d+: [A-Z]\d+: .*line )(\d+)(.*)', [2, 4]), (r'(.*:)(\d+)(:\d+: [A-Z]\d+: .*)', [2]), + (r'(.*:)(\d+)(: [a-zA-Z]*Warning.*)', [2]), ] output_lines = [] for line in error.splitlines(): @@ -205,6 +252,17 @@ def adjust_error_line_nums(self, error): output_lines.append(line) return '\n'.join(output_lines) + def simplify_error(self, error): + """Return a simplified version of a pylint error with Line inserted in + lieu of __source.py:

: Xnnnn + """ + pattern = r'_?_?source.py:(\d+): *\d+: *[A-Z]\d+: (.*)' + match = re.match(pattern, error) + if match: + return f"Line {match.group(1)}: {match.group(2)}" + else: + return error + def main_hacks(self): """Modify the code to be tested if params stripmain or stripmainifpresent' are specified. Returns a list of errors encountered while so doing. diff --git a/python3_stage1_gapfiller/template2.py b/python3_stage1_gapfiller/template2.py new file mode 100644 index 0000000..ea625e9 --- /dev/null +++ b/python3_stage1_gapfiller/template2.py @@ -0,0 +1,332 @@ +import html +import locale +import json +import os +import re +import sys +import py_compile + +from pytester import PyTester + +locale.setlocale(locale.LC_ALL, 'C.UTF-8') + +# NOTE: the following are most of the parameters from the python3_stage1 +# question type, included here so that the standard python3_stage1 support +# files can be used without changed. However, only the ones prefixed by +# '*' can be used in this question type (without the asterisk). + +KNOWN_PARAMS = { + 'abortonerror': True, + 'allowglobals': True, + 'allownestedfunctions': False, + 'checktemplateparams': True, + 'echostandardinput': True, + '*extra': 'None', + '*floattolerance': None, + 'globalextra': 'None', + '*imagewidth': None, + 'imports': [], + 'isfunction': False, + 'maxfunctionlength': 30, + '*maxfieldlength': None, + 'maxnumconstants': 4, + 'maxoutputbytes': 10000, + '*maxstringlength': 2000, + 'norun': False, + 'nostylechecks': True, + 'notest': False, + '*parsegaps': True, + 'precheckers': ['pylint'], + '*prelude': '', + '*proscribedbuiltins': ['exec', 'eval'], + 'proscribedfunctions': [], + 'proscribedconstructs': [], + '*proscribedsubstrings': [";", "print", "import"], + 'pylintoptions': [], + 'requiredconstructs': [], + 'requiredfunctiondefinitions': [], + 'requiredfunctioncalls': [], + '*requiredsubstrings': [], + 'requiretypehints': False, + 'restrictedfiles': { + 'disallow': ['__.*', 'prog.*', 'pytester.py'], + }, + 'restrictedmodules': { + 'builtins': { + 'onlyallow': [] + }, + 'imp': { + 'onlyallow': [] + }, + 'importlib': { + 'onlyallow': [] + }, + 'os': { + 'disallow': ['system', '_exit', '_.*'] + }, + 'subprocess': { + 'onlyallow': [] + }, + 'sys': { + 'disallow': ['_.*'] + }, + }, + 'runextra': False, + 'stdinfromextra': False, + '*strictwhitespace': True, + 'stripmain': False, + 'stripmainifpresent': False, + 'testisbash': False, + '*timeout': 5, + '*totaltimeout': 50, + 'suppresspassiveoutput': False, + '*useanswerfortests': False, + '*usesmatplotlib': False, + '*usesnumpy': False, + 'usesubprocess': False, + 'warnifpassiveoutput': False, +} + +GLOBAL_ERRORS = [] + +class TestCase: + def __init__(self, dict_rep, test_code_html=None): + """Construct a testcase from a dictionary representation obtained via JSON""" + self.testcode = dict_rep['testcode'] + self.stdin = dict_rep['stdin'] + self.expected = dict_rep['expected'] + self.extra = dict_rep['extra'] + self.display = dict_rep['display'] + try: + self.testtype = int(dict_rep['testtype']) + except: + self.testtype = 0 + self.hiderestiffail = bool(int(dict_rep['hiderestiffail'])) + self.useasexample = bool(int(dict_rep['useasexample'])) + self.mark = float(dict_rep['mark']) + self.test_code_html = test_code_html + + +# ================= CODE TO DO ALL TWIG PARAMETER PROCESSING =================== + +def process_template_params(): + """Extract the template params into a global dictionary PARAMS""" + global PARAMS, GLOBAL_ERRORS + PARAMS = json.loads("""{{ QUESTION.parameters | json_encode | e('py') }}""") + checktemplateparams = PARAMS.get('checktemplateparams', True) + + # Check for parameters that aren't allowed + if checktemplateparams: + author_params = set(PARAMS.keys()) + allowed_params = set(key[1:] for key in KNOWN_PARAMS.keys() if key.startswith('*')) + unknown_params = author_params - allowed_params + filtered_params = [param for param in unknown_params if not param.startswith('_')] + if filtered_params: + GLOBAL_ERRORS.append(f"Unexpected template parameter(s): {list(sorted(filtered_params))}") + + # Now merge all the KNOWN_PARAMS into PARAMS + for param_name, default in KNOWN_PARAMS.items(): + if param_name.startswith('*'): + param_name = param_name[1:] + if param_name in PARAMS: + param = PARAMS[param_name] + if type(param) != type(default) and default is not None: + GLOBAL_ERRORS.append(f"Template parameter {param_name} has wrong type (expected {type(default)})") + else: + PARAMS[param_name] = default; + + if PARAMS['extra'] == 'stdin': + PARAMS['stdinfromextra'] = True + if PARAMS['runextra']: + PARAMS['extra'] = 'pretest' # Legacy support + if PARAMS['timeout'] < 2: + PARAMS['timeout'] = 2 # Allow 1 extra second freeboard + + +def update_test_cases(test_cases, outcome): + """Return the updated testcases after replacing all empty expected fields with those from the + given outcome's test_results which must have a column header 'Got'. Non-empty existing expected + fields are left unchanged. + If any errors occur, the return value will be None and the outcome parameter will have had its prologuehtml + value updated to include an error message. + """ + try: + results = outcome['testresults'] + col_num = results[0].index('Got') + for i in range(len(test_cases)): + if test_cases[i].expected.strip() == '': + test_cases[i].expected = results[i + 1][col_num] + except ValueError: + outcome['prologuehtml'] = "No 'Got' column in result table from which to get testcase expecteds" + test_cases = None + except Exception as e: + outcome['prologuehtml'] = "Unexpected error ({}) extracting testcase expecteds from sample answer output".format(e) + test_cases = None + return test_cases + + + +def get_expecteds_from_answer(params, test_cases, ui_source, answer_field_values): + """Run all tests using the sample answer rather than the student answer. + Fill in the expected field of each test case using the sample answer and return + the updated test case list. + Return None if the sample answer gave any sort of runtime error + """ + # Insert answer fields into test cases. + answer_test_cases = get_test_cases(ui_source, answer_field_values) + new_params = {key: value for key, value in params.items()} + new_params['IS_PRECHECK'] = False + new_params['nostylechecks'] = True + new_params['STUDENT_ANSWER'] = get_student_answer(ui_source, answer_field_values) + new_params['running_sample_answer'] = True + tester = PyTester(new_params, answer_test_cases) + outcome = tester.test_code() + if 'prologuehtml' in outcome: + outcome['prologuehtml'] = "

ERROR IN QUESTION'S SAMPLE ANSWER. PLEASE REPORT

\n" + outcome['prologuehtml'] + return outcome, None + else: + return outcome, update_test_cases(test_cases, outcome) + + +# ================= CODE TO DO GAPFILLER STUFF =================== +def get_test_cases(ui_source, field_values): + """Return an array of Test objects from the template parameter TESTCASES""" + test_cases = [] + tests = json.loads("""{{ TESTCASES | json_encode | e('py') }}""") + + for test in tests: + # If gaps come from test cases then we fill student answer into gaps. + + if ui_source == 'test0' and test['testcode'].strip() != '': + test_code_html = insert_fields(htmlize(test['testcode']), field_values, highlight=True) + test['testcode'] = insert_fields(test['testcode'], field_values) + test_case = TestCase(test, test_code_html=test_code_html) + else: + test_case = TestCase(test) + test_cases.append(test_case) + return test_cases + +# Expand the given code by inserting the given fields into the +# given code, after splitting it with the given regular expression. +# If highlight is true, the inserted text is wrapped in a element +# with a special blue-grey background. +def insert_fields(code, fields, splitter=r"\{\[.*?\]\}", highlight=False): + global GLOBAL_ERRORS + bits = re.split(splitter, code) + if len(bits) != len(fields) + 1: + GLOBAL_ERRORS.append("Richard has goofed. Please report") + sys.exit() + + prog = bits[0] + i = 1 + for value in fields: + if len(value.splitlines()) > 1: + # Indent all lines in a textarea by the indent of the first line + indent = len(prog.splitlines()[-1]) + value_bits = value.splitlines() + for j in range(1, len(value_bits)): + value_bits[j] = indent * ' ' + value_bits[j] + value = '\n'.join(value_bits) + '\n' + if highlight: + value = f"{value}" + prog += value + bits[i] + i += 1 + return prog + +def get_student_answer(ui_source, field_values): + """Gets what should be considered the 'student answer'. If global extra + is being used as the UI source, the student answer is the global + extra with all the various gaps filled in. The tests then follow that + code. However, if test0 is the UI source, the student answer is empty + (apart from a possibly spurious module docstring) as the tests will + all have had the test0 gap contents plugged into them too. + """ + if ui_source == 'test0': + return "'''docstring'''" + else: + return insert_fields("""{{ QUESTION.globalextra | e('py')}}""", field_values) + +def snip(field, maxlen=10): + """Return the given field, snipped to a max of maxlen chars""" + return field if len(field) <= maxlen else field[0:maxlen] + '...' + +def fields_precheck(field_values): + """Checks field lengths (legacy) and no proscribed strings present in fields. + Also do a syntax check if the 'parsegaps' parameter is true. + """ + errors = [] + all_constructs = set() + for field in field_values: + if PARAMS['maxfieldlength'] is not None and len(field) > PARAMS['maxfieldlength']: + errors.append(f"Field '{snip(field)}' is too long. Maximum length is {PARAMS['maxfieldlength']}") + for bad in PARAMS['proscribedsubstrings']: + if bad in field: + errors.append(f"Field '{snip(field)}' contains the banned string '{bad}'") + if PARAMS['parsegaps']: + with open('__gap.py', 'w') as outfile: + outfile.write(field) + try: + py_compile.compile('__gap.py', doraise=True) + except Exception as e: + errors.append(f"Syntax error in field '{snip(field, 30)}'") + for filename in ['__gap.py', '__gap.pyc']: + try: + os.remove(filename) + except FileNotFoundError: + pass + for reqd in PARAMS['requiredsubstrings']: + if not any(reqd in field for field in field_values): + errors.append(f"Required substring '{reqd}' not found") + return errors + +def htmlize(message): + """An html version of the given error message""" + return '
' + html.escape(message) + '
' if message else '' + +def process_global_params(ui_source, field_values): + """Plug the globals STUDENT_ANSWER, IS_PRECHECK and QUESTION_PRECHECK into the global PARAMS """ + PARAMS['STUDENT_ANSWER'] = get_student_answer(ui_source, field_values) + '\n' + PARAMS['SEPARATOR'] = "##" + PARAMS['IS_PRECHECK'] = "{{ IS_PRECHECK }}" == "1" + PARAMS['QUESTION_PRECHECK'] = {{ QUESTION.precheck }} # Type of precheck: 0 = None, 1 = Empty etc + PARAMS['ALL_OR_NOTHING'] = "{{ QUESTION.allornothing }}" == "1" # Whether or not all-or-nothing grading is being used + PARAMS['GLOBAL_EXTRA'] = """{{ QUESTION.globalextra | e('py') }}\n""" + +# Get fields and ui params. +ui_params = json.loads("""{{ QUESTION.uiparameters | json_encode | e('py') }}""") +if ui_params and 'ui_source' in ui_params: + ui_source = ui_params['ui_source'] +else: + ui_source = 'globalextra' + +field_values = json.loads(""" {{ STUDENT_ANSWER | e('py') }}""") +answer_field_values = json.loads(""" {{ QUESTION.answer | e('py') }}""") + + +process_template_params() +test_cases = get_test_cases(ui_source, field_values) +process_global_params(ui_source, field_values) + +if GLOBAL_ERRORS: + errors = GLOBAL_ERRORS[:] +else: + errors = fields_precheck(field_values) + + +if errors: + prologue = '

' + htmlize('\n'.join(errors)) + '

' + prologue += '

Sorry, precheck failed 😞

' + outcome = {'fraction': 0, "prologuehtml": prologue} +else: + # For gap filler we don't want to apply proscribedsubstrings to all code, only the gaps as above. + # We also turn off all standard style checks. + PARAMS['proscribedsubstrings'] = [] + PARAMS['nostylechecks'] = True + PARAMS['stylechecks'] = False # For the future! + if PARAMS['useanswerfortests']: + outcome, test_cases = get_expecteds_from_answer(PARAMS, test_cases, ui_source, answer_field_values) + + if test_cases: + tester = PyTester(PARAMS, test_cases) + outcome = tester.test_code() +print(json.dumps(outcome))