-
-
Notifications
You must be signed in to change notification settings - Fork 762
feat(js_analyze): implement useRegexpExec
#8034
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e102487 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
27349e6 to
e0755fc
Compare
CodSpeed Performance ReportMerging #8034 will not alter performanceComparing Summary
Footnotes
|
9c31558 to
b4237d6
Compare
e60eac6 to
e102487
Compare
| AnyJsExpression::JsIdentifierExpression(identifier) => { | ||
| // TODO: get static regexp value | ||
| return None; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how how to resolve the value of a reference.
Could also use the type of the expression, then check the flags within the regex type? Not sure if that's valid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does the source rule do? https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They seem to call an Eslint API to get the value of a given node if it can be decided statically. (not through types) But not sure if biome provides such API?
https://eslint-community.github.io/eslint-utils/api/ast-utils.html#getstaticvalue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the semantic model and the control flow graph, but iirc typed rules shouldn't be using those because they use the module graph. @arendjr, I'd appreciate your input here, you have more context on the type system and the module graph than I do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typed rules can also use the semantic model and the control flow graph, but the semantic model is largely redundant with functionality that is already provided by the typed service, which provides similar things, but with a higher level of abstraction that includes types and cross-module inference.
So far the abstract way of looking at it :)
Unfortunately we don't really have an API yet that can determine static values (AFAIK), but the typed service actually does try to determine static values when it can determine them. Simple example:
const foo = 1;
foo;The type system will be able to determine that the reference to foo on the second line resolves to the concrete value 1, because in TypeScript concrete values can be used as types as well and we try to infer as specific as we can. So yes, I think using the type system is probably the way to go here, but some more work will be necessary. I don't think we have any specific detection for RegExp yet, and I haven't looked yet into what flags need to be checked, but that might require some custom logic too?
If you like, I might be interested to look into this next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be awesome. For now I only need to check on the global flag. But some API to the regexp type could be to check if it has any given flag could be nice
Summary
Port Typescript Eslint's
prefer-regexp-execCloses #7797
Test Plan
Docs