Eslint: fix ignored files

[simison]

Split out from another PR adding similar mechanism, see #48487 (comment)

Proposed changes

• Fixes lint issues in "excluded from lint" files; most were simple JSDoc lint issues
• Removes the tooling for managing tools/eslint-excludelist.json which excluded specific files from the linter. In Eslint: adopt @wordpress/use-recommended-components #48487 and Lint: use recommended components instead of deprecated Jetpack components #49284 we're adding back a mechanism that lists ignored files with counters for specific issues, which is helpful for large-scale slower migrations like componentry. Some code from this PR might make its way to above PRs for CI integration if it makes sense.

Related product discussion/links

Does this pull request change what data or activity we track or use?

Testing instructions

• CI should pass

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WoA dev site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin (Jetpack), and enable the update/eslint-fixes branch.
• To test on Simple, run the following command on your sandbox:

bin/jetpack-downloader test jetpack update/eslint-fixes

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Review, ...).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

---

Jetpack plugin:

No scheduled milestone found for this plugin.

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

[jp-launch-control]

Code Coverage Summary

This PR did not change code coverage!

That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷

Full summary · PHP report · JS report

[anomiex]

Fixes lint issues in "excluded from lint" files; most were simple JSDoc lint issues

This is good, thank you!

Removes the tooling for managing tools/eslint-excludelist.json which excluded specific files from the linter.

This fundamentally misunderstands what that tooling does. What you're removing here does a better job at the task than the replacement you're pushing in #48487.

I recommend you re-read the code you're removing from .github/workflows/linting.yml lines 410–422.

[simison]

Removes the tooling for managing tools/eslint-excludelist.json which excluded specific files from the linter.

This fundamentally misunderstands what that tooling does. What you're removing here does a better job at the task than the replacement you're pushing in #48487.

I recommend you re-read the code you're removing from .github/workflows/linting.yml lines 410–422.

Can you elaborate which mechanisms/aspects you'd prefer to keep running in this PR?

As I wrote in the description, I'm intending to add back parts in the other PR that make sense, but I'm splitting these changes into two separate PRs. If you'd prefer, I can merge everything into one (likely prune this down to just lint fix + empty [] as exclude list, so my plan will be a bit more visible?

[anomiex]

Your fundamental misunderstanding is in thinking that tools/eslint-excludelist.json excludes specific files from the linter. That is not what it does.

We have two JS linting runs. One lints files normally, except those listed in tools/eslint-excludelist.json. The second specifically lints files that are listed in tools/eslint-excludelist.json using @automattic/eslint-changed rather than vanilla eslint.

eslint-changed compares the linting of the old and new versions of the file, reporting any lints that are newly added and any lints that are present on lines being changed in the PR. This has advantages and disavantages compared to the "baseline file" approach you're pushing in your other PR.

• eslint-changed will flag existing issues in code that is being touched anyway, encouraging devs to fix them while touching the code. The baseline approach does not, as long as the count of issues doesn't change.
• eslint-changed will flag existing issues only in code that is being touched anyway. The baseline approach re-flags all issues if the count increases.
• eslint-changed does not complain when an existing issue is fixed (although our CI does call for cleanup when all issues in a file are fixed). The baseline approach wants an update every time any issue is fixed.
• OTOH, eslint-changed won't notice if an update of an eslint plugin or an eslint config change adds new issues to an unchanged file (because the file itself isn't changed), while the baseline approach would.
• Also, eslint-changed will be slightly slower, as it has to lint each changed file twice.

I think the advantages of the eslint-changed approach outweigh the disadvantages, particularly when most files get the "just run normal eslint" happy path to avoid all the disadvantages.

Can you elaborate which mechanisms/aspects you'd prefer to keep running in this PR?

I'd prefer keeping the code for running eslint-changed, even with an empty exclusion list, so it's there if we need to use it again. For example, for your PR that enables @wordpress/use-recommended-components without fixing the issues.

[simison]

Aight, thanks for elaborating! It was helpful since it wasn't clear from code.

So I can trim this PR down to fixing the lints, and then rework the other PRs to use the existing list. I don't really have strong opinion on how you lint the repo, I care more about having the lint in first place to guide right component usage. ;-)

addressed

[anomiex]

Looks reasonable, docs read ok. I don't know enough about any of this code to know if there are errors in the added docs though. 😅