Skip to content

Commit

Permalink
Avoid reporting custom class instances with repr
Browse files Browse the repository at this point in the history
  • Loading branch information
pschanely committed Oct 12, 2022
1 parent 46522a0 commit 98ec6da
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 62 deletions.
40 changes: 20 additions & 20 deletions crosshair/condition_parser.py
Expand Up @@ -55,10 +55,12 @@
from crosshair.util import (
DynamicScopeVar,
EvalFriendlyReprContext,
IdKeyedDict,
IgnoreAttempt,
UnexploredPath,
debug,
eval_friendly_repr,
format_boundargs,
frame_summary_for_fn,
is_pure_python,
sourcelines,
Expand Down Expand Up @@ -174,21 +176,11 @@ def default_counterexample(
fn_name: str,
bound_args: BoundArguments,
return_val: object,
repr_overrides: IdKeyedDict,
) -> Tuple[str, str]:
arg_strings = []
with EvalFriendlyReprContext() as ctx:
for (name, param) in bound_args.signature.parameters.items():
strval = repr(bound_args.arguments[name])
use_keyword = param.default is not Parameter.empty
if param.kind is Parameter.POSITIONAL_ONLY:
use_keyword = False
elif param.kind is Parameter.KEYWORD_ONLY:
use_keyword = True
if use_keyword:
arg_strings.append(f"{name} = {strval}")
else:
arg_strings.append(strval)
call_desc = f"{fn_name}({ctx.cleanup(', '.join(arg_strings))})"
with EvalFriendlyReprContext(repr_overrides) as ctx:
args_string = format_boundargs(bound_args)
call_desc = f"{fn_name}({ctx.cleanup(args_string)})"
return (call_desc, eval_friendly_repr(return_val))


Expand Down Expand Up @@ -270,7 +262,7 @@ class Conditions:
"""

counterexample_description_maker: Optional[
Callable[[BoundArguments, object], Tuple[str, str]]
Callable[[BoundArguments, object, IdKeyedDict], Tuple[str, str]]
] = None
"""
An optional callback that formats a counterexample invocation as text.
Expand All @@ -288,11 +280,15 @@ def syntax_messages(self) -> Iterator[ConditionSyntaxMessage]:
yield from self.fn_syntax_messages

def format_counterexample(
self, args: BoundArguments, return_val: object
self, args: BoundArguments, return_val: object, repr_overrides: IdKeyedDict
) -> Tuple[str, str]:
if self.counterexample_description_maker is not None:
return self.counterexample_description_maker(args, return_val)
return default_counterexample(self.src_fn.__name__, args, return_val)
return self.counterexample_description_maker(
args, return_val, repr_overrides
)
return default_counterexample(
self.src_fn.__name__, args, return_val, repr_overrides
)


@dataclass(frozen=True)
Expand Down Expand Up @@ -1119,13 +1115,17 @@ def _generate_args(self, payload: bytes, decorated_fn: Callable):
return ConjectureData.for_buffer(payload).draw(strategy)

def _format_counterexample(
self, fn: Callable, args: BoundArguments, return_val: object
self,
fn: Callable,
args: BoundArguments,
return_val: object,
repr_overrides: IdKeyedDict,
) -> Tuple[str, str]:
payload_bytes = args.arguments["payload"]
kwargs = self._generate_args(payload_bytes, fn)
sig = inspect.signature(fn.hypothesis.inner_test) # type: ignore
real_args = sig.bind(**kwargs)
return default_counterexample(fn.__name__, real_args, None)
return default_counterexample(fn.__name__, real_args, None, repr_overrides)

def get_fn_conditions(self, ctxfn: FunctionInfo) -> Optional[Conditions]:
fn_and_sig = ctxfn.get_callable()
Expand Down
7 changes: 5 additions & 2 deletions crosshair/condition_parser_test.py
Expand Up @@ -502,7 +502,10 @@ def foo(a=10, /, b=20):
foo = ns["foo"]
args = inspect.BoundArguments(inspect.signature(foo), {"a": 1, "b": 2})
conditions = Pep316Parser().get_fn_conditions(FunctionInfo.from_fn(foo))
assert conditions.format_counterexample(args, None) == ("foo(1, b = 2)", "None")
assert conditions.format_counterexample(args, None, {}) == (
"foo(1, b = 2)",
"None",
)


def test_format_counterexample_keyword_only():
Expand All @@ -511,7 +514,7 @@ def foo(a, *, b):

args = inspect.BoundArguments(inspect.signature(foo), {"a": 1, "b": 2})
conditions = Pep316Parser().get_fn_conditions(FunctionInfo.from_fn(foo))
assert conditions.format_counterexample(args, None) == ("foo(1, b = 2)", "None")
assert conditions.format_counterexample(args, None, {}) == ("foo(1, b = 2)", "None")


@pytest.mark.skipif(not hypothesis, reason="hypothesis is not installed")
Expand Down
50 changes: 42 additions & 8 deletions crosshair/core.py
Expand Up @@ -101,10 +101,12 @@
AttributeHolder,
CrosshairInternal,
CrosshairUnsupported,
IdKeyedDict,
IgnoreAttempt,
UnexploredPath,
debug,
eval_friendly_repr,
format_boundargs,
frame_summary_for_fn,
name_of_type,
samefile,
Expand Down Expand Up @@ -420,6 +422,7 @@ def proxy_for_class(typ: Type, varname: str) -> object:
f"unable to create concrete instance of {typ} due to bad constructor"
)
args = gen_args(constructor_sig)
typename = name_of_type(typ)
try:
with ResumedTracing():
obj = WithEnforcement(typ)(*args.args, **args.kwargs)
Expand All @@ -430,10 +433,13 @@ def proxy_for_class(typ: Type, varname: str) -> object:
except BaseException as e:
debug("Root-cause type construction traceback:", test_stack(e.__traceback__))
raise CrosshairUnsupported(
f"error constructing {name_of_type(typ)} instance: {name_of_type(type(e))}: {e}",
f"error constructing {typename} instance: {name_of_type(type(e))}: {e}",
) from e

debug("Proxy as a concrete instance of", name_of_type(typ))
debug("Proxy as a concrete instance of", typename)
repr_overrides = context_statespace().extra(LazyCreationRepr).reprs
repr_overrides[obj] = lambda _: f"{typename}({realize(format_boundargs(args))})"
debug("repr register lazy", hex(id(obj)), typename)
return obj


Expand Down Expand Up @@ -524,6 +530,12 @@ def register_type(typ: Type, creator: SymbolicCreationCallback) -> None:
_SIMPLE_PROXIES[typ] = creator


@dataclass
class LazyCreationRepr:
def __init__(self, *a):
self.reprs = IdKeyedDict()


@overload
def proxy_for_type(
typ: Callable[..., _T],
Expand Down Expand Up @@ -609,7 +621,13 @@ def proxy_maker(typ):
value = proxy_for_type(param.annotation, smt_name, allow_subtypes)
else:
value = proxy_for_type(cast(type, Any), smt_name, allow_subtypes)
debug("created proxy for", param.name, "as type:", name_of_type(type(value)))
debug(
"created proxy for",
param.name,
"as type:",
name_of_type(type(value)),
hex(id(value)),
)
args.arguments[param.name] = value
return args

Expand Down Expand Up @@ -1208,10 +1226,23 @@ def make(
def make_counterexample_message(
conditions: Conditions, args: BoundArguments, return_val: object = None
) -> str:
args = deep_realize(args)
repr_overrides = context_statespace().extra(LazyCreationRepr).reprs

with NoTracing():
arg_memo: dict = {}
args = deepcopyext(args, CopyMode.REALIZE, arg_memo)
for orig_id, new_obj in arg_memo.items():
old_repr = repr_overrides.inner.get(orig_id, None)
if old_repr:
repr_overrides.inner[id(new_obj)] = old_repr

return_val = deep_realize(return_val)

invocation, retstring = conditions.format_counterexample(
args, return_val, repr_overrides
)
with NoTracing():
invocation, retstring = conditions.format_counterexample(args, return_val)

patch_expr = context_statespace().extra(FunctionInterps).patch_string()
if patch_expr:
invocation += f" with {patch_expr}"
Expand All @@ -1231,9 +1262,9 @@ def attempt_call(
space = context_statespace()
msg_gen = MessageGenerator(conditions.src_fn)
with enforced_conditions.enabled_enforcement(), NoTracing():
bound_args = gen_args(conditions.sig)
original_args = deepcopyext(bound_args, CopyMode.BEST_EFFORT, {})
space.checkpoint()
original_args = gen_args(conditions.sig)
space.checkpoint()
bound_args = deepcopyext(original_args, CopyMode.BEST_EFFORT, {})

lcls: Mapping[str, object] = bound_args.arguments
# In preconditions, __old__ exists but is just bound to the same args.
Expand Down Expand Up @@ -1364,6 +1395,9 @@ def attempt_call(
return CallAnalysis(VerificationStatus.CONFIRMED)
else:
space.detach_path()
debug(
"AOUT TO DO THINGS", [hex(id(v)) for v in original_args.arguments.values()]
)
detail = "false " + make_counterexample_message(
conditions, original_args, __return__
)
Expand Down
24 changes: 15 additions & 9 deletions crosshair/core_test.py
Expand Up @@ -103,9 +103,6 @@ def __init__(self) -> None:
def weakenedfunc(self, x: int) -> None:
pass

def __repr__(self) -> str:
return "instance of A"

@icontract.invariant(lambda self: self.x < 100)
class IcontractB(IcontractA):
def break_parent_invariant(self):
Expand All @@ -118,9 +115,6 @@ def break_my_invariant(self):
def weakenedfunc(self, x: int) -> None:
pass

def __repr__(self) -> str:
return f"instance of B({self.x})"


class ShippingContainer:
container_weight = 4
Expand Down Expand Up @@ -618,6 +612,18 @@ def f(foo: Cat) -> int:
type_repo._add_class(BiggerCat)
check_states(f, POST_FAIL)

def test_does_not_report_with_actual_repr(self):
def f(foo: BiggerCat) -> int:
"""post: False"""
return foo.size()

(actual, expected) = check_messages(
analyze_function(f),
state=MessageType.POST_FAIL,
message="false when calling f(BiggerCat()) " "(which returns 2)",
)
assert expected == actual

def test_check_parent_conditions(self):
# Ensure that conditions of parent classes are checked in children
# even when not overridden.
Expand Down Expand Up @@ -905,7 +911,7 @@ def f(foo: List[Pokeable]) -> None:
*check_messages(
analyze_function(f),
state=MessageType.POST_FAIL,
message="false when calling f([Pokeable(x=10)])",
message="false when calling f([Pokeable(x = 10)])",
)
)

Expand Down Expand Up @@ -1063,13 +1069,13 @@ def test_icontract_class(self):
MessageType.POST_FAIL,
line_gt0,
'"@icontract.invariant(lambda self: self.x > 0)" yields false '
"when calling break_parent_invariant(instance of B(10))",
"when calling break_parent_invariant(IcontractB())",
),
(
MessageType.POST_FAIL,
line_lt100,
'"@icontract.invariant(lambda self: self.x < 100)" yields false '
"when calling break_my_invariant(instance of B(10))",
"when calling break_my_invariant(IcontractB())",
),
},
)
Expand Down
11 changes: 4 additions & 7 deletions crosshair/simplestructs.py
Expand Up @@ -52,6 +52,10 @@ def copy(self):
def __ch_pytype__(self):
return dict

