Skip to content

chore: added jsdoc/check-tag-names to ESLint config#65660

Merged
typescript-bot merged 6 commits intoDefinitelyTyped:masterfrom
JoshuaKGoldberg:check-tag-names-in-config
Jun 12, 2023
Merged

chore: added jsdoc/check-tag-names to ESLint config#65660
typescript-bot merged 6 commits intoDefinitelyTyped:masterfrom
JoshuaKGoldberg:check-tag-names-in-config

Conversation

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator

@JoshuaKGoldberg JoshuaKGoldberg commented Jun 1, 2023

Context: Part of part of #648, porting old TSLint rules to ESLint equivalents. This finishes migrating no-redundant-jsdoc2 to jsdoc/check-tag-names.

This finishes the work in #65080, #65315, and #65445 by finally checking the rule into the ESLint config. Which I also converted to .cjs so I could add comments about trimming down that big list of allowed tags.

Also adds a stub tsconfig.json so project: true always has a fallback.

@JoshuaKGoldberg JoshuaKGoldberg force-pushed the check-tag-names-in-config branch from 0708693 to cef47d5 Compare June 1, 2023 14:12
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as ready for review June 1, 2023 15:13
@typescript-bot
Copy link
Copy Markdown
Contributor

typescript-bot commented Jun 1, 2023

@JoshuaKGoldberg Thank you for submitting this PR!

This is a live comment which I will keep updated.

This PR touches some part of DefinitelyTyped infrastructure, so a DT maintainer will need to review it. This is rare — did you mean to do this?

8 packages in this PR (and infra files)

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

You can test the changes of this PR in the Playground.

Status

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.


