Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Have namepath checks in valid-types only allow strict namepaths #374

Open
brettz9 opened this issue Sep 1, 2019 · 2 comments
Open

Have namepath checks in valid-types only allow strict namepaths #374

brettz9 opened this issue Sep 1, 2019 · 2 comments

Comments

@brettz9
Copy link
Collaborator

brettz9 commented Sep 1, 2019

As raised in #308, contexts where we are checking namepaths in valid-types also mistakenly allow for more broad types, e.g.,Array<string> should get reported in @typedef {StringArray} Array<string> but it currently is not.

I've filed jsdoctypeparser/jsdoctypeparser#86 to possibly have jsdoctypeparser facilitate strict namepath checking rather than examining the parse results more closely here.

See also jsdoc-type-pratt-parser/jsdoc-type-pratt-parser#77

Ideally, @memberof variation pointers would be valid as well (see #521 ).


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@jaydenseric
Copy link
Contributor

Also, we should lint that certain kinds of child members have valid relationships to the parent in the namepath; for example (I'm not 100% sure about these because the JSDoc spec is poor):

  • It is only valid for an event to have an instance membership (#).

    • Valid: @event FooClass#event:EventName or @event FooClass#EventName
    • Invalid: @event FooClass~event:EventName or @event FooClass~EventName

    As a side note, it would also be nice to autofix event names to either have or not have the event: prefix for consistency. It's my preference to use the prefix everywhere instead of remembering it can be omitted in the @event and @fires tags.

  • A typedef can only have an inner membership (~).

    • Valid: @typedef {Object} FooClass~FooType
    • Invalid: @typedef {Object} FooClass#FooType

I spent a lot of time in jsdoc-md coming up with an elegant regex to reliably deconstruct and validate JSDoc namepaths, maybe you will find it useful:

If you do want to use it, let me know and I will publish a package for it. It's my preference to share a proper dependency than have you cut and paste the code in with the MIT license mandated credit in a comment or something.

@brettz9
Copy link
Collaborator Author

brettz9 commented Nov 28, 2020

Also, we should lint that certain kinds of child members have valid relationships to the parent in the namepath; for example (I'm not 100% sure about these because the JSDoc spec is poor):

* It is only valid for an event to have an instance membership (`#`).
  
  * Valid: `@event FooClass#event:EventName` or `@event FooClass#EventName`
  * Invalid: `@event FooClass~event:EventName` or `@event FooClass~EventName`

I think it should be possible that an event is defined as an inner object yet such an event instance is fired by a public method on that class and thus needs to document it is arising from a private.

class EventName {}

class FooClass {
  triggerEventName () {
    return new EventName();
  }
}

// ...

class PublicAPI {
  /**
   * @event FooClass~EventName
   */
  publicMethod () {
    return new FooClass().triggerEventName();
  }
}
  As a side note, it would also be nice to autofix event names to either have or not have the `event:` prefix for consistency. It's my preference to use the prefix everywhere instead of remembering it can be omitted in the `@event` and `@fires` tags.

Feel free to file a separate issue for this. We couldn't really determine if the event prefix was missing, e.g., on a @param unless on a tag like event, fires, listens (since we wouldn't know whether it was an event or something else).

* A typedef can only have an inner membership (`~`).
  
  * Valid: `@typedef {Object} FooClass~FooType`
  * Invalid: `@typedef {Object} FooClass#FooType`

I guess the idea here is that a public item would not just be abstract, and so a @typedef would not be suitable in such a case, right? (i.e., one would have to use a @namespace instead to define such a visible object.) Yes, I see the docs do at least suggest this is often the case even if they are not explicitly or firmly recommending this.

Such a change sounds reasonable though and would fit well with the change I think will be needed for this issue–to supply startRule: 'NamepathExpr' to our jsdoctypeparser call (we'd have to introspect on the owner of the parsed result to determine whether it is "MEMBER", "INNER_MEMBER", or "INSTANCE_MEMBER").

However, I think it should probably be added as a default-on option since the docs are not adamant that instance members cannot be used.

I spent a lot of time in jsdoc-md coming up with an elegant regex to reliably deconstruct and validate JSDoc namepaths, maybe you will find it useful:

* Source: https://github.com/jaydenseric/jsdoc-md/blob/v8.0.0/private/deconstructJsdocNamepath.js

* Tests: https://github.com/jaydenseric/jsdoc-md/blob/v8.0.0/test/private/deconstructJsdocNamepath.test.js

If you do want to use it, let me know and I will publish a package for it. It's my preference to share a proper dependency than have you cut and paste the code in with the MIT license mandated credit in a comment or something.

Thank you for the offer, but I really think this should be handled in jsdoctypeparser. The grammar makes it easy to understand and maintain (and ensure parsing improvements benefit other parsing uses within our project(s))--might even be useful for your project.

Btw, if anyone wants to help with a real bottleneck on jsdoctypeparser that is stalling some things on this project, we haven't been able to get semantic-release working, so any suggestions or help would be appreciated: jsdoctypeparser/jsdoctypeparser#122 . With such a fix in place, we'd be in a better position to make releases there which can be more quickly surfaced in eslint-plugin-jsdoc.

I do have to mention that my health/energy has not been great though, so I'm having to slow down and am not necessarily going to be able to be reliable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants