In typescript mode, only allow 'object'

[thernstig]

Motivation

TypeScript Do's and Dont's (https://www.typescriptlang.org/docs/handbook/declaration-files/do-s-and-don-ts.html#general-types) says to never use Object (capital O). So when in typescript mode, only ever object should be allowed as default, and not Object.

The reason is that a ton of users in our project always incorrectly uses Object in JSDoc, when it pretty much always should be object since we use typescript mode, which causes a lot of unnecessary code review comments.

Current behavior

When eslint-plugin-jsdoc is set to "mode": "typescript", then both Object and object is allowed. See https://github.com/gajus/eslint-plugin-jsdoc#check-types.

Desired behavior

When "mode": "typescript" is set, only object should be allowed by default.

Optional: Also object<>, object.<>, Object<> and Object.<> should be disallowed. One can use {s: string, n: number} instead when in typescript mode.

Alternatives considered

I could define this setting:

      "preferredTypes": {
        "Object": "object",
        "Object<>": false,
        "Object.<>": false,
        "object<>": false,
        "object.<>": false
      },

But the problem with that is that pretty much anyone using typescript mode would have to use that setting. It'd be much better to change the default, as that is an absolute "do and don't" to follow.

Those who want to allow Object or Object<> etc. can already do that via preferredTypes.

Slightly related #709.

[thernstig]

I realized that maybe Object<> and Object.<> should be allowed till jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#101 has been fixed. And that any object<> or object.<> should be converted to their uppercase versions.

[thernstig]

Why doesn't this change Object[] to object[]?

    "jsdoc": {
      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object"
      }
    }

I even tried:

    "jsdoc": {
      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object",
        "Object[]": "object[]",
      }
    }

But that did not work either. Is that a bug?

[brettz9]

Why doesn't this change Object[] to object[]?

    "jsdoc": {
      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object"
      }
    }

I even tried:

    "jsdoc": {
      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object",
        "Object[]": "object[]",
      }
    }

But that did not work either. Is that a bug?

Yes, this was a bug. I've fixed this in https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v37.6.3 to allow a mapping of Object: 'object', to cause replacements, but before doing anything further (e.g., to support Object<> conversions), I think we should come to a consensus about what to do between this issue and #709. Previously, we were just assuming people wanted to allow Object, but I see from your reference that it is discouraged at least as a general type. But this doesn't mean it can't be used as a generic, no? (as I understand #709 to be actually preferring and which I think has since been allowed)

[thernstig]

@brettz9 great that you fixed it in 37.6.3, thanks!

I am not sure about the answer to your question. My intent is to use this in JSDoc as soon as jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#101 has been fixed:

      "preferredTypes": {
        "Object": "object",
        "Object<>": false,
        "Object.<>": false,
        "object<>": false,
        "object.<>": false
      },

Afaik Object<string, string> is just a JSDoc syntax, that TypeScript added for support for in JSDoc (not sure if it can be used in non-JSDoc TypeScript code). As I am using "mode": "typescript" I really prefer to just use TypeScript as much as possible, so when jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#101 has been fixed I will just resort to forbidding the ones above.

[thernstig]

@brettz9 the fix made in 37.6.3 broke it in other, unforseen ways:

  "settings": {
    "jsdoc": {
      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object",
        "object<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
        "object.<>": "Object<>" // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
      }
    }
  }

This config now changes (via --fix):

  /**
   * @returns {Object<string, string>} variables passed to the dashboard
   */

into

  /**
   * @returns {object<string, string>} variables passed to the dashboard
   */

But the latter is reported as Invalid JSDoc @returns type "object"; prefer: "Object<>".eslintjsdoc/check-types.

So I am in a deadloop here.

[brettz9]

I'm afraid I don't have time to look at this now. If you want to fix it, feel free to send in a PR.

I'm also sorry about there being in a sense a regression, but I think it is preferable to at least have a single substitution working as intended, and deal with the circular issue in another fix (and then make it the default for TS if that is the consensus here).

[thernstig]

@brettz9 that is fine, I can wait.

[thernstig]

I am afraid the changes in https://github.com/gajus/eslint-plugin-jsdoc/releases/tag/v37.6.3 blocks us from updating to that version or later. object is preferred for everything but Object<>, which we need support for as jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#101 is not supported yet. I.e. this is needed for a good experience right now:

      "mode": "typescript",
      "preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object",
        "object<>": "Object<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
        "object.<>": "Object<>" // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
      },

[brettz9]

No promises, but I hope to take a look at this within the next day or so.

[brettz9]

With a little exploring. in trying TS within JSDoc...

It appears that TS simply doesn't allow: Object<string, number>. It gives the message:Type 'Object' is not generic.

Trying object.<string, number> (with a dot) also fails, giving: Cannot find name 'object'. Did you mean 'Object'?

And object<string, number> gives a multiplicity of errors.

However, TS does support Object.<string, number> with upper case and a dot, per the compiler and per their jsdoc page.

Both Object and object work too (though as you point out the plain Object is discouraged`.

So I think we want the following instead (adding dots to your last two, and adding a new case to convert `Object<>"):

"preferredTypes": {
        // Use 'object' in typescript mode, see TypeScript's Do's and Dont's
        "Object": "object",
        "object<>": "Object.<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
        "object.<>": "Object.<>", // see https://github.com/jsdoc-type-pratt-parser/jsdoc-type-pratt-parser/issues/101
        "Object<>": "Object.<>"
      },

I think we should probably make this the default for JSDoc too so as to foster better compatibility. Apparently TS uses XYZ<> for generics and didn't want or it wasn't appropriate for Object<> to be one of them that was supported (I don't know enough to say why); but it is clear that they could support the JSDoc approach of Object.<>, apparently choosing Object.<> over object.<>, because JSDoc uses it (also perhaps, Object is more common than an Object.create(null) object).

[thernstig]

@brettz9 Object<string, number> is supported in JSDoc with typescript mode and TypeScript 4.4.3, see microsoft/TypeScript#15105 and microsoft/TypeScript#17254. I.e the dot is not needed as in Object.<string, number>.

I just tested it locally in VS Code, where hovering function definitions with this shows proper TypeScript parsing. I am not sure why that did not work for you.

[brettz9]

Ah, sorry--I had been switching branches, so perhaps that was getting me to use an older version.

Yeah, I see that it works now. I also see that if microsoft/TypeScript#20555 is followed, there will eventually be no differences between the casings as well.

But frankly, I find that confusing, and think it's better to follow the TypeScript differentiation of casing (and our convention) though admittedly with TypeScript changing the meaning of "object" unrelated just to Object.create(null), it's already become rather messy.

But I guess we can go with preferring the non-dot form even if the TS JSDoc page uses that in its example as does the JSDoc type docs.

[brettz9]

I've submitted #835 to fix this if someone else wants to try that before I apply it @thernstig / @jaydenseric .

[brettz9]

Note that the PR does not switch the defaults nor apply to non-TypeScript mode. I figure we could roll that up with with #834

[thernstig]

@brettz9 it works! I get a bit of a strange output though:

16:0 warning Invalid JSDoc @type "-" type "object"; prefer: "Object<>" jsdoc/check-types

[brettz9]

@thernstig : Those quotes are for the name portion of the tag, i.e., the string of non-whitespace after the type, e.g., you could get that result with @type {SomeType} - A desc. The intent is to be more descriptive for the user yet without including the whole tag description.

[brettz9]

I think I will go ahead and release. I'll mark this as fixed and people can track #834 if we make this the default (as I think we should).

[gajus]

🎉 This issue has been resolved in version 37.7.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[gajus]

🎉 This issue has been resolved in version 38.1.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