Skip to content

Commit

Permalink
Improve non scalar parameter error message
Browse files Browse the repository at this point in the history
Ref. eng/recordflux/RecordFlux#1740
  • Loading branch information
Volham22 committed Jul 31, 2024
1 parent 18dff5e commit 47c7fb7
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 10 deletions.
28 changes: 22 additions & 6 deletions rflx/model/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -924,12 +924,28 @@ def _validate_types(

if f in parameters:
if not isinstance(t, type_decl.Scalar):
self.error.push(
ErrorEntry(
"parameters must have a scalar type",
Severity.ERROR,
f.identifier.location,
),
assert f.identifier.location is not None
additionnal_annotations = []
if not (
type_decl.is_builtin_type(t.identifier)
or type_decl.is_internal_type(t.identifier)
):
assert t.identifier.location is not None
additionnal_annotations.append(
Annotation(
"type declared here",
Severity.NOTE,
t.identifier.location,
),
)

self.error.extend(
rty.check_type_instance(
t.type_,
(rty.Enumeration, rty.AnyInteger),
location=f.identifier.location,
additionnal_annotations=additionnal_annotations,
).entries,
)
elif isinstance(t, type_decl.Enumeration) and t.always_valid:
self.error.push(
Expand Down
7 changes: 6 additions & 1 deletion rflx/typing_.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ def check_type_instance(
expected: Union[type[Type], tuple[type[Type], ...]],
location: Optional[Location],
description: str = "",
additionnal_annotations: abc.Sequence[Annotation] | None = None,
) -> RecordFluxError:
assert expected, "empty expected types"

Expand All @@ -359,6 +360,7 @@ def check_type_instance(
error = RecordFluxError()

if not isinstance(actual, expected) and actual != Any():
additionnal_annotations = additionnal_annotations or []
desc = (
" or ".join(e.DESCRIPTIVE_NAME for e in expected)
if isinstance(expected, tuple)
Expand All @@ -370,7 +372,10 @@ def check_type_instance(
f"expected {desc}",
Severity.ERROR,
location,
annotations=[Annotation(f"found {actual}", Severity.ERROR, location)],
annotations=[
Annotation(f"found {actual}", Severity.ERROR, location),
*additionnal_annotations,
],
generate_default_annotation=False,
),
)
Expand Down
4 changes: 2 additions & 2 deletions tests/data/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@

@lru_cache
def null_message() -> Message:
return Message("Null_Msg::Message", [], {})
return Message(ID("Null_Msg::Message", location=Location((1, 1))), [], {})


@lru_cache
Expand Down Expand Up @@ -459,7 +459,7 @@ def sequence_integer() -> Integer:

@lru_cache
def sequence_integer_vector() -> Sequence:
return Sequence("Sequence::Integer_Vector", sequence_integer())
return Sequence(ID("Sequence::Integer_Vector", location=Location((1, 1))), sequence_integer())


@lru_cache
Expand Down
86 changes: 86 additions & 0 deletions tests/integration/specification_model_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2070,3 +2070,89 @@ def test_aggregate_type_in_size(
),
capfd,
)


def test_parameter_non_scalar(
tmp_path: Path,
capfd: pytest.CaptureFixture[str],
) -> None:
file_path = tmp_path / "test.rflx"
file_path.write_text(
textwrap.dedent(
"""\
package Test is
type N is
message
D : Opaque with Size => 8;
end message;
type M (x : N) is
message
A : Opaque with Size => 8;
end message;
end Test;
""",
),
)
assert_error_full_message(
file_path,
textwrap.dedent(
f"""\
info: Parsing {file_path}
info: Processing Test
info: Verifying __BUILTINS__::Boolean
info: Verifying __INTERNAL__::Opaque
info: Verifying Test::N
error: expected enumeration type or integer type
--> {file_path}:7:12
|
2 | type N is
| - note: type declared here
3 | message
...
6 |
7 | type M (x : N) is
| ^ found message type "Test::N"
|
""",
),
capfd,
)


def test_parameter_non_scalar_and_builtin_type(
tmp_path: Path,
capfd: pytest.CaptureFixture[str],
) -> None:
file_path = tmp_path / "test.rflx"
file_path.write_text(
textwrap.dedent(
"""\
package Test is
type M (x : Opaque) is
message
A : Opaque with Size => 8;
end message;
end Test;
""",
),
)
assert_error_full_message(
file_path,
textwrap.dedent(
f"""\
info: Parsing {file_path}
info: Processing Test
info: Verifying __BUILTINS__::Boolean
info: Verifying __INTERNAL__::Opaque
error: expected enumeration type or integer type
--> {file_path}:2:12
|
2 | type M (x : Opaque) is
| ^ found sequence type "__INTERNAL__::Opaque" with element integer type \
"Byte" (0 .. 255)
|
""",
),
capfd,
)
12 changes: 11 additions & 1 deletion tests/unit/model/message_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
UncheckedDerivedMessage,
UncheckedMessage,
UncheckedRefinement,
type_decl,
)
from rflx.model.message import ByteOrder, annotate_path
from rflx.rapidflux import Annotation, Location, RecordFluxError, Severity
Expand Down Expand Up @@ -247,10 +248,19 @@ def test_invalid_parameter_type_composite(parameter_type: abc.Callable[[], TypeD
structure = [Link(INITIAL, Field("X")), Link(Field("X"), FINAL)]
types = {Field(ID("P", Location((1, 2)))): parameter_type(), Field("X"): models.integer()}

declared_location = (
r"\n<stdin>:1:1: note: type declared here"
if not (
type_decl.is_internal_type(parameter_type().identifier)
or type_decl.is_builtin_type(parameter_type().identifier)
)
else ""
)
assert_message_model_error(
structure,
types,
"^<stdin>:1:2: error: parameters must have a scalar type$",
r"^<stdin>:1:2: error: expected enumeration type or integer type\n"
rf"<stdin>:1:2: error: found {re.escape(str(parameter_type().type_))}{declared_location}$",
)


Expand Down

0 comments on commit 47c7fb7

Please sign in to comment.