Diagnostic Information: What the bot saw about this PR
{
  "type": "info",
  "now": "-",
  "pr_number": 65660,
  "author": "JoshuaKGoldberg",
  "headCommitOid": "30799962864493f80931672691c03eedac8d9d3c",
  "mergeBaseOid": "748cf8849b6c67026557632fc1fe2ebbbef0700c",
  "lastPushDate": "2023-06-07T21:56:23.000Z",
  "lastActivityDate": "2023-06-12T18:17:09.000Z",
  "mergeOfferDate": "2023-06-12T18:14:54.000Z",
  "mergeRequestDate": "2023-06-12T18:17:09.000Z",
  "mergeRequestUser": "JoshuaKGoldberg",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Critical",
  "pkgInfo": [
    {
      "name": null,
      "kind": "edit",
      "files": [
        {
          "path": ".eslintrc.cjs",
          "kind": "infrastructure"
        },
        {
          "path": ".eslintrc.json",
          "kind": "infrastructure"
        },
        {
          "path": "package.json",
          "kind": "infrastructure"
        },
        {
          "path": "tsconfig.json",
          "kind": "infrastructure"
        }
      ],
      "owners": [],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "ember",
      "kind": "edit",
      "files": [
        {
          "path": "types/ember/test/debug.ts",
          "kind": "test"
        },
        {
          "path": "types/ember/v3/test/debug.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "chriskrycho",
        "jamescdavis",
        "wagenet",
        "dfreeman"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "ember__debug",
      "kind": "edit",
      "files": [
        {
          "path": "types/ember__debug/ember__debug-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/ember__debug/v3/ember__debug-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "chriskrycho",
        "dfreeman",
        "jamescdavis",
        "wagenet"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "gulp-help-doc",
      "kind": "edit",
      "files": [
        {
          "path": "types/gulp-help-doc/gulp-help-doc-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Mikhus"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "openfin",
      "kind": "edit",
      "files": [
        {
          "path": "types/openfin/.eslintrc.json",
          "kind": "package-meta",
          "suspect": "edited"
        }
      ],
      "owners": [
        "chrisbarker",
        "rdepena",
        "whyn07m3",
        "licui3936",
        "tomer-openfin"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "openui5",
      "kind": "edit",
      "files": [
        {
          "path": "types/openui5/sap.f.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/openui5/sap.ui.mdc.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "openui5bot",
        "petermuessig",
        "codeworrior",
        "akudev"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    },
    {
      "name": "react-native",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-native/v0.71/Libraries/ActionSheetIOS/ActionSheetIOS.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.71/Libraries/ReactNative/RootTag.d.ts",
          "kind": "definition"
        },
        {
          "path": "types/react-native/v0.71/Libraries/TurboModule/RCTExport.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "alloy",
        "huhuanming",
        "iRoachie",
        "timwangdev",
        "kamal",
        "alexdunne",
        "swissmanu",
        "bm-software",
        "mvdam",
        "esemesek",
        "mrnickel",
        "souvik-ghosh",
        "nossbigg",
        "saranshkataria",
        "tykus160",
        "jakebloom",
        "ceyhun",
        "mcmar",
        "theohdv",
        "romain-faust",
        "bebebebebe",
        "Naturalclar",
        "chinesedfan",
        "vtolochk",
        "SychevSP",
        "sasurau4",
        "256hz",
        "doumart",
        "drmas",
        "jeremybarbet",
        "ds300",
        "natsathorn",
        "connectdotz",
        "alexeymolchan",
        "alexbrazier",
        "kuasha420",
        "phvillegas",
        "eps1lon",
        "ZihanChen-MSFT",
        "kelset",
        "MateWW",
        "lunaleaps",
        "saadnajmi"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Critical"
    },
    {
      "name": "react-reconciler",
      "kind": "edit",
      "files": [
        {
          "path": "types/react-reconciler/test/ReactFiberHostConfigWithNoTestSelectors.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "Methuselah96",
        "zhanghaocong",
        "mathieudutour"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    },
    {
      "name": "subset-font",
      "kind": "edit",
      "files": [
        {
          "path": "types/subset-font/subset-font-tests.ts",
          "kind": "test"
        }
      ],
      "owners": [
        "omacranger"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Well-liked by everyone"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "sheetalkamat",
      "date": "2023-06-12T18:14:07.000Z",
      "isMaintainer": true
    },
    {
      "type": "stale",
      "reviewer": "sandersn",
      "date": "2023-06-01T20:22:59.000Z",
      "abbrOid": "18e3302"
    }
  ],
  "mainBotCommentID": 1572246033,
  "ciResult": "pass"
}

@typescript-bot
Copy link
Copy Markdown
Contributor

🔔 @JoshuaKGoldberg — there are no owners, but it would still be good if you find someone to review this PR in the next few days, otherwise a maintainer will look at it. (And if you do find someone, maybe even recruit them to be a second owner to make future changes easier...)

Comment thread .eslintrc.cjs
{
// TODO: Some (but not all) of these tags should likely be removed from this list.
// Additionally, some may need to be contributed to eslint-plugin-jsdoc.
definedTags: [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this a list of tags that are used on DT and explicitly allowed, or is it the opposite: tags that are explicitly disallowed (and used to be on DT)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@sandersn
Copy link
Copy Markdown
Contributor

sandersn commented Jun 1, 2023

Here are the 21 packages with failures for a local run on all packages:

  1:Error in ember
 12:Error in ember/v3
 23:Error in ember__debug
 34:Error in ember__debug/v3
 45:Error in gulp-help-doc
 58:Error in ink-table
 69:Error in ink-testing-library
 80:Error in ink-text-input
 91:Error in node/v16
110:Error in node
130:Error in node/v18
150:Error in openfin/v50
296:Error in openfin
434:Error in openui5
449:Error in react-blessed
460:Error in react-native/v0.71
477:Error in react-reconciler
488:Error in subset-font
500:Error in vhtml
511:Error in xmpp__xml
522:Error in yeoman-test

The ember packages all have a tag @ember/debug; looks like a mis-parse which might be hard to avoid.

@DangerBotOSS
Copy link
Copy Markdown

Inspecting the JavaScript source for this package found some properties that are not in the .d.ts files.
The check for missing properties isn't always right, so take this list as advice, not a requirement.

react-reconciler (unpkg)

was missing the following properties:

  1. attemptContinuousHydration
  2. attemptDiscreteHydration
  3. attemptHydrationAtCurrentPriority
  4. attemptSynchronousHydration
  5. batchedUpdates
as well as these 30 other properties...

createComponentSelector, createContainer, createHasPseudoClassSelector, createHydrationContainer, createPortal, createRoleSelector, createTestNameSelector, createTextSelector, deferredUpdates, discreteUpdates, findAllNodes, findBoundingRects, findHostInstance, findHostInstanceWithNoPortals, findHostInstanceWithWarning, flushControlled, flushPassiveEffects, flushSync, focusWithin, getCurrentUpdatePriority, getFindAllNodesFailureDescription, getPublicRootInstance, injectIntoDevTools, isAlreadyRendering, observeVisibleRects, registerMutableSourceForHydration, runWithPriority, shouldError, shouldSuspend, updateContainer

Generated by 🚫 dangerJS against 3079996

Comment thread types/ember/test/debug.ts

/**
* @ember/debug tests
* `@ember/debug` tests
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pending gajus/eslint-plugin-jsdoc#1116, this seemed like the most straightforward resolution.

@typescript-bot
Copy link
Copy Markdown
Contributor

⚠️ There are too many reviewers for this PR change (61). Merging can only be handled by a DT maintainer.

People who would have been pinged chriskrycho jamescdavis wagenet dfreeman Mikhus chrisbarker rdepena whyn07m3 licui3936 tomer-openfin openui5bot petermuessig codeworrior akudev alloy huhuanming iRoachie timwangdev kamal alexdunne swissmanu bm-software mvdam esemesek mrnickel souvik-ghosh nossbigg saranshkataria tykus160 jakebloom ceyhun mcmar theohdv romain-faust bebebebebe Naturalclar chinesedfan vtolochk SychevSP sasurau4 256hz doumart drmas jeremybarbet ds300 natsathorn connectdotz alexeymolchan alexbrazier kuasha420 phvillegas eps1lon ZihanChen-MSFT kelset MateWW lunaleaps saadnajmi Methuselah96 zhanghaocong mathieudutour omacranger

@typescript-bot
Copy link
Copy Markdown
Contributor

@sandersn Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

Note that the test failures in eb565bf are unrelated to this PR. The later commit 3079996 undid changes to those packages. I'd only touched the packages to make sure that they don't have ESLint complaints.

Error in react-blessed
Error: Errors in [email protected] for external dependencies:
Error: ../react/jsx-runtime.d.ts(4,34): error TS2694: Namespace 'React.JSX' has no exported member 'ElementType'.

    at testTypesVersion (/home/runner/work/DefinitelyTyped/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:235:11)
    at runTests (/home/runner/work/DefinitelyTyped/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:197:7)


Error in yeoman-test
Error: 
A module look-up failed, this often occurs when you need to run `npm install` on a dependent module before you can lint.

Before you debug, first try running:

   npm install --prefix /home/runner/work/DefinitelyTyped/DefinitelyTyped/types/inquirer

Then re-run. Full error logs are below.

Errors in [email protected] for external dependencies:
Error: ../inquirer/index.d.ts(16,28): error TS2307: Cannot find module 'rxjs' or its corresponding type declarations.
Error: ../inquirer/lib/prompts/base.d.ts(2,28): error TS2307: Cannot find module 'rxjs' or its corresponding type declarations.
Error: ../inquirer/lib/prompts/editor.d.ts(2,39): error TS2307: Cannot find module 'rxjs' or its corresponding type declarations.
Error: ../inquirer/lib/utils/events.d.ts(2,28): error TS2307: Cannot find module 'rxjs' or its corresponding type declarations.

    at testTypesVersion (/home/runner/work/DefinitelyTyped/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:235:11)
    at runTests (/home/runner/work/DefinitelyTyped/DefinitelyTyped/node_modules/@definitelytyped/dtslint/src/index.ts:197:7)


Error in artillery
Error: Errors in [email protected] for external dependencies:
Error: node_modules/cacheable-request/dist/index.d.ts(3,8): error TS1259: Module '"node:events"' can only be default-imported using the 'esModuleInterop' flag
Error: node_modules/got/dist/source/core/index.d.ts(484,39): error TS2694: Namespace '"/home/runner/work/DefinitelyTyped/DefinitelyTyped/types/artillery/node_modules/cacheable-request/dist/index"' has no exported member 'StorageAdapter'.
Error: node_modules/got/dist/source/core/index.d.ts(747,39): error TS2694: Namespace '"/home/runner/work/DefinitelyTyped/DefinitelyTyped/types/artillery/node_modules/cacheable-request/dist/index"' has no exported member 'StorageAdapter'.
Error: node_modules/got/dist/source/core/index.d.ts(779,39): error TS2694: Namespace '"/home/runner/work/DefinitelyTyped/DefinitelyTyped/types/artillery/node_modules/cacheable-request/dist/index"' has no exported member 'StorageAdapter'.

@typescript-bot typescript-bot added the Self Merge This PR can now be self-merged by the PR author or an owner label Jun 12, 2023
@typescript-bot
Copy link
Copy Markdown
Contributor

@JoshuaKGoldberg: Everything looks good here. I am ready to merge this PR (at 3079996) on your behalf whenever you think it's ready.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

@JoshuaKGoldberg
Copy link
Copy Markdown
Collaborator Author

Ready to merge

@typescript-bot typescript-bot merged commit ac3c8e5 into DefinitelyTyped:master Jun 12, 2023
@JoshuaKGoldberg JoshuaKGoldberg deleted the check-tag-names-in-config branch June 12, 2023 18:18
Desplandis pushed a commit to Desplandis/DefinitelyTyped that referenced this pull request Jul 3, 2023
…o ESLint config by @JoshuaKGoldberg

* chore: added jsdoc/check-tag-names to ESLint config

* Added a stub tsconfig.json

* Temporarily add comment to files to force DT run

* Assorted fixes

* Switch to backticks

* Remove temporary touch tags
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants