fix(eslint-plugin): [prefer-return-this-type] check `accessor` properties with a function initializer #10794

ronami · 2025-02-05T17:28:47Z

PR Checklist

Addresses an existing open issue: fixes Bug: [prefer-return-this-type] doesn't report on an accessor property with a function initializer #10791
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

This PR addresses #10791 and adds checks for accessor properties.

typescript-eslint · 2025-02-05T17:28:50Z

Thanks for the PR, @ronami!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

netlify · 2025-02-05T17:29:09Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`b463fe5`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/67a4b390c2386600099e5cdc
😎 Deploy Preview	https://deploy-preview-10794--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

nx-cloud · 2025-02-05T17:29:39Z

View your CI Pipeline Execution ↗ for commit b463fe5.

Command	Status	Duration	Result
`nx test eslint-plugin --coverage=false`	✅ Succeeded	7m 28s	View ↗
`nx test eslint-plugin`	✅ Succeeded	5m 52s	View ↗
`nx test typescript-eslint --coverage=false`	✅ Succeeded	27s	View ↗
`nx run types:build`	✅ Succeeded	<1s	View ↗
`nx run-many --target=build --exclude website --...`	✅ Succeeded	31s	View ↗
`nx test utils --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test visitor-keys --coverage=false`	✅ Succeeded	<1s	View ↗
`nx test visitor-keys`	✅ Succeeded	<1s	View ↗
`Additional runs (24)`	✅ Succeeded	...	View ↗

☁️ Nx Cloud last updated this comment at 2025-02-06 13:20:32 UTC

codecov · 2025-02-05T17:53:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.29%. Comparing base (844608b) to head (b463fe5).
Report is 22 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10794      +/-   ##
==========================================
+ Coverage   87.28%   87.29%   +0.01%     
==========================================
  Files         450      450              
  Lines       15739    15739              
  Branches     4609     4609              
==========================================
+ Hits        13737    13739       +2     
  Misses       1645     1645              
+ Partials      357      355       -2

Flag	Coverage Δ
unittest	`87.29% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...eslint-plugin/src/rules/prefer-return-this-type.ts	`96.72% <100.00%> (+3.27%)`	⬆️

... and 1 file with indirect coverage changes

ronami · 2025-02-06T15:21:35Z

packages/eslint-plugin/src/rules/prefer-return-this-type.ts

-        checkFunction(node.value, node.parent.parent as ClassLikeDeclaration);
+        checkFunction(node.value, node.parent.parent);


The type assertion seems unnecessary since #9560.

This is an unrelated change, but I decided to include it as it's a tiny change and on the same part of the code that the PR changes anyway.

I wonder if this can be caught by no-unnecessary-type-assertion 🤔

Since ClassLikeDeclaration is a different type alias (and thus a different type), the comparison seems to fail (even though ClassLikeDeclaration is assigned the same type as the un-asserted type). Inlining the value of the type alias seem to cause no-unnecessary-type-assertion to report this.

Yeah that would be nice. My hunch is it's some nuance about generic functions having type parameters inferred from types of arguments.

ronami · 2025-02-06T15:24:27Z

packages/eslint-plugin/tests/rules/prefer-return-this-type.test.ts

+    `
+class Foo {
+  f?: string;
 }
    `,


This is a test addition that isn't related to the accessor changes, but was previously uncovered. This fails codecov (as the uncovered code has been moved to a function), and because it's such a tiny addition, I decided to include it in this PR.

JoshuaKGoldberg

🧹

JoshuaKGoldberg · 2025-02-17T13:26:28Z

packages/eslint-plugin/src/rules/prefer-return-this-type.ts

-        checkFunction(node.value, node.parent.parent as ClassLikeDeclaration);
+        checkFunction(node.value, node.parent.parent);


Yeah that would be nice. My hunch is it's some nuance about generic functions having type parameters inferred from types of arguments.

| datasource | package | from | to | | ---------- | -------------------------------- | ------ | ------ | | npm | @typescript-eslint/eslint-plugin | 8.24.0 | 8.24.1 | | npm | @typescript-eslint/parser | 8.24.0 | 8.24.1 | ## [v8.24.1](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8241-2025-02-17) ##### 🩹 Fixes - **eslint-plugin:** \[class-methods-use-this] check `accessor` methods with a function initializer ([#10796](typescript-eslint/typescript-eslint#10796)) - **eslint-plugin:** \[no-misused-promises] don't report on `static` `accessor` properties ([#10814](typescript-eslint/typescript-eslint#10814)) - **eslint-plugin:** \[no-deprecated] don't report on deprecated `accessor` property declaration ([#10813](typescript-eslint/typescript-eslint#10813)) - **eslint-plugin:** \[explicit-member-accessibility] check `accessor` class properties for missing accessibility modifier ([#10805](typescript-eslint/typescript-eslint#10805)) - **eslint-plugin:** \[explicit-module-boundary-types] check `accessor` class properties with a function initializer ([#10804](typescript-eslint/typescript-eslint#10804)) - **eslint-plugin:** \[prefer-return-this-type] check `accessor` properties with a function initializer ([#10794](typescript-eslint/typescript-eslint#10794)) - **eslint-plugin:** \[consistent-generic-constructors] check `accessor` class properties ([#10789](typescript-eslint/typescript-eslint#10789)) - **eslint-plugin:** \[no-unsafe-assignment] report on an `any` value assigned as an initializer of an `accessor` property ([#10785](typescript-eslint/typescript-eslint#10785)) - **eslint-plugin:** \[no-unnecessary-template-expression] ignore enum and enum members ([#10782](typescript-eslint/typescript-eslint#10782)) - **eslint-plugin:** \[no-inferrable-types] handle accessor ([#10780](typescript-eslint/typescript-eslint#10780)) ##### ❤️ Thank You - Ronen Amiel - YeonJuan You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.

handle accessor property with prefer-return-this-type

b71a9fe

refactor and add missing tests

b463fe5

ronami commented Feb 6, 2025

View reviewed changes

ronami marked this pull request as ready for review February 6, 2025 15:25

JoshuaKGoldberg approved these changes Feb 17, 2025

View reviewed changes

JoshuaKGoldberg merged commit 02d4779 into typescript-eslint:main Feb 17, 2025
60 of 61 checks passed

ronami deleted the prefer-return-this-type-accessor branch February 17, 2025 20:50

github-actions bot locked as resolved and limited conversation to collaborators Feb 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [prefer-return-this-type] check `accessor` properties with a function initializer #10794

fix(eslint-plugin): [prefer-return-this-type] check `accessor` properties with a function initializer #10794

ronami commented Feb 5, 2025 •

edited

Loading

typescript-eslint bot commented Feb 5, 2025

netlify bot commented Feb 5, 2025 •

edited

Loading

nx-cloud bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

ronami Feb 6, 2025 •

edited

Loading

ronami Feb 6, 2025 •

edited

Loading

JoshuaKGoldberg Feb 17, 2025

ronami Feb 6, 2025 •

edited

Loading

JoshuaKGoldberg left a comment

JoshuaKGoldberg Feb 17, 2025

		checkFunction(node.value, node.parent.parent as ClassLikeDeclaration);
		checkFunction(node.value, node.parent.parent);

fix(eslint-plugin): [prefer-return-this-type] check accessor properties with a function initializer #10794

fix(eslint-plugin): [prefer-return-this-type] check accessor properties with a function initializer #10794

Conversation

ronami commented Feb 5, 2025 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Feb 5, 2025

netlify bot commented Feb 5, 2025 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Feb 5, 2025 • edited Loading

codecov bot commented Feb 5, 2025 • edited Loading

Codecov Report

ronami Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

ronami Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg Feb 17, 2025

Choose a reason for hiding this comment

ronami Feb 6, 2025 • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg Feb 17, 2025

Choose a reason for hiding this comment

fix(eslint-plugin): [prefer-return-this-type] check `accessor` properties with a function initializer #10794

fix(eslint-plugin): [prefer-return-this-type] check `accessor` properties with a function initializer #10794

ronami commented Feb 5, 2025 •

edited

Loading

netlify bot commented Feb 5, 2025 •

edited

Loading

nx-cloud bot commented Feb 5, 2025 •

edited

Loading

codecov bot commented Feb 5, 2025 •

edited

Loading

ronami Feb 6, 2025 •

edited

Loading

ronami Feb 6, 2025 •

edited

Loading

ronami Feb 6, 2025 •

edited

Loading