diff --git a/src/erc7730/lint/v2/lint_validate_max_length.py b/src/erc7730/lint/v2/lint_validate_max_length.py index 8a826899..40cbb9fc 100644 --- a/src/erc7730/lint/v2/lint_validate_max_length.py +++ b/src/erc7730/lint/v2/lint_validate_max_length.py @@ -4,6 +4,7 @@ Adapted from v1 ValidateMaxLengthLinter to use v2 model types. """ +import re from typing import final, override from erc7730.common.ledger import ( @@ -18,8 +19,15 @@ from erc7730.common.output import OutputAdder from erc7730.lint.v2 import ERC7730Linter from erc7730.model.input.v2.descriptor import InputERC7730Descriptor +from erc7730.model.input.v2.format import VisibilityRule from erc7730.model.resolved.v2.descriptor import ResolvedERC7730Descriptor -from erc7730.model.resolved.v2.display import ResolvedField, ResolvedFieldDescription, ResolvedFieldGroup +from erc7730.model.resolved.v2.display import ( + ResolvedField, + ResolvedFieldDescription, + ResolvedFieldGroup, + ResolvedVisibilityConditions, + ResolvedVisibilityRules, +) @final @@ -79,8 +87,8 @@ def _validate_display_lengths(cls, descriptor: ResolvedERC7730Descriptor, out: O for fmt in descriptor.display.formats.values(): if fmt.intent is not None and isinstance(fmt.intent, str) and len(fmt.intent) > OPERATION_TYPE_MAX_LENGTH: too_long_intents.add(fmt.intent) - if fmt.interpolatedIntent is not None and len(fmt.interpolatedIntent) > OPERATION_TYPE_MAX_LENGTH: - too_long_intents.add(fmt.interpolatedIntent) + if (ii := fmt.interpolatedIntent) is not None and cls._literal_length(ii) > OPERATION_TYPE_MAX_LENGTH: + too_long_intents.add(ii) if fmt.id is not None and len(fmt.id) > OPERATION_TYPE_MAX_LENGTH: too_long_ids.add(fmt.id) @@ -106,10 +114,39 @@ def _validate_display_lengths(cls, descriptor: ResolvedERC7730Descriptor, out: O f"{FIELD_NAME_MAX_LENGTH} characters and may be truncated on Ledger devices.", ) + @staticmethod + def _is_never_displayed(visible: ResolvedVisibilityRules | None) -> bool: + """Whether the field is only ever checked, never put on a screen. + + A ``mustMatch`` field is a constraint the device enforces without displaying it, which + is why the descriptor may omit its label altogether. ``ifNotIn`` and ``optional`` both + do reach a screen, so their labels still count. + """ + if visible == VisibilityRule.NEVER: + return True + return isinstance(visible, ResolvedVisibilityConditions) and visible.mustMatch is not None + + _PLACEHOLDER = re.compile(r"\{[^}]*\}") + + @classmethod + def _literal_length(cls, interpolated_intent: str) -> int: + """Length of the text an interpolated intent is guaranteed to display. + + `{path}` is template syntax. Neither the braces nor the path inside them reaches a + screen, and the length of the value that replaces them is not known here -- an amount + may render as "1 ETH" or as eighteen decimal places. Only the literal text around the + placeholders is certain to be displayed, so it is the only part that can be measured. + Over the limit on that alone means the intent truncates whatever the values are. + """ + return len(cls._PLACEHOLDER.sub("", interpolated_intent)) + @classmethod def _collect_long_labels(cls, field: ResolvedField, too_long: set[str]) -> None: match field: case ResolvedFieldDescription(): + # a label that never reaches a screen cannot be truncated on one + if cls._is_never_displayed(field.visible): + return if field.label is not None and len(field.label) > FIELD_NAME_MAX_LENGTH: too_long.add(field.label) case ResolvedFieldGroup(): diff --git a/tests/v2/lint/test_lint_validate_max_length.py b/tests/v2/lint/test_lint_validate_max_length.py new file mode 100644 index 00000000..75025fad --- /dev/null +++ b/tests/v2/lint/test_lint_validate_max_length.py @@ -0,0 +1,89 @@ +import pytest + +from erc7730.lint.v2.lint_validate_max_length import ValidateMaxLengthLinter +from erc7730.model.input.v2.format import VisibilityRule +from erc7730.model.paths.path_parser import to_path +from erc7730.model.resolved.v2.display import ( + ResolvedFieldDescription, + ResolvedFieldGroup, + ResolvedVisibilityConditions, + ResolvedVisibilityRules, +) + +# 28 characters, against the 20-character field name limit +LONG = "Referrer Config Basis Points" + + +def field(visible: ResolvedVisibilityRules | None) -> ResolvedFieldDescription: + return ResolvedFieldDescription(label=LONG, path=to_path("#.referrerConfig.basisPoints"), visible=visible) + + +def collected(visible: ResolvedVisibilityRules | None) -> set[str]: + out: set[str] = set() + ValidateMaxLengthLinter._collect_long_labels(field(visible), out) + return out + + +@pytest.mark.parametrize( + "visible", + [ + pytest.param(VisibilityRule.ALWAYS, id="always"), + pytest.param(VisibilityRule.OPTIONAL, id="optional"), + pytest.param(None, id="unset"), + pytest.param(ResolvedVisibilityConditions(ifNotIn=["0"]), id="ifNotIn"), + ], +) +def test_label_that_can_reach_a_screen_is_reported(visible: ResolvedVisibilityRules | None) -> None: + """always, optional, unset and ifNotIn all display the field at least sometimes.""" + assert collected(visible) == {LONG} + + +@pytest.mark.parametrize( + "visible", + [ + pytest.param(VisibilityRule.NEVER, id="never"), + pytest.param(ResolvedVisibilityConditions(mustMatch=["0"]), id="mustMatch"), + ], +) +def test_label_that_never_reaches_a_screen_is_not_reported(visible: ResolvedVisibilityRules) -> None: + """A hidden label cannot truncate, and a mustMatch field may legitimately carry no label.""" + assert collected(visible) == set() + + +def test_a_group_does_not_hide_the_labels_inside_it() -> None: + """ResolvedFieldGroup carries no visibility of its own, so each child decides for itself.""" + group = ResolvedFieldGroup(fields=[field(VisibilityRule.NEVER), field(VisibilityRule.ALWAYS)]) + + out: set[str] = set() + ValidateMaxLengthLinter._collect_long_labels(group, out) + + assert out == {LONG} + + +@pytest.mark.parametrize( + "intent,expected", + [ + pytest.param("Withdraw {_amounts.[]}", 9, id="one_placeholder"), + pytest.param("Send {sharesAmount} to {recipient}", 9, id="two_placeholders"), + pytest.param("Stake ETH with SSV", 18, id="no_placeholder"), + pytest.param("{_approved} unstETH NFTs", 13, id="leading_placeholder"), + ], +) +def test_literal_length_counts_only_what_is_certain_to_be_displayed(intent: str, expected: int) -> None: + """A `{path}` is template syntax, and the value replacing it has no length known here.""" + assert ValidateMaxLengthLinter._literal_length(intent) == expected + + +def test_a_long_path_inside_a_placeholder_does_not_make_the_intent_long() -> None: + """`{execution.desc.minReturnAmount}` is 32 characters of path that never reaches a screen.""" + intent = "Swap for at least {execution.desc.minReturnAmount}" + + assert len(intent) > 30 + assert ValidateMaxLengthLinter._literal_length(intent) == 18 + + +def test_literal_text_over_the_limit_is_still_measured() -> None: + """Literal text alone over the limit truncates whatever the values render to.""" + intent = "Authorize decryption on behalf of {delegator} for {contracts} for {days} days" + + assert ValidateMaxLengthLinter._literal_length(intent) > 30