-
Notifications
You must be signed in to change notification settings - Fork 2
Implement duplicated code detection prompts, supported by tools. #109
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?
Changes from all commits
194af18
44c7e28
91a3ebf
240244a
f7b63ba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,206 @@ | ||||||||||||||||||||||
| --- | ||||||||||||||||||||||
| agent: agent | ||||||||||||||||||||||
| --- | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Check for Duplicated Code | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Use the MCP server tools to identify classes, modules, and predicates defined in a | ||||||||||||||||||||||
| `.ql` or `.qll` file and check for possible "duplicated code," where duplicated code | ||||||||||||||||||||||
| is defined to be: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - Reimplementing functionality that already exists in the standard library, or shared project `.qll` files, and | ||||||||||||||||||||||
| - The local definition is identical, or semantically equivalent, or superior to the library definition, or | ||||||||||||||||||||||
| - The local definition could be simplified by reusing the existing definition (e.g. a base class already exists that captures some of the shared logic) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Here are some examples: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ```ql | ||||||||||||||||||||||
| import cpp | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Duplicated: `StandardNamespace` already exists in the standard library and is identical | ||||||||||||||||||||||
| class NamespaceStd extends Namespace { | ||||||||||||||||||||||
| NamespaceStd() { this.getName() = "std" } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Duplicated: class should extend `Operator`, not `Function` | ||||||||||||||||||||||
| class ThrowingOperator extends Function { | ||||||||||||||||||||||
| ThrowingOperator() { | ||||||||||||||||||||||
| // Duplicated: this check is implied by using base class `Operator` | ||||||||||||||||||||||
| this.getName().matches("%operator%") and | ||||||||||||||||||||||
| and exists(ThrowExpr te | | ||||||||||||||||||||||
| // Duplicated: this is equivalent to `te.getEnclosingFunction() = this` | ||||||||||||||||||||||
| te.getParent*() = this.getAChild() | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Duplicated: member predicate `getDeclaringType()` already does this. | ||||||||||||||||||||||
| Class getDefiningClass() { ... } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Duplicated: `ControlFlowNode.getASuccessor()` already exists in `cpp` and is superior | ||||||||||||||||||||||
| predicate getASuccessor(Stmt a, Stmt b) { | ||||||||||||||||||||||
| exists(Block b, int i | a = b.getChild(i) and b = b.getChild(i + 1)) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Duplicated: prefer to import `semmle.code.cpp.controlflow.Dominance`, defined in dependency pack `cpp-all` | ||||||||||||||||||||||
| predicate dominates(Block a, Block b) { ... } | ||||||||||||||||||||||
| ``` | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Duplicate code removal isn't done arbitrarily, but for several key reasons: | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - **Maintainability**: Duplicated code must be maintained separately, may diverge and have different bugs | ||||||||||||||||||||||
| - **Simplicity**: Relying on existing definitions reduces the amount of code to read and understand | ||||||||||||||||||||||
| - **Readability**: Existing definitions map wrap complex ideas into readable names | ||||||||||||||||||||||
| - **Consistency**: A single source of truth makes for a more consistent user experience across queries | ||||||||||||||||||||||
| - **Completeness/Correctness**: Recreating an already-existing definition can miss edge cases, resulting in false positives or false negatives | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Use This Prompt When | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| - A query file defines a class or predicate whose name sounds generic (e.g. | ||||||||||||||||||||||
| `StandardNamespace`, `Callable`, `SecurityFeature`) | ||||||||||||||||||||||
| - Refactoring a query that was written before a relevant library predicate existed | ||||||||||||||||||||||
| - Reviewing a shared `.qll` file to check whether its helpers have been upstreamed | ||||||||||||||||||||||
| into the standard library in a newer CodeQL version | ||||||||||||||||||||||
| - Performing a code-quality audit across a suite of custom queries | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| ## Prerequisites | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| 1. The file path of the `.ql` or `.qll` file to audit for code duplication | ||||||||||||||||||||||
| 2. Understand which packs are imported by `qlpack.yml` | ||||||||||||||||||||||
|
||||||||||||||||||||||
| 2. Understand which packs are imported by `qlpack.yml` | |
| 2. Understand which packs are imported by `codeql-pack.yml` |
Copilot
AI
Mar 11, 2026
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.
This section documents the codeql_lsp_document_symbols response as if each symbols entry contains fields like kind, range, selectionRange, and children, but the example call sets names_only: true (which the tool implementation returns as a string[] of names). Either set names_only: false for this explanation, or update the description to match the names_only output shape.
Copilot
AI
Mar 11, 2026
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 example “Report Findings” table has a duplicated myHelper row, which makes the guidance look accidental/noisy. Remove the duplicate row or replace it with a different example entry.
| | Local name | Local file | import path | Notes | | |
| | ------------------- | ------------- | -------------------------------- | ---------- | | |
| | `StandardNamespace` | `query.ql:42` | already imported in `import cpp` | Identical | | |
| | `myHelper` | `query.ql:80` | `import myproject.Helpers` | Equivalent | | |
| | `myHelper` | `query.ql:80` | `import myproject.Helpers` | Equivalent | | |
| | Local name | Local file | import path | Notes | | |
| | ------------------- | ------------- | -------------------------------- | ------------------- | | |
| | `StandardNamespace` | `query.ql:42` | already imported in `import cpp` | Identical | | |
| | `myHelper` | `query.ql:80` | `import myproject.Helpers` | Equivalent | | |
| | `myWrapper` | `query.ql:95` | `import myproject.Helpers` | Thin wrapper only | |
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 QL example in this prompt contains invalid syntax and confusing shadowing (e.g.
...matches(...) andfollowed immediately byand exists(...), andpredicate getASuccessor(Stmt a, Stmt b)thenexists(Block b, ...)reusesb). Since these are teaching examples, they should be syntactically correct and avoid name shadowing to prevent users copying broken code.