feat(eslint-plugin): [no-array-delete] add rule #5852

mahdi-farnia · 2022-10-20T11:03:08Z

PR Checklist

Addresses an existing open issue: Rule proposal: no-array-delete #4432
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

Before:
Using delete operator for arrays in JavaScript leaves an empty value & unchanged length property as the actual index is not deleted, Just its value replaced by empty.

declare const array: any[];
declare const i: number;

delete array[i];

After:

declare const array: any[];
declare const i: number;

array.splice(i, 1);

Closes #4432

I rewrite it again after one month, and i figured out that the last clone is outdated and i cloned the latest repo and just copied & pasted the file. Because of that there is no history available for this project.

Unnecessary Information Removed Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

Rule Is Now Off By Default Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

My bad!😱 Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

This option is unneeded because users can simply turn off the rule

Support for literal method names & better error message for computed and non-computed method names.

First commit before PR

nx-cloud · 2022-10-20T11:03:10Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit bf6b1b6. 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
`nx run-many --target=lint --parallel`

✅ Successfully ran 45 targets

Sent with 💌 from NxCloud.

typescript-eslint · 2022-10-20T11:03:11Z

Thanks for the PR, @mahdi-farnia!

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 visitors per day.

netlify · 2022-10-20T11:03:14Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`04ec031`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/64c6b38a9021530008153a33
😎 Deploy Preview	https://deploy-preview-5852--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

codecov · 2022-10-20T11:18:45Z

Codecov Report

Merging #5852 (8230f09) into main (78a5e62) will increase coverage by 0.04%.
Report is 369 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 8230f09 differs from pull request most recent head 04ec031. Consider uploading reports for the commit 04ec031 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5852      +/-   ##
==========================================
+ Coverage   87.36%   87.41%   +0.04%     
==========================================
  Files         386      387       +1     
  Lines       13190    13217      +27     
  Branches     3867     3870       +3     
==========================================
+ Hits        11524    11553      +29     
+ Misses       1300     1298       -2     
  Partials      366      366

Flag	Coverage Δ
unittest	`87.41% <100.00%> (+0.04%)`	⬆️

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

Files Changed	Coverage Δ
...ackages/eslint-plugin/src/rules/no-array-delete.ts	`100.00% <100.00%> (ø)`

... and 5 files with indirect coverage changes

JoshuaKGoldberg

Thanks for sending this in - I'm excited to see the no-array-delete rule land soon*! 🙌

Requesting changes on some structural smoothing-out and more test cases, as well as splitting out the unrelated code into their own PRs.

Let me know if any of this is unclear - and thanks for working with us on it all @mahdi-farnia!

*sorry it took three weeks to get to the review 😬 - it should be sooner after this!

packages/eslint-plugin/docs/rules/no-in-array.md

packages/eslint-plugin/src/rules/no-array-delete.ts

packages/eslint-plugin/tests/rules/no-array-delete.test.ts

packages/eslint-plugin/src/rules/no-array-delete.ts

JoshuaKGoldberg · 2023-01-26T15:59:03Z

Ping @mahdi-farnia - is this still something you have time to work on? No worries if not, just checking in - thanks!

mahdi-farnia · 2023-01-26T16:02:29Z

Ping @mahdi-farnia - is this still something you have time to work on? No worries if not, just checking in - thanks!

Hi, Mmm... No nothing left hear.
But it seems I forget to correct my mistake 😅

That should be done in a few minutes...👍

JoshuaKGoldberg · 2023-02-19T04:16:52Z

@mahdi-farnia I normally wait till folks re-request review - but it looks like this might be ready for review. Do you want me to take another look? 🙂

mahdi-farnia · 2023-02-19T05:05:11Z

@mahdi-farnia I normally wait till folks re-request review - but it looks like this might be ready for review. Do you want me to take another look? 🙂

Omg, I thought i should wait till you review them and not to mention you again. 🫤

Yes, please 😄

packages/eslint-plugin/src/rules/no-array-delete.ts

mahdi-farnia · 2023-04-07T14:39:55Z

The lint problem resolved, that was due to uninstalled (i don't know why) package eslint-plugin-unicorn.
Because of that i could not figure out linting problems

packages/eslint-plugin/src/rules/no-array-delete.ts

by split if conditions Co-authored-by: Armano <armano2@users.noreply.github.com>

remove extra if statement

mahdi-farnia · 2023-04-07T14:59:20Z

@armano2 thank you so much. your review saved a review and an edit by me.

mahdi-farnia · 2023-04-07T15:02:03Z

Actually this is my first serious contribution to an open-source project.

If you see something strange in my code or something that experts don't do in their code, please don't blame me

JoshuaKGoldberg

🔥 Looks great - very close to merge I think! I did a closer pass and think it's just a few small-ish things blocking review now. Mostly the bug around suggestions.

packages/eslint-plugin/src/rules/no-array-delete.ts

packages/eslint-plugin/docs/rules/no-array-delete.md

packages/eslint-plugin/src/rules/no-array-delete.ts

mahdi-farnia · 2023-07-18T17:21:53Z

Seems this path is too long 😄

I'll apply suggested changes in the next weekend... (or as soon as I can)

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

apply suggested changes

mahdi-farnia · 2023-07-30T19:15:18Z

After all, please help me resolving conflicts with packages/eslint-plugin/src/configs/strict.ts 🙏

JoshuaKGoldberg · 2023-07-31T02:23:34Z

You should be able to run the command mentioned at the top of the file:

// For developers working in the typescript-eslint monorepo:
// You can regenerate it using `yarn generate:configs`

My typical strategy for autogenerated files that are giving me Git merge woes is:

git checkout main -- path/to/file to undo any of my branch's changes to the file
Re-run yarn to reinstall and rebuild everything
Run the command to generate them: here, that's running yarn generate:configs from the packages/eslint-plugin directory

Does that help?

JoshuaKGoldberg · 2023-10-17T19:32:18Z

👋 Hey @mahdi-farnia! Just checking in, is this still something you have time for? No worries if not - I just don't want to leave it hanging.

mahdi-farnia · 2023-10-18T15:43:00Z

👋 Hey @mahdi-farnia! Just checking in, is this still something you have time for? No worries if not - I just don't want to leave it hanging.

I was waiting for you :) (i requested a review)

JoshuaKGoldberg · 2023-10-18T15:47:26Z

Ah, I was under the impression you were waiting for help on resolving the merge conflict with strict.ts. Is that not the case? #5852 (comment) -> #5852 (comment)

JoshuaKGoldberg · 2023-10-19T17:59:15Z

packages/eslint-plugin/tests/rules/no-array-delete.test.ts

