[flake8-return] Exempt cached properties and other property-like decorators from explicit return rule (RET501)

[epenet]

Summary

Follow-up to #12243, in which regular properties were exempted but not cached properties.

Still linked to home-assistant/core#115031

Test Plan

[codspeed-hq]

CodSpeed Performance Report

Merging #12563 will not alter performance

_{Comparing epenet:20240729-0822 (c2c96bc) with epenet:20240729-0822 (f452453)}

Summary

✅ 33 untouched benchmarks

[github-actions]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

[MichaReiser]

LGTM. Thank you. I let @AlexWaygood merge to get the blessing of someone who actually understands Python ;)

[AlexWaygood]

LGTM. I added support for other stdlib property-like decorators as well, such as @abc.abstractproperty, @enum.property, and @types.DynamicClassAttribute. I think the same rationale applies for special-casing these decorators as it does for special-casing @functools.cached_property.

[AlexWaygood]

@epenet -- you had semantic.resolve_qualified_name(map_callable(&decorator.expression)) here. I think using map_callable would be incorrect in this instance. map_callable() is a convenience function for when we don't care whether e.g. a function is decorated with @functools.lru_cache or @functools.lru_cache() (whether the decorator is a call-expression or not is irrelevant). But for all of these property-like decorators, you can only use them as @property; it's invalid to use them as @property(). So for these decorators, we just want to call resolve_qualified_name directly on the expression, instead of passing it through map_callable first.

[epenet]

I copied it from elsewhere in the code base.
I wonder if this applies there as well...
And would it make sense to move this function to a shared location?

[AlexWaygood]

I copied it from elsewhere in the code base.
I wonder if this applies there as well...

It's hard to say in general. For a lot of decorators (like @lru_cache), you really don't care about whether it's been decorated with @lru_cache or @lru_cache(). So while there may be other places in the codebase where it's being used incorrectly, I'm pretty confident they're not all incorrect ;-)

And would it make sense to move this function to a shared location?

Possibly...

[AlexWaygood]

Good that you asked. It seems we already have a general-purpose function for this:

ruff/crates/ruff_python_semantic/src/analyze/visibility.rs

Lines 63 to 83 in aaa56eb

	/// Returns `true` if a function definition is a `@property`.
	/// `extra_properties` can be used to check additional non-standard
	/// `@property`-like decorators.
	pub fn is_property(
	decorator_list: &[Decorator],
	extra_properties: &[QualifiedName],
	semantic: &SemanticModel,
	) -> bool {
	decorator_list.iter().any(|decorator| {
	semantic
	.resolve_qualified_name(map_callable(&decorator.expression))
	.is_some_and(|qualified_name| {
	matches!(
	qualified_name.segments(),
	["" | "builtins", "property"] | ["functools", "cached_property"]
	) || extra_properties
	.iter()
	.any(|extra_property| extra_property.segments() == qualified_name.segments())
	})
	})
	}

We should just use that for now. I'll apply some fixes to it in a followup PR.

[epenet]

For reference, I wonder if it might make sense to look at rules/pydoclint/rules/check_docstring.rs also:

ruff/crates/ruff_linter/src/rules/pydoclint/rules/check_docstring.rs

Lines 442 to 461 in 459c85b

	// Checks if a function has a `@property` decorator
	fn is_property(definition: &Definition, checker: &Checker) -> bool {
	let Some(function) = definition.as_function_def() else {
	return false;
	};
	let Some(last_decorator) = function.decorator_list.last() else {
	return false;
	};
	checker
	.semantic()
	.resolve_qualified_name(&last_decorator.expression)
	.is_some_and(|qualified_name| {
	matches!(
	qualified_name.segments(),
	["", "property"] | ["functools", "cached_property"]
	)
	})
	}

[AlexWaygood]

nice, thanks