Skip to content

Conversation

michaelnebel
Copy link
Contributor

@michaelnebel michaelnebel commented Sep 1, 2025

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.

@github-actions github-actions bot added the Rust Pull requests that update Rust code label Sep 1, 2025
@michaelnebel michaelnebel added the no-change-note-required This PR does not need a change note label Sep 2, 2025
@michaelnebel michaelnebel marked this pull request as ready for review September 2, 2025 08:07
@michaelnebel michaelnebel requested a review from a team as a code owner September 2, 2025 08:07
@Copilot Copilot AI review requested due to automatic review settings September 2, 2025 08:07
Copy link
Contributor

@Copilot Copilot AI left a 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

Copy link
Contributor

@paldepind paldepind left a 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.

Comment on lines 53 to +55
* 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.
Copy link
Contributor

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?

Suggested change
* 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants