Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(eslint-plugin): [member-ordering] add support for grouping readonly fields#6349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@IronGeek
Copy link
Contributor

PR Checklist

Overview

This PR introduces new member attribute/type/Kind:readonly-field, a superset offield, which denotes all fields with readonly modifiers. This makes it possible to group/ sort fields based on their readonly modifiers and optionally with their Accessibility, and or Scope too.

Like other member types,readonly-field can also be joined with accessibility and scope attributes to create more specific type; the following is the complete combination ofreadonly-field with Scope and Accessibility introduced in this PR:

"readonly-field""public-readonly-field""public-decorated-readonly-field""decorated-readonly-field""static-readonly-field""public-static-readonly-field""instance-readonly-field""public-instance-readonly-field""abstract-readonly-field""public-abstract-readonly-field""protected-readonly-field""protected-decorated-readonly-field""protected-static-readonly-field""protected-instance-readonly-field""protected-abstract-readonly-field""private-readonly-field""private-decorated-readonly-field""private-static-readonly-field""private-instance-readonly-field""#private-readonly-field""#private-static-readonly-field""#private-instance-readonly-field"

For example, with this configuration the following code would be treated as valid:

{"rules": {"@typescript-eslint/member-ordering": ["error", {"default": ["decorated-readonly-field",            ["public-readonly-field","readonly-field"]"protected-readonly-field","private-readonly-field","abstract-readonly-field","field"        ]       }    ]  }}
abstractclassFoo{  @DecpublicreadonlyDA:string;  @DecprotectedreadonlyDB:string;  @DecprivatereadonlyDC:string;publicstaticreadonlySA:string;publicreadonlyIA:string;staticreadonly #SD:string;readonly #ID:string;protectedstaticreadonlySB:string;protectedreadonlyIB:string;privatestaticreadonlySC:string;privatereadonlyIC:string;publicabstractreadonlyAA:string;protectedabstractreadonlyAB:string;publicRA:string;RB:string;privateRC:string;  #RD:string;}

@nx-cloud
Copy link

nx-cloudbot commentedJan 18, 2023•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit534ac14. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

🟥 Failed Commands
Node 14 - nx test typescript-estree --coverage=false
✅ Successfully ran 31 targets

Sent with 💌 fromNxCloud.

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@IronGeek!

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 onhttps://opencollective.com/typescript-eslint.

@netlify
Copy link

netlifybot commentedJan 18, 2023•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit534ac14
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/640ea9bbfa9041000889496e
😎 Deploy Previewhttps://deploy-preview-6349--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to yourNetlify site settings.

@codecov
Copy link

codecovbot commentedJan 18, 2023•
edited
Loading

Codecov Report

Merging#6349 (9f2e32c) intomain (36ef0e1) willincrease coverage by2.65%.
The diff coverage is100.00%.

❗ Current head9f2e32c differs from pull request most recent head534ac14. Consider uploading reports for the commit534ac14 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@##             main    #6349      +/-   ##==========================================+ Coverage   88.62%   91.28%   +2.65%==========================================  Files         382      366      -16       Lines       12883    12461     -422       Branches     3787     3656     -131     ==========================================- Hits        11418    11375      -43+ Misses       1124      772     -352+ Partials      341      314      -27
FlagCoverage Δ
unittest91.28% <100.00%> (+2.65%)⬆️

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

Impacted FilesCoverage Δ
...ackages/eslint-plugin/src/rules/member-ordering.ts96.41% <100.00%> (+0.27%)⬆️

... and56 files with indirect coverage changes

Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A good start - thanks for getting this PR up! 🙌

Requesting changes on more test coverage & handling index signatures.

Aside: this rule gets more complicated and hard to deal with in every PR. But I don't see a way around it 😞. This comment isn't actionable and you can ignore my sadness. I'm just ... musing.

@JoshuaKGoldbergJoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party labelJan 22, 2023
@JoshuaKGoldbergJoshuaKGoldberg removed the awaiting responseIssues waiting for a reply from the OP or another party labelMar 13, 2023
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Whew, looks good to me! Thanks@IronGeek! 🙌

In theory one might want something likereadonly-method, for cases like{ readonly A: () => void }. But that can be a feature add if requested after this PR. The rule is (and already was) complex enough as it is.

@JoshuaKGoldberg
Copy link
Member

Btw, sorry for the month-long wait time. This fell off my radar. After the v6 release I'm hopeful we can work on#6033 as a process improvement so it doesn't happen again.

@JoshuaKGoldbergJoshuaKGoldberg merged commit9d3bdfc intotypescript-eslint:mainMar 13, 2023
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsMar 21, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[member-ordering] separating readonly members is not supported

2 participants

@IronGeek@JoshuaKGoldberg
[8]ページ先頭
©2009-2025 Movatter.jp