+
+
+A value is assigned to a local variable, but either that variable is never read
+later on, or its value is always overwritten before being read. This means
+that the original assignment has no effect, and could indicate a logic error or
+incomplete code.
+
+
+
+
+
+
+Ensure that you check the control and data flow in the method carefully.
+If a value is really not needed, consider omitting the assignment. Be careful,
+though: if the right-hand side has a side-effect (like performing a method call),
+it is important to keep this to preserve the overall behavior.
+
+
+
+
+
+
+In the following example, the return value of the call to send on line 2
+is assigned to the local variable result, but then never used.
+
+
+
+
+
+Assuming that send returns a status code indicating whether the operation
+succeeded or not, the value of result should be checked, perhaps like this:
+
+
+
+
+
+
+
+
+Wikipedia: Dead store.
+
+
+
+
+
diff --git a/ruby/ql/src/queries/variables/DeadStoreOfLocal.ql b/ruby/ql/src/queries/variables/DeadStoreOfLocal.ql
index 2cf36bdc3e50..8717047e9954 100644
--- a/ruby/ql/src/queries/variables/DeadStoreOfLocal.ql
+++ b/ruby/ql/src/queries/variables/DeadStoreOfLocal.ql
@@ -7,16 +7,25 @@
* @id rb/useless-assignment-to-local
* @tags maintainability
* external/cwe/cwe-563
- * @precision low
+ * @precision medium
*/
import codeql.ruby.AST
import codeql.ruby.dataflow.SSA
+import codeql.ruby.ApiGraphs
class RelevantLocalVariableWriteAccess extends LocalVariableWriteAccess {
RelevantLocalVariableWriteAccess() {
not this.getVariable().getName().charAt(0) = "_" and
- not this = any(Parameter p).getAVariable().getDefiningAccess()
+ not this = any(Parameter p).getAVariable().getDefiningAccess() and
+ not API::getTopLevelMember("ERB").getInstance().getAMethodCall("result").asExpr().getScope() =
+ this.getCfgScope() and
+ not exists(RetryStmt r | r.getCfgScope() = this.getCfgScope()) and
+ not exists(MethodCall c |
+ c.getReceiver() instanceof SelfVariableAccess and
+ c.getMethodName() = "binding" and
+ c.getCfgScope() = this.getCfgScope()
+ )
}
}
diff --git a/ruby/ql/src/queries/variables/examples/DeadStoreOfLocal.rb b/ruby/ql/src/queries/variables/examples/DeadStoreOfLocal.rb
new file mode 100644
index 000000000000..0b5dc3d6c2ee
--- /dev/null
+++ b/ruby/ql/src/queries/variables/examples/DeadStoreOfLocal.rb
@@ -0,0 +1,5 @@
+def f(x)
+ result = send(x)
+ waitForResponse
+ return getResponse
+end
\ No newline at end of file
diff --git a/ruby/ql/src/queries/variables/examples/DeadStoreOfLocalGood.rb b/ruby/ql/src/queries/variables/examples/DeadStoreOfLocalGood.rb
new file mode 100644
index 000000000000..fc40a7fc4ebd
--- /dev/null
+++ b/ruby/ql/src/queries/variables/examples/DeadStoreOfLocalGood.rb
@@ -0,0 +1,9 @@
+def f(x)
+ result = send(x)
+ # check for error
+ if (result == -1)
+ raise "Unable to send, check network."
+ end
+ waitForResponse
+ return getResponse
+end
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.expected b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.expected
new file mode 100644
index 000000000000..572308ee51e4
--- /dev/null
+++ b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.expected
@@ -0,0 +1 @@
+| DeadStoreOfLocal.rb:2:5:2:5 | y | This assignment to $@ is useless, since its value is never read. | DeadStoreOfLocal.rb:2:5:2:5 | y | y |
diff --git a/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.qlref b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.qlref
new file mode 100644
index 000000000000..222bba3622b5
--- /dev/null
+++ b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.qlref
@@ -0,0 +1,2 @@
+query: queries/variables/DeadStoreOfLocal.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.rb b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.rb
new file mode 100644
index 000000000000..7f3252a58fdc
--- /dev/null
+++ b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/DeadStoreOfLocal.rb
@@ -0,0 +1,36 @@
+def test_basic(x)
+ y = x #$ Alert
+ y = x + 2
+ return y
+end
+
+def test_retry
+ x = 0
+ begin
+ if x == 0
+ raise "error"
+ end
+ rescue
+ x = 2 # OK - the retry will allow a later read
+ retry
+ end
+ return 42
+end
+
+def test_binding
+ x = 4 # OK - the binding collects the value of x
+ return binding
+end
+
+class Sup
+ def m(x)
+ print(x + 1)
+ end
+end
+
+class Sub < Sup
+ def m(y)
+ y = 3 # OK - the call to `super` sees the value of `y``
+ super
+ end
+end
\ No newline at end of file
diff --git a/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/TestTemplate.rb b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/TestTemplate.rb
new file mode 100644
index 000000000000..e13b54d0e038
--- /dev/null
+++ b/ruby/ql/test/query-tests/variables/DeadStoreOfLocal/TestTemplate.rb
@@ -0,0 +1,10 @@
+# -*- coding: utf-8 -*-
+require 'erb'
+
+template = ERB.new <