From e5da8131995444b107baba13d21e2e49be531e4e Mon Sep 17 00:00:00 2001 From: Neil Brown Date: Tue, 4 Aug 2026 09:49:20 +0100 Subject: [PATCH] Fix list.pop() return type inference (Strype issue #1003) pop() was hard-coded to return the list type itself instead of the item type, so e.g. get_actors().pop().remove() resolved .remove() against list.remove(x) instead of the actual item class's remove(), leaking the wrong signature into autocomplete. Also fixed append/extend/insert/remove/ reverse/sort, which had the same hard-coded-to-self bug but should return None. Mirrors the BuiltinMethod/getMethodType pattern used for dict.keys() rather than an AST-walker echo marker: MUTABLE_SEQ registers "pop" as a shared BuiltinMethod template, and TypeAstWalker.getTypeOfAttr binds it to the receiver's own PrimitiveType (e.g. list[Actor] rather than plain list) at each attribute access, since a field shared across every instance of a type can't otherwise see which concrete instance it was accessed on. ListType overrides getMethodType to answer "pop" with its item type. --- .../utilities/types/BuiltinMethod.scala | 41 +++++++++++++++++++ .../utilities/types/BuiltinTypes.scala | 14 +++---- .../utilities/types/ListType.scala | 9 ++++ .../utilities/types/PrimitiveType.scala | 6 +++ .../utilities/types/TypeAstWalker.scala | 23 ++++++++++- .../params_pop_item_type_issue1003.py | 8 ++++ 6 files changed, 93 insertions(+), 8 deletions(-) create mode 100644 tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinMethod.scala create mode 100644 tpParser/shared/src/test/programs/completer_params/params_pop_item_type_issue1003.py diff --git a/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinMethod.scala b/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinMethod.scala new file mode 100644 index 0000000..4fdc0ad --- /dev/null +++ b/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinMethod.scala @@ -0,0 +1,41 @@ +package tigerpython.utilities.types + +/** + * The `BuiltinMethod` represents special methods such as `list.pop()` where we need to access the type information + * of the underlying receiver in order to get accurate types. + */ +class BuiltinMethod(var parent: PrimitiveType, + val name: String, + val params: Array[String], + val defaultReturnType: DataType) extends FunctionType { + + override def getFullName: String = if (parent != null) parent.getFullName + "." + name else name + + override def getParamsString: String = params.mkString(", ") + + override def getReturnType: DataType = + if (parent != null) + parent.getMethodType(name) + else + defaultReturnType + + // `parent` is bound once, when the field is registered on its declaring type (e.g. ``), + // so it never reflects the receiver's own type (e.g. `list[Actor]` rather than plain `list`). Every + // attribute access must therefore call `boundTo` with the actual receiver type so `getMethodType` sees it. + def boundTo(receiver: PrimitiveType): BuiltinMethod = { + val result = new BuiltinMethod(receiver, name, params, defaultReturnType) + result.docString = docString + result + } + + override def toString: String = "%s(%s)".format(name, getParamsString) +} +object BuiltinMethod { + + def apply(parent: PrimitiveType, name: String, params: Array[String], docString: String, + defaultReturnType: DataType = BuiltinTypes.ANY_TYPE): BuiltinMethod = { + val result = new BuiltinMethod(parent, name, params, defaultReturnType) + result.docString = docString + result + } +} diff --git a/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinTypes.scala b/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinTypes.scala index 2d4f029..248c8ef 100644 --- a/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinTypes.scala +++ b/tpParser/shared/src/main/scala/tigerpython/utilities/types/BuiltinTypes.scala @@ -99,13 +99,13 @@ object BuiltinTypes { val MUTABLE_SEQ = PrimitiveType("", SEQ_TYPE) MUTABLE_SEQ.addFields( - BuiltinFunction("append", Array("x"), MUTABLE_SEQ, null), - BuiltinFunction("extend", Array("x"), MUTABLE_SEQ, null), - BuiltinFunction("insert", Array(), MUTABLE_SEQ, null), - BuiltinFunction("pop", Array(), MUTABLE_SEQ, null), - BuiltinFunction("remove", Array("x"), MUTABLE_SEQ, null), - BuiltinFunction("reverse", Array(), MUTABLE_SEQ, "reverses the items of the sequence in place"), - BuiltinFunction("sort", Array(), MUTABLE_SEQ, "sort the items of the sequence in place") + BuiltinFunction("append", Array("x"), NONE_TYPE, null), + BuiltinFunction("extend", Array("x"), NONE_TYPE, null), + BuiltinFunction("insert", Array(), NONE_TYPE, null), + BuiltinMethod(null, "pop", Array(), null), + BuiltinFunction("remove", Array("x"), NONE_TYPE, null), + BuiltinFunction("reverse", Array(), NONE_TYPE, "reverses the items of the sequence in place"), + BuiltinFunction("sort", Array(), NONE_TYPE, "sort the items of the sequence in place") ) val LIST_TYPE = PrimitiveType("list", MUTABLE_SEQ) val TUPLE_TYPE = PrimitiveType("tuple", SEQ_TYPE) diff --git a/tpParser/shared/src/main/scala/tigerpython/utilities/types/ListType.scala b/tpParser/shared/src/main/scala/tigerpython/utilities/types/ListType.scala index df57999..245511b 100644 --- a/tpParser/shared/src/main/scala/tigerpython/utilities/types/ListType.scala +++ b/tpParser/shared/src/main/scala/tigerpython/utilities/types/ListType.scala @@ -12,6 +12,15 @@ class ListType(val itemType: DataType) extends //override def getFields: Map[String, DataType] = super.fields override def getItemType: DataType = itemType + + protected[types] + override def getMethodType(methodName: String): DataType = + methodName match { + case "pop" => + itemType + case _ => + BuiltinTypes.ANY_TYPE + } } object ListType { private val listTypes = collection.mutable.Map[DataType, ListType]() diff --git a/tpParser/shared/src/main/scala/tigerpython/utilities/types/PrimitiveType.scala b/tpParser/shared/src/main/scala/tigerpython/utilities/types/PrimitiveType.scala index 272f408..447a876 100644 --- a/tpParser/shared/src/main/scala/tigerpython/utilities/types/PrimitiveType.scala +++ b/tpParser/shared/src/main/scala/tigerpython/utilities/types/PrimitiveType.scala @@ -27,6 +27,12 @@ class PrimitiveType(val name: String, def getInstanceFields: Map[String, DataType] = fields + /** + * This method is used by `BuiltinMethod` to retrieve contextual type information. + */ + protected[types] + def getMethodType(methodName: String): DataType = BuiltinTypes.ANY_TYPE + override def isCallable: Boolean = true def isSubclassOf(base: DataType): Boolean = diff --git a/tpParser/shared/src/main/scala/tigerpython/utilities/types/TypeAstWalker.scala b/tpParser/shared/src/main/scala/tigerpython/utilities/types/TypeAstWalker.scala index fc37f51..aeab640 100644 --- a/tpParser/shared/src/main/scala/tigerpython/utilities/types/TypeAstWalker.scala +++ b/tpParser/shared/src/main/scala/tigerpython/utilities/types/TypeAstWalker.scala @@ -86,7 +86,15 @@ class TypeAstWalker { } protected def getTypeOfAttr(attr: AstNode.Attribute): DataType = { - getType(attr.base).findField(attr.attr.name) match { + val baseType = getType(attr.base) + baseType.findField(attr.attr.name) match { + case Some(method: BuiltinMethod) => + getPrimitiveType(baseType) match { + case Some(receiver) => + method.boundTo(receiver) + case None => + method + } case Some(result) => validateDataType(result) case None => @@ -94,6 +102,19 @@ class TypeAstWalker { } } + // A `BuiltinMethod` field is shared by every instance of its declaring type (e.g. all lists share + // the same "pop" field object), so it can't carry the receiver's own type (e.g. `list[Actor]` rather + // than plain `list`) - that has to be bound in at each attribute access instead, from here. + private def getPrimitiveType(dataType: DataType): Option[PrimitiveType] = + dataType match { + case instance: Instance => + getPrimitiveType(instance.baseType) + case primitive: PrimitiveType => + Some(primitive) + case _ => + None + } + protected def getTypeOfBinaryOp(binOp: AstNode.BinaryOp): DataType = { val left = getType(binOp.left) val right = getType(binOp.right) diff --git a/tpParser/shared/src/test/programs/completer_params/params_pop_item_type_issue1003.py b/tpParser/shared/src/test/programs/completer_params/params_pop_item_type_issue1003.py new file mode 100644 index 0000000..da3dcf7 --- /dev/null +++ b/tpParser/shared/src/test/programs/completer_params/params_pop_item_type_issue1003.py @@ -0,0 +1,8 @@ +# pyi:issue1003.actors +# class Actor: +# def remove(self) -> None: ... +# def get_actors() -> list[Actor]: ... +# 50 +# +from issue1003.actors import * +get_actors().pop().remove()