-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Rust: Fix some Ql4Ql violations. #20333
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
This PR fixes various Ql4Ql violations in the Rust codeql library based on specific checks for code quality improvements. The changes focus on removing unused variables, improving documentation, and simplifying code patterns.
Key changes:
- Removes unused private field variables in favor of inline expressions
- Adds missing parameter documentation for extensible predicates
- Improves code clarity by eliminating unnecessary variable declarations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
rust/ql/lib/codeql/rust/internal/Type.qll | Removes unused private field and simplifies constructor logic |
rust/ql/lib/codeql/rust/elements/internal/CallImpl.qll | Removes unused qualifier field in CallExprMethodCall class |
rust/ql/lib/codeql/rust/dataflow/internal/ModelsAsData.qll | Adds missing documentation for madId parameter in extensible predicates |
rust/ql/lib/codeql/rust/dataflow/internal/DataFlowImpl.qll | Adds missing documentation for preservesValue parameter |
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.
Looks good. One small nitpick.
* Holds if in a call to the function with canonical path `path`, the value referred | ||
* to by `output` is a flow source of the given `kind`. | ||
* The `madId` is the data extension row number. |
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.
Putting "the" in front of a parameter name sounds odd to me? What about this?
* Holds if in a call to the function with canonical path `path`, the value referred | |
* to by `output` is a flow source of the given `kind`. | |
* The `madId` is the data extension row number. | |
* Holds if in a call to the function with canonical path `path`, the value referred | |
* to by `output` is a flow source of the given `kind` and `madId` is the data | |
* extension row number. |
Same suggestion for the two similar cases below.
Fix some Ql4Ql violations based on the following checks
ql/field-only-used-in-charpred
ql/could-be-cast
ql/counting-to-zero
ql/dataflow-module-naming-convention
ql/if-with-none
ql/missing-parameter-qldoc
ql/misspelling
DCA looks good.