Repository: quora/pyanalyze
Pull request #486 (2022-03-02 22:59:14)

Add hasattr_static


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 062d355..0bd0911 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Add mechanism to prevent crashes on objects
+  with unusual `__getattr__` methods (#486)
 - Infer callable signatures for objects with a
   `__getattr__` method (#485)
 - Do not treat attributes that raise an exception on access
diff --git a/pyanalyze/arg_spec.py b/pyanalyze/arg_spec.py
index 2b0f850..5f46c52 100644
--- a/pyanalyze/arg_spec.py
+++ b/pyanalyze/arg_spec.py
@@ -12,9 +12,9 @@ from .find_unused import used
 from . import implementation
 from .safe import (
     all_of_type,
+    hasattr_static,
     is_newtype,
     safe_equals,
-    safe_hasattr,
     safe_issubclass,
     is_typing_name,
     safe_isinstance,
@@ -327,7 +327,7 @@ class ArgSpecCache:
         # because @functools.wraps copies the __annotations__ of the wrapped function. We
         # don't want that, because the wrapper may have changed the return type.
         # This caused problems with @contextlib.contextmanager.
-        is_wrapped = safe_hasattr(function_object, "__wrapped__")
+        is_wrapped = hasattr_static(function_object, "__wrapped__")
 
         if returns is not None:
             has_return_annotation = True
@@ -608,8 +608,8 @@ class ArgSpecCache:
             )
             return make_bound_method(argspec, Composite(KnownValue(obj.__self__)))
 
-        if hasattr(obj, "fn") or hasattr(obj, "original_fn"):
-            is_asynq = is_asynq or hasattr(obj, "asynq")
+        if hasattr_static(obj, "fn") or hasattr_static(obj, "original_fn"):
+            is_asynq = is_asynq or hasattr_static(obj, "asynq")
             # many decorators put the original function in the .fn attribute
             try:
                 original_fn = qcore.get_original_fn(obj)
@@ -662,7 +662,7 @@ class ArgSpecCache:
             )
 
         if inspect.isfunction(obj):
-            if hasattr(obj, "inner"):
+            if hasattr_static(obj, "inner"):
                 # @qclient.task_queue.exec_after_request() puts the original function in .inner
                 return self._cached_get_argspec(
                     obj.inner, impl, is_asynq, in_overload_resolution
@@ -759,7 +759,7 @@ class ArgSpecCache:
                 return self.from_signature(inspect_sig, function_object=obj)
             return self._make_any_sig(obj)
 
-        if hasattr(obj, "__call__"):
+        if hasattr_static(obj, "__call__"):
             # we could get an argspec here in some cases, but it's impossible to figure out
             # the argspec for some builtin methods (e.g., dict.__init__), and no way to detect
             # these with inspect, so just give up.
@@ -907,9 +907,9 @@ class ArgSpecCache:
 def _is_qcore_decorator(obj: object) -> TypeGuard[Any]:
     try:
         return (
-            hasattr(obj, "is_decorator")
+            hasattr_static(obj, "is_decorator")
             and obj.is_decorator()
-            and hasattr(obj, "decorator")
+            and hasattr_static(obj, "decorator")
         )
     except Exception:
         # black.Line has an is_decorator attribute but it is not a method
@@ -917,7 +917,7 @@ def _is_qcore_decorator(obj: object) -> TypeGuard[Any]:
 
 
 def _get_class_name(obj: object) -> Optional[str]:
-    if hasattr(obj, "__qualname__"):
+    if hasattr_static(obj, "__qualname__"):
         pieces = obj.__qualname__.split(".")
         if len(pieces) >= 2:
             return pieces[-2]
diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py
index d1b2d44..ee3b8d3 100644
--- a/pyanalyze/implementation.py
+++ b/pyanalyze/implementation.py
@@ -3,7 +3,7 @@ from .error_code import ErrorCode
 from .extensions import assert_type, reveal_locals, reveal_type
 from .format_strings import parse_format_string
 from .predicates import IsAssignablePredicate
-from .safe import safe_hasattr, safe_isinstance, safe_issubclass
+from .safe import hasattr_static, safe_isinstance, safe_issubclass
 from .stacked_scopes import (
     NULL_CONSTRAINT,
     AbstractConstraint,
@@ -215,7 +215,7 @@ def _hasattr_impl(ctx: CallContext) -> Value:
         name.val, ctx.visitor
     ):
         return_value = KnownValue(True)
-    elif isinstance(obj, KnownValue) and safe_hasattr(obj.val, name.val):
+    elif isinstance(obj, KnownValue) and hasattr_static(obj.val, name.val):
         return_value = KnownValue(True)
     else:
         return_value = TypedValue(bool)
@@ -1349,6 +1349,14 @@ def get_default_argspecs() -> Dict[object, Signature]:
             impl=_hasattr_impl,
             callable=hasattr,
         ),
+        Signature.make(
+            [
+                SigParameter("object", _POS_ONLY),
+                SigParameter("name", _POS_ONLY, annotation=TypedValue(str)),
+            ],
+            impl=_hasattr_impl,
+            callable=hasattr_static,
+        ),
         Signature.make(
             [
                 SigParameter("object", _POS_ONLY),
diff --git a/pyanalyze/safe.py b/pyanalyze/safe.py
index 735f16f..df1c504 100644
--- a/pyanalyze/safe.py
+++ b/pyanalyze/safe.py
@@ -28,6 +28,16 @@ except ImportError:
 T = TypeVar("T")
 
 
+def hasattr_static(object: object, name: str) -> bool:
+    """Similar to ``inspect.getattr_static()``."""
+    try:
+        inspect.getattr_static(object, name)
+    except AttributeError:
+        return False
+    else:
+        return True
+
+
 def safe_hasattr(item: object, member: str) -> bool:
     """Safe version of ``hasattr()``."""
     try:
diff --git a/pyanalyze/typeshed.py b/pyanalyze/typeshed.py
index db781c0..20c5d5d 100644
--- a/pyanalyze/typeshed.py
+++ b/pyanalyze/typeshed.py
@@ -18,7 +18,7 @@ from .annotations import (
 )
 from .error_code import ErrorCode
 from .extensions import overload, real_overload
-from .safe import all_of_type, is_typing_name, safe_isinstance
+from .safe import all_of_type, hasattr_static, is_typing_name, safe_isinstance
 from .stacked_scopes import uniq_chain, Composite
 from .signature import (
     ConcreteSignature,
@@ -196,7 +196,7 @@ class TypeshedFinder:
             return self.get_argspec_for_fully_qualified_name(
                 obj, obj, type_params=type_params
             )
-        if inspect.ismethoddescriptor(obj) and hasattr(obj, "__objclass__"):
+        if inspect.ismethoddescriptor(obj) and hasattr_static(obj, "__objclass__"):
             objclass = obj.__objclass__
             fq_name = self._get_fq_name(objclass)
             if fq_name is None:
@@ -211,9 +211,10 @@ class TypeshedFinder:
             self.log("Ignoring method", obj)
             return None
         if (
-            hasattr(obj, "__qualname__")
-            and hasattr(obj, "__name__")
-            and hasattr(obj, "__module__")
+            hasattr_static(obj, "__qualname__")
+            and hasattr_static(obj, "__name__")
+            and hasattr_static(obj, "__module__")
+            and isinstance(obj.__qualname__, str)
             and obj.__qualname__ != obj.__name__
             and "." in obj.__qualname__
         ):


Test patch:
diff --git a/pyanalyze/test_arg_spec.py b/pyanalyze/test_arg_spec.py
index 114006e..0835d62 100644
--- a/pyanalyze/test_arg_spec.py
+++ b/pyanalyze/test_arg_spec.py
@@ -1,7 +1,8 @@
 # static analysis: ignore
+from dataclasses import dataclass
 from asynq import asynq
 import functools
-from typing import TypeVar, NewType
+from typing import List, TypeVar, NewType
 
 from .checker import Checker
 from .test_name_check_visitor import (
@@ -89,6 +90,14 @@ def decorator(fn):
     return wrapper
 
 
+@dataclass
+class AllTheAttrs:
+    x: List[str]
+
+    def __getattr__(self, attr: str) -> "AllTheAttrs":
+        return AllTheAttrs([*self.x, attr])
+
+
 def test_get_argspec():
     checker = Checker()
     visitor = ConfiguredNameCheckVisitor(
@@ -106,6 +115,9 @@ def test_get_argspec():
             [SigParameter("arg")], callable=ClassWithCall.__call__
         ) == visitor.signature_from_value(cwc_typed)
 
+        ata = AllTheAttrs([])
+        assert asc.get_argspec(ata) is None
+
         assert BoundMethodSignature(
             Signature.make(
                 [SigParameter("cls")],