C++: Fix some Ql4Ql violations. #20325
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
michaelnebel
added
the
no-change-note-required
|
The "experimental" crypto stuff should not be touched. That's actively being worked on. Or, it should be a separate PR at the very least so |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Ok, I will split it then. |
michaelnebel
force-pushed
the
cpp/ql4ql
branch
from
441e5a3 to
5199829
Compare
michaelnebel
force-pushed
the
cpp/ql4ql
branch
from
5199829 to
83d53ba
Compare
| * The location spans column `sc` of line `sl` to | ||
| * column `ec` of line `el` in file `path`. |
There was a problem hiding this comment.
I think we should just rename the predicate arguments here. This makes things less clear.
| * The location spans column `sc` of line `sl` to | ||
| * column `ec` of line `el` in file `path`. |
There was a problem hiding this comment.
As above, rename the arguments.
| count(e.getAChild()) = 0 and | ||
| not exists(e.getAChild()) and |
There was a problem hiding this comment.
This is not a library that we test against in DCA, but it is used by the field team e.g., so if we want to change this we need to make sure in some way that it's not changing performance.
| /** | ||
| * Gets a constructor call, if any. | ||
| */ | ||
| ConstructorCall getAConstructorCall() { |
| @@ -164,12 +164,17 @@ predicate valueOccurrenceCount(string value, int n) { | |||
| n > 20 | |||
| } | |||
|
|
|||
| predicate occurenceCount(Literal lit, string value, int n) { | |||
| predicate occurrenceCount(Literal lit, string value, int n) { | |||
| @@ -128,11 +128,18 @@ abstract class LeapYearFieldAccess extends YearFieldAccess { | |||
| /** | |||
| * Holds if the top-level binary operation includes an addition or subtraction operator with an operand specified by `valueToCheck`. | |||
| */ | |||
| predicate additionalAdditionOrSubstractionCheckForLeapYear(int valueToCheck) { | |||
| predicate additionalAdditionOrSubtractionCheckForLeapYear(int valueToCheck) { | |||
| * The location spans column `sc` of line `sl` to | ||
| * column `ec` of line `el` in file `path`. |
There was a problem hiding this comment.
As earlier, rename the arguments.
| * column `startcol` of line `startline` to column `endcol` of line `endline` | ||
| * in file `file`. |
There was a problem hiding this comment.
As earlier, rename the arguments.
There was a problem hiding this comment.
Do you know, if that will cause any problems when this is an external predicate?
There was a problem hiding this comment.
That's a good question. I honestly don't know. Feel free to ignore this review comment.
| * column `startcol` of line `startline` to column `endcol` of line `endline` | ||
| * in file `file`. |
There was a problem hiding this comment.
As above, rename the arguments.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes several Ql4Ql violations identified by code quality checks for C++ CodeQL queries and libraries. The changes address issues including spelling corrections, parameter documentation improvements, code simplification, and method naming corrections.
Key changes:
- Fixed spelling errors in comments and documentation
- Simplified conditional logic in several files
- Corrected method names and parameter documentation
- Removed unused variables and improved code structure
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql | Simplified exists statement by removing unused parameter variable |
| cpp/ql/src/external/MetricFilter.qll | Updated parameter names in documentation to match predicate signature |
| cpp/ql/src/external/DefectFilter.qll | Updated parameter names in documentation to match predicate signature |
| cpp/ql/src/experimental/Security/CWE/CWE-416/UseAfterExpiredLifetime.ql | Simplified conditional logic using instanceof check |
| cpp/ql/src/experimental/Security/CWE/CWE-243/IncorrectChangingWorkingDirectory.ql | Fixed parameter name in documentation comments |
| cpp/ql/src/experimental/Security/CWE/CWE-125/DangerousWorksWithMultibyteOrWideCharacters.ql | Added missing parameter documentation |
| cpp/ql/src/experimental/Security/CWE/CWE-1126/DeclarationOfVariableWithUnnecessarilyWideScope.ql | Removed unused field and restructured class logic |
| cpp/ql/src/experimental/Likely Bugs/RedundantNullCheckParam.ql | Simplified count check using exists |
| cpp/ql/src/definitions.ql | Fixed spelling of "behavior" |
| cpp/ql/src/change-notes/2025-09-03-rename-api.md | Added change note documenting deprecated predicates |
| Multiple library files | Various corrections including parameter documentation, method naming, spelling fixes, and code simplification |
Comments suppressed due to low confidence (1)
cpp/ql/src/jsf/4.10 Classes/AV Rule 96.ql:1
- Typo in the message: 'teated' should be 'treated'.
/**
|
Thank you for the review @jketema ! |
Fix some Ql4Ql violations based on the following checks
ql/field-only-used-in-charpredql/could-be-castql/counting-to-zeroql/dataflow-module-naming-conventionql/if-with-noneql/missing-parameter-qldocql/misspellingDCA looks good.