Skip to content

Commit

Permalink
Improve case expression error messages
Browse files Browse the repository at this point in the history 
Ref. #907
  • Loading branch information
@senier
senier committed Jul 12, 2022
1 parent 80f3940 commit b8a5e0a
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 25 deletions.
43 changes: 29 additions & 14 deletions rflx/expression.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from dataclasses import dataclass
from enum import Enum
from functools import lru_cache
from itertools import groupby
from operator import itemgetter
from sys import intern
from typing import Callable, Iterable, List, Mapping, Optional, Sequence, Tuple, Type, Union

Expand Down Expand Up @@ -2811,7 +2813,7 @@ def _check_enumeration(self) -> RecordFluxError:
(
f'missing literal "{l.name}"',
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
self.expr.type_.location,
)
for l in missing
Expand All @@ -2831,9 +2833,9 @@ def _check_enumeration(self) -> RecordFluxError:
),
*[
(
f'literal "{l.name}" not part of {self.expr.type_.identifier}',
f'literal "{l.name}" not part of "{self.expr.type_.identifier}"',
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
self.expr.type_.location,
)
for l in invalid
Expand All @@ -2855,23 +2857,30 @@ def _check_integer(self) -> RecordFluxError:

missing = set(type_literals) - set(literals)
if missing:
missing_ranges = []
for _, g in groupby(enumerate(missing), lambda i: i[0] - i[1]):
group = list(map(itemgetter(1), g))
missing_ranges.append((group[0], group[-1]))

error.extend(
[
(
f"case expression does not cover full range of "
f"{self.expr.type_.identifier}",
f'"{self.expr.type_.identifier}"',
Subsystem.MODEL,
Severity.ERROR,
self.location,
),
*[
(
f'missing literal "{l}"',
f"missing range {r[0]}-{r[1]}"
if r[0] != r[1]
else f"missing value {r[0]}",
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
self.expr.type_.location,
)
for l in missing
for r in missing_ranges
],
]
)
Expand All @@ -2888,9 +2897,9 @@ def _check_integer(self) -> RecordFluxError:
),
*[
(
f'literal "{l}" not part of {self.expr.type_.identifier}',
f'value {l} not part of "{self.expr.type_.identifier}"',
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
self.expr.type_.location,
)
for l in invalid
Expand All @@ -2916,15 +2925,15 @@ def _check_type_subexpr(self) -> RecordFluxError:
error.extend(
[
(
f'dependent expression "{e1}" has incompatible type {e1.type_}',
f'dependent expression "{e1}" has incompatible {e1.type_}',
Subsystem.MODEL,
Severity.ERROR,
e1.location,
),
(
f'conflicting with "{e2}" which has type {e2.type_}',
f'conflicting with "{e2}" which has {e2.type_}',
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
e2.location,
),
]
Expand Down Expand Up @@ -2952,7 +2961,7 @@ def _check_type_subexpr(self) -> RecordFluxError:
(
f'duplicate literal "{l}"',
Subsystem.MODEL,
Severity.WARNING,
Severity.INFO,
l.location,
)
for l in duplicates
Expand All @@ -2968,11 +2977,17 @@ def _check_type_subexpr(self) -> RecordFluxError:
error.extend(
[
(
f'invalid discrete choice type "{self.expr.type_}"',
f"invalid discrete choice with {self.expr.type_}",
Subsystem.MODEL,
Severity.ERROR,
self.expr.location,
),
(
"expected enumeration or integer type",
Subsystem.MODEL,
Severity.INFO,
self.expr.location,
),
]
)

Expand Down
32 changes: 21 additions & 11 deletions tests/unit/expression_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2526,14 +2526,15 @@ def test_case_type() -> None:

assert_type_error(
CaseExpr(c1, [([ID("V1"), ID("V2")], TRUE), ([ID("V3")], Number(1))]),
r'^model: error: dependent expression "True" has incompatible type enumeration type '
r'^model: error: dependent expression "True" has incompatible enumeration type '
r'"__BUILTINS__::Boolean"\n'
r'model: warning: conflicting with "1" which has type type universal integer \(1\)$',
r'model: info: conflicting with "1" which has type universal integer \(1\)$',
)
assert_type_error(
CaseExpr(Opaque(Variable("X", type_=rty.Message("A"))), [([ID("V")], Number(1))]),
r'^model: error: invalid discrete choice type "sequence type "__INTERNAL__::Opaque" '
r'with element integer type "Byte" \(0 .. 255\)"$',
r'^model: error: invalid discrete choice with sequence type "__INTERNAL__::Opaque" '
r'with element integer type "Byte" \(0 .. 255\)\n'
r"model: info: expected enumeration or integer type$",
)


Expand All @@ -2554,7 +2555,7 @@ def test_case_invalid() -> None:
location=Location((1, 2)),
),
"^<stdin>:1:2: model: error: not all enumeration literals covered by case expression\n"
'<stdin>:10:2: model: warning: missing literal "Two"$',
'<stdin>:10:2: model: info: missing literal "Two"$',
)
assert_type_error(
CaseExpr(
Expand All @@ -2563,7 +2564,7 @@ def test_case_invalid() -> None:
location=Location((1, 2)),
),
"^<stdin>:1:2: model: error: invalid literals used in case expression\n"
'<stdin>:10:2: model: warning: literal "Invalid" not part of P::Enumeration$',
'<stdin>:10:2: model: info: literal "Invalid" not part of "P::Enumeration"$',
)
assert_type_error(
CaseExpr(
Expand All @@ -2575,7 +2576,7 @@ def test_case_invalid() -> None:
location=Location((1, 2)),
),
"^<stdin>:1:2: model: error: duplicate literals used in case expression\n"
'<stdin>:3:2: model: warning: duplicate literal "One"$',
'<stdin>:3:2: model: info: duplicate literal "One"$',
)

assert_type_error(
Expand All @@ -2584,8 +2585,17 @@ def test_case_invalid() -> None:
[([Number(1)], TRUE), ([Number(2)], FALSE)],
location=Location((2, 2)),
),
"^<stdin>:2:2: model: error: case expression does not cover full range of P::Tiny\n"
'<stdin>:1:2: model: warning: missing literal "3"$',
'^<stdin>:2:2: model: error: case expression does not cover full range of "P::Tiny"\n'
"<stdin>:1:2: model: info: missing value 3$",
)
assert_type_error(
CaseExpr(
Variable("C", type_=TINY_INT.type_),
[([Number(1)], TRUE)],
location=Location((2, 2)),
),
'^<stdin>:2:2: model: error: case expression does not cover full range of "P::Tiny"\n'
"<stdin>:1:2: model: info: missing range 2-3$",
)
assert_type_error(
CaseExpr(
Expand All @@ -2594,7 +2604,7 @@ def test_case_invalid() -> None:
location=Location((2, 2)),
),
"^<stdin>:2:2: model: error: invalid literals used in case expression\n"
'<stdin>:1:2: model: warning: literal "4" not part of P::Tiny$',
'<stdin>:1:2: model: info: value 4 not part of "P::Tiny"$',
)
assert_type_error(
CaseExpr(
Expand All @@ -2606,5 +2616,5 @@ def test_case_invalid() -> None:
location=Location((1, 2)),
),
"^<stdin>:1:2: model: error: duplicate literals used in case expression\n"
'<stdin>:1:14: model: warning: duplicate literal "2"$',
'<stdin>:1:14: model: info: duplicate literal "2"$',
)

0 comments on commit b8a5e0a

Please sign in to comment.