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

Arguably terrible enum identifiers causes breakage. #595

Open
oBusk opened this issue Apr 19, 2024 · 1 comment
Open

Arguably terrible enum identifiers causes breakage. #595

oBusk opened this issue Apr 19, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@oBusk
Copy link

oBusk commented Apr 19, 2024

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 oBusk added the bug Something isn't working label Apr 19, 2024
@webpro
Copy link
Owner

webpro commented Apr 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants