mirror of
https://github.com/github/codeql.git
synced 2025-12-17 01:03:14 +01:00
ruby: improve help file
This has improved autofixes I hope it also helps humans
This commit is contained in:
41
ruby/ql/src/queries/variables/UninitializedLocal.md
Normal file
41
ruby/ql/src/queries/variables/UninitializedLocal.md
Normal file
@@ -0,0 +1,41 @@
|
||||
# Method call on `nil`
|
||||
|
||||
## Description
|
||||
In Ruby, it is not necessary to explicitly initialize variables.
|
||||
If a local variable has not been explicitly initialized, it will have the value `nil`. If this happens unintended, though, the variable will not represent an object with the expected methods, and a method call on the variable will raise a `NoMethodError`.
|
||||
|
||||
## Recommendation
|
||||
|
||||
Ensure that the variable cannot be `nil` at the point hightligted by the alert.
|
||||
This can be achieved by using a safe navigation or adding a check for `nil`.
|
||||
|
||||
Note: You do not need to explicitly initialize the variable, if you can make the program deal with the possible `nil` value. In particular, initializing the variable to `nil` will have no effect, as this is already the value of the variable. If `nil` is the only possibly default value, you need to handle the `nil` value instead of initializing the variable.
|
||||
|
||||
## Examples
|
||||
|
||||
In the following code, the call to `create_file` may fail and then the call `f.close` will raise a `NoMethodError` since `f` will be `nil` at that point.
|
||||
|
||||
```ruby
|
||||
def dump(x)
|
||||
f = create_file
|
||||
f.puts(x)
|
||||
ensure
|
||||
f.close
|
||||
end
|
||||
```
|
||||
|
||||
We can fix this by using safe navigation:
|
||||
```ruby
|
||||
def dump(x)
|
||||
f = create_file
|
||||
f.puts(x)
|
||||
ensure
|
||||
f&.close
|
||||
end
|
||||
```
|
||||
|
||||
## References
|
||||
|
||||
- https://www.rubyguides.com/: [Nil](https://www.rubyguides.com/2018/01/ruby-nil/)
|
||||
- https://ruby-doc.org/: [NoMethodError](https://ruby-doc.org/core-2.6.5/NoMethodError.html)
|
||||
|
||||
@@ -1,37 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>
|
||||
In Ruby, raw identifiers like <code>x</code> can be both local variable accesses and method calls. It is a local variable access iff it is syntactically preceded by something that binds it (like an assignment).
|
||||
Consider the following example:
|
||||
</p>
|
||||
|
||||
<sample src="examples/UninitializedLocal.rb" />
|
||||
|
||||
<p>
|
||||
This will generate an alert on the last access to <code>m</code>, where it is not clear that the programmer intended to read from the local variable.
|
||||
In fact, the last access to <code>m</code> is a method call, and the value of the local variable is <code>nil</code>,
|
||||
so this will raise a <code>NoMethodError</code>.
|
||||
</p>
|
||||
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>
|
||||
Ensure that you check the control and data flow in the method carefully.
|
||||
Add a check for <code>nil</code> before the read, or rewrite the code to ensure that the variable is always initialized before being read.
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<references>
|
||||
|
||||
|
||||
<li>https://www.rubyguides.com/: <a href="https://www.rubyguides.com/2018/01/ruby-nil/">Nil</a>.</li>
|
||||
<li>https://ruby-doc.org/: <a href="https://ruby-doc.org/core-2.6.5/NoMethodError.html">NoMethodError</a>.</li>
|
||||
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -72,13 +72,24 @@ private predicate isNilChecked(LocalVariableReadAccess read) {
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `name` is the name of a method defined on `nil`.
|
||||
* See https://ruby-doc.org/core-2.5.8/NilClass.html
|
||||
*/
|
||||
private predicate isNilMethodName(string name) {
|
||||
name in [
|
||||
"inspect", "instance_of?", "is_a?", "kind_of?", "method", "nil?", "rationalize", "to_a",
|
||||
"to_c", "to_f", "to_h", "to_i", "to_r", "to_s"
|
||||
]
|
||||
}
|
||||
|
||||
class RelevantLocalVariableReadAccess extends LocalVariableReadAccess instanceof TVariableAccessReal
|
||||
{
|
||||
RelevantLocalVariableReadAccess() {
|
||||
not isInBooleanContext(this) and
|
||||
not isNilChecked(this) and
|
||||
not isGuarded(this) and
|
||||
this = any(MethodCall m).getReceiver()
|
||||
this = any(MethodCall m | not isNilMethodName(m.getMethodName())).getReceiver()
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user