Skip to content

Commit 0ae66a4

Browse files
committed
ci(codeql): use custom rule for safe final-field-copy shadowing pattern
1 parent 4721b99 commit 0ae66a4

3 files changed

Lines changed: 63 additions & 1 deletion

File tree

.github/codeql/codeql-config.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#using-a-custom-configuration-file
2+
name: "CodeQL config"
3+
queries:
4+
- uses: security-and-quality
5+
- uses: ./.github/codeql/queries/java # load our custom CodeQL rules
6+
7+
query-filters:
8+
- exclude:
9+
# Exclude the built-in shadowing rule.
10+
# We intentionally use final locals that copy instance fields
11+
# (e.g. `final var name = this.name`) to support Eclipse null analysis
12+
# without introducing noisy renaming. This pattern is deliberate and safe,
13+
# so the built-in rule is disabled in favor of our custom rule
14+
# (local-shadows-instance-field.ql).
15+
id: java/local-shadows-field
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* @name Local variable shadows instance field (except final this-copy)
3+
* @description Flags local variables that shadow an instance field, unless they are final
4+
* and initialized directly from that same field (for example `final var name = this.name;`).
5+
* @id custom-java/local-shadows-instance-field-strict
6+
* @kind problem
7+
* @problem.severity warning
8+
* @precision medium
9+
* @tags correctness readability maintainability
10+
*/
11+
import java
12+
import semmle.code.java.Member
13+
import semmle.code.java.Variable
14+
15+
/**
16+
* True if this local variable is an allowed shadowing of the given instance field:
17+
*
18+
* final var name = this.name;
19+
* final var name = name; // implicit this
20+
*/
21+
predicate allowedShadowing(LocalVariableDecl local, Field field) {
22+
local.isFinal() and
23+
exists(FieldAccess fa |
24+
fa = local.getInitializer().getUnderlyingExpr().(FieldAccess) and
25+
fa.getField() = field and
26+
fa.isOwnFieldAccess()
27+
)
28+
}
29+
30+
from LocalVariableDecl local, Field field, Callable c
31+
where
32+
// same simple name
33+
local.getName() = field.getName() and
34+
35+
// only consider instance fields
36+
not field.isStatic() and
37+
38+
// local is inside a callable of a class related to the field's declaring type
39+
c = local.getCallable() and
40+
c.getDeclaringType().getASupertype*() = field.getDeclaringType() and
41+
42+
// shadowing is NOT in the allowed final-this-copy form
43+
not allowedShadowing(local, field)
44+
select local,
45+
"Local variable '" + local.getName() + "' shadows instance field '" +
46+
field.getQualifiedName() +
47+
"'. Shadowing is only allowed for 'final' locals directly initialized from this field."

.github/workflows/codeql.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ jobs:
133133
# https://github.com/github/codeql-action#build-modes
134134
build-mode: ${{ matrix.build-mode }}
135135
# https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning#using-queries-in-ql-packs
136-
queries: +security-and-quality
136+
config-file: ./.github/codeql/codeql-config.yml
137137

138138

139139
- name: "Build with Maven 🔨"

0 commit comments

Comments
 (0)