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

Go: database local sources for MongoDB #18493

Merged

Conversation

egregius313
Copy link
Contributor

Models the go.mongodb.org/mongo-driver/mongo package.

Pull Request checklist

All query authors

  • A change note is added if necessary. See the documentation in this repository.
    - [ ] All new queries have appropriate .qhelp. See the documentation in this repository.
    - [ ] QL tests are added if necessary. See Testing custom queries in the GitHub documentation.
    - [ ] New and changed queries have correct query metadata. See the documentation in this repository.

Internal query authors only

- [ ] Autofixes generated based on these changes are valid, only needed if this PR makes significant changes to .ql, .qll, or .qhelp files. See the documentation (internal access required).

@Copilot Copilot bot review requested due to automatic review settings January 15, 2025 03:41
@egregius313 egregius313 requested a review from a team as a code owner January 15, 2025 03:41
@github-actions github-actions bot added the Go label Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • go/ql/test/library-tests/semmle/go/dataflow/flowsources/local/database/go.mod: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Copy link
Contributor

github-actions bot commented Jan 15, 2025

⚠️ The head of this PR and the base branch were compared for differences in the framework coverage reports. The generated reports are available in the artifacts of this workflow run. The differences will be picked up by the nightly job after the PR gets merged.

Click to show differences in coverage

go

Generated file changes for go

  • Changes to framework-coverage-go.rst:
-    `MongoDB Go Driver <https://www.mongodb.com/docs/drivers/go/current/>`_,``go.mongodb.org/mongo-driver*``,,,14
+    `MongoDB Go Driver <https://www.mongodb.com/docs/drivers/go/current/>`_,``go.mongodb.org/mongo-driver*``,11,5,14
-    Totals,,459,947,1532
+    Totals,,470,952,1532
  • Changes to framework-coverage-go.csv:
- go.mongodb.org/mongo-driver/mongo,14,,,,,,,14,,,,,,,,,,,,,,,,,,,
+ go.mongodb.org/mongo-driver/mongo,14,11,5,,,,,14,,,,,,,,,,,,,11,,,,,5,

Copy link
Contributor

@owen-mc owen-mc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here looks good. Two points:

  • I think we should also model Client.Watch.
  • We normally record the depstubber invocation as a go generate comment. If you search for "//go:generate depstubber" you'll see what I mean. This makes it easy to regenerate the stubs in future, e.g. if the library changes, or if you want some extra stubs. If you still have the depstubber command available then it would be helpful to record it in this way.

@@ -1,8 +1,9 @@
//go:generate depstubber -vendor go.mongodb.org/mongo-driver/mongo Client,Collection,Database
Copy link
Contributor

@owen-mc owen-mc Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should instead go at the top of the related test. That way it won't be deleted if the file is ever regenerated.

@egregius313 egregius313 merged commit c93fb4c into github:main Feb 21, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants