Skip to content

Commit

Permalink
Refactoring: make the type of fullname str instead of Bogus[str] (#14435
Browse files Browse the repository at this point in the history
)

The type Bogus[X] is treated as Any when the code is compiled with
mypyc, while it's equivalent to X when only type checking. They are
sometimes used when X is not actually the real type of a value, but
changing it to the correct type would be complicated.

Bogus[str] types are pretty awkward, since we are lying to the type
checker and mypyc only sees Any types.

An empty fullname is now represented by "" instead of None, so we no
longer need a Bogus[str] type.

This might break some plugins, so we should document this in release
notes and the relevant issue that tracks plugin incompatibilities.

(Various small optimizations, including this, together netted a 6%
performance improvement in self check.)
JukkaL authored Jan 13, 2023
1 parent 1c5eeb8 commit 28e5436
Showing 15 changed files with 65 additions and 61 deletions.
4 changes: 2 additions & 2 deletions mypy/checker.py
Original file line number Diff line number Diff line change
@@ -2178,7 +2178,7 @@ def visit_class_def(self, defn: ClassDef) -> None:
temp = self.temp_node(sig, context=decorator)
fullname = None
if isinstance(decorator, RefExpr):
fullname = decorator.fullname
fullname = decorator.fullname or None

# TODO: Figure out how to have clearer error messages.
# (e.g. "class decorator must be a function that accepts a type."
@@ -4598,7 +4598,7 @@ def visit_decorator(self, e: Decorator) -> None:
temp = self.temp_node(sig, context=e)
fullname = None
if isinstance(d, RefExpr):
fullname = d.fullname
fullname = d.fullname or None
# if this is a expression like @b.a where b is an object, get the type of b
# so we can pass it the method hook in the plugins
object_type: Type | None = None
14 changes: 7 additions & 7 deletions mypy/checkexpr.py
Original file line number Diff line number Diff line change
@@ -216,8 +216,8 @@ def extract_refexpr_names(expr: RefExpr) -> set[str]:
Note that currently, the only two subclasses of RefExpr are NameExpr and
MemberExpr."""
output: set[str] = set()
while isinstance(expr.node, MypyFile) or expr.fullname is not None:
if isinstance(expr.node, MypyFile) and expr.fullname is not None:
while isinstance(expr.node, MypyFile) or expr.fullname:
if isinstance(expr.node, MypyFile) and expr.fullname:
# If it's None, something's wrong (perhaps due to an
# import cycle or a suppressed error). For now we just
# skip it.
@@ -228,7 +228,7 @@ def extract_refexpr_names(expr: RefExpr) -> set[str]:
if isinstance(expr.node, TypeInfo):
# Reference to a class or a nested class
output.update(split_module_names(expr.node.module_name))
elif expr.fullname is not None and "." in expr.fullname and not is_suppressed_import:
elif "." in expr.fullname and not is_suppressed_import:
# Everything else (that is not a silenced import within a class)
output.add(expr.fullname.rsplit(".", 1)[0])
break
@@ -526,7 +526,7 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) ->
# There are two special cases where plugins might act:
# * A "static" reference/alias to a class or function;
# get_function_hook() will be invoked for these.
fullname = e.callee.fullname
fullname = e.callee.fullname or None
if isinstance(e.callee.node, TypeAlias):
target = get_proper_type(e.callee.node.target)
if isinstance(target, Instance):
@@ -536,7 +536,7 @@ def visit_call_expr_inner(self, e: CallExpr, allow_none_return: bool = False) ->
# get_method_hook() and get_method_signature_hook() will
# be invoked for these.
if (
fullname is None
not fullname
and isinstance(e.callee, MemberExpr)
and self.chk.has_type(e.callee.expr)
):
@@ -605,7 +605,7 @@ def method_fullname(self, object_type: Type, method_name: str) -> str | None:
elif isinstance(object_type, TupleType):
type_name = tuple_fallback(object_type).type.fullname

if type_name is not None:
if type_name:
return f"{type_name}.{method_name}"
else:
return None
@@ -5489,7 +5489,7 @@ def type_info_from_type(typ: Type) -> TypeInfo | None:


def is_operator_method(fullname: str | None) -> bool:
if fullname is None:
if not fullname:
return False
short_name = fullname.split(".")[-1]
return (
58 changes: 33 additions & 25 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@
from mypy_extensions import trait

import mypy.strconv
from mypy.bogus_type import Bogus
from mypy.util import short_type
from mypy.visitor import ExpressionVisitor, NodeVisitor, StatementVisitor

@@ -247,12 +246,10 @@ class SymbolNode(Node):
def name(self) -> str:
pass

# fullname can often be None even though the type system
# disagrees. We mark this with Bogus to let mypyc know not to
# worry about it.
# Fully qualified name
@property
@abstractmethod
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
pass

@abstractmethod
@@ -294,7 +291,7 @@ class MypyFile(SymbolNode):
__match_args__ = ("name", "path", "defs")

# Fully qualified module name
_fullname: Bogus[str]
_fullname: str
# Path to the file (empty string if not known)
path: str
# Top-level definitions and statements
@@ -361,7 +358,7 @@ def name(self) -> str:
return "" if not self._fullname else self._fullname.split(".")[-1]

@property
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
return self._fullname

def accept(self, visitor: NodeVisitor[T]) -> T:
@@ -526,16 +523,15 @@ def __init__(self) -> None:
self.is_static = False
self.is_final = False
# Name with module prefix
# TODO: Type should be Optional[str]
self._fullname = cast(Bogus[str], None)
self._fullname = ""

@property
@abstractmethod
def name(self) -> str:
pass

@property
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
return self._fullname


@@ -871,7 +867,7 @@ def name(self) -> str:
return self.func.name

@property
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
return self.func.fullname

@property
@@ -967,7 +963,7 @@ def __init__(self, name: str, type: mypy.types.Type | None = None) -> None:
super().__init__()
self._name = name # Name without module prefix
# TODO: Should be Optional[str]
self._fullname = cast("Bogus[str]", None) # Name with module prefix
self._fullname = "" # Name with module prefix
# TODO: Should be Optional[TypeInfo]
self.info = VAR_NO_INFO
self.type: mypy.types.Type | None = type # Declared or inferred type, or None
@@ -1019,7 +1015,7 @@ def name(self) -> str:
return self._name

@property
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
return self._fullname

def accept(self, visitor: NodeVisitor[T]) -> T:
@@ -1057,7 +1053,7 @@ class ClassDef(Statement):

__slots__ = (
"name",
"fullname",
"_fullname",
"defs",
"type_vars",
"base_type_exprs",
@@ -1075,7 +1071,7 @@ class ClassDef(Statement):
__match_args__ = ("name", "defs")

name: str # Name of the class without module prefix
fullname: Bogus[str] # Fully qualified name of the class
_fullname: str # Fully qualified name of the class
defs: Block
type_vars: list[mypy.types.TypeVarLikeType]
# Base class expressions (not semantically analyzed -- can be arbitrary expressions)
@@ -1102,7 +1098,7 @@ def __init__(
) -> None:
super().__init__()
self.name = name
self.fullname = None # type: ignore[assignment]
self._fullname = ""
self.defs = defs
self.type_vars = type_vars or []
self.base_type_exprs = base_type_exprs or []
@@ -1117,6 +1113,14 @@ def __init__(
self.deco_line: int | None = None
self.removed_statements = []

@property
def fullname(self) -> str:
return self._fullname

@fullname.setter
def fullname(self, v: str) -> None:
self._fullname = v

def accept(self, visitor: StatementVisitor[T]) -> T:
return visitor.visit_class_def(self)

@@ -1725,7 +1729,7 @@ class RefExpr(Expression):
__slots__ = (
"kind",
"node",
"fullname",
"_fullname",
"is_new_def",
"is_inferred_def",
"is_alias_rvalue",
@@ -1739,7 +1743,7 @@ def __init__(self) -> None:
# Var, FuncDef or TypeInfo that describes this
self.node: SymbolNode | None = None
# Fully qualified name (or name if not global)
self.fullname: str | None = None
self._fullname = ""
# Does this define a new name?
self.is_new_def = False
# Does this define a new name with inferred type?
@@ -1752,6 +1756,14 @@ def __init__(self) -> None:
# Cache type guard from callable_type.type_guard
self.type_guard: mypy.types.Type | None = None

@property
def fullname(self) -> str:
return self._fullname

@fullname.setter
def fullname(self, v: str) -> None:
self._fullname = v


class NameExpr(RefExpr):
"""Name expression
@@ -2806,7 +2818,7 @@ class is generic then it will be a type constructor of higher kind.
"self_type",
)