def __repr__(self):
contents = ", ".join(f"{repr(k)}: {repr(v)}" for (k, v) in self.items())
return "{" + contents + "}"

if sys.version_info >= (3, 9):

def __or__(self, other: Mapping) -> Mapping:
Expand Down Expand Up @@ -135,9 +139,6 @@ def __bool__(self):
def __len__(self):
return self.contents_.__len__()

def __repr__(self):
return repr(dict(self.items()))

def items(self):
return self.contents_

Expand Down Expand Up @@ -234,10 +235,6 @@ def __delitem__(self, key):
self._mutations[key] = _DELETED
self._len -= 1

def __repr__(self):
contents = ", ".join(f"{repr(k)}: {repr(v)}" for (k, v) in self.items())
return "{" + contents + "}"

def _lastitem(self):
raise KeyError

Expand Down
17 changes: 16 additions & 1 deletion crosshair/util.py
Expand Up @@ -16,7 +16,6 @@
import time
import traceback
import types
import typing
from types import BuiltinFunctionType, FunctionType, MethodDescriptorType, TracebackType
from typing import (
Any,
Expand Down Expand Up @@ -366,6 +365,22 @@ def import_alternative(name: str, suppress: Tuple[str, ...] = ()):
pass


def format_boundargs(bound_args: inspect.BoundArguments) -> str:
arg_strings = []
for (name, param) in bound_args.signature.parameters.items():
strval = repr(bound_args.arguments[name])
use_keyword = param.default is not inspect.Parameter.empty
if param.kind is inspect.Parameter.POSITIONAL_ONLY:
use_keyword = False
elif param.kind is inspect.Parameter.KEYWORD_ONLY:
use_keyword = True
if use_keyword:
arg_strings.append(f"{name} = {strval}")
else:
arg_strings.append(strval)
return ", ".join(arg_strings)


UNABLE_TO_REPR_TEXT = "<unable to repr>"


Expand Down
27 changes: 12 additions & 15 deletions doc/source/hints_for_your_classes.rst
Expand Up @@ -4,18 +4,15 @@ Hints for Your Classes
**********************

CrossHair can reason about classes that you write, and may likely do the right
thing out of the box. To be sure it does, though, follow the following guidlines for
writing classes.
thing out of the box.

1. **Type your __init__ arguments.** When needed, CrossHair will attempt to
construct your class with symbolic arguments. To do this, it looks up types on the
``__init__`` arguments.
2. **Ensure your instances return good repr() strings.** You can do this by defining
``__repr__``. CrossHair uses ``repr()`` to desribe your instances in counterexamples,
so you'll want that to return useful information.
To be sure it does, though, **add types for your __init__() arguments**.
When needed, CrossHair will attempt to construct your class with symbolic arguments.
To do this, it looks up types on the parameters to the ``__init__`` method.

One of the easiest way to do **both** of these is to use `Python's dataclass module`_.
CrossHair will work out-of-the-box with definitions like this::
If you use `Python's dataclass module`_, your generated ``__init__`` arguments will be
already typed.
That means CrossHair will also work out-of-the-box with definitions like this::

import dataclasses
@dataclasses.dataclass
Expand All @@ -27,11 +24,6 @@ Customizing Creation
====================


.. note::
This capabliity and the interface to it is under active development and unstable.
That said, if you are willing to try it out, please ask questions and let us know
how it goes.

There are a variety of times where you may need to customize the way CrossHair
constructs instances of your class.
For example,
Expand All @@ -42,6 +34,11 @@ For example,
* The class has valid/reachable states that aren't directly constructable.
* The type is implemented in C.

.. note::
This capabliity and the interface to it is under active development and unstable.
That said, if you are willing to try it out, please ask questions and let us know
how it goes.

In such cases, you may register a creation callback with
:func:`crosshair.register_type`.
Your creation callback will be given a :class:`crosshair.SymbolicFactory` instance that
Expand Down

0 comments on commit 98ec6da

Please sign in to comment.