-
Notifications
You must be signed in to change notification settings - Fork 1.7k
JS: ClientRequests Axios Instance support #19655
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
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.
Pull Request Overview
Adds support for Axios instances in the ClientRequests framework and updates tests accordingly.
- Introduces a new test file (
axios.ts
) that usesaxios.create({ ... })
and exercises.get
and.patch
calls. - Updates the expected outputs (
ClientRequests.expected
) to include those new axios instance calls. - Extends the QL library (
ClientRequests.qll
) with a newAxiosInstanceRequest
class and adds support for thepatch
method.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
javascript/ql/test/library-tests/frameworks/ClientRequests/axios.ts | Added tests for axios instances and their .get / .patch calls |
javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequests.expected | Updated expected results to include axios.ts calls |
javascript/ql/lib/semmle/javascript/frameworks/ClientRequests.qll | Added handling for axios instances and included patch method |
javascript/ql/lib/change-notes/2025-06-03-axios-instance-support.md | Documented the new axios instance support |
Comments suppressed due to low confidence (1)
javascript/ql/test/library-tests/frameworks/ClientRequests/axios.ts:24
- [nitpick] The function is named
updateUser
but it actually patches a repository endpoint. Consider renaming it toupdateRepo
(or adjust the endpoint if it should update a user).
export async function updateUser(owner: string, repo: string, data: any) {
javascript/ql/test/library-tests/frameworks/ClientRequests/axios.ts
Outdated
Show resolved
Hide resolved
javascript/ql/test/library-tests/frameworks/ClientRequests/axios.ts
Outdated
Show resolved
Hide resolved
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.
The duplication between the two classes is a bit unfortunate but not blocking. I'll try and clean it up post-merge.
I've started a DCA run.
AxiosInstanceRequest() { | ||
instance = axios().getMember(["create", "createInstance"]).getACall() and | ||
method = [httpMethodName(), "request", "postForm", "putForm", "patchForm", "getUri"] and | ||
this = instance.getReturn().getMember(method).getACall() |
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.
There's a case on line 227 that ought to be moved here:
this = axios().getMember("create").getReturn().getACall() and
method = "request"
That is, if the result of create()
is invoked directly, it should be treated as a call to request()
.
I've added support for creation Axios instances using
create({ ... })