darraghoriordan / eslint-plugin-nestjs-typed

Some eslint rules for working with NestJs projects
http://www.darraghoriordan.com
171 stars 34 forks source link

`param-decorator-name-matches-route-param` needs to handle certain edge cases #62

Closed mcmxcdev closed 1 year ago

mcmxcdev commented 1 year ago

We do have routes which have the URL put into a variable like @Put(FILTERS_URL) leading to Param decorators with identifiers e.g. Param("myvar") should match a specified route parameter - e.g. Get(":myvar")

The value of FILTERS_URL is ":employerId/major-medical-plans/filters" and the param is @Param('employerId', new ParseUUIDPipe()) employerId: string

I would recommend to just not report on issues like there were no certainty is guaranteed.


The other edge case is similar: it's a route with template literals like

@Put(`onboarding-periods/:id/${MAJOR_MEDICAL_PLAN_URL}`)

where the param is @Param('id') periodId: string. Changing the template literal temporarily to double or single quotes removes the eslint issue.

darraghoriordan commented 1 year ago

Thanks for the report

This is just a regex if i remember correctly. Feel free to submit a PR to update the regex if you have time. I guess it needs to ignore anything not in literal string quotes

darraghoriordan commented 1 year ago

This is fixed in 3.22.2

the rule ignores template literals and identifiers

mcmxcdev commented 1 year ago

We have upgraded to the latest version but unfortunately the fix doesn't solve any of our false positives:

  @Put(`onboarding-periods/:id/${MAJOR_MEDICAL_PLAN_URL}`)
  async upsertMajorMedicalPlan(
    @Param('id') periodId: string,
  @Get(FILTERS_URL) // ":employerId/major-medical-plans/filters"
  async getMajorMedicalPlanFilters(
    @Param('employerId', new ParseUUIDPipe()) employerId: string
darraghoriordan commented 1 year ago

I just added both of those to the test suite and it passes as expected (they are ignored). Are you certain you're using the latest version?

I can't recreate this with the samples you provided, perhaps you can take the time to debug this one yourself?

mcmxcdev commented 1 year ago

You are correct, my bad, I guess I was in some half-updated state in VSCode. The warnings are gone, thank you!