fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists #8277

yeonjuan · 2024-01-20T07:58:17Z

PR Checklist

Addresses an existing open issue: fixes Bug: [class-literal-property-style] A literal getter with a setter does not pass lint, and incorrectly auto-fix #5961
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

typescript-eslint · 2024-01-20T07:58:21Z

Thanks for the PR, @yeonjuan!

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 · 2024-01-20T07:58:23Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`9af90d0`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/65bc5ee9fd4f6400088df997
😎 Deploy Preview	https://deploy-preview-8277--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 95 (🟢 up 5 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.

packages/eslint-plugin/src/rules/class-literal-property-style.ts

JoshuaKGoldberg

I think there's an edge case that needs to be handled? But otherwise this looks good to me!

packages/eslint-plugin/src/rules/class-literal-property-style.ts

…same key setter exists

codecov · 2024-02-01T13:47:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (5c253a8) 87.71% compared to head (9af90d0) 87.72%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8277   +/-   ##
=======================================
  Coverage   87.71%   87.72%           
=======================================
  Files         397      397           
  Lines       13963    13971    +8     
  Branches     4061     4065    +4     
=======================================
+ Hits        12248    12256    +8     
  Misses       1403     1403           
  Partials      312      312

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

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

Files	Coverage Δ
...t-plugin/src/rules/class-literal-property-style.ts	`100.00% <100.00%> (ø)`

JoshuaKGoldberg · 2024-02-01T16:49:19Z

Aside: could you please not force push PRs? It's a small thing, no worries, but from https://typescript-eslint.io/contributing/pull-requests:

Reasoning: GitHub is not able to track changes across force pushes, which makes it take longer for us to perform incremental reviews

JoshuaKGoldberg

Looks great, thanks! 🔥

Just one suggested added test. I can merge this in too to get it in quickly.

#8323 tracks adding a shared utility around the static names. Linking in this comment for tracking.

packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts

…e.test.ts

yeonjuan · 2024-02-01T16:53:02Z

Aside: could you please not force push PRs? It's a small thing, no worries, but from https://typescript-eslint.io/contributing/pull-requests:

Oh OK! Sorry about that. I was making the review difficult. 😅

JoshuaKGoldberg · 2024-02-01T16:53:27Z

No worries at all 😄 TIL I (re-)learned that gh pr checkout has a --force.

yeonjuan · 2024-02-01T17:07:12Z

@JoshuaKGoldberg Oops, Maybe this rule should report error on this cases?

https://github.com/typescript-eslint/typescript-eslint/actions/runs/7744772276/job/21119385280?pr=8277

https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/website/src/components/config/ConfigTypeScript.tsx#L38

          // group: Record<string, ConfigOptionsType>
          group[category] ??= {
            heading: category,
            fields: [],
          };

JoshuaKGoldberg · 2024-02-01T19:00:19Z

I don't know how that comment got hidden - but, yes - could you take a look please actually?

(I'm between things atm)

yeonjuan · 2024-02-01T19:05:39Z

@JoshuaKGoldberg I made a pr for fixing it (#8336)
I thinks the lint error was false positive of no-unnecessary-condition. And the false positive fixed in #8249.

…same key setter exists (typescript-eslint#8277)

yeonjuan commented Jan 20, 2024

View reviewed changes

packages/eslint-plugin/src/rules/class-literal-property-style.ts Show resolved Hide resolved

yeonjuan changed the title ~~fix(eslint-plugin): [class-literal-property-style] allow getter when …~~ fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists Jan 20, 2024

yeonjuan force-pushed the fix#5961 branch 4 times, most recently from 38a2136 to 348267b Compare January 28, 2024 04:09

JoshuaKGoldberg requested changes Jan 28, 2024

View reviewed changes

packages/eslint-plugin/src/rules/class-literal-property-style.ts Outdated Show resolved Hide resolved

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jan 28, 2024

yeonjuan force-pushed the fix#5961 branch 6 times, most recently from 7c28999 to a4cb56f Compare January 30, 2024 13:43

yeonjuan requested a review from JoshuaKGoldberg January 30, 2024 14:56

github-actions bot removed the awaiting response Issues waiting for a reply from the OP or another party label Jan 30, 2024

fix(eslint-plugin): [class-literal-property-style] allow getter when …

9701126

…same key setter exists

yeonjuan force-pushed the fix#5961 branch 4 times, most recently from e7e2b70 to 819b411 Compare February 1, 2024 12:40

apply review

b511b8a

yeonjuan force-pushed the fix#5961 branch from 819b411 to b511b8a Compare February 1, 2024 13:29

JoshuaKGoldberg previously approved these changes Feb 1, 2024

View reviewed changes

packages/eslint-plugin/tests/rules/class-literal-property-style.test.ts Show resolved Hide resolved

Update packages/eslint-plugin/tests/rules/class-literal-property-styl…

dbf2f67

…e.test.ts

JoshuaKGoldberg dismissed their stale review via dbf2f67 February 1, 2024 16:53

JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Feb 1, 2024

bradzacher and others added 2 commits February 2, 2024 12:19

Merge branch 'main' into fix#5961

58ced5a

Merge branch 'main' into fix#5961

9af90d0

bradzacher merged commit 69bd501 into typescript-eslint:main Feb 2, 2024

danvk pushed a commit to danvk/typescript-eslint that referenced this pull request Feb 4, 2024

fix(eslint-plugin): [class-literal-property-style] allow getter when …

55a56ff

…same key setter exists (typescript-eslint#8277)

github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists #8277

fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists #8277

yeonjuan commented Jan 20, 2024 •

edited

Loading

typescript-eslint bot commented Jan 20, 2024

netlify bot commented Jan 20, 2024 •

edited

Loading

JoshuaKGoldberg left a comment

codecov bot commented Feb 1, 2024 •

edited

Loading

JoshuaKGoldberg commented Feb 1, 2024

JoshuaKGoldberg left a comment

yeonjuan commented Feb 1, 2024

JoshuaKGoldberg commented Feb 1, 2024

yeonjuan commented Feb 1, 2024 •

edited

Loading

JoshuaKGoldberg commented Feb 1, 2024

yeonjuan commented Feb 1, 2024 •

edited

Loading

fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists #8277

fix(eslint-plugin): [class-literal-property-style] allow getter when same key setter exists #8277

Conversation

yeonjuan commented Jan 20, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Jan 20, 2024

netlify bot commented Jan 20, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 1, 2024 • edited Loading

Codecov Report

JoshuaKGoldberg commented Feb 1, 2024

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

yeonjuan commented Feb 1, 2024

JoshuaKGoldberg commented Feb 1, 2024

yeonjuan commented Feb 1, 2024 • edited Loading

JoshuaKGoldberg commented Feb 1, 2024

yeonjuan commented Feb 1, 2024 • edited Loading

yeonjuan commented Jan 20, 2024 •

edited

Loading

netlify bot commented Jan 20, 2024 •

edited

Loading

codecov bot commented Feb 1, 2024 •

edited

Loading

yeonjuan commented Feb 1, 2024 •

edited

Loading

yeonjuan commented Feb 1, 2024 •

edited

Loading