typescript-eslint/typescript-eslintPublic

NotificationsYou must be signed in to change notification settings
Fork2.8k
Star16k

feat(eslint-plugin): [explicit-function-return-type] add allowedNames#4440

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

Jump to bottom

Merged

bradzacher merged 15 commits intotypescript-eslint:mainfromislandryu:featAddallowedNames

Feb 2, 2022

Merged

feat(eslint-plugin): [explicit-function-return-type] add allowedNames#4440

bradzacher merged 15 commits intotypescript-eslint:mainfromislandryu:featAddallowedNames

Feb 2, 2022

+275 −0

Conversation

Copy link

Contributor

islandryu commentedJan 14, 2022•
edited by bradzacher
Loading

Add optionallowedNames to explicit-function-return-type

PR Checklist

Addresses an existing issue:fixes[explicit-function-return-type] add an exclude list option #4402
That issue was marked asaccepting prs
Steps inCONTRIBUTING.md were taken

Overview

You may pass function/method names you would like this rule to ignore, like so:

{"@typescript-eslint/explicit-function-return-type": ["error",    {"allowedNames": ["ignoredFunctionName","ignoredMethodName"]    }  ]}

feat(eslint-plugin): [explicit-function-return-type] add allowedNames…

5e2c4fd

… option

Copy link

nx-cloudbot commentedJan 14, 2022•
edited
Loading

☁️ Nx Cloud Report

CI ran the following commands for commit65103ff. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 48 targets

Sent with 💌 fromNxCloud.

Copy link

Contributor

typescript-eslintbot commentedJan 14, 2022

Thanks for the PR,@islandryu!

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. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitorsper day.

Copy link

netlifybot commentedJan 14, 2022•
edited
Loading

❌ Deploy Preview fortypescript-eslint failed.

🔨 Explore the source changes:65103ff

🔍 Inspect the deploy log:https://app.netlify.com/sites/typescript-eslint/deploys/61f8ee03e1dd9e00086c7f6a

feat(eslint-plugin): [explicit-function-return-type] afix typecheck

f931a38

Copy link

codecovbot commentedJan 14, 2022•
edited
Loading

Codecov Report

Merging#4440 (65103ff) intomain (04baac8) willincrease coverage by0.01%.
The diff coverage is100.00%.

@@            Coverage Diff             @@##             main    #4440      +/-   ##==========================================+ Coverage   94.54%   94.56%   +0.01%==========================================  Files         147      147                Lines        7851     7877      +26       Branches     2516     2530      +14     ==========================================+ Hits         7423     7449      +26  Misses        234      234                Partials      194      194

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

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

Impacted Files	Coverage Δ
...-plugin/src/rules/explicit-function-return-type.ts	`100.00% <100.00%> (ø)`

islandryu marked this pull request as ready for review

January 14, 2022 14:43

bradzacher added the enhancement: plugin rule optionNew rule option for an existing eslint-plugin rule label

Jan 14, 2022

JoshuaKGoldberg requested changes

Jan 23, 2022

View reviewed changes

Copy link

Member

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

LGTM in general, thanks!

Codecov is right that there's a gap in test coverage; could you please
add a bit more testing and resolve the conflicts from themain branch? Once you do I think we should be good to merge.

packages/eslint-plugin/src/rules/explicit-function-return-type.ts OutdatedShow resolvedHide resolved

JoshuaKGoldberg added the awaiting responseIssues waiting for a reply from the OP or another party label

Jan 23, 2022

islandryuand others added9 commits

January 24, 2022 11:59

Merge branch 'main'

edd64e8

Update packages/eslint-plugin/src/rules/explicit-function-return-type.ts

b383ff6

Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>

Merge branch 'featAddallowedNames' ofhttps://github.com/islandryu/ty…

f48f8d7

…pescript-eslint into featAddallowedNames

feat(eslint-plugin): [explicit-function-return-type] Change to allowe…

f03d6e2

