Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/dayamlchecker/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ class MessageDefinition:
severity=Severity.ERROR,
finding_class=FindingClass.GENERAL,
summary="Unknown YAML keys",
template="keys that should not appear in a docassemble block: {keys}",
template="keys that should not appear in a docassemble block: {keys}{suggestions}",
),
MessageId.SHOW_IF_MALFORMED: MessageDefinition(
code="EG302",
Expand Down Expand Up @@ -629,10 +629,11 @@ class MessageDefinition:
code="EG413",
severity=Severity.ERROR,
finding_class=FindingClass.GENERAL,
summary="Invalid field modifier key casing",
summary="Invalid field modifier key",
template=(
'invalid field key "{field_key}". docassemble field modifier keys are '
'case-sensitive; use "{suggested_key}"'
'invalid field key "{field_key}". Did you mean "{suggested_key}"? '
'If so, use "{suggested_key}" exactly; docassemble field modifier keys '
"are case-sensitive"
),
),
MessageId.MISSING_QUESTION_ID: MessageDefinition(
Expand Down
153 changes: 146 additions & 7 deletions src/dayamlchecker/yaml_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -762,14 +762,30 @@ def _line_for(self, field_item, code_line=1):
return field_line + max(code_line - 1, 0)

def _extract_field_name(self, field_item):
field_key = self._extract_field_key(field_item)
if field_key is None:
return None
return field_item[field_key]

def _extract_field_key(self, field_item):
if not isinstance(field_item, dict):
return None
if "field" in field_item and isinstance(field_item["field"], str):
return "field"

possible_keys = []
for key, value in field_item.items():
if key in self.modifier_keys:
continue
if isinstance(value, str):
return value
return None
possible_keys.append(key)

# A field's arbitrary label key is not a modifier typo. Prefer a key that
# does not resemble a modifier when deciding which key declares the field.
Comment on lines +782 to +783

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot about that with fields. I'm okay with just not using this feature for fields, rather than introducing confusion about what field names are valid or not

for key in possible_keys:
if not isinstance(key, str) or _suggest_known_field_modifier(key) is None:
return key
return possible_keys[0] if possible_keys else None

def _validate_python_modifier(
self, modifier_key, modifier_value, field_item, screen_variables
Expand Down Expand Up @@ -903,6 +919,8 @@ def _validate_field_modifiers(self, fields_list):
if not isinstance(field_item, dict):
continue

field_variable_key = self._extract_field_key(field_item)

for field_key, field_value in field_item.items():
if (
field_key not in self.modifier_keys
Expand Down Expand Up @@ -930,16 +948,22 @@ def _validate_field_modifiers(self, fields_list):
**dict(err.context),
)
)
if (
field_key not in self.modifier_keys
and field_key.lower() in self.modifier_keys
):
suggested_key = None
if field_key not in self.modifier_keys:
modifier_case_key = field_key.lower()
if modifier_case_key in self.modifier_keys:
# Preserve the existing casing check even when the key
# could otherwise be interpreted as a shorthand label.
suggested_key = modifier_case_key
elif field_key != field_variable_key:
suggested_key = _suggest_known_field_modifier(field_key)
if suggested_key is not None:
self.errors.append(
draft(
MessageId.FIELD_MODIFIER_CASE,
line_number=self._line_for(field_item),
field_key=field_key,
suggested_key=field_key.lower(),
suggested_key=suggested_key,
)
)
if field_key in self.mako_keys:
Expand Down Expand Up @@ -1431,6 +1455,111 @@ def _variable_candidates(self, var_expr):
)


def _normalize_key_for_similarity(key: str) -> str:
"""Case-fold a key and remove whitespace and punctuation."""
return "".join(character for character in key.casefold() if character.isalnum())


_KNOWN_DICT_KEYS_BY_NORMALIZED: dict[str, set[str]] = {}
for _known_dict_key in all_dict_keys:
_KNOWN_DICT_KEYS_BY_NORMALIZED.setdefault(
_normalize_key_for_similarity(_known_dict_key), set()
).add(_known_dict_key)
_MAX_NORMALIZED_DICT_KEY_LENGTH = max(map(len, _KNOWN_DICT_KEYS_BY_NORMALIZED))

_KNOWN_FIELD_MODIFIER_KEYS_BY_NORMALIZED: dict[str, set[str]] = {}
for _field_modifier_key in DAFields.modifier_keys - {"__line__"}:
_KNOWN_FIELD_MODIFIER_KEYS_BY_NORMALIZED.setdefault(
_normalize_key_for_similarity(_field_modifier_key), set()
).add(_field_modifier_key)
_MAX_NORMALIZED_FIELD_MODIFIER_KEY_LENGTH = max(
map(len, _KNOWN_FIELD_MODIFIER_KEYS_BY_NORMALIZED)
)


def _damerau_levenshtein_distance(left: str, right: str) -> int:
"""Return edit distance, treating an adjacent transposition as one edit."""
previous_previous_row: list[int] | None = None
previous_row = list(range(len(right) + 1))

for left_index, left_character in enumerate(left, start=1):
current_row = [left_index]
for right_index, right_character in enumerate(right, start=1):
current_row.append(
min(
current_row[right_index - 1] + 1,
previous_row[right_index] + 1,
previous_row[right_index - 1] + (left_character != right_character),
)
)
if (
previous_previous_row is not None
and left_index > 1
and right_index > 1
and left_character == right[right_index - 2]
and left[left_index - 2] == right_character
):
current_row[right_index] = min(
current_row[right_index],
previous_previous_row[right_index - 2] + 1,
)
previous_previous_row, previous_row = previous_row, current_row

return previous_row[-1]


def _suggest_known_key(
invalid_key: str,
known_keys_by_normalized: dict[str, set[str]],
max_known_key_length: int,
) -> str | None:
"""Return a unique, conservatively close known key."""
normalized_key = _normalize_key_for_similarity(invalid_key)
exact_matches = known_keys_by_normalized.get(normalized_key, set())
if len(exact_matches) == 1:
return next(iter(exact_matches))

# Short keys are too easy to match accidentally. Longer keys can tolerate two
# edits while still requiring one unique best match.
if len(normalized_key) < 5:
return None
max_distance = 1 if len(normalized_key) < 9 else 2
if len(normalized_key) > max_known_key_length + max_distance:
return None

best_distance = max_distance + 1
best_matches: set[str] = set()
for known_normalized, known_keys in known_keys_by_normalized.items():
if abs(len(normalized_key) - len(known_normalized)) > max_distance:
continue
distance = _damerau_levenshtein_distance(normalized_key, known_normalized)
if distance < best_distance:
best_distance = distance
best_matches = set(known_keys)
elif distance == best_distance:
best_matches.update(known_keys)

if best_distance <= max_distance and len(best_matches) == 1:
return next(iter(best_matches))
return None


def _suggest_known_dict_key(invalid_key: str) -> str | None:
return _suggest_known_key(
invalid_key,
_KNOWN_DICT_KEYS_BY_NORMALIZED,
_MAX_NORMALIZED_DICT_KEY_LENGTH,
)


def _suggest_known_field_modifier(invalid_key: str) -> str | None:
return _suggest_known_key(
invalid_key,
_KNOWN_FIELD_MODIFIER_KEYS_BY_NORMALIZED,
_MAX_NORMALIZED_FIELD_MODIFIER_KEY_LENGTH,
)