_fullname: Bogus[str] # Fully qualified name
_fullname: str # Fully qualified name
# Fully qualified name for the module this type was defined in. This
# information is also in the fullname, but is harder to extract in the
# case of nested class definitions.
@@ -3023,7 +3035,7 @@ def name(self) -> str:
return self.defn.name

@property
def fullname(self) -> Bogus[str]:
def fullname(self) -> str:
return self._fullname

def is_generic(self) -> bool:
@@ -3739,11 +3751,7 @@ def serialize(self, prefix: str, name: str) -> JsonDict:
if prefix is not None:
fullname = self.node.fullname
if (
# See the comment above SymbolNode.fullname -- fullname can often be None,
# but for complex reasons it's annotated as being `Bogus[str]` instead of `str | None`,
# meaning mypy erroneously thinks the `fullname is not None` check here is redundant
fullname is not None # type: ignore[redundant-expr]
and "." in fullname
"." in fullname
and fullname != prefix + "." + name
and not (isinstance(self.node, Var) and self.node.from_module_getattr)
):
2 changes: 1 addition & 1 deletion mypy/partially_defined.py
Original file line number Diff line number Diff line change
@@ -287,7 +287,7 @@ def is_undefined(self, name: str) -> bool:


def refers_to_builtin(o: RefExpr) -> bool:
return o.fullname is not None and o.fullname.startswith("builtins.")
return o.fullname.startswith("builtins.")


