From 2b09df9b28dffba6ee9efb13adff73ecc2fc722d Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Thu, 25 Aug 2022 22:13:58 -0700 Subject: [PATCH 1/6] Introduce ownership --- pyanalyze/error_code.py | 2 ++ pyanalyze/extensions.py | 22 ++++++++++++++ pyanalyze/implementation.py | 50 +++++++++++++++++++++++--------- pyanalyze/name_check_visitor.py | 20 +++++++------ pyanalyze/signature.py | 51 +++++++++++++++++++++------------ pyanalyze/value.py | 16 ++++++++++- 6 files changed, 119 insertions(+), 42 deletions(-) diff --git a/pyanalyze/error_code.py b/pyanalyze/error_code.py index 34825365..55efffd3 100644 --- a/pyanalyze/error_code.py +++ b/pyanalyze/error_code.py @@ -99,6 +99,7 @@ class ErrorCode(enum.Enum): invalid_annotated_assignment = 79 unused_assignment = 80 incompatible_yield = 81 + disallowed_mutation = 82 # Allow testing unannotated functions without too much fuss @@ -219,6 +220,7 @@ class ErrorCode(enum.Enum): ErrorCode.invalid_annotated_assignment: "Invalid annotated assignment", ErrorCode.unused_assignment: "Assigned value is never used", ErrorCode.incompatible_yield: "Incompatible yield type", + ErrorCode.disallowed_mutation: "Mutation of unowned object", } diff --git a/pyanalyze/extensions.py b/pyanalyze/extensions.py index 8b853d28..8bd777c1 100644 --- a/pyanalyze/extensions.py +++ b/pyanalyze/extensions.py @@ -146,6 +146,28 @@ def _is_disallowed(self, value: "Value") -> bool: ) +@dataclass(frozen=True) +class Owned(CustomCheck): + """Custom check that indicates that a mutable value is mutated. For example, + a function that mutates a list should accept an argument of type + ``Annotated[List[T], Owned()]``. + """ + + def can_assign(self, value: "Value", ctx: "CanAssignContext") -> "CanAssign": + for val in pyanalyze.value.flatten_values(value, unwrap_annotated=False): + if isinstance(val, pyanalyze.value.AnnotatedValue): + if any(val.get_custom_check_of_type(Owned)): + continue + val = val.value + if isinstance(val, pyanalyze.value.AnyValue): + continue + return pyanalyze.value.CanAssignError( + f"Value {val} is not owned and may not be mutated", + error_code=pyanalyze.error_code.ErrorCode.disallowed_mutation, + ) + return {} + + class _AsynqCallableMeta(type): def __getitem__( self, params: Tuple[Union[Literal[Ellipsis], List[object]], object] diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py index dfb11609..0633e443 100644 --- a/pyanalyze/implementation.py +++ b/pyanalyze/implementation.py @@ -52,6 +52,7 @@ KnownValue, kv_pairs_from_mapping, KVPair, + make_owned, MultiValuedValue, NO_RETURN_VALUE, ParameterTypeGuardExtension, @@ -326,8 +327,11 @@ def _set_impl(ctx: CallContext) -> ImplReturn: def _sequence_impl(typ: type, ctx: CallContext) -> ImplReturn: iterable = ctx.vars["iterable"] + maybe_owned: Callable[[Value], Value] = ( + (lambda x: x) if typ is tuple else make_owned + ) if iterable is _NO_ARG_SENTINEL: - return ImplReturn(KnownValue(typ())) + return ImplReturn(maybe_owned(KnownValue(typ()))) def inner(iterable: Value) -> Value: cvi = concrete_values_from_iterable(iterable, ctx.visitor) @@ -338,12 +342,14 @@ def inner(iterable: Value) -> Value: arg="iterable", detail=str(cvi), ) - return TypedValue(typ) + return maybe_owned(TypedValue(typ)) elif isinstance(cvi, Value): - return GenericValue(typ, [cvi]) + return maybe_owned(GenericValue(typ, [cvi])) else: # TODO: Consider changing concrete_values_from_iterable to preserve unpacked bits - return SequenceValue.make_or_known(typ, [(False, elt) for elt in cvi]) + return maybe_owned( + SequenceValue.make_or_known(typ, [(False, elt) for elt in cvi]) + ) return flatten_unions(inner, iterable) @@ -1468,7 +1474,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(list)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + ), SigParameter("object", _POS_ONLY), ], callable=list.append, @@ -1484,7 +1492,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(list)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + ), SigParameter( "x", _POS_ONLY, annotation=TypedValue(collections.abc.Iterable) ), @@ -1494,7 +1504,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(list)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + ), SigParameter( "iterable", _POS_ONLY, @@ -1522,7 +1534,7 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(set)), + SigParameter("self", _POS_ONLY, annotation=make_owned(TypedValue(set))), SigParameter("object", _POS_ONLY), ], callable=set.add, @@ -1530,7 +1542,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(dict)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + ), SigParameter("k", _POS_ONLY), SigParameter("v", _POS_ONLY), ], @@ -1556,7 +1570,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(dict)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + ), SigParameter("key", _POS_ONLY), SigParameter("default", _POS_ONLY, default=KnownValue(None)), ], @@ -1565,7 +1581,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(dict)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + ), SigParameter("key", _POS_ONLY), SigParameter("default", _POS_ONLY, default=_NO_ARG_SENTINEL), ], @@ -1574,7 +1592,9 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=TypedValue(dict)), + SigParameter( + "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + ), SigParameter("m", _POS_ONLY, default=_NO_ARG_SENTINEL), SigParameter("kwargs", ParameterKind.VAR_KEYWORD), ], @@ -1590,8 +1610,10 @@ def get_default_argspecs() -> Dict[object, Signature]: annotation=GenericValue(dict, [TypeVarValue(K), TypeVarValue(V)]), ) ], - DictIncompleteValue( - dict, [KVPair(TypeVarValue(K), TypeVarValue(V), is_many=True)] + make_owned( + DictIncompleteValue( + dict, [KVPair(TypeVarValue(K), TypeVarValue(V), is_many=True)] + ) ), callable=dict.copy, ), diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py index b8766541..70151d38 100644 --- a/pyanalyze/name_check_visitor.py +++ b/pyanalyze/name_check_visitor.py @@ -177,6 +177,7 @@ NoReturnConstraintExtension, ReferencingValue, SequenceValue, + make_owned, set_self, SubclassValue, TypedValue, @@ -2395,13 +2396,13 @@ def _handle_imports( # Comprehensions def visit_DictComp(self, node: ast.DictComp) -> Value: - return self._visit_sequence_comp(node, dict) + return make_owned(self._visit_sequence_comp(node, dict)) def visit_ListComp(self, node: ast.ListComp) -> Value: - return self._visit_sequence_comp(node, list) + return make_owned(self._visit_sequence_comp(node, list)) def visit_SetComp(self, node: ast.SetComp) -> Value: - return self._visit_sequence_comp(node, set) + return make_owned(self._visit_sequence_comp(node, set)) def visit_GeneratorExp(self, node: ast.GeneratorExp) -> Value: return self._visit_sequence_comp(node, types.GeneratorType) @@ -2655,7 +2656,7 @@ def visit_Dict(self, node: ast.Dict) -> Value: ErrorCode.unsupported_operation, detail=str(new_pairs), ) - return TypedValue(dict) + return make_owned(TypedValue(dict)) all_pairs += new_pairs continue key_val = self.visit(key_node) @@ -2695,15 +2696,18 @@ def visit_Dict(self, node: ast.Dict) -> Value: ret[key] = value if has_non_literal: - return DictIncompleteValue(dict, all_pairs) + return make_owned(DictIncompleteValue(dict, all_pairs)) else: - return KnownValue(ret) + return make_owned(KnownValue(ret)) def visit_Set(self, node: ast.Set) -> Value: - return self._visit_display_read(node, set) + return make_owned(self._visit_display_read(node, set)) def visit_List(self, node: ast.List) -> Optional[Value]: - return self._visit_display(node, list) + val = self._visit_display(node, list) + if val is not None: + return make_owned(val) + return None def visit_Tuple(self, node: ast.Tuple) -> Optional[Value]: return self._visit_display(node, tuple) diff --git a/pyanalyze/signature.py b/pyanalyze/signature.py index 84af5bd9..a064e588 100644 --- a/pyanalyze/signature.py +++ b/pyanalyze/signature.py @@ -638,7 +638,8 @@ def _check_param_type_compatibility( ctx.on_error( f"Incompatible argument type for {param.name}: expected" f" {param_typ} but got {composite.value}", - code=ErrorCode.incompatible_argument, + code=bounds_map.get_error_code() + or ErrorCode.incompatible_argument, node=composite.node if composite.node is not None else None, detail=str(bounds_map), ) @@ -1253,41 +1254,45 @@ def _maybe_perform_call( args = [] kwargs = {} for definitely_present, composite in actual_args.positionals: - if definitely_present and isinstance(composite.value, KnownValue): - args.append(composite.value.val) - else: + if not definitely_present: + return None + arg = _extract_known_value(composite.value) + if arg is None: return None + args.append(arg.val) if actual_args.star_args is not None: values = concrete_values_from_iterable( actual_args.star_args, ctx.can_assign_ctx ) - if isinstance(values, collections.abc.Sequence) and all_of_type( - values, KnownValue - ): - args += [val.val for val in values] - else: + if not isinstance(values, collections.abc.Sequence): return None + for args_val in values: + arg = _extract_known_value(args_val) + if arg is None: + return None + args.append(arg.val) for kwarg, (required, composite) in actual_args.keywords.items(): if not required: return None - if isinstance(composite.value, KnownValue): - kwargs[kwarg] = composite.value.val - else: + kwarg_value = _extract_known_value(composite.value) + if kwarg_value is None: return None + kwargs[kwarg] = kwarg_value if actual_args.star_kwargs is not None: value = replace_known_sequence_value(actual_args.star_kwargs) if isinstance(value, DictIncompleteValue): for pair in value.kv_pairs: + if pair.is_many or not pair.is_required: + return None + key_val = _extract_known_value(pair.key) + value_val = _extract_known_value(pair.value) if ( - pair.is_required - and not pair.is_many - and isinstance(pair.key, KnownValue) - and isinstance(pair.key.val, str) - and isinstance(pair.value, KnownValue) + key_val is None + or value_val is None + or not isinstance(key_val.val, str) ): - kwargs[pair.key.val] = pair.value.val - else: return None + kwargs[key_val.val] = value_val.val else: return None @@ -2517,3 +2522,11 @@ def decompose_union( ), f"all union members matched between {expected_type} and {parent_value}" return bounds_map, union_used_any, unite_values(*remaining_values) return None + + +def _extract_known_value(val: Value) -> Optional[KnownValue]: + if isinstance(val, AnnotatedValue): + val = val.value + if isinstance(val, KnownValue): + return val + return None diff --git a/pyanalyze/value.py b/pyanalyze/value.py index 55afecc9..82bcdd74 100644 --- a/pyanalyze/value.py +++ b/pyanalyze/value.py @@ -50,7 +50,8 @@ def function(x: int, y: list[int], z: Any): from typing_extensions import Literal, ParamSpec, Protocol import pyanalyze -from pyanalyze.extensions import CustomCheck +from pyanalyze.error_code import ErrorCode +from pyanalyze.extensions import CustomCheck, Owned from .safe import all_of_type, safe_equals, safe_isinstance, safe_issubclass @@ -269,6 +270,7 @@ class CanAssignError: message: str = "" children: List["CanAssignError"] = field(default_factory=list) + error_code: Optional[ErrorCode] = None def display(self, depth: int = 2) -> str: """Display all errors in a human-readable format.""" @@ -281,6 +283,14 @@ def display(self, depth: int = 2) -> str: else: return child_result + def get_error_code(self) -> Optional[ErrorCode]: + errors = {child.get_error_code() for child in self.children} + if self.error_code: + errors.add(self.error_code) + if len(errors) == 1: + return next(iter(errors)) + return None + def __str__(self) -> str: return self.display() @@ -2637,3 +2647,7 @@ def can_assign_and_used_any( def is_overlapping(left: Value, right: Value, ctx: CanAssignContext) -> bool: return left.is_assignable(right, ctx) or right.is_assignable(left, ctx) + + +def make_owned(typ: Value) -> Value: + return AnnotatedValue(typ, [CustomCheckExtension(Owned())]) From 954e0390755385ed6820ec39d26e6d42020dda48 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Thu, 1 Sep 2022 19:57:17 -0700 Subject: [PATCH 2/6] Add ownership to self and its attributes --- pyanalyze/attributes.py | 9 ++++++++- pyanalyze/functions.py | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/pyanalyze/attributes.py b/pyanalyze/attributes.py index eb23a444..6794e107 100644 --- a/pyanalyze/attributes.py +++ b/pyanalyze/attributes.py @@ -15,6 +15,7 @@ import qcore from .annotations import Context, type_from_annotations, type_from_runtime +from .extensions import Owned from .options import Options, PyObjectSequenceOption from .safe import safe_isinstance, safe_issubclass from .signature import MaybeSignature @@ -30,6 +31,7 @@ KnownValue, KnownValueWithTypeVars, MultiValuedValue, + make_owned, set_self, SubclassValue, TypedValue, @@ -89,9 +91,11 @@ def get_generic_bases( def get_attribute(ctx: AttrContext) -> Value: root_value = ctx.root_value + should_own = False if isinstance(root_value, TypeVarValue): root_value = root_value.get_fallback_value() elif isinstance(root_value, AnnotatedValue): + should_own = any(root_value.get_custom_check_of_type(Owned)) root_value = root_value.value if isinstance(root_value, KnownValue): attribute_value = _get_attribute_from_known(root_value.val, ctx) @@ -137,7 +141,10 @@ def get_attribute(ctx: AttrContext) -> Value: ) and isinstance(ctx.root_value, AnnotatedValue): for guard in ctx.root_value.get_metadata_of_type(HasAttrExtension): if guard.attribute_name == KnownValue(ctx.attr): - return guard.attribute_type + attribute_value = guard.attribute_type + break + if should_own: + attribute_value = make_owned(attribute_value) return attribute_value diff --git a/pyanalyze/functions.py b/pyanalyze/functions.py index 346fadd7..ae94ed38 100644 --- a/pyanalyze/functions.py +++ b/pyanalyze/functions.py @@ -36,6 +36,7 @@ SubclassValue, TypedValue, TypeVarValue, + make_owned, unite_values, UnpackedValue, Value, @@ -340,6 +341,7 @@ def compute_parameters( else: # normal method value = enclosing_class + value = make_owned(value) else: # This is meant to exclude methods in nested classes. It's a bit too # conservative for cases such as a function nested in a method nested in a From 10b7e553d7e499db2fb0d53ec3a2894238798e81 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Thu, 1 Sep 2022 20:32:55 -0700 Subject: [PATCH 3/6] many fixes --- pyanalyze/attributes.py | 2 +- pyanalyze/implementation.py | 63 +++++++++++++++++++++++++-------- pyanalyze/name_check_visitor.py | 28 +++++++++------ pyanalyze/value.py | 4 +++ 4 files changed, 71 insertions(+), 26 deletions(-) diff --git a/pyanalyze/attributes.py b/pyanalyze/attributes.py index dc75a516..490c87f3 100644 --- a/pyanalyze/attributes.py +++ b/pyanalyze/attributes.py @@ -143,7 +143,7 @@ def get_attribute(ctx: AttrContext) -> Value: if guard.attribute_name == KnownValue(ctx.attr): attribute_value = guard.attribute_type break - if should_own: + if should_own and attribute_value is not UNINITIALIZED_VALUE: attribute_value = make_owned(attribute_value) return attribute_value diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py index 0633e443..f6a97725 100644 --- a/pyanalyze/implementation.py +++ b/pyanalyze/implementation.py @@ -50,6 +50,7 @@ HasAttrGuardExtension, KNOWN_MUTABLE_TYPES, KnownValue, + is_owned, kv_pairs_from_mapping, KVPair, make_owned, @@ -98,6 +99,20 @@ def flatten_unions( return ImplReturn.unite_impl_rets(results) +def inherit_ownership( + impl: Callable[[CallContext], ImplReturn] +) -> Callable[[CallContext], ImplReturn]: + def wrapper(ctx: CallContext) -> ImplReturn: + ret = impl(ctx) + if is_owned(ctx.vars["self"]): + return ImplReturn( + make_owned(ret.return_value), ret.constraint, ret.no_return_unless + ) + return ret + + return wrapper + + # Implementations of some important functions for use in their ExtendedArgSpecs (see above). These # are called when the test_scope checker encounters call to these functions. @@ -364,7 +379,9 @@ def _list_append_impl(ctx: CallContext) -> ImplReturn: varname, ConstraintType.is_value_object, True, - SequenceValue.make_or_known(list, (*lst.members, (False, element))), + make_owned( + SequenceValue.make_or_known(list, (*lst.members, (False, element))) + ), ) return ImplReturn(KnownValue(None), no_return_unless=no_return_unless) if isinstance(lst, GenericValue): @@ -430,8 +447,10 @@ def inner(key: Value) -> Value: if isinstance(self_value, SequenceValue): members = self_value.get_member_sequence() if members is not None: - return SequenceValue.make_or_known( - typ, [(False, m) for m in members[key.val]] + return make_owned( + SequenceValue.make_or_known( + typ, [(False, m) for m in members[key.val]] + ) ) else: # If the value contains unpacked values, we don't attempt @@ -446,7 +465,7 @@ def inner(key: Value) -> Value: # __getitem__, but then we wouldn't get here). # TODO return a more precise type if the class inherits # from a generic list/tuple. - return TypedValue(typ) + return make_owned(TypedValue(typ)) else: ctx.show_error(f"Invalid {typ.__name__} key {key}") return AnyValue(AnySource.error) @@ -468,6 +487,7 @@ def inner(key: Value) -> Value: return flatten_unions(inner, ctx.vars["obj"], unwrap_annotated=True) +@inherit_ownership def _list_getitem_impl(ctx: CallContext) -> ImplReturn: return _sequence_getitem_impl(ctx, list) @@ -525,6 +545,7 @@ def _dict_setitem_impl(ctx: CallContext) -> ImplReturn: return _add_pairs_to_dict(ctx.vars["self"], [pair], ctx, varname) +@inherit_ownership def _dict_getitem_impl(ctx: CallContext) -> ImplReturn: def inner(key: Value) -> Value: self_value = ctx.vars["self"] @@ -588,6 +609,7 @@ def inner(key: Value) -> Value: return flatten_unions(inner, ctx.vars["k"]) +@inherit_ownership def _dict_get_impl(ctx: CallContext) -> ImplReturn: default = ctx.vars["default"] @@ -655,6 +677,7 @@ def inner(key: Value) -> Value: return flatten_unions(inner, ctx.vars["key"]) +@inherit_ownership def _dict_pop_impl(ctx: CallContext) -> ImplReturn: key = ctx.vars["key"] default = ctx.vars["default"] @@ -739,6 +762,7 @@ def _maybe_unite(value: Value, default: Value) -> Value: return unite_values(value, default) +@inherit_ownership def _dict_setdefault_impl(ctx: CallContext) -> ImplReturn: key = ctx.vars["key"] default = ctx.vars["default"] @@ -782,9 +806,14 @@ def _dict_setdefault_impl(ctx: CallContext) -> ImplReturn: elif isinstance(self_value, DictIncompleteValue): existing_value = self_value.get_value(key, ctx.visitor) is_present = existing_value is not UNINITIALIZED_VALUE - new_value = DictIncompleteValue( - self_value.typ, - [*self_value.kv_pairs, KVPair(key, default, is_required=not is_present)], + new_value = make_owned( + DictIncompleteValue( + self_value.typ, + [ + *self_value.kv_pairs, + KVPair(key, default, is_required=not is_present), + ], + ) ) if varname is not None: no_return_unless = Constraint( @@ -851,8 +880,10 @@ def _update_incomplete_dict( varname, ConstraintType.is_value_object, True, - DictIncompleteValue( - self_val.typ if isinstance(self_val, TypedValue) else dict, pairs + make_owned( + DictIncompleteValue( + self_val.typ if isinstance(self_val, TypedValue) else dict, pairs + ) ), ) return ImplReturn(KnownValue(None), no_return_unless=no_return_unless) @@ -963,13 +994,14 @@ def inner(left: Value, right: Value) -> Value: left = replace_known_sequence_value(left) right = replace_known_sequence_value(right) if isinstance(left, SequenceValue) and isinstance(right, SequenceValue): - return SequenceValue.make_or_known(list, [*left.members, *right.members]) + val = SequenceValue.make_or_known(list, [*left.members, *right.members]) elif isinstance(left, TypedValue) and isinstance(right, TypedValue): left_arg = left.get_generic_arg_for_type(list, ctx.visitor, 0) right_arg = right.get_generic_arg_for_type(list, ctx.visitor, 0) - return GenericValue(list, [unite_values(left_arg, right_arg)]) + val = GenericValue(list, [unite_values(left_arg, right_arg)]) else: - return TypedValue(list) + val = TypedValue(list) + return make_owned(val) return flatten_unions(inner, ctx.vars["self"], ctx.vars["x"]) @@ -997,6 +1029,7 @@ def inner(lst: Value, iterable: Value) -> ImplReturn: constrained_value = SequenceValue( list, [*cleaned_lst.members, (True, arg_type)] ) + constrained_value = make_owned(constrained_value) if return_container: return ImplReturn(constrained_value) if varname is not None: @@ -1066,8 +1099,10 @@ def _set_add_impl(ctx: CallContext) -> ImplReturn: varname, ConstraintType.is_value_object, True, - SequenceValue.make_or_known( - set, (*set_value.members, (False, element)) + make_owned( + SequenceValue.make_or_known( + set, (*set_value.members, (False, element)) + ) ), ) return ImplReturn(KnownValue(None), no_return_unless=no_return_unless) diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py index 70151d38..e1c29d48 100644 --- a/pyanalyze/name_check_visitor.py +++ b/pyanalyze/name_check_visitor.py @@ -112,6 +112,7 @@ KWARGS, MaybeSignature, OverloadedSignature, + ParameterKind, Signature, SigParameter, ) @@ -1914,11 +1915,12 @@ def _visit_function_body(self, function_info: FunctionInfo) -> FunctionResult: "%first_arg", VisitorState.check_names, ) + if info.param.kind is ParameterKind.VAR_KEYWORD: + annotation = make_owned(info.param.annotation) + else: + annotation = info.param.annotation self.scopes.set( - info.param.name, - info.param.annotation, - info.node, - VisitorState.check_names, + info.param.name, annotation, info.node, VisitorState.check_names ) with qcore.override( @@ -3987,7 +3989,10 @@ def visit_AnnAssign(self, node: ast.AnnAssign) -> None: ) # We set the declared type on initial assignment, so that the # annotation can be used to adjust pyanalyze's type inference. - value = expected_type + if isinstance(value, AnnotatedValue): + value = annotate_value(expected_type, value.metadata) + else: + value = expected_type else: is_yield = False @@ -4338,12 +4343,13 @@ def composite_from_attribute(self, node: ast.Attribute) -> Composite: composite.get_varname(), self.being_assigned, node, self.state ) - if isinstance(root_composite.value, TypedValue): - typ = root_composite.value.typ - if isinstance(typ, type): - self._record_type_attr_set( - typ, node.attr, node, self.being_assigned - ) + for root_val in flatten_values(root_composite.value, unwrap_annotated=True): + if isinstance(root_val, TypedValue): + typ = root_val.typ + if isinstance(typ, type): + self._record_type_attr_set( + typ, node.attr, node, self.being_assigned + ) return Composite(self.being_assigned, composite, node) elif self._is_read_ctx(node.ctx): if self._is_checking(): diff --git a/pyanalyze/value.py b/pyanalyze/value.py index 82bcdd74..a16401a5 100644 --- a/pyanalyze/value.py +++ b/pyanalyze/value.py @@ -2651,3 +2651,7 @@ def is_overlapping(left: Value, right: Value, ctx: CanAssignContext) -> bool: def make_owned(typ: Value) -> Value: return AnnotatedValue(typ, [CustomCheckExtension(Owned())]) + + +def is_owned(val: Value) -> bool: + return isinstance(val, AnnotatedValue) and any(val.get_custom_check_of_type(Owned)) From 8e5dee1edde597335d91debfe198f8cfbefea1cc Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Thu, 1 Sep 2022 20:38:24 -0700 Subject: [PATCH 4/6] fix dict.pop, dict() --- docs/changelog.md | 2 ++ pyanalyze/implementation.py | 8 +++++--- pyanalyze/typeshed.py | 2 ++ 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 25bd9a7c..895bc4bb 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,8 @@ ## Unreleased +- Add concept of ownership: only containers owned by calling + code may be mutated (#542) - Preserve `Annotated` annotations on access to methods of literals (#541) - `allow_call` callables are now also called if the arguments diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py index f6a97725..b95188ba 100644 --- a/pyanalyze/implementation.py +++ b/pyanalyze/implementation.py @@ -720,9 +720,11 @@ def _dict_pop_impl(ctx: CallContext) -> ImplReturn: existing_value = self_value.get_value(key, ctx.visitor) is_present = existing_value is not UNINITIALIZED_VALUE if varname is not None and isinstance(key, KnownValue): - new_value = DictIncompleteValue( - self_value.typ, - [pair for pair in self_value.kv_pairs if pair.key != key], + new_value = make_owned( + DictIncompleteValue( + self_value.typ, + [pair for pair in self_value.kv_pairs if pair.key != key], + ) ) no_return_unless = Constraint( varname, ConstraintType.is_value_object, True, new_value diff --git a/pyanalyze/typeshed.py b/pyanalyze/typeshed.py index accdf642..82ac37d1 100644 --- a/pyanalyze/typeshed.py +++ b/pyanalyze/typeshed.py @@ -79,6 +79,7 @@ TypeVarValue, UNINITIALIZED_VALUE, Value, + make_owned, ) @@ -770,6 +771,7 @@ def _get_signature_from_info( sig = sig.replace_return_value(self_val) else: self_val = SubclassValue(self_val) + sig = sig.replace_return_value(make_owned(sig.return_value)) bound_sig = make_bound_method(sig, Composite(self_val)) if bound_sig is None: return None From 38c15fd91d9101c4aa6b22d6ee1a455fcaea0298 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 2 Sep 2022 11:36:34 -0700 Subject: [PATCH 5/6] rename Owned -> Mutable; add make_mutable --- pyanalyze/attributes.py | 8 +++---- pyanalyze/error_code.py | 2 +- pyanalyze/extensions.py | 18 ++++++++------ pyanalyze/functions.py | 4 ++-- pyanalyze/implementation.py | 42 ++++++++++++++++----------------- pyanalyze/name_check_visitor.py | 20 ++++++++-------- pyanalyze/typeshed.py | 4 ++-- pyanalyze/value.py | 8 +++---- 8 files changed, 55 insertions(+), 51 deletions(-) diff --git a/pyanalyze/attributes.py b/pyanalyze/attributes.py index ba2a23bb..af27347c 100644 --- a/pyanalyze/attributes.py +++ b/pyanalyze/attributes.py @@ -15,7 +15,7 @@ import qcore from .annotations import Context, type_from_annotations, type_from_runtime -from .extensions import Owned +from .extensions import Mutable from .options import Options, PyObjectSequenceOption from .safe import safe_isinstance, safe_issubclass from .signature import MaybeSignature @@ -31,7 +31,7 @@ KnownValue, KnownValueWithTypeVars, MultiValuedValue, - make_owned, + make_mutable, set_self, SubclassValue, TypedValue, @@ -95,7 +95,7 @@ def get_attribute(ctx: AttrContext) -> Value: if isinstance(root_value, TypeVarValue): root_value = root_value.get_fallback_value() elif isinstance(root_value, AnnotatedValue): - should_own = any(root_value.get_custom_check_of_type(Owned)) + should_own = any(root_value.get_custom_check_of_type(Mutable)) root_value = root_value.value if isinstance(root_value, KnownValue): attribute_value = _get_attribute_from_known(root_value.val, ctx) @@ -144,7 +144,7 @@ def get_attribute(ctx: AttrContext) -> Value: attribute_value = guard.attribute_type break if should_own and attribute_value is not UNINITIALIZED_VALUE: - attribute_value = make_owned(attribute_value) + attribute_value = make_mutable(attribute_value) return attribute_value diff --git a/pyanalyze/error_code.py b/pyanalyze/error_code.py index 55efffd3..913acfd5 100644 --- a/pyanalyze/error_code.py +++ b/pyanalyze/error_code.py @@ -220,7 +220,7 @@ class ErrorCode(enum.Enum): ErrorCode.invalid_annotated_assignment: "Invalid annotated assignment", ErrorCode.unused_assignment: "Assigned value is never used", ErrorCode.incompatible_yield: "Incompatible yield type", - ErrorCode.disallowed_mutation: "Mutation of unowned object", + ErrorCode.disallowed_mutation: "Mutation of object that does not allow mutation", } diff --git a/pyanalyze/extensions.py b/pyanalyze/extensions.py index 8bd777c1..188bdeb0 100644 --- a/pyanalyze/extensions.py +++ b/pyanalyze/extensions.py @@ -30,7 +30,7 @@ ) import typing_extensions -from typing_extensions import Literal, NoReturn +from typing_extensions import Annotated, Literal, NoReturn import pyanalyze @@ -39,6 +39,8 @@ if TYPE_CHECKING: from .value import AnySource, CanAssign, CanAssignContext, TypeVarMap, Value +_T = TypeVar("_T") + class CustomCheck: """A mechanism for extending the type system with user-defined checks. @@ -147,16 +149,16 @@ def _is_disallowed(self, value: "Value") -> bool: @dataclass(frozen=True) -class Owned(CustomCheck): +class Mutable(CustomCheck): """Custom check that indicates that a mutable value is mutated. For example, a function that mutates a list should accept an argument of type - ``Annotated[List[T], Owned()]``. + ``Annotated[List[T], Mutable()]``. """ def can_assign(self, value: "Value", ctx: "CanAssignContext") -> "CanAssign": for val in pyanalyze.value.flatten_values(value, unwrap_annotated=False): if isinstance(val, pyanalyze.value.AnnotatedValue): - if any(val.get_custom_check_of_type(Owned)): + if any(val.get_custom_check_of_type(Mutable)): continue val = val.value if isinstance(val, pyanalyze.value.AnyValue): @@ -168,6 +170,11 @@ def can_assign(self, value: "Value", ctx: "CanAssignContext") -> "CanAssign": return {} +def make_mutable(obj: _T) -> Annotated[_T, Mutable()]: + """Unsafely mark an object as mutable.""" + return obj + + class _AsynqCallableMeta(type): def __getitem__( self, params: Tuple[Union[Literal[Ellipsis], List[object]], object] @@ -394,9 +401,6 @@ def __call__(self) -> NoReturn: raise NotImplementedError("just here to fool typing._type_check") -_T = TypeVar("_T") - - def reveal_type(value: _T) -> _T: """Inspect the inferred type of an expression. diff --git a/pyanalyze/functions.py b/pyanalyze/functions.py index 7b96c117..e49b8daa 100644 --- a/pyanalyze/functions.py +++ b/pyanalyze/functions.py @@ -37,7 +37,7 @@ SubclassValue, TypedValue, TypeVarValue, - make_owned, + make_mutable, unite_values, UnpackedValue, Value, @@ -342,7 +342,7 @@ def compute_parameters( else: # normal method value = enclosing_class - value = make_owned(value) + value = make_mutable(value) else: # This is meant to exclude methods in nested classes. It's a bit too # conservative for cases such as a function nested in a method nested in a diff --git a/pyanalyze/implementation.py b/pyanalyze/implementation.py index b95188ba..baa0eb93 100644 --- a/pyanalyze/implementation.py +++ b/pyanalyze/implementation.py @@ -53,7 +53,7 @@ is_owned, kv_pairs_from_mapping, KVPair, - make_owned, + make_mutable, MultiValuedValue, NO_RETURN_VALUE, ParameterTypeGuardExtension, @@ -106,7 +106,7 @@ def wrapper(ctx: CallContext) -> ImplReturn: ret = impl(ctx) if is_owned(ctx.vars["self"]): return ImplReturn( - make_owned(ret.return_value), ret.constraint, ret.no_return_unless + make_mutable(ret.return_value), ret.constraint, ret.no_return_unless ) return ret @@ -343,7 +343,7 @@ def _set_impl(ctx: CallContext) -> ImplReturn: def _sequence_impl(typ: type, ctx: CallContext) -> ImplReturn: iterable = ctx.vars["iterable"] maybe_owned: Callable[[Value], Value] = ( - (lambda x: x) if typ is tuple else make_owned + (lambda x: x) if typ is tuple else make_mutable ) if iterable is _NO_ARG_SENTINEL: return ImplReturn(maybe_owned(KnownValue(typ()))) @@ -379,7 +379,7 @@ def _list_append_impl(ctx: CallContext) -> ImplReturn: varname, ConstraintType.is_value_object, True, - make_owned( + make_mutable( SequenceValue.make_or_known(list, (*lst.members, (False, element))) ), ) @@ -447,7 +447,7 @@ def inner(key: Value) -> Value: if isinstance(self_value, SequenceValue): members = self_value.get_member_sequence() if members is not None: - return make_owned( + return make_mutable( SequenceValue.make_or_known( typ, [(False, m) for m in members[key.val]] ) @@ -465,7 +465,7 @@ def inner(key: Value) -> Value: # __getitem__, but then we wouldn't get here). # TODO return a more precise type if the class inherits # from a generic list/tuple. - return make_owned(TypedValue(typ)) + return make_mutable(TypedValue(typ)) else: ctx.show_error(f"Invalid {typ.__name__} key {key}") return AnyValue(AnySource.error) @@ -720,7 +720,7 @@ def _dict_pop_impl(ctx: CallContext) -> ImplReturn: existing_value = self_value.get_value(key, ctx.visitor) is_present = existing_value is not UNINITIALIZED_VALUE if varname is not None and isinstance(key, KnownValue): - new_value = make_owned( + new_value = make_mutable( DictIncompleteValue( self_value.typ, [pair for pair in self_value.kv_pairs if pair.key != key], @@ -808,7 +808,7 @@ def _dict_setdefault_impl(ctx: CallContext) -> ImplReturn: elif isinstance(self_value, DictIncompleteValue): existing_value = self_value.get_value(key, ctx.visitor) is_present = existing_value is not UNINITIALIZED_VALUE - new_value = make_owned( + new_value = make_mutable( DictIncompleteValue( self_value.typ, [ @@ -882,7 +882,7 @@ def _update_incomplete_dict( varname, ConstraintType.is_value_object, True, - make_owned( + make_mutable( DictIncompleteValue( self_val.typ if isinstance(self_val, TypedValue) else dict, pairs ) @@ -1003,7 +1003,7 @@ def inner(left: Value, right: Value) -> Value: val = GenericValue(list, [unite_values(left_arg, right_arg)]) else: val = TypedValue(list) - return make_owned(val) + return make_mutable(val) return flatten_unions(inner, ctx.vars["self"], ctx.vars["x"]) @@ -1031,7 +1031,7 @@ def inner(lst: Value, iterable: Value) -> ImplReturn: constrained_value = SequenceValue( list, [*cleaned_lst.members, (True, arg_type)] ) - constrained_value = make_owned(constrained_value) + constrained_value = make_mutable(constrained_value) if return_container: return ImplReturn(constrained_value) if varname is not None: @@ -1101,7 +1101,7 @@ def _set_add_impl(ctx: CallContext) -> ImplReturn: varname, ConstraintType.is_value_object, True, - make_owned( + make_mutable( SequenceValue.make_or_known( set, (*set_value.members, (False, element)) ) @@ -1512,7 +1512,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(list)) ), SigParameter("object", _POS_ONLY), ], @@ -1530,7 +1530,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(list)) ), SigParameter( "x", _POS_ONLY, annotation=TypedValue(collections.abc.Iterable) @@ -1542,7 +1542,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(list)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(list)) ), SigParameter( "iterable", @@ -1571,7 +1571,7 @@ def get_default_argspecs() -> Dict[object, Signature]: ), Signature.make( [ - SigParameter("self", _POS_ONLY, annotation=make_owned(TypedValue(set))), + SigParameter("self", _POS_ONLY, annotation=make_mutable(TypedValue(set))), SigParameter("object", _POS_ONLY), ], callable=set.add, @@ -1580,7 +1580,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(dict)) ), SigParameter("k", _POS_ONLY), SigParameter("v", _POS_ONLY), @@ -1608,7 +1608,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(dict)) ), SigParameter("key", _POS_ONLY), SigParameter("default", _POS_ONLY, default=KnownValue(None)), @@ -1619,7 +1619,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(dict)) ), SigParameter("key", _POS_ONLY), SigParameter("default", _POS_ONLY, default=_NO_ARG_SENTINEL), @@ -1630,7 +1630,7 @@ def get_default_argspecs() -> Dict[object, Signature]: Signature.make( [ SigParameter( - "self", _POS_ONLY, annotation=make_owned(TypedValue(dict)) + "self", _POS_ONLY, annotation=make_mutable(TypedValue(dict)) ), SigParameter("m", _POS_ONLY, default=_NO_ARG_SENTINEL), SigParameter("kwargs", ParameterKind.VAR_KEYWORD), @@ -1647,7 +1647,7 @@ def get_default_argspecs() -> Dict[object, Signature]: annotation=GenericValue(dict, [TypeVarValue(K), TypeVarValue(V)]), ) ], - make_owned( + make_mutable( DictIncompleteValue( dict, [KVPair(TypeVarValue(K), TypeVarValue(V), is_many=True)] ) diff --git a/pyanalyze/name_check_visitor.py b/pyanalyze/name_check_visitor.py index c4434b2f..2eface00 100644 --- a/pyanalyze/name_check_visitor.py +++ b/pyanalyze/name_check_visitor.py @@ -180,7 +180,7 @@ NoReturnConstraintExtension, ReferencingValue, SequenceValue, - make_owned, + make_mutable, set_self, SubclassValue, TypedValue, @@ -1921,7 +1921,7 @@ def _visit_function_body(self, function_info: FunctionInfo) -> FunctionResult: VisitorState.check_names, ) if info.param.kind is ParameterKind.VAR_KEYWORD: - annotation = make_owned(info.param.annotation) + annotation = make_mutable(info.param.annotation) else: annotation = info.param.annotation self.scopes.set( @@ -2403,13 +2403,13 @@ def _handle_imports( # Comprehensions def visit_DictComp(self, node: ast.DictComp) -> Value: - return make_owned(self._visit_sequence_comp(node, dict)) + return make_mutable(self._visit_sequence_comp(node, dict)) def visit_ListComp(self, node: ast.ListComp) -> Value: - return make_owned(self._visit_sequence_comp(node, list)) + return make_mutable(self._visit_sequence_comp(node, list)) def visit_SetComp(self, node: ast.SetComp) -> Value: - return make_owned(self._visit_sequence_comp(node, set)) + return make_mutable(self._visit_sequence_comp(node, set)) def visit_GeneratorExp(self, node: ast.GeneratorExp) -> Value: return self._visit_sequence_comp(node, types.GeneratorType) @@ -2663,7 +2663,7 @@ def visit_Dict(self, node: ast.Dict) -> Value: ErrorCode.unsupported_operation, detail=str(new_pairs), ) - return make_owned(TypedValue(dict)) + return make_mutable(TypedValue(dict)) all_pairs += new_pairs continue key_val = self.visit(key_node) @@ -2703,17 +2703,17 @@ def visit_Dict(self, node: ast.Dict) -> Value: ret[key] = value if has_non_literal: - return make_owned(DictIncompleteValue(dict, all_pairs)) + return make_mutable(DictIncompleteValue(dict, all_pairs)) else: - return make_owned(KnownValue(ret)) + return make_mutable(KnownValue(ret)) def visit_Set(self, node: ast.Set) -> Value: - return make_owned(self._visit_display_read(node, set)) + return make_mutable(self._visit_display_read(node, set)) def visit_List(self, node: ast.List) -> Optional[Value]: val = self._visit_display(node, list) if val is not None: - return make_owned(val) + return make_mutable(val) return None def visit_Tuple(self, node: ast.Tuple) -> Optional[Value]: diff --git a/pyanalyze/typeshed.py b/pyanalyze/typeshed.py index 72ed5028..608fa3c5 100644 --- a/pyanalyze/typeshed.py +++ b/pyanalyze/typeshed.py @@ -73,7 +73,7 @@ TypeVarValue, UNINITIALIZED_VALUE, Value, - make_owned, + make_mutable, ) @@ -756,7 +756,7 @@ def _get_signature_from_info( sig = sig.replace_return_value(self_val) else: self_val = SubclassValue(self_val) - sig = sig.replace_return_value(make_owned(sig.return_value)) + sig = sig.replace_return_value(make_mutable(sig.return_value)) bound_sig = make_bound_method(sig, Composite(self_val)) if bound_sig is None: return None diff --git a/pyanalyze/value.py b/pyanalyze/value.py index 5682f7a9..85b580f6 100644 --- a/pyanalyze/value.py +++ b/pyanalyze/value.py @@ -50,7 +50,7 @@ def function(x: int, y: list[int], z: Any): import pyanalyze from pyanalyze.error_code import ErrorCode -from pyanalyze.extensions import CustomCheck, Owned +from pyanalyze.extensions import CustomCheck, Mutable from .safe import all_of_type, safe_equals, safe_isinstance, safe_issubclass @@ -2652,12 +2652,12 @@ def is_overlapping(left: Value, right: Value, ctx: CanAssignContext) -> bool: return left.is_assignable(right, ctx) or right.is_assignable(left, ctx) -def make_owned(typ: Value) -> Value: - return AnnotatedValue(typ, [CustomCheckExtension(Owned())]) +def make_mutable(typ: Value) -> Value: + return AnnotatedValue(typ, [CustomCheckExtension(Mutable())]) def is_owned(val: Value) -> bool: - return isinstance(val, AnnotatedValue) and any(val.get_custom_check_of_type(Owned)) + return isinstance(val, AnnotatedValue) and any(val.get_custom_check_of_type(Mutable)) def make_coro_type(return_type: Value) -> GenericValue: From 5fb22f359ecfbb2cac2d0c2028dbae107a60120b Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sun, 6 Nov 2022 18:27:39 -0800 Subject: [PATCH 6/6] add test --- pyanalyze/test_ownership.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 pyanalyze/test_ownership.py diff --git a/pyanalyze/test_ownership.py b/pyanalyze/test_ownership.py new file mode 100644 index 00000000..98bc74e3 --- /dev/null +++ b/pyanalyze/test_ownership.py @@ -0,0 +1,21 @@ +# static analysis: ignore +from .test_name_check_visitor import TestNameCheckVisitorBase +from .test_node_visitor import assert_passes + + +class TestOwnership(TestNameCheckVisitorBase): + @assert_passes() + def test(self): + from typing import List + + def capybara(x: List[str]): + x.append("x") # E: disallowed_mutation + y = list(x) + y.append("x") + + z = [a for a in x] + z.append("x") + + # TODO make it work for an all-literal list + alpha = ["a", "b", "c", str(x)] + alpha.append("x")