+delete multiDimesnional[i][i][i][i][i].someProp;
+    `,
+  ],
+  invalid: [


A few more test cases off the top of my head:

ReadonlyArray<...>

readonly ...[]

Readonly<...[]>

Readonly<Array<...>>

StyleShit · 2023-10-23T17:15:33Z

packages/eslint-plugin/src/rules/no-array-delete.ts

+import { unionTypeParts } from 'tsutils';
+import * as ts from 'typescript';
+
+import * as util from '../util';


util should probably be switched to named imports

mahdi-farnia · 2023-10-23T17:45:32Z

Sorry for late response. I am busy these days. I'll apply every requested changes as soon as I touch my computer. 🙂

JoshuaKGoldberg · 2023-11-17T18:02:45Z

@mahdi-farnia since it's been a month I'm going to go ahead and close this PR to keep our queue small. Totally understand not having time for this - thanks for all the work you did to get it started!

If you end up having time to pick it back up, feel free to send a new PR (or re-open this one if it hasn't been auto-locked).

If anybody else ends up wanting to take on these changes in the interim, please do add a co-author attribution if you use any code from this PR.

Thanks!

mahdi-farnia and others added 13 commits October 19, 2022 00:00

Complete Super Method Checking

6ec086d

I rewrite it again after one month, and i figured out that the last clone is outdated and i cloned the latest repo and just copied & pasted the file. Because of that there is no history available for this project.

Update packages/eslint-plugin/docs/rules/call-super-on-override.md

31be88c

Unnecessary Information Removed Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

Update packages/eslint-plugin/src/rules/call-super-on-override.ts

5138d1d

Rule Is Now Off By Default Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

Update packages/eslint-plugin/src/rules/call-super-on-override.ts

b8444c3

My bad!😱 Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>

Unnecessary ignoreMethods Option Removed

a31ade2

This option is unneeded because users can simply turn off the rule

Support For Literal Method Names

2b65003

Support for literal method names & better error message for computed and non-computed method names.

Merge branch 'typescript-eslint:main' into main

ab81978

Merge branch 'typescript-eslint:main' into main

1fe9429

Merge remote-tracking branch 'refs/remotes/origin/main'

7f588b3

Merge branch 'typescript-eslint:main' into main

35c527c

Resolve Failed CI Tests & TopLevel Option Removed

e95c87f

Rule no-in-array

4e9da68

Rule no-array-delete Completed

75c1979

First commit before PR

bradzacher changed the title ~~feat(no-array-delete): Disallow delete operator for arrays~~ feat(eslint-plugin): [no-array-delete] add rule Oct 24, 2022

bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Oct 24, 2022

Merge branch 'main' into no-array-delete

8d31382

JoshuaKGoldberg requested changes Nov 13, 2022

View reviewed changes

bradzacher added the awaiting response Issues waiting for a reply from the OP or another party label Nov 16, 2022

mahdi-farnia added 2 commits January 26, 2023 20:01

remove unneeded files from other branch

f397a33

remove unneeded files from other branch

c32bb0a

JoshuaKGoldberg self-requested a review February 19, 2023 05:29

JoshuaKGoldberg mentioned this pull request Feb 19, 2023

Docs: Mention re-requesting review in Contributing > Pull Requests #6492

Closed

2 tasks

armano2 reviewed Apr 7, 2023

View reviewed changes

packages/eslint-plugin/src/rules/no-array-delete.ts Outdated Show resolved Hide resolved

armano2 reviewed Apr 7, 2023

View reviewed changes

packages/eslint-plugin/src/rules/no-array-delete.ts Outdated Show resolved Hide resolved

armano2 reviewed Apr 7, 2023

View reviewed changes

packages/eslint-plugin/src/rules/no-array-delete.ts Outdated Show resolved Hide resolved

mahdi-farnia and others added 2 commits April 7, 2023 18:16

reduce amount of type request

a3290ec

by split if conditions Co-authored-by: Armano <armano2@users.noreply.github.com>

apply suggested changes

b5d99e0

remove extra if statement

bradzacher removed the awaiting response Issues waiting for a reply from the OP or another party label May 6, 2023

Update no-array-delete.md

8230f09

JoshuaKGoldberg requested changes Jul 17, 2023

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Jul 17, 2023

mahdi-farnia and others added 2 commits July 21, 2023 15:06

make doc more clearer

2392654

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

fix(no-array-delete): buggy suggestion

04ec031

apply suggested changes

mahdi-farnia requested a review from JoshuaKGoldberg July 30, 2023 19:04

JoshuaKGoldberg added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Oct 17, 2023

JoshuaKGoldberg reviewed Oct 19, 2023

View reviewed changes

StyleShit suggested changes Oct 23, 2023

View reviewed changes

JoshuaKGoldberg removed the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Oct 23, 2023

JoshuaKGoldberg closed this Nov 17, 2023

JoshuaKGoldberg added the stale PRs or Issues that are at risk of being or have been closed due to inactivity for a prolonged period label Nov 17, 2023

github-actions bot locked as resolved and limited conversation to collaborators Nov 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(eslint-plugin): [no-array-delete] add rule #5852

feat(eslint-plugin): [no-array-delete] add rule #5852

mahdi-farnia commented Oct 20, 2022 •

edited by Josh-Cena

Loading

nx-cloud bot commented Oct 20, 2022 •

edited

Loading

typescript-eslint bot commented Oct 20, 2022

netlify bot commented Oct 20, 2022 •

edited

Loading

codecov bot commented Oct 20, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

JoshuaKGoldberg commented Jan 26, 2023

mahdi-farnia commented Jan 26, 2023

JoshuaKGoldberg commented Feb 19, 2023 •

edited

Loading

mahdi-farnia commented Feb 19, 2023

mahdi-farnia commented Apr 7, 2023

mahdi-farnia commented Apr 7, 2023

mahdi-farnia commented Apr 7, 2023

JoshuaKGoldberg left a comment

mahdi-farnia commented Jul 18, 2023

mahdi-farnia commented Jul 30, 2023

JoshuaKGoldberg commented Jul 31, 2023

JoshuaKGoldberg commented Oct 17, 2023

mahdi-farnia commented Oct 18, 2023

JoshuaKGoldberg commented Oct 18, 2023

JoshuaKGoldberg Oct 19, 2023

StyleShit Oct 23, 2023 •

edited

Loading

mahdi-farnia commented Oct 23, 2023

JoshuaKGoldberg commented Nov 17, 2023

feat(eslint-plugin): [no-array-delete] add rule #5852

feat(eslint-plugin): [no-array-delete] add rule #5852

Conversation

mahdi-farnia commented Oct 20, 2022 • edited by Josh-Cena Loading

PR Checklist

Overview

nx-cloud bot commented Oct 20, 2022 • edited Loading

☁️ Nx Cloud Report

typescript-eslint bot commented Oct 20, 2022

netlify bot commented Oct 20, 2022 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

codecov bot commented Oct 20, 2022 • edited Loading

Codecov Report

JoshuaKGoldberg left a comment • edited Loading

Choose a reason for hiding this comment

JoshuaKGoldberg commented Jan 26, 2023

mahdi-farnia commented Jan 26, 2023

JoshuaKGoldberg commented Feb 19, 2023 • edited Loading

mahdi-farnia commented Feb 19, 2023

mahdi-farnia commented Apr 7, 2023

mahdi-farnia commented Apr 7, 2023

mahdi-farnia commented Apr 7, 2023

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

mahdi-farnia commented Jul 18, 2023

mahdi-farnia commented Jul 30, 2023

JoshuaKGoldberg commented Jul 31, 2023

JoshuaKGoldberg commented Oct 17, 2023

mahdi-farnia commented Oct 18, 2023

JoshuaKGoldberg commented Oct 18, 2023

JoshuaKGoldberg Oct 19, 2023

Choose a reason for hiding this comment

StyleShit Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

mahdi-farnia commented Oct 23, 2023

JoshuaKGoldberg commented Nov 17, 2023

mahdi-farnia commented Oct 20, 2022 •

edited by Josh-Cena

Loading

nx-cloud bot commented Oct 20, 2022 •

edited

Loading

netlify bot commented Oct 20, 2022 •

edited

Loading

codecov bot commented Oct 20, 2022 •

edited

Loading

JoshuaKGoldberg left a comment •

edited

Loading

JoshuaKGoldberg commented Feb 19, 2023 •

edited

Loading

StyleShit Oct 23, 2023 •

edited

Loading