linter: `jsdoc/require-property`

[JakobJingleheimer]

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?

.oxlintrc.json5

{
	"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 erroneously errors when the @prop shorthand is used

  × eslint-plugin-jsdoc(require-property): The `@typedef` and `@namespace` tags must include a `@property` tag with the type Object.
    ╭─[packages/media/media.loader.mjs:46:4]
 45 │  *
 46 │  * @typedef {object} MediaExtensionAddRemoveConfig
    ·    ─────────────────
 47 │  * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
    ╰────
  help: Consider adding a `@property` tag or replacing it with a more specific type.

[camc314]

can you supply the shape of your .oxlintrc.json ?

https://github.com/oxc-project/oxc/blob/03a40df31b9fd58646c963351a745529d04a6c53/crates/oxc_linter/src/rules/jsdoc/require_property.rs#L144-L159

i believe we follow the same as the eslint rule, which, when settings is provided does not report this error

[JakobJingleheimer]

can you supply the shape of your .oxlintrc.json ?

It's in the OP within the <details>

I'll try manually setting the alias, thanks 🙂

[camc314]

ah didn't see that - thanks

[camc314]

yeah so adding:

    "settings": {
        "jsdoc": {
            "tagNamePreference": {
                "property": "prop"
            }
        }
    }

to the oxlint config hides the warning for me.

let me know if it's the same and this can be closed.

Also, were you using eslint before? was the issue present there? did you have settings there?

[camchenry]

I think this is working as expected, unless we want to diverge on the behavior here from ESLint. I just tried this out of the box with [email protected] and [email protected] with the following eslint.config.js:

import jsdoc from "eslint-plugin-jsdoc";

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

export default config;

and the output was:

  1:1  warning  Missing JSDoc @property                                                   jsdoc/require-property
  3:1  warning  Invalid JSDoc tag (preference). Replace "prop" JSDoc tag with "property"  jsdoc/check-tag-names

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

for this file:

/**
 * @typedef {object} SomeTypedef
 * @prop {string} propName Prop description
 */

Although, I don't feel like this is a very productive default to me (if I were designing this plugin), since it seems like it is just emitting errors needlessly. But I'm not sure it's worth diverging on the behavior here currently.

[JakobJingleheimer]

Adding that bit in "settings" does work, thanks!

Although, I don't feel like this is a very productive default to me (if I were designing this plugin), since it seems like it is just emitting errors needlessly.

Hmm, yes. I would expect official aliases to JustWork™; IMO only something nonstandard should require configuration.

But I'm not sure it's worth diverging on the behavior here currently.

If oxlint supports the official aliases out of the box, I think that's not exactly divergent: something from eslint doesn't stop working in oxlint (someone could still include those "settings" config—that would just do nothing because it already works). But if you want 1-to-1, then yes, don't improve on eslint, but I think do document it (probably in jsdoc/check-type-names and then link to that from the related rules).

If you prefer the documentation approach, I can do.

I could maybe add out-of-the-box support for the aliases too, but I've used Rust only a little bit.

There's also a third option: get eslint to support the aliases out-of-the-box too (and then it's non-divergent). I know one of the maintainers, so I could ask him about how they'd feel about it.

[camc314]

i'm back to looking at this again, so let me summarize.

Given the following JS:

/**
 * @typedef {Array<string>|Set<string>} FileExtensionsList
 *
 * @typedef {object} MediaExtensionAddRemoveConfig
 * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
 * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.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.
 */
/**
 * @type {import('node:module').InitializeHook}
 * @param {MediaExtensionAddRemoveConfig|MediaExtensionReplacementConfig} config Data to configure media loader file extensions.
 */
function initialiseMedia(config) {
	if (config == null) return;

	if (isList(config)) {
		exts.clear();
		for (const r of /** @type {Iterable} */ (config)) exts.add(r);
	}

	if ('additions' in config && isList(config.additions)) {
		for (const a of config.additions) exts.add(a);
	}
	if ('deletions' in config && isList(config.deletions)) {
		for (const d of config.deletions) exts.delete(d);
	}
}

and the default oxlint configuration.

The following currently outputs from oxc:

  × eslint-plugin-jsdoc(require-property): The `@typedef` and `@namespace` tags must include a `@property` tag with the type Object.
    ╭─[packages/media/media.loader.mjs:47:4]
 46 │  *
 47 │  * @typedef {object} MediaExtensionAddRemoveConfig
    ·    ─────────────────
 48 │  * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
    ╰────
  help: Consider adding a `@property` tag or replacing it with a more specific type.

A workaround to this issue is to specify the following in your .oxlintrc.json

    "settings": {
        "jsdoc": {
            "tagNamePreference": {
                "property": "prop"
            }
        }
    }

This behaviour aligns with eslint-plugin-jsdoc.

However, it does appear that typescript handles both cases: https://github.com/microsoft/typescript/blob/5170645f4e4b395891fad823bfe03ac628411afb/src/compiler/parser.ts#L9767-L9770

Given that when using this rule, in combination with check-tag-names, we report that @prop should be replaced with @property I think this is ok

  × eslint-plugin-jsdoc(check-tag-names): Invalid tag name found.
    ╭─[packages/media/media.loader.mjs:49:4]
 48 │  * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.additions A list of file extensions to add to the default list.
 49 │  * @prop {FileExtensionsList} MediaExtensionAddRemoveConfig.deletions A list of file extensions to remove from the default list.
    ·    ─────
 50 │  *
    ╰────
  help: Replace tag `@prop` with `@property`.

In conclution, I think the best solve for this ticket, is to add in documentation about how to customize the @prop tag that this lint rule expects to look for

[JakobJingleheimer]

Works for me! And I see you opened a PR for the doc update; thanks!