knip icon indicating copy to clipboard operation
knip copied to clipboard

Arguably terrible enum identifiers causes breakage.

Open oBusk opened this issue 10 months ago • 1 comments

So I was trying to run knip on a larger project and was runing into errors that was not very clear to me at first.

file:///Z:/My_Large_Repo/node_modules/knip/dist/typescript/getImportsAndExports.js:298
        for (const match of sourceFile.text.matchAll(new RegExp(item.identifier.replace(/\$/g, '\\$'), 'g'))) {
                                                     ^

SyntaxError: Invalid regular expression: /+/g: Nothing to repeat
    at new RegExp (<anonymous>)
    at setRefs (file:///Z:/My_Large_Repo/node_modules/knip/dist/typescript/getImportsAndExports.js:298:54)     
    at getImportsAndExports (file:///Z:/My_Large_Repo/node_modules/knip/dist/typescript/getImportsAndExports.js:314:13)
    at ProjectPrincipal.analyzeSourceFile (file:///Z:/My_Large_Repo/node_modules/knip/dist/ProjectPrincipal.js:140:47)
    at analyzeSourceFile (file:///Z:/My_Large_Repo/node_modules/knip/dist/index.js:187:66)
    at main (file:///Z:/My_Large_Repo/node_modules/knip/dist/index.js:268:17)
    at async run (file:///Z:/My_Large_Repo/node_modules/knip/dist/cli.js:25:73)
    at async file:///Z:/My_Large_Repo/node_modules/knip/dist/cli.js:85:1

After adding some logging into the getImportsAndExports.js I managed to find that it was choking parsing an enum we have to handle potential values of KeyboardEvent.key, including keys like * and +, which is arguably a terrible idea, but is allowed in Typescript, so I guess knip shouldn't fail on it either.

Here's a repro: https://stackblitz.com/edit/github-4tmeki?file=enum.ts%3AL21

I think it should be pretty easy to amend, I might send a PR too

oBusk avatar Apr 19 '24 10:04 oBusk

Knip should definitely not choke on those indeed. A PR would be great, otherwise I'll pick it up after the weekend.

webpro avatar Apr 19 '24 10:04 webpro

Knip should definitely not choke on those indeed. A PR would be great, otherwise I'll pick it up after the weekend.

Hey, bro, it looks like this problem still hasn’t been solved. Do you have any plans to fix it soon?

mahoushoujoarale avatar May 30 '24 03:05 mahoushoujoarale

:rocket: This issue has been resolved in v5.17.4. See Release 5.17.4 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.

webpro avatar Jun 03 '24 08:06 webpro