diff --git a/ql/src/queries/performance/UseDetect.ql b/ql/src/queries/performance/UseDetect.ql new file mode 100644 index 00000000000..d17f5d6f8f9 --- /dev/null +++ b/ql/src/queries/performance/UseDetect.ql @@ -0,0 +1,65 @@ +/** + * @name Use detect + * @description Use 'detect' instead of 'select' followed by 'first' or 'last'. + * @kind problem + * @problem.severity warning + * @id rb/use-detect + * @tags performance rubocop + * @precision high + * + * This is an implementation of Rubocop rule + * https://github.com/rubocop/rubocop-performance/blob/master/lib/rubocop/cop/performance/detect.rb + */ + +import ruby +import codeql_ruby.dataflow.SSA + +/** A call that extracts the first or last element of a list. */ +class EndCall extends MethodCall { + string detect; + + EndCall() { + detect = "detect" and + ( + this.getMethodName() = "first" and + this.getNumberOfArguments() = 0 + or + this.getNumberOfArguments() = 1 and + this.getArgument(0).(IntegerLiteral).getValueText() = "0" + ) + or + detect = "reverse_detect" and + ( + this.getMethodName() = "last" and + this.getNumberOfArguments() = 0 + or + this.getNumberOfArguments() = 1 and + this.getArgument(0).(UnaryMinusExpr).getOperand().(IntegerLiteral).getValueText() = "1" + ) + } + + string detectCall() { result = detect } +} + +Expr getUniqueRead(Expr e) { + exists(AssignExpr ae | + e = ae.getRightOperand() and + forex(Ssa::WriteDefinition def | def.getWriteAccess() = ae.getLeftOperand() | + strictcount(def.getARead()) = 1 and + not def = any(Ssa::PhiNode phi).getAnInput() and + def.getARead() = result.getAControlFlowNode() + ) + ) +} + +class SelectBlock extends MethodCall { + SelectBlock() { + this.getMethodName() in ["select", "filter", "find_all"] and + exists(this.getBlock()) + } +} + +from EndCall call, SelectBlock selectBlock +where getUniqueRead*(selectBlock) = call.getReceiver() +select call, "Replace this call and $@ with '" + call.detectCall() + "'.", selectBlock, + "'select' call" diff --git a/ql/test/query-tests/performance/UseDetect/UseDetect.expected b/ql/test/query-tests/performance/UseDetect/UseDetect.expected new file mode 100644 index 00000000000..d8a5ec8dd41 --- /dev/null +++ b/ql/test/query-tests/performance/UseDetect/UseDetect.expected @@ -0,0 +1,7 @@ +| UseDetect.rb:5:9:5:36 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:5:9:5:30 | call to select | 'select' call | +| UseDetect.rb:6:9:6:35 | call to last | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:6:9:6:30 | call to select | 'select' call | +| UseDetect.rb:7:9:7:33 | ...[...] | Replace this call and $@ with 'detect'. | UseDetect.rb:7:9:7:30 | call to select | 'select' call | +| UseDetect.rb:8:9:8:34 | ...[...] | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:8:9:8:30 | call to select | 'select' call | +| UseDetect.rb:9:9:9:36 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:9:9:9:30 | call to filter | 'select' call | +| UseDetect.rb:10:9:10:37 | call to last | Replace this call and $@ with 'reverse_detect'. | UseDetect.rb:10:9:10:32 | call to find_all | 'select' call | +| UseDetect.rb:12:9:12:24 | call to first | Replace this call and $@ with 'detect'. | UseDetect.rb:11:22:11:43 | call to select | 'select' call | diff --git a/ql/test/query-tests/performance/UseDetect/UseDetect.qlref b/ql/test/query-tests/performance/UseDetect/UseDetect.qlref new file mode 100644 index 00000000000..6d920e39cff --- /dev/null +++ b/ql/test/query-tests/performance/UseDetect/UseDetect.qlref @@ -0,0 +1 @@ +queries/performance/UseDetect.ql \ No newline at end of file diff --git a/ql/test/query-tests/performance/UseDetect/UseDetect.rb b/ql/test/query-tests/performance/UseDetect/UseDetect.rb new file mode 100644 index 00000000000..e1d2d9b91ba --- /dev/null +++ b/ql/test/query-tests/performance/UseDetect/UseDetect.rb @@ -0,0 +1,21 @@ + +class DetectTest + def test + # These are bad + [].select { |i| true }.first + [].select { |i| true }.last + [].select { |i| true }[0] + [].select { |i| true }[-1] + [].filter { |i| true }.first + [].find_all { |i| true }.last + selection1 = [].select { |i| true } + selection1.first + + # These are good + [].select("").first # Selecting a string + [].select { |i| true }[1] # Selecting the second element + selection2 = [].select { |i| true } + selection2.first # Selection used elsewhere + selection3 = selection2 + end +end