ember-tooling / ember-eslint-parser

This is the eslint parser for ember's gjs and gts files (using <template>).
4 stars 4 forks source link

`jsdoc/require-jsdoc` crashes ESLint when there is an un-exported class with a `<template>` tag #78

Closed chancancode closed 1 month ago

chancancode commented 7 months ago
// foo.gts

class Foo {
  <template>...</template>
}

export default class Bar {
  <template>...</template>
}

Something about this causes this code to blow up: https://github.com/gajus/eslint-plugin-jsdoc/blob/v48.0.6/src/exportParser.js#L216-L220

I think it was not expecting the GlimmerTemplate node inside the class body, and so it tried to access node.key.name on it and it blows up because GlimmerTemplate doesn't have a key. The fact that the code only runs when there is an unexported class is incidental.

I think the bottomline is there are probably code out there outside of our control (well, we can try to PR a fix, but would have to explain a lot of background and convince people to care, and it's kind of a whack-a-mole situation), that have some implicit expectation on what can appear inside a class body.

I think either:

  1. Somehow we filter out the GlimmerTemplate node by default and only expose it with some kind of opt-in
  2. Try to mimic the structure of an existing node type as much as possible (I think a method is probably the best fit indeed) so that code that aren't written defensively will be able to "get by"
patricklx commented 7 months ago

Do they support StaticBlock?

chancancode commented 7 months ago

Possibly, it's more about what doesn't cause their code (and other plugins) to not blow up; I won't have time to look into it until next week at the earliest. In this case it is missing node.key which they seem to assume (reasonably or not) every node inside the class body to have.

I suspect for linting purposes, pretending to be a method body is closer to the actual semantics, if some plugin checks the content and see this it's more likely to assume the correct thing over when it's inside a static block.

patricklx commented 7 months ago

static block is also part if the body and doesn't have key. So they probably need to validate if a key is available. https://astexplorer.net/#/gist/8eeee786f08a003c55e6851cfe95bb46/latest

patricklx commented 7 months ago

Making template a method ast node has other issues. Like eslint plugins that try to order members and methods

patricklx commented 1 month ago

fixed in https://github.com/gajus/eslint-plugin-jsdoc/pull/1313