diff --git a/src/dayamlchecker/accessibility.py b/src/dayamlchecker/accessibility.py index a5cb4e2..c7084b8 100644 --- a/src/dayamlchecker/accessibility.py +++ b/src/dayamlchecker/accessibility.py @@ -17,8 +17,10 @@ "datatype", "choices", "value", + "validate", "validation code", "validation message", + "validation messages", "show if", "hide if", "js show if", @@ -311,7 +313,11 @@ def _check_yesno_shortcuts( def _check_multifield_no_label_usage( doc: dict[str, Any], document_start_line: int ) -> list[FindingDraft]: - fields = _iter_fields(doc) + fields = [ + field + for field in _iter_fields(doc) + if _field_collects_user_input(field) or "no label" in field + ] if len(fields) <= 1: return [] @@ -482,24 +488,7 @@ def _check_validation_guidance( for field in _iter_fields(doc): if "code" in field: continue - has_constraint = bool( - str(field.get("validation code") or "").strip() - or field.get("min") - or field.get("max") - or field.get("minlength") - or field.get("maxlength") - or field.get("required") - ) - if not has_constraint: - continue - validation_messages = _collect_validation_messages(field) - has_guidance = bool( - str(field.get("hint") or "").strip() - or str(field.get("help") or "").strip() - or str(field.get("note") or "").strip() - or validation_messages - ) - if has_guidance: + if not _has_inline_custom_validation_without_message(field): continue findings.append( draft( @@ -1244,9 +1233,25 @@ def _collect_validation_messages( for value in validation_message.values(): if isinstance(value, str) and value.strip(): messages.append((value.strip(), validation_message.get("__line__"))) + validation_messages = field.get("validation messages") + if isinstance(validation_messages, dict): + for value in validation_messages.values(): + if isinstance(value, str) and value.strip(): + messages.append((value.strip(), validation_messages.get("__line__"))) return messages +def _has_inline_custom_validation_without_message(field: dict[str, Any]) -> bool: + validate = field.get("validate") + if not isinstance(validate, str): + return False + if not validate.strip().startswith("lambda"): + return False + if "validation_error" in validate: + return False + return not _collect_validation_messages(field) + + def _is_truthy(value: Any) -> bool: if isinstance(value, bool): return value diff --git a/src/dayamlchecker/messages.py b/src/dayamlchecker/messages.py index df7b975..912388f 100644 --- a/src/dayamlchecker/messages.py +++ b/src/dayamlchecker/messages.py @@ -738,8 +738,8 @@ class MessageDefinition: code="WA526", severity=Severity.WARNING, finding_class=FindingClass.ACCESSIBILITY, - summary="Validation constraints lack guidance", - template="field has validation constraints but no hint, help, or validation message: {snippet}", + summary="Inline custom validation lacks a validation message", + template="field uses inline custom validation without a clear validation message: {snippet}", ), MessageId.ACCESSIBILITY_GENERIC_VALIDATION_MESSAGE: MessageDefinition( code="WA527", diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index 35b4ce1..64a9900 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/src/dayamlchecker/yaml_structure.py @@ -484,7 +484,9 @@ class DAFields: "label", "datatype", "choices", + "validate", "validation code", + "validation messages", "show if", "hide if", "js show if", diff --git a/tests/test_yaml_structure.py b/tests/test_yaml_structure.py index b54f1ff..c29d306 100644 --- a/tests/test_yaml_structure.py +++ b/tests/test_yaml_structure.py @@ -596,6 +596,35 @@ def test_accessibility_dynamic_code_field_no_error_on_multi_field_screen(self): f"Did not expect no-label accessibility error for dynamic code field, got: {errs}", ) + def test_accessibility_note_field_does_not_need_label(self): + yaml_content = """id: report_type_and_period +question: | + Which report do you need to file? +subquestion: | + Choose the 60-day form or the annual form. +fields: + - Which form is this?: report_type + datatype: radio + choices: + - 60-day report: sixty_day + - Annual report: annual + - note: | + What dates does this filing cover? + - Start date: reportperiod_beginning + datatype: date + - End date: reportperiod_end + datatype: date +""" + errs = find_errors_from_string( + yaml_content, + input_file="", + lint_mode="accessibility", + ) + self.assertFalse( + any("single-field screens" in e.err_str.lower() for e in errs), + f"Did not expect no-label accessibility error for note field, got: {errs}", + ) + def test_accessibility_no_label_uses_variable_from_no_label_key(self): yaml_content = """question: | Reorder fields @@ -2000,6 +2029,120 @@ def test_accessibility_field_shortcut_and_validation_rules_are_reported(self): f"Expected new accessibility parity findings, got: {errs}", ) + def test_accessibility_validation_guidance_reports_inline_validate_lambda(self): + yaml_text = """ +id: custom_validation +question: | + Tell us more +fields: + - Details: case_notes + validate: | + lambda y: len(y or "") >= 10 +""" + errs = find_errors_from_string( + yaml_text, + input_file="", + lint_mode="accessibility", + ) + self.assertTrue( + _has_code(errs, "WA526"), + f"Expected inline validate lambda without separate guidance to be reported, got: {errs}", + ) + + def test_accessibility_validation_guidance_accepts_inline_validation_error(self): + yaml_text = """ +id: inline_validation_error +question: | + Tell us more +fields: + - Details: case_notes + validate: | + lambda y: len(y or "") >= 10 or validation_error("Enter at least 10 characters.") +""" + errs = find_errors_from_string( + yaml_text, + input_file="", + lint_mode="accessibility", + ) + self.assertFalse( + _has_code(errs, "WA526"), + f"Did not expect inline validate lambda with validation_error() to trigger WA526, got: {errs}", + ) + + def test_accessibility_validation_guidance_ignores_named_validate_function(self): + yaml_text = """ +id: named_validation +question: | + Tell us more +fields: + - Details: case_notes + validate: case_notes_are_valid +""" + errs = find_errors_from_string( + yaml_text, + input_file="", + lint_mode="accessibility", + ) + self.assertFalse( + _has_code(errs, "WA526"), + f"Did not expect named validate function to trigger WA526, got: {errs}", + ) + + def test_accessibility_validation_guidance_accepts_validation_messages_modifier(self): + yaml_text = """ +id: inline_validation_with_message +question: | + Tell us more +fields: + - Details: case_notes + validate: | + lambda y: len(y or "") >= 10 + validation messages: + validate: Enter at least 10 characters. +""" + errs = find_errors_from_string( + yaml_text, + input_file="", + lint_mode="accessibility", + ) + self.assertFalse( + _has_code(errs, "WA526"), + f"Did not expect inline validate lambda with validation messages to trigger WA526, got: {errs}", + ) + + def test_accessibility_validation_guidance_ignores_docassemble_builtin_constraints( + self, + ): + yaml_text = """ +id: builtin_validation +question: | + Tell us more +fields: + - Required name: required_name + required: True + - Short code: short_code + maxlength: 10 + - Long code: long_code + minlength: 3 + - Age: age + datatype: number + min: 18 + max: 120 + - Email: email_address + datatype: email + - Birth date: birth_date + datatype: date +""" + errs = find_errors_from_string( + yaml_text, + input_file="", + lint_mode="accessibility", + ) + self.assertFalse( + _has_code(errs, "WA526"), + f"Did not expect built-in Docassemble constraints to trigger WA526, got: {errs}", + ) + def test_accessibility_html_rules_are_reported(self): yaml_text = """ id: content