oxc icon indicating copy to clipboard operation
oxc copied to clipboard

linter: `jsdoc/check-tag-names`

Open JakobJingleheimer opened this issue 8 months ago • 2 comments

What version of Oxlint are you using?

0.16.4

What command did you run?

oxlint --config ./.oxlintrc.json5

What does your .oxlintrc.json config file look like?

{
	"ignorePatterns": [
		"**/fixtures/**",
		"**/fixture.*",
		"**/*.snap.cjs",
	],
	"categories": {
		"correctness": "error",
		"perf": "error",
		"pedantic": "error",
		"suspicious": "warn",
	},
	"plugins": [
		"import",
		"jsdoc",
		"jsx-a11y",
		"node",
		"oxc",
		"react-perf",
		"react",
		"recommended",
		"typescript",
		"unicorn",
	],
	"rules": {
		"eslint/no-case-declarations": "off",
		"eslint/no-constructor-return": "off",
		"eslint/require-await": "off",
		"import/max-dependencies": "off",
		"eslint/max-depth": ["error", 3],
		"eslint/max-lines-per-function": ["error", 30],
		"eslint/max-nested-callbacks": ["error", 3],
		"jsdoc/require-returns": "off",
		"jsdoc/require-returns-description": "off",
		"no-console": "error",
		"no-import-type-side-effects": "error",
		"typescript/ban-types": "off",
		"unicorn/explicit-length-check": "off",
		"unicorn/no-typeof-undefined": "off",
	},
	"overrides": [
		{
			"files": [
				"*.spec.mjs",
				"*.spec.jsx",
				"*.test.mjs",
				"*.test.jsx",
			],
			"rules": {
				"max-depth": "off",
				"max-lines-per-function": "off",
				"max-nested-callbacks": "off",
			},
		},
	],
}

What happened?

oxlint does not recognise jsdoc @prop

Synonyms prop

related: https://github.com/oxc-project/oxc/issues/10252

When switched to @property, it erroneously fails with a different error (it validates successfully with every tool I check, including tsc which uses it to build type declaration files):

  × eslint-plugin-jsdoc(check-property-names): No root defined for @property path.
    ╭─[packages/media/media.loader.mjs:48:35]
 46 │  * @typedef {object} MediaExtensionAddRemoveConfig
 47 │  * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
 48 │  * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.deletions A list of file extensions to remove from the default list.
    ·                                   ───────────────────────────────────────
 49 │  *
    ╰────
  help: @property path declaration `MediaExtensionAddRemoveConfig.deletions` appears before any real property

JakobJingleheimer avatar Apr 05 '25 13:04 JakobJingleheimer

Besides the @prop/@property issue (being discussed in #10252), I believe this is consistent with eslint-plugin-jsdoc. Using this file:

/**
 * @typedef {Array<string>|Set<string>} FileExtensionsList
 *
 * @typedef {object} MediaExtensionAddRemoveConfig
 * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
 * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.deletions A list of file extensions to remove from the default list.
 */

with this eslint.config.js:

import jsdoc from "eslint-plugin-jsdoc";

const config = [
  // configuration included in plugin
  jsdoc.configs["flat/recommended"],
];

export default config;

when I run eslint . I get this error:

  5:1  warning  Expected no lines between tags                                                                           jsdoc/tag-lines
  7:1  warning  @property path declaration ("MediaExtensionAddRemoveConfig.additions") appears before any real property  jsdoc/check-property-names

✖ 2 problems (0 errors, 2 warnings)
  0 errors and 1 warning potentially fixable with the `--fix` option.

It seems like tsc handles this no problem and just pretends like you meant to write the following:

/**
 * @typedef {object} MediaExtensionAddRemoveConfig
 * @property {FileExtensionsList} additions A list of file extensions to add to the default list.
 * @property {FileExtensionsList} deletions A list of file extensions to remove from the default list.
 */

I feel like tsc is probably right here, but we're aligned with eslint-plugin-jsdoc at the moment.

camchenry avatar Apr 07 '25 02:04 camchenry

Looking at the jsdoc documentation for property, the first example includes the parent; but then the second example does not. IIR, tsc does not accept it without the parent (the generated type decl files are broken).

EDIT: Ah, so I think there is a situation where the parent would be required: deeply nested properties: @property {number} defaults.treasure.gold. So I think it needs to support the parent, otherwise it's not possible to define those deeply nested props (the only way would be to create a typedef for each level, which could get real tedious).

JakobJingleheimer avatar Apr 07 '25 14:04 JakobJingleheimer

TS ref > https://github.com/microsoft/typescript/blob/6f75783184328087627da983fe30a05fca4f6314/src/compiler/utilitiesPublic.ts#L953-L960

camc314 avatar Apr 29 '25 20:04 camc314

I think we're going to choose to be consistent with jsdoc here. While TypeScript appears to support both, I think (at least for the moment), it makes sense to be stricter, as this will enforce more consistenty across codebases using the rule.

Looking at the jsdoc documentation for property, the first example includes the parent; but then the second example does not. IIR, tsc does not accept it without the parent (the generated type decl files are broken).

I don't believe this is quite right In the original example of this ticket, the TS type is effectvly:

{
    additions: FileExtensionsList;
    deletions: FileExtensionsList;
}

where as in the first example here, the player/level ect are nested under default rather than being top level.

TypeScript effectivly has special handling for @property {type} <name of the type>.topLevelProperty, where if the <name of the type> is the same as the top level property, it collapses it.

Given this, the diff to avoid seeing the error from oxlint in your code is this diff (remove the leading MediaExtensionAddRemoveConfig from the @property jsdoc.

@@ -45,8 +45,8 @@ export { loadMedia as load };
  * @typedef {Array<string>|Set<string>} FileExtensionsList
  *
  * @typedef {object} MediaExtensionAddRemoveConfig
- * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
- * @property {FileExtensionsList} MediaExtensionAddRemoveConfig.deletions A list of file extensions to remove from the default list.
+ * @property {FileExtensionsList} additions A list of file extensions to add to the default list.
+ * @property {FileExtensionsList} deletions A list of file extensions to remove from the default list.
  *
  * @typedef {FileExtensionsList} MediaExtensionReplacementConfig A list of file extensions to REPLACE the default list.
  */

camc314 avatar May 07 '25 17:05 camc314

Ah, you're right: tsc does handle it properly:

/**
 * @typedef {object} MediaExtensionAddRemoveConfig
 * @prop {FileExtensionsList} additions A list of file extensions to add to the default list.
 * @prop {FileExtensionsList} deletions A list of file extensions to remove from the default list.
 */
// media.d.ts
export type MediaExtensionAddRemoveConfig = {
    /**
     * A list of file extensions to add to the default list.
     */
    additions: FileExtensionsList;
    /**
     * A list of file extensions to remove from the default list.
     */
    deletions: FileExtensionsList;
};

JakobJingleheimer avatar May 11 '25 13:05 JakobJingleheimer