Merge pull request #22657 from asgerf/unified/unused-variable

Unified: Add unused-variable query

Author: asgerf

unified/extractor/src/languages/swift/swift.rs

@@ -531,6 +531,11 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
             =>
             (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!(
@@ -694,44 +699,11 @@ fn translation_rules() -> Vec<Rule<SwiftContext>> {
                 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.

unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.output

@@ -0,0 +1,76 @@
+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⟩ ⟨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"

unified/extractor/tests/corpus/swift/closures/nested-trailing-closure.swift

@@ -0,0 +1 @@
+print(xs.map { $0 * 2 })

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() + "'"

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:16:17:16:17 | b | Unused local variable 'b' |

unified/ql/test/query-tests/unusedentities/UnusedVariable.qlref

@@ -0,0 +1,2 @@
+query: queries/unusedentities/UnusedVariable.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql

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"
+    }
+    let a = 1
+    print(
+        foo { _ in
+            print(a)
+            let b = 2  // $ Alert[unified/unused-variable]
+        })
+}