diff --git a/src/dayamlchecker/__init__.py b/src/dayamlchecker/__init__.py index c077a01..fdc61a6 100644 --- a/src/dayamlchecker/__init__.py +++ b/src/dayamlchecker/__init__.py @@ -1,4 +1,7 @@ -from dayamlchecker.messages import Finding, FindingClass +from dayamlchecker.messages import ( + Finding, + FindingClass, +) from dayamlchecker.yaml_structure import ( RuntimeOptions, find_errors, diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index a5a598a..7489cfe 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -32,6 +32,7 @@ class MessageId(StrEnum): PYTHON_CODE_TYPE = "python_code_type" PYTHON_SYNTAX_ERROR = "python_syntax_error" + PYTHON_CODE_FUNCTION_DEF = "python_code_function_def" VALIDATION_CODE_MISSING_VALIDATION_ERROR = ( "validation_code_missing_validation_error" ) @@ -273,6 +274,16 @@ class MessageDefinition: summary="Python syntax error", template="Python syntax error: {error}", ), + MessageId.PYTHON_CODE_FUNCTION_DEF: MessageDefinition( + code="WG123", + severity=Severity.WARNING, + finding_class=FindingClass.GENERAL, + summary="Code block defines a Python function", + template=( + "code block defines function `{function_name}`; move reusable " + "helper functions to a Python module instead, or use inline code for small snippets that are only used once" + ), + ), MessageId.VALIDATION_CODE_MISSING_VALIDATION_ERROR: MessageDefinition( code="WG101", severity=Severity.WARNING, diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index 8399077..382526f 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -185,7 +185,7 @@ def __init__(self, x): ] return try: - ast.parse(x) + tree = ast.parse(x) except SyntaxError as ex: # ex.lineno gives line number within the code block lineno = ex.lineno or 1 @@ -193,6 +193,18 @@ def __init__(self, x): self.errors = [ draft(MessageId.PYTHON_SYNTAX_ERROR, line_number=lineno, error=msg) ] + return + + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + self.errors.append( + draft( + MessageId.PYTHON_CODE_FUNCTION_DEF, + line_number=getattr(node, "lineno", 1) or 1, + function_name=node.name, + ) + ) + break class ValidationCode(PythonText): diff --git a/tests/test_yaml_structure.py b/tests/test_yaml_structure.py index cdc5e8c..67e0d6c 100644 --- a/tests/test_yaml_structure.py +++ b/tests/test_yaml_structure.py @@ -19,6 +19,35 @@ def test_valid_question_no_errors(self): errs = find_errors_from_string(valid, input_file="") self.assertEqual(len(errs), 0, f"Expected no errors, got: {errs}") + def test_code_block_function_def_warns(self): + yaml_content = """code: | + answer = 1 + def helper(): + return answer +""" + errs = find_errors_from_string(yaml_content, input_file="") + + warning = next( + err for err in errs if err.message_id == "python_code_function_def" + ) + self.assertEqual(warning.code, "WG123") + self.assertEqual(warning.line_number, 4) + self.assertEqual(warning.context["function_name"], "helper") + self.assertIn("Python module", warning.message) + + def test_code_block_without_function_def_does_not_warn(self): + yaml_content = """code: | + answer = 1 + if answer: + result = answer +""" + errs = find_errors_from_string(yaml_content, input_file="") + + self.assertFalse( + any(err.message_id == "python_code_function_def" for err in errs), + f"Did not expect function-def warning, got: {errs}", + ) + def test_question_and_template_exclusive_error(self): invalid = """ question: | @@ -513,8 +542,14 @@ def test_accessibility_this_website_link_text_fails(self): input_file="", lint_mode="accessibility", ) - generic_errs = [e for e in errs if "link text" in e.err_str.lower() and "too generic" in e.err_str.lower()] - self.assertEqual(len(generic_errs), 3, f"Expected 3 generic link-text errors, got: {errs}") + generic_errs = [ + e + for e in errs + if "link text" in e.err_str.lower() and "too generic" in e.err_str.lower() + ] + self.assertEqual( + len(generic_errs), 3, f"Expected 3 generic link-text errors, got: {errs}" + ) self.assertTrue(all(e.severity.value == "error" for e in generic_errs)) def test_accessibility_sitio_web_link_text_fails(self): @@ -528,8 +563,14 @@ def test_accessibility_sitio_web_link_text_fails(self): input_file="", lint_mode="accessibility", ) - generic_errs = [e for e in errs if "link text" in e.err_str.lower() and "too generic" in e.err_str.lower()] - self.assertEqual(len(generic_errs), 3, f"Expected 3 generic link-text errors, got: {errs}") + generic_errs = [ + e + for e in errs + if "link text" in e.err_str.lower() and "too generic" in e.err_str.lower() + ] + self.assertEqual( + len(generic_errs), 3, f"Expected 3 generic link-text errors, got: {errs}" + ) self.assertTrue(all(e.severity.value == "error" for e in generic_errs)) def test_accessibility_empty_markdown_link_fails(self):