From 9fd747823d75789a1593ccf91b2bf87ae594e7a1 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 14 Aug 2026 11:22:09 +0200 Subject: [PATCH 1/7] unified: Add unused variable query --- .../queries/unusedentities/UnusedVariable.ql | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 unified/ql/src/queries/unusedentities/UnusedVariable.ql diff --git a/unified/ql/src/queries/unusedentities/UnusedVariable.ql b/unified/ql/src/queries/unusedentities/UnusedVariable.ql new file mode 100644 index 000000000000..d5747df44863 --- /dev/null +++ b/unified/ql/src/queries/unusedentities/UnusedVariable.ql @@ -0,0 +1,40 @@ +/** + * @name Unused variable + * @description Unused variables may be an indication that the code is incomplete or has a typo. + * @kind problem + * @problem.severity recommendation + * @id unified/unused-variable + * @precision high + */ + +private import unified + +private predicate isUnusedLocal(LocalName local) { + local.getABinding().fromSource() and // ignore unused implicit locals, and ignore locals in built-ins + not ignoreDeclaration(local.getABinding().getDeclaration()) and // ignore fields, method, etc + not local.getName().regexpMatch("_.*") and + not exists(LocalNameAccess access | + access = local.getAnAccess() and + not access instanceof NameBinding + ) and + not local = any(UnqualifiedMemberAccess access).getImplicitQualifierVariable() +} + +private predicate ignoreDeclaration(AstNode n) { + // ignore fields and methods etc + n = any(ClassLikeDeclaration cls).getAMember() + or + // ignore top-level statements, which are often exported + n = any(TopLevel t).getBody().getAStmt() + or + // Parameters are often needed to satisfy an interface + n instanceof Parameter +} + +string getKind(LocalName local) { + if local instanceof LocalVariable then result = "local variable" else result = "declaration" +} + +from LocalName local +where isUnusedLocal(local) +select local, "Unused " + getKind(local) + " '" + local.getName() + "'" From a1d83ae9da94674911a38dec67a5f7ca490c4296 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 14 Aug 2026 11:36:15 +0200 Subject: [PATCH 2/7] unified: Add basic test --- .../unusedentities/UnusedVariable.expected | 2 ++ .../unusedentities/UnusedVariable.qlref | 2 ++ .../test/query-tests/unusedentities/test.swift | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 unified/ql/test/query-tests/unusedentities/UnusedVariable.expected create mode 100644 unified/ql/test/query-tests/unusedentities/UnusedVariable.qlref create mode 100644 unified/ql/test/query-tests/unusedentities/test.swift diff --git a/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected new file mode 100644 index 000000000000..249ff7a1fd60 --- /dev/null +++ b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected @@ -0,0 +1,2 @@ +| test.swift:2:9:2:9 | a | Unused local variable 'a' | +| test.swift:12:9:12:9 | a | Unused local variable 'a' | diff --git a/unified/ql/test/query-tests/unusedentities/UnusedVariable.qlref b/unified/ql/test/query-tests/unusedentities/UnusedVariable.qlref new file mode 100644 index 000000000000..67f81cd140b8 --- /dev/null +++ b/unified/ql/test/query-tests/unusedentities/UnusedVariable.qlref @@ -0,0 +1,2 @@ +query: queries/unusedentities/UnusedVariable.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql diff --git a/unified/ql/test/query-tests/unusedentities/test.swift b/unified/ql/test/query-tests/unusedentities/test.swift new file mode 100644 index 000000000000..9c2d5b892934 --- /dev/null +++ b/unified/ql/test/query-tests/unusedentities/test.swift @@ -0,0 +1,18 @@ +func t1() -> Int { + let a = 1 // $ Alert[unified/unused-variable] + let b = 2 + return b +} + +func t2() -> Int { + func foo(callback: (Int) -> Void) -> String { + callback() + return "df" + } + // Note: This currently fails because the trailing closure is not extracted correctly + let a = 1 // $ SPURIOUS: Alert + print(foo() { _ in + print(a) + let b = 2 // $ MISSING: Alert[unified/unused-variable] + }) +} From 373ec130a03c21c8abc4f40302a171d3f832f7a2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 23 Sep 2026 10:26:04 +0200 Subject: [PATCH 3/7] unified: Add corpus test --- .../closures/nested-trailing-closure.output | 65 +++++++++++++++++++ .../closures/nested-trailing-closure.swift | 1 + 2 files changed, 66 insertions(+) create mode 100644 unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output create mode 100644 unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.swift diff --git a/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output new file mode 100644 index 000000000000..48039239a182 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output @@ -0,0 +1,65 @@ +print(xs.map { $0 * 2 }) + +--- + +sourceFile + endOfFileToken: endOfFile + statements: + codeBlockItem + item: + functionCallExpr + leftParen: ( + rightParen: ) + arguments: + labeledExpr + expression: + functionCallExpr + arguments: + additionalTrailingClosures: + calledExpression: + memberAccessExpr + period: . + declName: + declReferenceExpr + baseName: identifier "map" + base: + declReferenceExpr + baseName: identifier "xs" + trailingClosure: + closureExpr + leftBrace: { + rightBrace: } + statements: + codeBlockItem + item: + infixOperatorExpr + operator: + binaryOperatorExpr + operator: binaryOperator "*" + leftOperand: + declReferenceExpr + baseName: dollarIdentifier "$0" + rightOperand: + integerLiteralExpr + literal: integerLiteral "2" + additionalTrailingClosures: + calledExpression: + declReferenceExpr + baseName: identifier "print" + +--- + +top_level source="⟨body⟩" + body: + block source="⟨stmt⟩" + stmt: + call_expr source="⟨callee⟩(⟨argument⟩)" + callee: identifier "print" source="print" + argument: + argument source="⟨value⟩" + value: + call_expr source="⟨callee⟩ { $0 * 2 }" + callee: + member_access_expr source="⟨base⟩.⟨member_name_node⟩" + base: identifier "xs" source="xs" + member_name_node: identifier "map" source="map" diff --git a/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.swift b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.swift new file mode 100644 index 000000000000..b129e1fba835 --- /dev/null +++ b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.swift @@ -0,0 +1 @@ +print(xs.map { $0 * 2 }) From e3bbe46daef542acb067b898a8b589deadc44f6f Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 23 Sep 2026 09:49:34 +0200 Subject: [PATCH 4/7] unified: Fix extraction bug The special-cased labelExpr rules were problematic. They are now coveered by a combination of more general rules. --- .../extractor/src/languages/swift/swift.rs | 42 ++++--------------- .../closures/nested-trailing-closure.output | 13 +++++- .../unusedentities/UnusedVariable.expected | 2 +- .../query-tests/unusedentities/test.swift | 4 +- 4 files changed, 22 insertions(+), 39 deletions(-) diff --git a/unified/extractor/src/languages/swift/swift.rs b/unified/extractor/src/languages/swift/swift.rs index c6624d6e2493..7926faf01819 100644 --- a/unified/extractor/src/languages/swift/swift.rs +++ b/unified/extractor/src/languages/swift/swift.rs @@ -533,6 +533,11 @@ fn translation_rules() -> Vec> { => (identifier #{name}) ), + rule!( + (patternExpr pattern: @p) + => + expr { p } + ), // A `let`/`var` value-binding pattern (`let x`) inside a case or `if case` // preserves the binding specifier around its inner pattern. rule!( @@ -696,44 +701,11 @@ fn translation_rules() -> Vec> { tree!((call_expr callee: {callee} argument: {args})) } ), - // A call or enum-case pattern argument. Both use the shared `argument` - // shape, preserving the optional label as `name` and the child as `value`. - // The pattern-only shapes (`patternExpr`, `discardAssignmentExpr`) are - // matched first; they never occur as ordinary call arguments. - rule!( - (labeledExpr - label: _? @@lbl - expression: (functionCallExpr - calledExpression: @constructor - arguments: _* @elements) @@call) - => - argument { - let value = tree_at!( - ctx, - call, - (call_expr callee: {constructor} argument: {elements}) - ); - tree!((argument - name_node: (identifier #{lbl})? - value: {value})) - } - ), - rule!( - (labeledExpr label: _? @@lbl expression: (patternExpr pattern: @p)) - => - (argument name_node: (identifier #{lbl})? value: {p}) - ), - rule!( - (labeledExpr label: _? @@lbl expression: (discardAssignmentExpr) @@wildcard) - => - (argument name_node: (identifier #{lbl})? value: (identifier #{wildcard})) - ), + // A call or enum-case pattern argument. rule!( (labeledExpr label: _? @@lbl expression: @val) => - argument { - tree!((argument name_node: (identifier #{lbl})? value: {val})) - } + (argument name_node: (identifier #{lbl})? value: {val}) ), // Member access (`list.append`). The `declName` is itself a // `declReferenceExpr`; pull its `baseName` out as the member identifier. diff --git a/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output index 48039239a182..a8594a175bac 100644 --- a/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output +++ b/unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output @@ -58,8 +58,19 @@ top_level source="⟨body⟩" argument: argument source="⟨value⟩" value: - call_expr source="⟨callee⟩ { $0 * 2 }" + call_expr source="⟨callee⟩ ⟨argument⟩" callee: member_access_expr source="⟨base⟩.⟨member_name_node⟩" base: identifier "xs" source="xs" member_name_node: identifier "map" source="map" + argument: + argument source="⟨value⟩" + value: + function_expr source="{ ⟨body⟩ }" + body: + block source="⟨stmt⟩" + stmt: + binary_expr source="⟨left⟩ ⟨operator⟩ ⟨right⟩" + left: identifier "$0" source="$0" + operator: infix_operator "*" source="*" + right: int_literal "2" source="2" diff --git a/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected index 249ff7a1fd60..5702ea57410c 100644 --- a/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected +++ b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected @@ -1,2 +1,2 @@ | test.swift:2:9:2:9 | a | Unused local variable 'a' | -| test.swift:12:9:12:9 | a | Unused local variable 'a' | +| test.swift:16:13:16:13 | b | Unused local variable 'b' | diff --git a/unified/ql/test/query-tests/unusedentities/test.swift b/unified/ql/test/query-tests/unusedentities/test.swift index 9c2d5b892934..6440779385b0 100644 --- a/unified/ql/test/query-tests/unusedentities/test.swift +++ b/unified/ql/test/query-tests/unusedentities/test.swift @@ -10,9 +10,9 @@ func t2() -> Int { return "df" } // Note: This currently fails because the trailing closure is not extracted correctly - let a = 1 // $ SPURIOUS: Alert + let a = 1 print(foo() { _ in print(a) - let b = 2 // $ MISSING: Alert[unified/unused-variable] + let b = 2 // $ Alert[unified/unused-variable] }) } From 700cf63260b3ac5c7bbe8635de657da2490f2104 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 23 Sep 2026 14:47:59 +0200 Subject: [PATCH 5/7] unified: Removestale comment --- unified/ql/test/query-tests/unusedentities/test.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/unified/ql/test/query-tests/unusedentities/test.swift b/unified/ql/test/query-tests/unusedentities/test.swift index 6440779385b0..e93d2ab2e3a2 100644 --- a/unified/ql/test/query-tests/unusedentities/test.swift +++ b/unified/ql/test/query-tests/unusedentities/test.swift @@ -9,7 +9,6 @@ func t2() -> Int { callback() return "df" } - // Note: This currently fails because the trailing closure is not extracted correctly let a = 1 print(foo() { _ in print(a) From 40152b7be5b2c1927aba1535541c174f8407ecd4 Mon Sep 17 00:00:00 2001 From: Asger F Date: Wed, 23 Sep 2026 14:48:11 +0200 Subject: [PATCH 6/7] unified: Autoformat test file --- unified/ql/test/query-tests/unusedentities/test.swift | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/unified/ql/test/query-tests/unusedentities/test.swift b/unified/ql/test/query-tests/unusedentities/test.swift index e93d2ab2e3a2..117168664652 100644 --- a/unified/ql/test/query-tests/unusedentities/test.swift +++ b/unified/ql/test/query-tests/unusedentities/test.swift @@ -1,5 +1,5 @@ func t1() -> Int { - let a = 1 // $ Alert[unified/unused-variable] + let a = 1 // $ Alert[unified/unused-variable] let b = 2 return b } @@ -10,8 +10,9 @@ func t2() -> Int { return "df" } let a = 1 - print(foo() { _ in - print(a) - let b = 2 // $ Alert[unified/unused-variable] - }) + print( + foo { _ in + print(a) + let b = 2 // $ Alert[unified/unused-variable] + }) } From 6af72284435f3f80372975a5ddc8ac96ae80ce4b Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 24 Sep 2026 09:35:34 +0200 Subject: [PATCH 7/7] unified: Update location in test --- .../ql/test/query-tests/unusedentities/UnusedVariable.expected | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected index 5702ea57410c..41b89fbc987d 100644 --- a/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected +++ b/unified/ql/test/query-tests/unusedentities/UnusedVariable.expected @@ -1,2 +1,2 @@ | test.swift:2:9:2:9 | a | Unused local variable 'a' | -| test.swift:16:13:16:13 | b | Unused local variable 'b' | +| test.swift:16:17:16:17 | b | Unused local variable 'b' |