Add rule to detect unnecessary class properties

[ShaharNaveh]

Summary

Fixes #21403

Test Plan

Added tests

[astral-sh-bot]

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+4 -0 violations, +0 -0 fixes in 2 projects; 53 projects unchanged)

pandas-dev/pandas (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ pandas/tests/io/excel/test_writers.py:1708:17: RUF069 `sheets` is a property without a `return` statement

pytest-dev/pytest (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview

+ testing/python/raises.py:346:17: RUF069 `__class__` is a property without a `return` statement
+ testing/test_compat.py:117:13: RUF069 `__class__` is a property without a `return` statement
+ testing/test_compat.py:91:9: RUF069 `raise_fail_outcome` is a property without a `return` statement

Changes by rule (1 rules affected)

code	total	+ violation	- violation	+ fix	- fix
RUF069	4	4	0	0	0

[ntBre]

Thanks! This looks good to me overall, just some minor nits and a suggestion about the rule name and code.

[ntBre]

"Unnecessary" doesn't feel like exactly the right description to me. I think something like property-without-return for the rule name, along the lines of the issue title, might make more sense.

[ShaharNaveh]

The initial name was something like that, but I decided it's no longer accurate after the inclusion of raise and yield, would you still want me to revert it to the old name?

[ntBre]

Ah, I see what you mean, but I think it's still okay for the name and diagnostic message only to mention return. @amyreese had a good point that return will probably be the most common fix when the diagnostic fires too, so it seems good to emphasize that option.

[ntBre]

Thank you, this looks great!

I think we just need to bump the preview_since version and update the rule name/docs and this is good to go.

[ntBre]

Ah, I see what you mean, but I think it's still okay for the name and diagnostic message only to mention return. @amyreese had a good point that return will probably be the most common fix when the diagnostic fires too, so it seems good to emphasize that option.

[amyreese]

Sorry to add one more change needed. We should pick a smaller range for the diagnostic, rather than throwing a yellow squiggle on an entire function in an editor, as changing the diagnostic range later will potentially be a breaking change.

I think I'd suggest just marking the @property decorator itself, since that's the part that's unfulfilled by the body of the function.

[MichaReiser]

In some cases, we highlight the range of the function's name. You can see other usages by searching for references to the Identifier trait

ruff/crates/ruff_python_ast/src/identifier.rs

Lines 18 to 21 in 9ae698f

	pub trait Identifier {
	/// Return the [`TextRange`] of the identifier in the given AST node.
	fn identifier(&self) -> TextRange;
	}

[ShaharNaveh]

Thank you @amyreese & @MichaReiser for the suggestions.

I agree with highlighting only one line instead of the entire function block.

I ended up highlighting only the function name (@MichaReiser suggestion), just because it's a personal preference of mine. LMK if you want me to change that to highlight something else instead

[amyreese]

One last nit, but otherwise looks good to me!

[MichaReiser]

Thanks this looks good. I just noticed that you recoded the rule from RUF066 to RUF069 before. Can you tell me why? It seems to me that RUF066 is still available.

[MichaReiser]

Oh, never mind. It's because there are other in-flight PRs using that code. I recoded it back to RUF066, because I want to avoid holes and your PR is going to land first :)