class Loop:
12 changes: 6 additions & 6 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
@@ -3022,13 +3022,13 @@ def analyze_lvalues(self, s: AssignmentStmt) -> None:
def apply_dynamic_class_hook(self, s: AssignmentStmt) -> None:
if not isinstance(s.rvalue, CallExpr):
return
fname = None
fname = ""
call = s.rvalue
while True:
if isinstance(call.callee, RefExpr):
fname = call.callee.fullname
# check if method call
if fname is None and isinstance(call.callee, MemberExpr):
if not fname and isinstance(call.callee, MemberExpr):
callee_expr = call.callee.expr
if isinstance(callee_expr, RefExpr) and callee_expr.fullname:
method_name = call.callee.name
@@ -4624,7 +4624,7 @@ def bind_name_expr(self, expr: NameExpr, sym: SymbolTableNode) -> None:
else:
expr.kind = sym.kind
expr.node = sym.node
expr.fullname = sym.fullname
expr.fullname = sym.fullname or ""

def visit_super_expr(self, expr: SuperExpr) -> None:
if not self.type and not expr.call.args:
@@ -4849,7 +4849,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
self.process_placeholder(expr.name, "attribute", expr)
return
expr.kind = sym.kind
expr.fullname = sym.fullname
expr.fullname = sym.fullname or ""
expr.node = sym.node
elif isinstance(base, RefExpr):
# This branch handles the case C.bar (or cls.bar or self.bar inside
@@ -4881,7 +4881,7 @@ def visit_member_expr(self, expr: MemberExpr) -> None:
if not n:
return
expr.kind = n.kind
expr.fullname = n.fullname
expr.fullname = n.fullname or ""
expr.node = n.node

