fix[ux]: show readable type names in type-mismatch errors by cristianizzo · Pull Request #5202 · vyperlang/vyper

cristianizzo · 2026-07-26T14:04:46Z

What I did

_GenericTypeAcceptor (used to represent the set of types a builtin accepts) rendered as its internal Python class repr in error messages. For example, len() on a wrong type or a module produced:

expected one of GenericTypeAcceptor(<class 'vyper.semantics.types.bytestrings.StringT'>), GenericTypeAcceptor(<class 'vyper.semantics.types.bytestrings.BytesT'>), GenericTypeAcceptor(<class 'vyper.semantics.types.subscriptable.DArrayT'>)

Now it produces:

expected one of String, Bytes, DynArray

How

The type-mismatch message renders acceptors via str(...) (analysis/utils.py), which fell back to __repr__. I added a __str__ that resolves the user-facing type name — _id when it's a plain class string (String/Bytes/DynArray), falling back to typeclass for parametric types whose _id is a property (e.g. IntegerT → integer), then the class name — so it is always a string. __repr__ is left as the unambiguous debug form (which also keeps it total, so e.g. TYPE_T.any() doesn't raise).

Tests

Added two regression tests to tests/functional/syntax/test_len.py: one asserting a len() type-mismatch message shows String/Bytes/DynArray (and no GenericTypeAcceptor), and one for the typeclass fallback branch (indexing with the wrong type → integer). Both fail on the current code and pass with this change; black/isort/flake8 are clean.

Copilot

Pull request overview

This PR improves Vyper’s type-mismatch UX by ensuring _GenericTypeAcceptor (used in builtin input type definitions) renders as a user-facing type name in error messages, avoiding leakage of internal Python class reprs as reported in #4955.

Changes:

Add __str__ to _GenericTypeAcceptor to emit readable, user-facing type identifiers for error formatting.
Add functional regression tests asserting len() and an indexing type-mismatch no longer include GenericTypeAcceptor(...) and instead show readable expected-type names.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
vyper/semantics/types/base.py	Adds `_GenericTypeAcceptor.__str__` to produce user-facing names in type-mismatch messages.
tests/functional/syntax/test_len.py	Adds regression tests for readable expected-type names and no `GenericTypeAcceptor` leakage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`_GenericTypeAcceptor` rendered as `GenericTypeAcceptor(<class '...StringT'>)` in error messages (e.g. `len()` on a wrong type or module), leaking internal Python class paths instead of readable Vyper type names. Add a `__str__` that resolves the user-facing name (`String`, `Bytes`, `DynArray`, ...), falling back to `typeclass` then the class name, and keep `__repr__` as the debug form. Fixes vyperlang#4955

Sporarum

Thank you for your work!

A couple points:

Given the tests are explicitly about error messages, I would prefer the check was of the form assert message == to have the full expected content on display, you can look how we did it in some other tests
(notably I don't think we do str(exc_info.value))

Sporarum · 2026-07-27T07:43:15Z

+    def __str__(self):
+        # User-facing type name (e.g. `String`, `Bytes`, `DynArray`) used in
+        # error messages, instead of the internal Python class repr. `_id` is a
+        # plain class attribute on most types, but a property on parametric
+        # types (e.g. `IntegerT`, `BytesM_T`), so fall back to `typeclass`
+        # (mapped to its user-facing spelling), then the class name, keeping
+        # this total (never returns a non-string).
+        name = getattr(self.type_, "_id", None)
+        if not isinstance(name, str):
+            typeclass = getattr(self.type_, "typeclass", None)
+            name = _TYPECLASS_DISPLAY_NAMES.get(typeclass, typeclass)
+        if not isinstance(name, str):
+            name = self.type_.__name__
+        return name


I'm not sure the typeclass is a good default name (somewhat highlighted by the need for _TYPECLASS_DISPLAY_NAMES), as far as I understand it's the name used when linearizing to json, and thus needs to be unique for us, but not a good user-facing name.

And the user-facing name might evolve differently than the typeclass, for backward compat reasons

I'm not sure what the "correct" solution is then, maybe have _id also work on String etc ?

Agreed — I dropped typeclass from the display path entirely (it is the json-linearization id and shouldn't double as a user-facing name), along with the _TYPECLASS_DISPLAY_NAMES map and the __name__ fallback.

Instead, __str__ now resolves the class-level _id first and falls back to a new explicit _generic_id:

name = getattr(self.type_, "_id", None) if not isinstance(name, str): name = self.type_._generic_id if not isinstance(name, str): raise CompilerPanic(f"{self.type_.__name__} has no user-facing name")

_generic_id: str = None # type: ignore is declared on VyperType (mirroring the adjacent typeclass declaration) and set on exactly the three types that .any() can hand us without a class-level _id: BytesM_T (_id is a property) → "bytesM", IntegerT (_id is a cached_property) → "integer", and TYPE_T (no _id at all) → "type". I checked every type reachable via .any() and they all now resolve to a readable name, so the CompilerPanic is unreachable but keeps a future parametric type from silently leaking a class repr instead of quietly degrading.

I also switched the tests to full-message equality as you asked (assert e.value.message == ..., no str(exc_info.value) — that appends the source location), e.g.:

assert e.value.message == "Given reference has type int128, expected one of String, Bytes, DynArray"

Why does _id on String (et al) work ?
Since if anything String is more parametrized than IntegerT

Good question, and the answer showed my comment was wrong (fixed below).

It is not about parametricity. It is about what _id names: the type constructor, or the fully applied type.

StringT _id = "String" (class attr) __repr__ -> "String[5]" BytesT _id = "Bytes" (class attr) __repr__ -> "Bytes[5]" DArrayT _id = "DynArray" (class attr) __repr__ -> "DynArray[uint256, 3]" IntegerT _id = cached_property -> "uint256" BytesM_T _id = property -> "bytes4"

So String is parameterized, as you say, but its _id holds the constructor name and the parameters are appended in __repr__. IntegerT/BytesM_T instead bake the parameters into _id itself, so there is no constructor-level name to read off the class — which is exactly what .any() needs. That is the gap _generic_id fills, and I have reworded the declaration comment accordingly.

Okay thanks, then I think we should instead do the same, and have _id not have parameters baked-in, and the repr outputing the full name

cristianizzo · 2026-07-28T16:09:54Z

Yeah, fair point. typeclass is really the serialization id (it's what we dump in to_dict), so using it for display and then patching it with _TYPECLASS_DISPLAY_NAMES is fixing the wrong layer, and you're right that it could drift from the user-facing name for backwards-compat reasons.

The reason it breaks is just that _id is a normal class attr on most types (String, Bytes, DynArray...) but a property on IntegerT and BytesM_T, so there's no value at the class level, which is what .any() hands us.

So instead of touching typeclass, I'd give those two an explicit class-level name:

class BytesM_T(_PrimT):
    _generic_id = "bytesM"

class IntegerT(...):
    _generic_id = "integer"

and make __str__ go _generic_id -> class-level _id -> __name__. That drops both the typeclass fallback and the map.

Or if you'd rather make _id itself resolve at the class level for every type, I can do it that way instead. Lmk which you prefer and I'll push it.

Sporarum · 2026-07-29T07:08:44Z

make __str__ go _generic_id -> class-level _id -> __name__. That drops both the typeclass fallback and the map.

Not sure I understand, but I think we should use _id if available and _generic_id otherwise (and never need __name__!)

Or if you'd rather make _id itself resolve at the class level for every type

I'm not sure what that would entail, if it can be done cleanly, it conceptually looks like the cleaner option

cristianizzo · 2026-07-29T16:21:40Z

Yeah, _id first then _generic_id makes more sense, I'll flip it.

On dropping __name__: mostly agree, one gotcha though — TYPE_T has no _id at all (only reachable via TYPE_T.any() in a couple of builtins), so it'd still hit __name__. If I give it a _generic_id too (like "type"), then we can drop __name__ entirely like you want — everything reachable via .any() then has either a real _id (String/Bytes/DynArray) or a _generic_id (bytesM/integer/type).

On folding it into _id: I looked at it and it's not really cleaner in practice. The parametric _ids are properties that need instance state (self.m, self.bits), so to make Cls._id return a generic name while instance._id stays specific you'd need a custom class/instance descriptor on each of them — more magic than a one-line _generic_id attr imo. So I'd lean _generic_id.

I'll push that (flip the order, add _generic_id to BytesM_T/IntegerT/TYPE_T, drop the map + __name__) unless you object.

Address review: `typeclass` is the json-serialization id, not a user-facing name, so reusing it (plus a `_TYPECLASS_DISPLAY_NAMES` map) for display was the wrong layer. Give the types whose `_id` is a property (`BytesM_T`, `IntegerT`) or absent (`TYPE_T`) an explicit `_generic_id`, and resolve `_id` -> `_generic_id` in `_GenericTypeAcceptor.__str__`, dropping the `typeclass` map and the `__name__` fallback.

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Compare `e.value.message` against the complete expected string instead of substring checks, per review feedback, so the exact user-facing message is visible in the test.

Sporarum · 2026-07-30T07:19:02Z

+    # `concat`'s accepted `bytesM` type must be shown by its user-facing name,
+    # not the internal `bytes_m` typeclass (nor a `GenericTypeAcceptor` repr).
+    # See issue #4955.


Nit:
I think the added tests are small enough they don't need an explanation

Removed from both tests. The one comment I kept is a single line on the index test, noting it is the _generic_id path rather than the _id path, since that is not obvious from reading the contract snippet.

Sporarum · 2026-07-30T07:21:50Z

 # position, ex. constructors (events, interfaces and structs), and also
 # certain builtins which take types as parameters
 class TYPE_T(VyperType):
+    _generic_id = "type"


Hmm this doesn't seem great, can you add a test so we can see what the error looks like ?

I think TYPE_T should instead use the name of the underlying object (event, interface, etc)

You were right that this was not great — I removed TYPE_T._generic_id entirely.

I could not produce an error that displays it. I tried every TYPE_T.any() call site with a variety of bad arguments:

empty(1) InvalidType: '1' is not a type! empty(self.bar) InvalidType: 'self.bar' is not a type! empty("uint256") InvalidType: '"uint256"' is not a type! epsilon(b"ab") InvalidType: 'b"ab"' is not a type! abi_decode(b, b"ab") InvalidType: 'b"ab"' is not a type! method_id("f()", output_type=b"ab") InvalidType: 'b"ab"' is not a type! extract32(b, 0, output_type=b"ab") InvalidType: 'b"ab"' is not a type!

In all of them the argument is resolved as a type before the expected-type name is ever formatted, so __str__ is never called (I confirmed this with the same tracing as in the IntegerT thread — zero renders). The only other use, TYPE_T.any().compare_type(...) in _signatures.py, does not stringify either. So there is nothing to write a test against; the name was dead code, and the CompilerPanic now documents that invariant.

On using the underlying object's name (event, interface, ...): that is not available here. .any() is a classmethod and hands _GenericTypeAcceptor the class, not a TYPE_T instance, so there is no typedef to read a name from at that point. If a code path ever does need to display it, the natural fix would be an instance-level name on TYPE_T rather than a class-level _generic_id — but today nothing needs it.

Sporarum · 2026-07-30T07:26:39Z

    """

    typeclass = "integer"
+    _generic_id = "integer"


I'm not sure this is reachable, since the .any() does not go through a GenericTypeAcceptator
Please look into it

I looked into it — IntegerT is reachable, so I kept it. .any() is _GenericTypeAcceptor(cls) (base.py), and IntegerT.any() has five call sites.

To be sure the string actually comes from this path rather than somewhere else, I traced _GenericTypeAcceptor.__str__ and recorded every render:

x[b"ab"] TypeMismatch: Expected integer but literal can only be cast as Bytes[2]. rendered via __str__: [(IntegerT, "integer")] shift(1, b"ab") TypeMismatch: Expected integer but literal can only be cast as Bytes[2]. rendered via __str__: [(IntegerT, "integer")]

Two independent paths (validate_expected_type(node, IntegerT.any()) in subscriptable.py for index validation, and the shift builtin). Without _generic_id these hit the CompilerPanic. The index case is covered by test_index_type_mismatch_message_uses_readable_type_names.

Note that IntegerT.any() inside a tuple of expected types behaves differently — uint2str(b"ab") expands to Expected one of uint8, uint16, ..., which never reaches __str__. So only the standalone uses matter here.

Sporarum · 2026-07-30T07:27:49Z

+    # user-facing name for parametric types whose `_id` is an instance
+    # property (e.g. `bytesM`, `integer`), so it has no value at the class
+    # level. see `_GenericTypeAcceptor.__str__`.
+    _generic_id: str = None  # type: ignore


Nit:
If it doesn't have the right type, I'm sure I'll get confused at some point, let's instead:

Suggested change

_generic_id: str = None # type: ignore

_generic_id: Optional[str] = None

Done, applied as suggested: _generic_id: Optional[str] = None, and the # type: ignore is gone with it. mypy -p vyper is clean.

Sporarum · 2026-07-30T07:34:41Z

+        # error messages, instead of the internal Python class repr. Prefer the
+        # class-level `_id`; parametric types (e.g. `IntegerT`, `BytesM_T`)
+        # define `_id` as a property, so it has no value at the class level that
+        # `.any()` hands us -- fall back to their `_generic_id`.


I don't think this needs a comment:
__str__ is for user-facing stuff
The rest just paraphrases the body

Also it seems incorrect:
IntegerT and BytesM_T are not parametric (uint256, bytes4)
(well as python values they are, but then so are String et al)

Right on both counts — comment removed, __str__ is now just the three lines of body.

And thanks for catching the inaccuracy: "parametric" was the wrong axis (String is parameterized too). The actual distinction is constructor name vs fully applied name in _id; I have written that up in your other thread and corrected the remaining comment on the _generic_id declaration.

Sporarum · 2026-07-30T07:36:24Z

+    def __str__(self):
+        # User-facing type name (e.g. `String`, `Bytes`, `DynArray`) used in
+        # error messages, instead of the internal Python class repr. `_id` is a
+        # plain class attribute on most types, but a property on parametric
+        # types (e.g. `IntegerT`, `BytesM_T`), so fall back to `typeclass`
+        # (mapped to its user-facing spelling), then the class name, keeping
+        # this total (never returns a non-string).
+        name = getattr(self.type_, "_id", None)
+        if not isinstance(name, str):
+            typeclass = getattr(self.type_, "typeclass", None)
+            name = _TYPECLASS_DISPLAY_NAMES.get(typeclass, typeclass)
+        if not isinstance(name, str):
+            name = self.type_.__name__
+        return name


Why does _id on String (et al) work ?
Since if anything String is more parametrized than IntegerT

Address review: - remove `TYPE_T._generic_id`: every `TYPE_T.any()` call site rejects a non-type argument with `InvalidType` before any type name is rendered, so the name was dead code. `CompilerPanic` now covers that case. - type `_generic_id` as `Optional[str]` instead of `str = None`. - drop the comments on `__str__` and the tests; fix the description of `_generic_id`, which is about `_id` naming the fully applied type rather than the type constructor, not about parametricity.

Copilot AI review requested due to automatic review settings July 26, 2026 14:04

Copilot started reviewing on behalf of cristianizzo July 26, 2026 14:05 View session

Copilot AI reviewed Jul 26, 2026

View reviewed changes

Comment thread vyper/semantics/types/base.py

cristianizzo force-pushed the fix/generic-type-acceptor-readable-names branch from ee1808b to 4f4f1ca Compare July 26, 2026 14:52

Sporarum reviewed Jul 27, 2026

View reviewed changes

cristianizzo requested a review from Copilot July 29, 2026 17:12

Copilot started reviewing on behalf of cristianizzo July 29, 2026 17:13 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

test: assert full error messages in type-name tests

ceb019d

Compare `e.value.message` against the complete expected string instead of substring checks, per review feedback, so the exact user-facing message is visible in the test.

Sporarum reviewed Jul 30, 2026

View reviewed changes

	_generic_id: str = None # type: ignore
	_generic_id: Optional[str] = None

Uh oh!

Uh oh!

Conversation

cristianizzo commented Jul 26, 2026

What I did

How

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Sporarum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cristianizzo commented Jul 28, 2026

Uh oh!

Sporarum commented Jul 29, 2026

Uh oh!

cristianizzo commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants