fix: missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) #5453

bmish · 2022-08-09T20:54:01Z

PR Checklist

Addresses an existing open issue: fixes Bug: no-unnecessary-type-constraint missing placeholder in suggestion message #5460 fixes Bug: no-unsafe-argument missing placeholders in violation message #5461
That issue was marked as accepting prs
Steps in CONTRIBUTING.md were taken

Overview

Fixes bugs caught by the eslint-plugin/no-missing-placeholders and eslint-plugin/no-unused-placeholders rules:

Enables the recommended config from eslint-plugin-eslint-plugin which catches mistakes and enforces best practices in eslint plugins. Note that I have been actively making fixes to eslint-plugin-eslint-plugin to eliminate any false positives with its rules.

If desired, I can extract these bug fixes into separate PRs to go in ahead of the rest of the eslint-plugin-eslint-plugin additions.

…pe-constraint and no-unsafe-argument (and enable eslint-plugin/recommended rules internally)

typescript-eslint · 2022-08-09T20:54:02Z

Thanks for the PR, @bmish!

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.

nx-cloud · 2022-08-09T20:54:03Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ec3fb4b. 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

✅ Successfully ran 47 targets

Sent with 💌 from NxCloud.

netlify · 2022-08-09T20:54:08Z

✅ Deploy Preview for typescript-eslint ready!

Name	Link
🔨 Latest commit	`ec3fb4b`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/62f5112688f7de00085d9c36
😎 Deploy Preview	https://deploy-preview-5453--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 settings.

bmish · 2022-08-09T20:54:34Z

.eslintrc.js

@@ -194,7 +195,7 @@ module.exports = {
        '@typescript-eslint/no-unsafe-call': 'off',
        '@typescript-eslint/no-unsafe-member-access': 'off',
        '@typescript-eslint/no-unsafe-return': 'off',
-        'eslint-plugin/no-identical-tests': 'error',


This rule is now included in the recommended config.

bmish · 2022-08-09T20:54:57Z

packages/eslint-plugin/src/rules/no-duplicate-enum-values.ts

@@ -9,7 +9,7 @@ export default util.createRule({
      description: 'Disallow duplicate enum member values',
      recommended: 'strict',
    },
-    hasSuggestions: true,
+    hasSuggestions: false,


Many of these rules indicated they had suggestions but do not actually provide suggestions.

Caught by the eslint-plugin/require-meta-has-suggestions rule.

bmish · 2022-08-09T20:55:41Z

packages/eslint-plugin/src/rules/no-empty-interface.ts

@@ -15,10 +15,9 @@ export default util.createRule<Options, MessageIds>({
    docs: {
      description: 'Disallow the declaration of empty interfaces',
      recommended: 'error',
-      suggestion: true,


This meta.docs.suggestion property was replaced by meta.hasSuggestions (more info in eslint/eslint#14312). It's confusing to leave the old property here especially when it's incorrectly specified in many of the rules.

bmish · 2022-08-09T20:56:41Z

packages/eslint-plugin/src/rules/no-unnecessary-type-constraint.ts

@@ -89,6 +88,9 @@ export default util.createRule({
          suggest: [
            {
              messageId: 'removeUnnecessaryConstraint',
+              data: {
+                constraint,


This is an actual bug. This data was missing for the suggestion message and would thus show as {{constraint}} to the user instead of the actual value. I updated the tests to test this data.

I can extract this bug fix into a separate PR if desired.

Caught by the eslint-plugin/no-missing-placeholders and eslint-plugin/no-unused-placeholders rules.

bmish · 2022-08-09T20:57:15Z

packages/eslint-plugin/src/rules/no-unsafe-argument.ts

@@ -142,7 +142,7 @@ export default util.createRule<[], MessageIds>({
      unsafeArgument:
        'Unsafe argument of type `{{sender}}` assigned to a parameter of type `{{receiver}}`.',
      unsafeTupleSpread:
-        'Unsafe spread of a tuple type. The {{index}} element is of type `{{sender}}` and is assigned to a parameter of type `{{reciever}}`.',
+        'Unsafe spread of a tuple type. The argument is of type `{{sender}}` and is assigned to a parameter of type `{{receiver}}`.',


This is an actual bug. The {{index}} placeholder was never provided for this message. It also doesn't seem necessary to me as the violation is already reported on the specific argument that it refers to.

I can extract this bug fix into a separate PR if desired.

Caught by the eslint-plugin/no-missing-placeholders and eslint-plugin/no-unused-placeholders rules.

bmish · 2022-08-09T20:57:41Z

packages/eslint-plugin/src/rules/no-unsafe-argument.ts

@@ -142,7 +142,7 @@ export default util.createRule<[], MessageIds>({
      unsafeArgument:
        'Unsafe argument of type `{{sender}}` assigned to a parameter of type `{{receiver}}`.',
      unsafeTupleSpread:
-        'Unsafe spread of a tuple type. The {{index}} element is of type `{{sender}}` and is assigned to a parameter of type `{{reciever}}`.',
+        'Unsafe spread of a tuple type. The argument is of type `{{sender}}` and is assigned to a parameter of type `{{receiver}}`.',


This is an actual bug. There is a typo in the receiver placeholder name.

I can extract this bug fix into a separate PR if desired.

Caught by the eslint-plugin/no-missing-placeholders and eslint-plugin/no-unused-placeholders rules.

JoshuaKGoldberg

Wow, I had no idea eslint-plugin-eslint-plugin (😂 what a name!) was so powerful! Fantastic stuff this PR brings in. Thanks for sending it over!

LGTM, just a few notes on filing & linking issues. I'm down to help fill those requests in if it'd be helpful!

packages/eslint-plugin/tests/rules/consistent-indexed-object-style.test.ts

JoshuaKGoldberg · 2022-08-10T00:57:29Z

.eslintrc.js

@@ -194,7 +195,7 @@ module.exports = {
        '@typescript-eslint/no-unsafe-call': 'off',
        '@typescript-eslint/no-unsafe-member-access': 'off',
        '@typescript-eslint/no-unsafe-return': 'off',
-        'eslint-plugin/no-identical-tests': 'error',
+        'eslint-plugin/consistent-output': 'off',


Out of curiosity, why is this still enabled in plugin:eslint-plugin/recommended? Is it just a legacy thing?

I disabled eslint-plugin/consistent-output for two reasons:

There are 100 violations so we could follow-up to enable it in a separate PR if we want to.

I think it's less useful now that ESLint v7 validates that test cases provide output when they produce an autofix, and it's tedious to include output: null when not necessary. It might be removed as recommended eventually.

Heh, yeah, now that ESLint is already at 8.21.0, it feels like removing from recommended is the right step 😄

Could you just add a todo comment around this line with a link to an issue? Either one on this repo if you think we should enable it, or one on the eslint-plugin repo if you think it should be removed from recommended?

Filed and added a link to this issue: eslint-community/eslint-plugin-eslint-plugin#284

JoshuaKGoldberg · 2022-08-10T00:58:10Z

packages/eslint-plugin/src/rules/no-unsafe-call.ts

@@ -1,3 +1,7 @@
+/* eslint-disable eslint-comments/no-use */
+/* eslint eslint-plugin/no-unused-message-ids:"off" -- disabled because the rule doesn't understand our helper function checkCall() */


We generally like filing issues when a bug is found, both to get a link to a full discussion & to help drive to fixing it. Do you have the bandwidth to file one so we can link it here?

(here and the other disable)

I went ahead and filed these bugs to capture the two @typescript-eslint/eslint-plugin bugs I'm fixing:

Bug: no-unnecessary-type-constraint missing placeholder in suggestion message #5460

Bug: no-unsafe-argument missing placeholders in violation message #5461

I also filed and added links for the remaining places a rule had to be disabled:

False positive with no-unused-message-ids when helper function parameter reassignment present eslint-community/eslint-plugin-eslint-plugin#282

False positive with no-unused-message-ids when violation reporting is done with external helper function eslint-community/eslint-plugin-eslint-plugin#283

Awesome, thanks! ✨

packages/eslint-plugin/src/rules/object-curly-spacing.ts

JoshuaKGoldberg

This is fantastic work, thanks so much for sending it in! 👏

Just requesting changes on adding some comments around disables, to help us track removing them over time. Otherwise I think this is good to ship!

JoshuaKGoldberg

Glorious, thanks for introducing eslint-plugin-eslint-plugin to the repo (and me)! Let's get these fixes in 💪

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | patch | [`5.33.0` -> `5.33.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.33.0/5.33.1) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | patch | [`5.33.0` -> `5.33.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.33.0/5.33.1) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.33.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5331-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5330v5331-2022-08-15) [Compare Source](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) ##### Bug Fixes - missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) ([#5453](typescript-eslint/typescript-eslint#5453)) ([d023910](typescript-eslint/typescript-eslint@d023910)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.33.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5331-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5330v5331-2022-08-15) [Compare Source](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] 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> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1509 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.33.1` -> `5.35.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2feslint-plugin/5.33.1/5.35.1) | | [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint) | devDependencies | minor | [`5.33.1` -> `5.35.1`](https://renovatebot.com/diffs/npm/@typescript-eslint%2fparser/5.33.1/5.35.1) | --- ### Release Notes <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/eslint-plugin)</summary> ### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5351-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5350v5351-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) ##### Bug Fixes - **eslint-plugin:** correct rule schemas to pass ajv validation ([#5531](typescript-eslint/typescript-eslint#5531)) ([dbf8b56](typescript-eslint/typescript-eslint@dbf8b56)) ### [`v5.35.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5350-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5340v5350-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.34.0...v5.35.0) ##### Features - **eslint-plugin:** \[explicit-member-accessibility] suggest adding explicit accessibility specifiers ([#5492](typescript-eslint/typescript-eslint#5492)) ([0edb94a](typescript-eslint/typescript-eslint@0edb94a)) ### [`v5.34.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#5340-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5331v5340-2022-08-22) [Compare Source](typescript-eslint/typescript-eslint@v5.33.1...v5.34.0) ##### Bug Fixes - **eslint-plugin:** \[no-useless-constructor] handle parameter decorator ([#5450](typescript-eslint/typescript-eslint#5450)) ([864dbcf](typescript-eslint/typescript-eslint@864dbcf)) ##### Features - **eslint-plugin:** \[prefer-optional-chain] support suggesting `!foo || !foo.bar` as a valid match for the rule ([#5266](typescript-eslint/typescript-eslint#5266)) ([aca935c](typescript-eslint/typescript-eslint@aca935c)) #### [5.33.1](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) (2022-08-15) ##### Bug Fixes - missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) ([#5453](typescript-eslint/typescript-eslint#5453)) ([d023910](typescript-eslint/typescript-eslint@d023910)) </details> <details> <summary>typescript-eslint/typescript-eslint (@typescript-eslint/parser)</summary> ### [`v5.35.1`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5351-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5350v5351-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.35.0...v5.35.1) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) ### [`v5.35.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5350-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5340v5350-2022-08-24) [Compare Source](typescript-eslint/typescript-eslint@v5.34.0...v5.35.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) ### [`v5.34.0`](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/parser/CHANGELOG.md#5340-httpsgithubcomtypescript-eslinttypescript-eslintcomparev5331v5340-2022-08-22) [Compare Source](typescript-eslint/typescript-eslint@v5.33.1...v5.34.0) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) #### [5.33.1](typescript-eslint/typescript-eslint@v5.33.0...v5.33.1) (2022-08-15) **Note:** Version bump only for package [@typescript-eslint/parser](https://github.com/typescript-eslint/parser) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [x] 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: Epsilon_02 <epsilon_02@noreply.codeberg.org> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1519 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

fix: missing placeholders in violation messages for no-unnecessary-ty…

6243463

…pe-constraint and no-unsafe-argument (and enable eslint-plugin/recommended rules internally)

add bug link

c4265a7

bmish mentioned this pull request Aug 9, 2022

fix: missing placeholders in violation messages for no-unnecessary-type-constraint and no-unsafe-argument (and enable eslint-plugin/recommended rules internally) #5414

Closed

bmish commented Aug 9, 2022

View reviewed changes

JoshuaKGoldberg previously approved these changes Aug 10, 2022

View reviewed changes

add bug links for eslint-plugin/no-unused-message-ids rule

2ec6bb8

bmish dismissed JoshuaKGoldberg’s stale review via 2ec6bb8 August 10, 2022 15:25

bmish requested a review from JoshuaKGoldberg August 10, 2022 18:23

JoshuaKGoldberg requested changes Aug 10, 2022

View reviewed changes

JoshuaKGoldberg added the awaiting response Issues waiting for a reply from the OP or another party label Aug 10, 2022

add comment about eslint-plugin/consistent-output

ab5d4d5

bmish requested a review from JoshuaKGoldberg August 11, 2022 14:21

JoshuaKGoldberg approved these changes Aug 11, 2022

View reviewed changes

Merge branch 'main' into eslint-plugin-eslint-plugin

ec3fb4b

JoshuaKGoldberg enabled auto-merge (squash) August 11, 2022 14:24

JoshuaKGoldberg merged commit d023910 into typescript-eslint:main Aug 11, 2022

bmish mentioned this pull request Aug 17, 2022

chore: update eslint-plugin-eslint-plugin to 5.0.5 #5493

Merged

3 tasks

takurinton mentioned this pull request Aug 17, 2022

chore(deps-dev): bump @typescript-eslint/parser from 5.31.0 to 5.33.1 voyagegroup/ingred-ui#935

Merged

github-actions bot locked as resolved and limited conversation to collaborators Sep 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) #5453

fix: missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) #5453

bmish commented Aug 9, 2022 •

edited by JoshuaKGoldberg

typescript-eslint bot commented Aug 9, 2022

nx-cloud bot commented Aug 9, 2022 •

edited

netlify bot commented Aug 9, 2022 •

edited

bmish Aug 9, 2022

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

JoshuaKGoldberg left a comment

JoshuaKGoldberg Aug 10, 2022

bmish Aug 10, 2022

JoshuaKGoldberg Aug 10, 2022

bmish Aug 10, 2022

JoshuaKGoldberg Aug 10, 2022

bmish Aug 10, 2022

JoshuaKGoldberg Aug 10, 2022

JoshuaKGoldberg left a comment

JoshuaKGoldberg left a comment

		@@ -1,3 +1,7 @@
		/* eslint-disable eslint-comments/no-use */
		/* eslint eslint-plugin/no-unused-message-ids:"off" -- disabled because the rule doesn't understand our helper function checkCall() */

fix: missing placeholders in violation messages for no-unnecessary-type-constraint and no-unsafe-argument (and enable eslint-plugin/recommended rules internally) #5453

fix: missing placeholders in violation messages for no-unnecessary-type-constraint and no-unsafe-argument (and enable eslint-plugin/recommended rules internally) #5453

Conversation

bmish commented Aug 9, 2022 • edited by JoshuaKGoldberg

PR Checklist

Overview

typescript-eslint bot commented Aug 9, 2022

nx-cloud bot commented Aug 9, 2022 • edited

☁️ Nx Cloud Report

netlify bot commented Aug 9, 2022 • edited

✅ Deploy Preview for typescript-eslint ready!

Choose a reason for hiding this comment

bmish Aug 9, 2022 • edited

Choose a reason for hiding this comment

bmish Aug 9, 2022 • edited

Choose a reason for hiding this comment

bmish Aug 9, 2022 • edited

Choose a reason for hiding this comment

bmish Aug 9, 2022 • edited

Choose a reason for hiding this comment

bmish Aug 9, 2022 • edited

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

fix: missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) #5453

fix: missing placeholders in violation messages for `no-unnecessary-type-constraint` and `no-unsafe-argument` (and enable `eslint-plugin/recommended` rules internally) #5453

bmish commented Aug 9, 2022 •

edited by JoshuaKGoldberg

nx-cloud bot commented Aug 9, 2022 •

edited

netlify bot commented Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited

bmish Aug 9, 2022 •

edited