refactor(rome_js_analyze): improve noDelete

[Conaclos]

Summary

This improves noDelete in several ways:

• Better documentation and diagnostics

• Add comments to understand implementation decisions

• Avoid recursion

• Avoid the introduction of a new enum type

• Relax the rule like discussed here

  This no longer rejects the following code:

  delete record[computed]

  Following code is still rejected:

  delete record["literal"]
  delete record[0]
  delete record.a

• Report delete on optional chains

  Previously the rule didn't report the following code:

  delete a?.b;
  delete a?.['b'];

  There are no explanation why it is not rejected.
  I think it is not rejected because delete can be without effect when the property is missing.

  The rule now reports delete on optional chains.

Test Plan

Updated and extended.

Documentation

Updated and improved.

• The PR requires documentation
• I will create a new PR to update the documentation

[netlify]

✅ Deploy Preview for docs-rometools ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	34d6799
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rometools/deploys/640a0bffd7790f00084e5d17
😎 Deploy Preview	https://deploy-preview-4272--docs-rometools.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 settings.

[Conaclos]

TypeScript ESLint has a rule in its strict preset that disallows delete on computed keys: no-dynamic-delete. This conflicts with the proposal I made of relaxing the rule.

Another possibility could be to keep the rule strict by rejecting delete on computed keys and removing the rule from the recommended preset.

[Conaclos]

TypeScript ESLint has a rule in its strict preset that disallows delete on computed keys: no-dynamic-delete. This conflicts with the proposal I made of relaxing the rule.

Another possibility could be to keep the rule strict by rejecting delete on computed keys and removing the rule from the recommended preset.

@ematipico What do you think about the second option: keeping the rule strict and removing the rule from the recommended preset?

In the meantime, I can remove the relaxing in order to ship the other improvements of the rule.

[ematipico]

Personally, I would keep it recommended but relax the logic. ESLint rule is very relaxed: https://eslint.org/docs/latest/rules/no-delete-var

[Conaclos]

Personally, I would keep it recommended but relax the logic. ESLint rule is very relaxed: https://eslint.org/docs/latest/rules/no-delete-var

In fact noDeelet and no-delete-var are different. no-delete-var reports only delete on variables that is undefined in JavaScript.
In my view we should create another rule noUselessDelete that reports delete on variables and expressions such as delete f() or extend noDelete to check for that.

[ematipico]

IMHO, we could relax the rule. It seems users are bothered by its strictness, not by having it in the recommended set.