…dNames to work for object properties and class methods

feat(eslint-plugin): [explicit-function-return-type] Change to allowe…

2728975

…dNames to work for object properties and class methods

fix(eslint-plugin): [explicit-function-return-type] fix for codecov

690986d

fix(eslint-plugin): [explicit-function-return-type] add test

1e38d1d

fix(eslint-plugin): [explicit-function-return-type] add test

d8d39e6

fix(eslint-plugin): [explicit-function-return-type] fix for codecov

39d6715

islandryu requested a review fromJoshuaKGoldberg

January 29, 2022 14:14

Copy link

ContributorAuthor

islandryu commentedJan 29, 2022

@JoshuaKGoldberg
Thank you for review!
I resolved conflict and improved coverage.

I apologize for the large number of commits.

Copy link

Member

JoshuaKGoldberg commentedJan 29, 2022•
edited
Loading

Large number of commits is no problem 😄, I do that all the time. We squash PR merges anyway and most PRs -including this one- are a small enough number of changes that we don't particularly need a Git walkthrough. Thanks!

JoshuaKGoldberg previously approved these changes

Jan 29, 2022

View reviewed changes

Copy link

Member

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Super, thanks so much@islandryu! 🚀

Merge branch 'main' into featAddallowedNames

a79ac5f

JoshuaKGoldbergenabled auto-merge (squash)

January 29, 2022 15:28

Merge branch 'main' into featAddallowedNames

8d66aa2

Copy link

ContributorAuthor

islandryu commentedJan 31, 2022

@JoshuaKGoldberg
Sorry, I updated the PR by mistake.
Is there anything left for me to do regarding this PR?

islandryu requested a review fromJoshuaKGoldberg

January 31, 2022 11:51

Copy link

Member

JoshuaKGoldberg commentedJan 31, 2022

Nope, it's ready to merge!

I don't have the ability to merge on a failing build and the Netlify deployment is nondeterministically failing. cc@bradzacher

Copy link

Member

bradzacher commentedJan 31, 2022

@JoshuaKGoldberg - I've removed netlify from the "required" list of checks, so you should be able to merge past that signal from now on!

bradzacherdisabled auto-merge

January 31, 2022 18:33

bradzacher requested changes

Jan 31, 2022

View reviewed changes

Copy link

Member

bradzacher left a comment

Choose a reason for hiding this comment

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

just one small comment about correctness - otherwise great work!

