Commit d15197be authored by Luke Mewburn's avatar Luke Mewburn
Browse files

asn_backwards_compat: improve container handling

Fix handling of skipping already checked types, including
ENUMERATED, SEQUENCE OF, SET OF.

Detect new tags in SEQUENCE and SET that aren't OPTIONAL.

Use the "tagpath" as the seenkey for fields that use base types.

Adapt check_asn_backwards_compat to new interfaces.
parent aaf8a3bd
Loading
Loading
Loading
Loading
Loading
+150 −35
Original line number Diff line number Diff line
@@ -20,6 +20,8 @@ import pycrate_asn1c.asnproc

# pylint: disable=logging-fstring-interpolation

TYPE_CONTAINER_OF = (pa_asnobj.TYPE_SEQ_OF, pa_asnobj.TYPE_SET_OF)


@dataclasses.dataclass
class Context:
@@ -31,7 +33,7 @@ class Context:
    seen: set[str] = dataclasses.field(default_factory=set)


def process_error(ctx: Context, message: str):
def process_error(ctx: Context, message: str) -> None:
    """Log `message` and append `message` to `errors`."""
    logging.info(f"Test Failure: {message}")
    ctx.errors.append(message)
@@ -40,7 +42,8 @@ def process_error(ctx: Context, message: str):
def dict_swap(dictionary: dict) -> dict:
    """Swap keys and values in `dictionary`.

    Note: Doesn't attempt to handle duplicates."""
    Note: Doesn't attempt to handle duplicates.
    """
    return {value: key for key, value in dictionary.items()}


@@ -52,11 +55,19 @@ def parse_asn1(asntext: str) -> pa_asnobj.ASN1Dict:
    return res


def repr_modname(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return name of `asnobj` as MODULE.NAME or NAME."""
    result = asnobj._name
    if (asnmod := getattr(asnobj, "_mod", None)) is not None:
        result = f"{asnmod}.{result}"
    return result


def repr_name(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return name of `asnobj` as MODULE.PARENT.NAME[TAG].

    "MODULE.", "PARENT.", and "[TAG]" are optional."""

    "MODULE.", "PARENT.", and "[TAG]" are optional.
    """
    result = f"{asnobj._name}"
    if (asnparent := getattr(asnobj, "_parent", None)) is not None:
        result = f"{repr_name(asnparent)}.{result}"
@@ -67,17 +78,59 @@ def repr_name(asnobj: pa_asnobj.ASN1Obj) -> str:
    return result


def repr_name_type(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return name and type of `asnobj`, using repr_name() and repr_type()."""
    return f"{repr_name(asnobj)} {repr_type(asnobj)}"


def repr_tagpath(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return tag path name of `asnobj` as MODULE.TYPE.TAG[.TAG ...].

    "MODULE." and ".TAG" are optional.
    """
    if asnobj._tag is not None:
        result = asnobj._tag[0]
    else:
        result = asnobj._name

    if (asnparent := getattr(asnobj, "_parent", None)) is not None:
        result = f"{repr_tagpath(asnparent)}.{result}"
    if (asnmod := getattr(asnobj, "_mod", None)) is not None:
        result = f"{asnmod}.{result}"
    return result


def repr_type(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return name of type of `asnobj`, as MODULE.TYPE.
    """Return name of type of `asnobj`, as MODULE.TYPE or BASETYPE."""
    if (tr := asnobj.get_typeref()) is not None:
        # type reference
        result = repr_modname(tr)
    elif asnobj._parent is None:
        # type in module
        result = repr_modname(asnobj)
    else:
        # base type
        result = asnobj.TYPE
    return result


    TODO: fix for anonymous types such as L2AccessPDU.L2CC.l2CCContents CHOICE.
    "MODULE." is optional."""
def repr_type_tagpath(asnobj: pa_asnobj.ASN1Obj) -> str:
    """Return type and/or tagpath for `asnobj`.

    Returns MODULE.TYPE if asnobj has a type reference or is a type,
    otherwise the tagpath (e.g., an unnamed inline container).
    """
    if (tr := asnobj.get_typeref()) is not None:
        return repr_name(tr)
        # reference to type in module
        result = repr_modname(tr)
    elif asnobj._parent is None:
        # type in module
        result = repr_modname(asnobj)
    else:
        # TODO: this isn't quite right
        return repr_name(asnobj)  # TODO asnobj._type ?
        # other field
        result = repr_tagpath(asnobj)
    logging.debug(f"repr_type_tagpath {asnobj} = '{result}'")
    return result


def container_to_tag_dict(ctx: Context, container: pa_asnobj.ASN1Dict) -> dict:
@@ -94,11 +147,35 @@ def container_to_tag_dict(ctx: Context, container: pa_asnobj.ASN1Dict) -> dict:
    return result


def compare_enum(ctx: Context, fa_enum: pa_asnobj.ENUM, fb_enum: pa_asnobj.ENUM):
def types_already_seen(
    ctx: Context, fa_type: pa_asnobj.ASN1Obj, fb_type: pa_asnobj.ASN1Obj
) -> bool:
    """Return True if fa_type and fb_type have already been seen."""
    fa_repr = repr_type_tagpath(fa_type)
    fb_repr = repr_type_tagpath(fb_type)
    seenkey = f"{fa_repr} and {fb_repr}"
    logging.info(f"Seen check {seenkey}")
    if seenkey in ctx.seen:
        logging.info(f"Skipping, already seen {seenkey}")
        return True
    ctx.seen.add(seenkey)
    return False


def compare_enum(
    ctx: Context, fa_enum: pa_asnobj.ENUM, fb_enum: pa_asnobj.ENUM
) -> None:
    """Compare ASN.1 ENUMERATED `fa_enum` and `fb_enum`."""
    assert fa_enum.TYPE == pa_asnobj.TYPE_ENUM
    assert fb_enum.TYPE == pa_asnobj.TYPE_ENUM

    fa_desc = repr_name_type(fa_enum)
    fb_desc = repr_name_type(fb_enum)
    logging.info(f"Comparing enum {fa_desc} with {fb_desc}")

    if types_already_seen(ctx, fa_enum, fb_enum):
        return

    fa_tags = dict_swap(fa_enum.get_cont())
    fb_tags = dict_swap(fb_enum.get_cont())

@@ -114,22 +191,23 @@ def compare_enum(ctx: Context, fa_enum: pa_asnobj.ENUM, fb_enum: pa_asnobj.ENUM)
        if ca_name != cb_name:
            process_error(
                ctx,
                f"{repr_name(fa_enum)}.{ca_name}({tag}) renamed to '{cb_name}({tag})'",
                f"{repr_name(fa_enum)}.{ca_name}({tag}) renamed to {cb_name}({tag})",
            )


def compare_container(
    ctx: Context, fa_type: pa_asnobj.ASN1Obj, fb_type: pa_asnobj.ASN1Obj
):
) -> None:
    """Compare ASN.1 containers `fa_type` and `fb_type`."""
    assert fa_type.TYPE in pa_asnobj.TYPE_CONSTRUCT
    assert fb_type.TYPE in pa_asnobj.TYPE_CONSTRUCT

    seenkey = f"{repr_type(fa_type)} and {repr_type(fb_type)}"
    logging.info(f"Comparing container {seenkey}")
    if seenkey in ctx.seen:
        logging.info(f"Skipping, already processed {seenkey}")
    ctx.seen.add(seenkey)
    fa_desc = repr_name_type(fa_type)
    fb_desc = repr_name_type(fb_type)
    logging.info(f"Comparing container {fa_desc} with {fb_desc}")

    if types_already_seen(ctx, fa_type, fb_type):
        return

    fa_tags = container_to_tag_dict(ctx=ctx, container=fa_type.get_cont())
    fb_tags = container_to_tag_dict(ctx=ctx, container=fb_type.get_cont())
@@ -152,30 +230,67 @@ def compare_container(
        if ca_name != cb_name:
            process_error(
                ctx,
                f"{repr_name(fa_type)}.{ca_name}[{tag}] renamed to '{cb_name}[{tag}]'",
                f"{repr_name(fa_type)}.{ca_name}[{tag}] renamed to {cb_name}[{tag}]",
            )

        if ca_typerepr != cb_typerepr:
            process_error(
                ctx,
                f"{repr_name(fa_type)}.{ca_name}[{tag}] type '{ca_typerepr}' changed to type '{cb_typerepr}'",
                f"{repr_name(fa_type)}.{ca_name}[{tag}] {ca_typerepr} type changed to "
                f"{repr_name(fb_type)}.{cb_name}[{tag}] {cb_typerepr}",
            )

        compare_asntype(ctx=ctx, fa_type=ca_child, fb_type=cb_child)

        # check new SEQUENCE and SET tags are OPTIONAL
    if fb_type.TYPE not in (pa_asnobj.TYPE_SEQ, pa_asnobj.TYPE_SET):
        return

    fb_newtags = {tag: fb_tags[tag] for tag in fb_tags.keys() - fa_tags.keys()}
    if fb_newtags:
        logging.info(f"Container {fb_desc} has new tags {set(fb_newtags.keys())}")
    for tag, child in fb_newtags.items():
        if child._flag is None or pa_asnobj.FLAG_OPT not in child._flag:
            process_error(
                ctx,
                f"{repr_name(fb_type)} new tag {child._name}[{tag}] is not OPTIONAL",
            )


def compare_container_of(
    ctx: Context, fa_type: pa_asnobj.ASN1Obj, fb_type: pa_asnobj.ASN1Obj
) -> None:
    """Compare ASN.1 containers SEQ OF or SET OF `fa_type` and `fb_type`."""
    assert fa_type.TYPE in TYPE_CONTAINER_OF
    assert fb_type.TYPE in TYPE_CONTAINER_OF

    fa_desc = repr_name_type(fa_type)
    fb_desc = repr_name_type(fb_type)
    logging.info(f"Comparing container OF {fa_desc} with {fb_desc}")

    # TODO: .name may end with "._item_"; removesuffix?

    if types_already_seen(ctx, fa_type, fb_type):
        return

    fa_of = fa_type.get_cont()
    fb_of = fa_type.get_cont()

    compare_asntype(ctx=ctx, fa_type=fa_of, fb_type=fb_of)


def compare_asntype(
    ctx: Context, fa_type: pa_asnobj.ASN1Obj, fb_type: pa_asnobj.ASN1Obj
):
) -> None:
    """Compare ASN.1 types `fa_type` and `fb_type`."""
    fa_desc = f"{repr_name(fa_type)} type {repr_type(fa_type)}"
    fb_desc = f"{repr_name(fb_type)} type {repr_type(fb_type)}"
    fa_desc = repr_name_type(fa_type)
    fb_desc = repr_name_type(fb_type)
    logging.info(f"Comparing {fa_desc} with {fb_desc}")

    if fa_type.TYPE != fb_type.TYPE:
        process_error(
            ctx,
            f"{fa_desc} type '{fa_type.TYPE}' mismatch with '{fb_type.TYPE}'",
            f"{fa_desc} {fa_type.TYPE} type mismatch with {fb_desc} {fb_type.TYPE}",
        )
        # No further processing of this type can be performed
        return
@@ -189,32 +304,33 @@ def compare_asntype(
        if fa_const != fb_const:
            process_error(
                ctx,
                f"{fa_desc} constraint '{','.join(fa_const)}' changed to {fb_desc} constraint '{','.join(fb_const)}'",
                f"{fa_desc} constraint {','.join(fa_const)} changed to {fb_desc} constraint {','.join(fb_const)}",
            )

    if fa_type.TYPE is pa_asnobj.TYPE_ENUM:
        compare_enum(ctx=ctx, fa_enum=fa_type, fb_enum=fb_type)
    elif fa_type.TYPE in TYPE_CONTAINER_OF:
        compare_container_of(ctx=ctx, fa_type=fa_type, fb_type=fb_type)
    elif fa_type.TYPE in pa_asnobj.TYPE_CONSTRUCT:
        compare_container(ctx=ctx, fa_type=fa_type, fb_type=fb_type)
    else:
        # TODO: process_error(ctx, f"Unimplemented check of type '{fa_type.TYPE}'")
        pass
        logging.debug(f"Unimplemented check of type '{fa_type.TYPE}'")


def compare_asntext(ctx: Context, fa_asntext: str, fb_asntext: str):
def compare_asntext(ctx: Context, fa_asntext: str, fb_asntext: str) -> None:
    """Compare ASN.1 text `fa_asntext` and `fb_asntext` for backwards compatibility
    issues introduced in `fb_asntext`:
    - Module is the same
      - Each type in the module:
         - Exists
         - Has the same member types
         - Exists.
         - Has the same member types.
         - Each complex member type has:
           - Same inner type at the tag
           - Warns if the type renamed (not fatal)
           - Same inner name at the tag.
           - Same inner type at the tag.
           - New tags in SEQUENCE and SET are OPTIONAL.

    Updates ctx.errors with a list of error messages (if any).
    """

    logging.info(f"Comparing file {ctx.fa_path} with {ctx.fb_path}")

    logging.info(f"Parsing {ctx.fa_path}")
@@ -246,9 +362,8 @@ def compare_asntext(ctx: Context, fa_asntext: str, fb_asntext: str):
            compare_asntype(ctx=ctx, fa_type=fa_type, fb_type=fb_type)


def compare_files(ctx: Context):
def compare_files(ctx: Context) -> None:
    """Read ASN.1 files `ctx.fa_path` and `ctx.fb_path` and compare with compare_asntext."""

    fa_text = pathlib.Path(ctx.fa_path).read_text()
    fb_text = pathlib.Path(ctx.fb_path).read_text()
    compare_asntext(ctx=ctx, fa_asntext=fa_text, fb_asntext=fb_text)
+7 −8
Original line number Diff line number Diff line
@@ -10,18 +10,17 @@ import asn_backwards_compat
def compare_releases(files):
    total_errors = 0
    for idx in range(len(files) - 1):
        file1 = files[idx]
        file2 = files[idx + 1]
        errors = asn_backwards_compat.compare_files(file1, file2)
        if errors:
        ctx = asn_backwards_compat.Context(fa_path=files[idx], fb_path=files[idx + 1])
        asn_backwards_compat.compare_files(ctx=ctx)
        if ctx.errors:
            print("-----------------------------")
            print(f"File 1: {file1}")
            print(f"File 2: {file2}")
            print(f"File 1: {ctx.fa_path}")
            print(f"File 2: {ctx.fb_path}")
            print("Errors:")
            for error in errors:
            for error in ctx.errors:
                print(f"  {error}")
            print("-----------------------------")
        total_errors += len(errors)
        total_errors += len(ctx.errors)
    return total_errors