def _lowercase_key_map(mapping: dict[Any, Any]) -> dict[str, str]:
return {
key.lower(): key
Expand Down Expand Up @@ -2074,6 +2203,7 @@ def find_errors_from_string(
)

weird_keys = []
key_suggestions = []
for attr in doc.keys():
if attr == "__line__":
continue
Expand All @@ -2082,13 +2212,22 @@ def find_errors_from_string(
weird_keys.append(str(attr))
elif attr.lower() not in all_dict_keys:
weird_keys.append(attr)
suggested_key = _suggest_known_dict_key(attr)
if suggested_key is not None:
key_suggestions.append(
f'"{attr}" should likely be "{suggested_key}"'
)
if len(weird_keys) > 0:
suggestions = ""
if key_suggestions:
suggestions = ". Did you mean: " + "; ".join(key_suggestions) + "?"
all_errors.append(
make_finding(
MessageId.UNKNOWN_KEYS,
line_number=line_number,
file_name=input_file,
keys=", ".join(weird_keys),
suggestions=suggestions,
)
)
for key in doc.keys():
Expand Down
107 changes: 107 additions & 0 deletions tests/test_yaml_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,59 @@ def test_unknown_block_type_error_is_clearer(self):
f"Expected clearer unknown block type error, got: {errs}",
)

def test_unknown_key_suggests_known_separator_variant(self):
invalid = """question: What should happen next?
depends_on: prior_answer
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

unknown_key_error = next(err for err in errs if err.code == "EG301")
self.assertIn(
'"depends_on" should likely be "depends on"', unknown_key_error.err_str
)

def test_unknown_key_suggestion_ignores_punctuation(self):
invalid = """question: What should happen next?
depends.on: prior_answer
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

unknown_key_error = next(err for err in errs if err.code == "EG301")
self.assertIn(
'"depends.on" should likely be "depends on"', unknown_key_error.err_str
)

def test_unknown_key_suggests_single_typo(self):
invalid = """question: What should happen next?
dependz on: prior_answer
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

unknown_key_error = next(err for err in errs if err.code == "EG301")
self.assertIn(
'"dependz on" should likely be "depends on"', unknown_key_error.err_str
)

def test_unknown_key_suggests_adjacent_transposition(self):
invalid = """question: What should happen next?
depneds on: prior_answer
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

unknown_key_error = next(err for err in errs if err.code == "EG301")
self.assertIn(
'"depneds on" should likely be "depends on"', unknown_key_error.err_str
)

def test_unknown_key_does_not_suggest_unrelated_key(self):
invalid = """question: What should happen next?
completely unrelated: prior_answer
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

unknown_key_error = next(err for err in errs if err.code == "EG301")
self.assertNotIn("Did you mean", unknown_key_error.err_str)

def test_mixed_case_question_keys_are_accepted(self):
invalid = """
Question: |
Expand Down Expand Up @@ -311,6 +364,60 @@ def test_mixed_case_field_modifier_is_not_accepted(self):
f"Expected mixed-case modifier key to be rejected, got: {errs}",
)

def test_field_modifier_suggests_punctuation_variant(self):
invalid = """question: |
What is your favorite fruit?
fields:
- label: Fruit
field: favorite_fruit
- label: Why
field: fruit_reason
show_if: favorite_fruit
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

self.assertTrue(
any(
err.code == "EG413"
and 'invalid field key "show_if"' in err.err_str
and 'Did you mean "show if"?' in err.err_str
for err in errs
),
f"Expected a field modifier suggestion, got: {errs}",
)

def test_field_modifier_suggests_typo(self):
invalid = """question: |
What is your favorite fruit?
fields:
- Fruit: favorite_fruit
- Why: fruit_reason
shwo if: favorite_fruit
"""
errs = find_errors_from_string(invalid, input_file="<string_invalid>")

self.assertTrue(
any(
err.code == "EG413"
and 'invalid field key "shwo if"' in err.err_str
and 'Did you mean "show if"?' in err.err_str
for err in errs
),
f"Expected a field modifier typo suggestion, got: {errs}",
)

def test_shorthand_field_label_is_not_treated_as_modifier_typo(self):
valid = """question: Choose an option
fields:
- Choices!: selected_choice
"""
errs = find_errors_from_string(valid, input_file="<string_valid>")

self.assertFalse(
any(err.code == "EG413" for err in errs),
f"Did not expect the shorthand label to be treated as a modifier: {errs}",
)

def test_duplicate_key_error(self):
duplicate = """
question: |
Expand Down
Loading