def visit_op_expr(self, expr: OpExpr) -> None:
@@ -5341,7 +5341,7 @@ def is_overloaded_item(self, node: SymbolNode, statement: Statement) -> bool:
return False

def is_defined_in_current_module(self, fullname: str | None) -> bool:
if fullname is None:
if not fullname:
return False
return module_prefix(self.modules, fullname) == self.cur_mod_id

2 changes: 1 addition & 1 deletion mypy/server/aststrip.py
Original file line number Diff line number Diff line change
@@ -230,7 +230,7 @@ def visit_op_expr(self, node: OpExpr) -> None:
def strip_ref_expr(self, node: RefExpr) -> None:
node.kind = None
node.node = None
node.fullname = None
node.fullname = ""
node.is_new_def = False
node.is_inferred_def = False

14 changes: 5 additions & 9 deletions mypy/server/deps.py
Original file line number Diff line number Diff line change
@@ -289,13 +289,9 @@ def visit_decorator(self, o: Decorator) -> None:
# all call sites, making them all `Any`.
for d in o.decorators:
tname: str | None = None
if isinstance(d, RefExpr) and d.fullname is not None:
if isinstance(d, RefExpr) and d.fullname:
tname = d.fullname
if (
isinstance(d, CallExpr)
and isinstance(d.callee, RefExpr)
and d.callee.fullname is not None
):
if isinstance(d, CallExpr) and isinstance(d.callee, RefExpr) and d.callee.fullname:
tname = d.callee.fullname
if tname is not None:
self.add_dependency(make_trigger(tname), make_trigger(o.func.fullname))
@@ -500,7 +496,7 @@ def visit_assignment_stmt(self, o: AssignmentStmt) -> None:
if (
isinstance(rvalue, CallExpr)
and isinstance(rvalue.callee, RefExpr)
and rvalue.callee.fullname is not None
and rvalue.callee.fullname
):
fname: str | None = None
if isinstance(rvalue.callee.node, TypeInfo):
@@ -510,7 +506,7 @@ def visit_assignment_stmt(self, o: AssignmentStmt) -> None:
fname = init.node.fullname
else:
fname = rvalue.callee.fullname
if fname is None:
if not fname:
return
for lv in o.lvalues:
if isinstance(lv, RefExpr) and lv.fullname and lv.is_new_def:
@@ -638,7 +634,7 @@ def visit_del_stmt(self, o: DelStmt) -> None:
# Expressions

def process_global_ref_expr(self, o: RefExpr) -> None:
if o.fullname is not None:
if o.fullname:
self.add_dependency(make_trigger(o.fullname))

# If this is a reference to a type, generate a dependency to its
2 changes: 1 addition & 1 deletion mypy/strconv.py
Original file line number Diff line number Diff line change
@@ -367,7 +367,7 @@ def pretty_name(
id = ""
if isinstance(target_node, mypy.nodes.MypyFile) and name == fullname:
n += id
elif kind == mypy.nodes.GDEF or (fullname != name and fullname is not None):
elif kind == mypy.nodes.GDEF or (fullname != name and fullname):
# Append fully qualified name for global references.
n += f" [{fullname}{id}]"
elif kind == mypy.nodes.LDEF:
2 changes: 1 addition & 1 deletion mypy/stubtest.py
Original file line number Diff line number Diff line change
@@ -1147,7 +1147,7 @@ def apply_decorator_to_funcitem(
) -> nodes.FuncItem | None:
if not isinstance(decorator, nodes.RefExpr):
return None
if decorator.fullname is None:
if not decorator.fullname:
# Happens with namedtuple
return None
if (
Loading

0 comments on commit 28e5436

Please sign in to comment.