packages/eslint-plugin/src/rules/explicit-function-return-type.ts Outdated

		caseAST_NODE_TYPES.MethodDefinition:
		caseAST_NODE_TYPES.PropertyDefinition:
		caseAST_NODE_TYPES.Property:{
		if(parent.key.type===AST_NODE_TYPES.Identifier){

Copy link

Member

bradzacherJan 31, 2022

Choose a reason for hiding this comment

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

one minor problem with this code:

constignoredName='notIgnoredName';classFoo{[ignoredName](){}}

This is valid code! Which obviously shouldnot be ignored.
Only checking for.key.type === 'Identifier' will match this case.
You should also assert that.computed === false.

cc@JoshuaKGoldberg as an FYI - this is a really common case that everyone (including me!) always forgets for properties of all kinds AND for member expressions as well.
I raised the issue with the estree spec but the outcome there was really "spec can't change".

Copy link

Member

JoshuaKGoldbergJan 31, 2022

Choose a reason for hiding this comment

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

Ah thanks for the spot!

Copy link

ContributorAuthor

islandryuFeb 1, 2022

Choose a reason for hiding this comment

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

Thank you!
I didn't know such a pattern existed.
@bradzacher
I changed the computed name so that it is not ignored by the allowedName.

islandryu added2 commits

February 1, 2022 17:20

fix(eslint-plugin): [explicit-function-return-type] Change allowedNam…

545b2ab

…e to not ignore computed property

Merge branch 'featAddallowedNames' ofhttps://github.com/islandryu/ty…

65103ff

…pescript-eslint into featAddallowedNames

islandryu dismissedJoshuaKGoldberg’sstale review via65103ff

February 1, 2022 08:23

islandryu requested a review frombradzacher

February 1, 2022 08:34

bradzacher approved these changes

Feb 2, 2022

View reviewed changes

Copy link

Member

bradzacher left a comment

Choose a reason for hiding this comment

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

nice one! LGTM

bradzacher merged commit936e252 intotypescript-eslint:main

Feb 2, 2022

islandryu deleted the featAddallowedNames branch

February 9, 2022 13:25

lonyele pushed a commit to lonyele/typescript-eslint that referenced this pull request

Feb 12, 2022

feat(eslint-plugin): [explicit-function-return-type] add allowedNames (…

aca65f3

…typescript-eslint#4440)* feat(eslint-plugin): [explicit-function-return-type] add allowedNames option* feat(eslint-plugin): [explicit-function-return-type] afix typecheck* Update packages/eslint-plugin/src/rules/explicit-function-return-type.tsCo-authored-by: Josh Goldberg <me@joshuakgoldberg.com>* feat(eslint-plugin): [explicit-function-return-type] Change to allowedNames to work for object properties and class methods* feat(eslint-plugin): [explicit-function-return-type] Change to allowedNames to work for object properties and class methods* fix(eslint-plugin): [explicit-function-return-type] fix for codecov* fix(eslint-plugin): [explicit-function-return-type] add test* fix(eslint-plugin): [explicit-function-return-type] add test* fix(eslint-plugin): [explicit-function-return-type] fix for codecov* fix(eslint-plugin): [explicit-function-return-type] Change allowedName to not ignore computed propertyCo-authored-by: Josh Goldberg <me@joshuakgoldberg.com>Co-authored-by: Josh Goldberg <joshuakgoldberg@outlook.com>

crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request

Feb 16, 2022

Update typescript-eslint monorepo to v5.12.0 (#1157)

ed8f8ea

This PR contains the following updates:| Package | Type | Update | Change ||---|---|---|---|| [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.10.2` -> `5.12.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.10.2/5.12.0) || [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.10.2` -> `5.12.0`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.10.2/5.12.0) |---### Release Notes<details><summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/eslint-plugin)</summary>### [`v5.12.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5120-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5110v5120-2022-02-14)[Compare Source](typescript-eslint/typescript-eslint@v5.11.0...v5.12.0)##### Bug Fixes-   **eslint-plugin:** \[init-declarations] fix nested namespace ([#&#8203;4544](typescript-eslint/typescript-eslint#4544)) ([fe910e6](typescript-eslint/typescript-eslint@fe910e6))-   **eslint-plugin:** \[no-unnecessary-type-arguments] Use Symbol to check if it's the same type ([#&#8203;4543](typescript-eslint/typescript-eslint#4543)) ([5b7d8df](typescript-eslint/typescript-eslint@5b7d8df))-   support nested object deconstructuring with type annotation ([#&#8203;4548](typescript-eslint/typescript-eslint#4548)) ([4da9278](typescript-eslint/typescript-eslint@4da9278))##### Features-   add checking property definition for allowNames option ([#&#8203;4542](typescript-eslint/typescript-eslint#4542)) ([e32bef6](typescript-eslint/typescript-eslint@e32bef6))### [`v5.11.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#&#8203;5110-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5102v5110-2022-02-07)[Compare Source](typescript-eslint/typescript-eslint@v5.10.2...v5.11.0)##### Bug Fixes-   **eslint-plugin:** \[no-magic-numbers] fix invalid schema merging ([#&#8203;4517](typescript-eslint/typescript-eslint#4517)) ([b95f796](typescript-eslint/typescript-eslint@b95f796))-   **eslint-plugin:** \[non-nullable-type-assertion-style] fix false positive when asserting to a generic type that might be nullish ([#&#8203;4509](typescript-eslint/typescript-eslint#4509)) ([4209362](typescript-eslint/typescript-eslint@4209362))##### Features-   **eslint-plugin:** \[explicit-function-return-type] add allowedNames ([#&#8203;4440](typescript-eslint/typescript-eslint#4440)) ([936e252](typescript-eslint/typescript-eslint@936e252))#### [5.10.2](typescript-eslint/typescript-eslint@v5.10.1...v5.10.2) (2022-01-31)##### Bug Fixes-   **eslint-plugin:** \[no-restricted-imports] allow relative type imports with patterns configured ([#&#8203;4494](typescript-eslint/typescript-eslint#4494)) ([4a6d217](typescript-eslint/typescript-eslint@4a6d217))#### [5.10.1](typescript-eslint/typescript-eslint@v5.10.0...v5.10.1) (2022-01-24)**Note:** Version bump only for package [@&#8203;typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/eslint-plugin)</details><details><summary>typescript-eslint/typescript-eslint (@&#8203;typescript-eslint/parser)</summary>### [`v5.12.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5120-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5110v5120-2022-02-14)[Compare Source](typescript-eslint/typescript-eslint@v5.11.0...v5.12.0)**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)### [`v5.11.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#&#8203;5110-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5102v5110-2022-02-07)[Compare Source](typescript-eslint/typescript-eslint@v5.10.2...v5.11.0)**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)#### [5.10.2](typescript-eslint/typescript-eslint@v5.10.1...v5.10.2) (2022-01-31)**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)#### [5.10.1](typescript-eslint/typescript-eslint@v5.10.0...v5.10.1) (2022-01-24)**Note:** Version bump only for package [@&#8203;typescript-eslint/parser](https://github.com/typescript-eslint/parser)</details>---### Configuration📅 **Schedule**: At any time (no schedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found.🔕 **Ignore**: Close this PR and you won't be reminded about these updates again.--- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.---This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).Co-authored-by: cabr2-bot <cabr2.help@gmail.com>Co-authored-by: crapStone <crapstone@noreply.codeberg.org>Reviewed-on:https://codeberg.org/Calciumdibromid/CaBr2/pulls/1157Reviewed-by: crapStone <crapstone@noreply.codeberg.org>Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

github-actionsbot locked asresolvedand limited conversation to collaborators

May 26, 2022

Labels

awaiting response

Issues waiting for a reply from the OP or another party

enhancement: plugin rule option

New rule option for an existing eslint-plugin rule

Movatterモバイル変換

Uh oh!

feat(eslint-plugin): [explicit-function-return-type] add allowedNames#4440

feat(eslint-plugin): [explicit-function-return-type] add allowedNames#4440

Conversation

islandryu commentedJan 14, 2022• edited by bradzacherLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

nx-cloudbot commentedJan 14, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

typescript-eslintbot commentedJan 14, 2022

Uh oh!

netlifybot commentedJan 14, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

codecovbot commentedJan 14, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

islandryu commentedJan 29, 2022

Uh oh!

JoshuaKGoldberg commentedJan 29, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Uh oh!

islandryu commentedJan 31, 2022

Uh oh!

JoshuaKGoldberg commentedJan 31, 2022

Uh oh!

bradzacher commentedJan 31, 2022

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

bradzacherJan 31, 2022

Choose a reason for hiding this comment

Uh oh!

JoshuaKGoldbergJan 31, 2022

Choose a reason for hiding this comment

Uh oh!

islandryuFeb 1, 2022

Choose a reason for hiding this comment

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

islandryu commentedJan 14, 2022•
edited by bradzacher
Loading

nx-cloudbot commentedJan 14, 2022•
edited
Loading

netlifybot commentedJan 14, 2022•
edited
Loading

codecovbot commentedJan 14, 2022•
edited
Loading

JoshuaKGoldberg commentedJan 29, 2022•
edited
Loading