Ruby: Rewrite inline expectation tests to use parameterized module

jketema · jketema · commit 4485560f4377 · 2023-06-09T10:43:05.000+02:00
diff --git a/ruby/ql/test/library-tests/concepts/CryptographicOperation.expected b/ruby/ql/test/library-tests/concepts/CryptographicOperation.expected
@@ -0,0 +1,2 @@
+failures
+testFailures
diff --git a/ruby/ql/test/library-tests/concepts/CryptographicOperation.ql b/ruby/ql/test/library-tests/concepts/CryptographicOperation.ql
@@ -2,17 +2,15 @@ import codeql.ruby.AST
 import codeql.ruby.Concepts
 import TestUtilities.InlineExpectationsTest
 
-class CryptographicOperationTest extends InlineExpectationsTest {
-  CryptographicOperationTest() { this = "CryptographicOperationTest" }
-
-  override string getARelevantTag() {
+module CryptographicOperationTest implements TestSig {
+  string getARelevantTag() {
     result in [
         "CryptographicOperation", "CryptographicOperationInput", "CryptographicOperationAlgorithm",
         "CryptographicOperationBlockMode"
       ]
   }
 
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     exists(Cryptography::CryptographicOperation cryptoOperation |
       location = cryptoOperation.getLocation() and
       (
@@ -31,3 +29,5 @@ class CryptographicOperationTest extends InlineExpectationsTest {
     )
   }
 }
+
+import MakeTest<CryptographicOperationTest>
diff --git a/ruby/ql/test/library-tests/dataflow/api-graphs/use.expected b/ruby/ql/test/library-tests/dataflow/api-graphs/use.expected
@@ -0,0 +1,2 @@
+failures
+testFailures
diff --git a/ruby/ql/test/library-tests/dataflow/api-graphs/use.ql b/ruby/ql/test/library-tests/dataflow/api-graphs/use.ql
@@ -17,10 +17,8 @@ class CustomEntryPointUse extends API::EntryPoint {
   }
 }
 
-class ApiUseTest extends InlineExpectationsTest {
-  ApiUseTest() { this = "ApiUseTest" }
-
-  override string getARelevantTag() { result = ["use", "def", "call"] }
+module ApiUseTest implements TestSig {
+  string getARelevantTag() { result = ["use", "def", "call"] }
 
   private predicate relevantNode(API::Node a, DataFlow::Node n, Location l, string tag) {
     l = n.getLocation() and
@@ -36,13 +34,13 @@ class ApiUseTest extends InlineExpectationsTest {
     )
   }
 
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "use" and // def tags are always optional
-    exists(DataFlow::Node n | this.relevantNode(_, n, location, tag) |
+    exists(DataFlow::Node n | relevantNode(_, n, location, tag) |
       // Only report the longest path on this line:
       value =
         max(API::Node a2, Location l2, DataFlow::Node n2 |
-          this.relevantNode(a2, n2, l2, tag) and
+          relevantNode(a2, n2, l2, tag) and
           l2.getFile() = location.getFile() and
           l2.getEndLine() = location.getEndLine()
         |
@@ -56,14 +54,16 @@ class ApiUseTest extends InlineExpectationsTest {
 
   // We also permit optional annotations for any other path on the line.
   // This is used to test subclass paths, which typically have a shorter canonical path.
-  override predicate hasOptionalResult(Location location, string element, string tag, string value) {
-    exists(API::Node a, DataFlow::Node n | this.relevantNode(a, n, location, tag) |
+  predicate hasOptionalResult(Location location, string element, string tag, string value) {
+    exists(API::Node a, DataFlow::Node n | relevantNode(a, n, location, tag) |
       element = n.toString() and
       value = getAPath(a, _)
     )
   }
 }
 
+import MakeTest<ApiUseTest>
+
 private int size(AstNode n) { not n instanceof StmtSequence and result = count(n.getAChild*()) }
 
 /**
diff --git a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.expected
@@ -1,5 +1,6 @@
 WARNING: Type BarrierGuard has been deprecated and may be removed in future (barrier-guards.ql:10,3-15)
 failures
+testFailures
 oldStyleBarrierGuards
 | barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:4:3:6 | foo | true |
 | barrier-guards.rb:3:4:3:15 | ... == ... | barrier-guards.rb:4:5:4:7 | foo | barrier-guards.rb:3:11:3:15 | "foo" | true |
diff --git a/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql b/ruby/ql/test/library-tests/dataflow/barrier-guards/barrier-guards.ql
@@ -24,12 +24,10 @@ query predicate controls(CfgNode condition, BasicBlock bb, SuccessorTypes::Condi
   )
 }
 
-class BarrierGuardTest extends InlineExpectationsTest {
-  BarrierGuardTest() { this = "BarrierGuardTest" }
+module BarrierGuardTest implements TestSig {
+  string getARelevantTag() { result = "guarded" }
 
-  override string getARelevantTag() { result = "guarded" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "guarded" and
     exists(DataFlow::Node n |
       newStyleBarrierGuards(n) and
@@ -39,3 +37,5 @@ class BarrierGuardTest extends InlineExpectationsTest {
     )
   }
 }
+
+import MakeTest<BarrierGuardTest>
diff --git a/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.expected b/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.expected
@@ -1,4 +1,5 @@
 failures
+testFailures
 | improper_memoization.rb:100:1:104:3 | m14 | Unexpected result: result=BAD |
 #select
 | improper_memoization.rb:50:1:55:3 | m7 | improper_memoization.rb:50:8:50:10 | arg | improper_memoization.rb:51:3:53:5 | ... \|\|= ... |
diff --git a/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.ql b/ruby/ql/test/query-tests/experimental/improper-memoization/ImproperMemoization.ql
@@ -2,12 +2,10 @@ import codeql.ruby.AST
 import TestUtilities.InlineExpectationsTest
 import codeql.ruby.security.ImproperMemoizationQuery
 
-class ImproperMemoizationTest extends InlineExpectationsTest {
-  ImproperMemoizationTest() { this = "ImproperMemoizationTest" }
+module ImproperMemoizationTest implements TestSig {
+  string getARelevantTag() { result = "result" }
 
-  override string getARelevantTag() { result = "result" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "result" and
     value = "BAD" and
     exists(Expr e |
@@ -18,6 +16,8 @@ class ImproperMemoizationTest extends InlineExpectationsTest {
   }
 }
 
+import MakeTest<ImproperMemoizationTest>
+
 from Method m, Parameter p, AssignLogicalOrExpr s
 where isImproperMemoizationMethod(m, p, s)
 select m, p, s
diff --git a/ruby/ql/test/query-tests/security/cwe-116/IncompleteMultiCharacterSanitization/IncompleteMultiCharacterSanitization.expected b/ruby/ql/test/query-tests/security/cwe-116/IncompleteMultiCharacterSanitization/IncompleteMultiCharacterSanitization.expected
@@ -0,0 +1,2 @@
+failures
+testFailures
diff --git a/ruby/ql/test/query-tests/security/cwe-116/IncompleteMultiCharacterSanitization/IncompleteMultiCharacterSanitization.ql b/ruby/ql/test/query-tests/security/cwe-116/IncompleteMultiCharacterSanitization/IncompleteMultiCharacterSanitization.ql
@@ -8,17 +8,17 @@ import codeql.ruby.DataFlow
 import codeql.ruby.security.IncompleteMultiCharacterSanitizationQuery as Query
 import TestUtilities.InlineExpectationsTest
 
-class Test extends InlineExpectationsTest {
-  Test() { this = "IncompleteMultiCharacterSanitizationTest" }
+module Test implements TestSig {
+  string getARelevantTag() { result = "hasResult" }
 
-  override string getARelevantTag() { result = "hasResult" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "hasResult" and
     hasResult(location, element, value)
   }
 }
 
+import MakeTest<Test>
+
 predicate hasResult(Location location, string element, string value) {
   exists(DataFlow::Node replace, string kind |
     replace.getLocation() = location and
diff --git a/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.expected b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.expected
@@ -1,4 +1,5 @@
 failures
+testFailures
 #select
 | Gemfile:2:8:2:28 | "http://rubygems.org" | Dependency source URL uses the unencrypted protocol HTTP. Use HTTPS instead. |
 | Gemfile:3:8:3:27 | "ftp://rubygems.org" | Dependency source URL uses the unencrypted protocol FTP. Use FTPS or SFTP instead. |
diff --git a/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql b/ruby/ql/test/query-tests/security/cwe-300/InsecureDependency.ql
@@ -2,12 +2,10 @@ import codeql.ruby.AST
 import TestUtilities.InlineExpectationsTest
 import codeql.ruby.security.InsecureDependencyQuery
 
-class InsecureDependencyTest extends InlineExpectationsTest {
-  InsecureDependencyTest() { this = "InsecureDependencyTest" }
+module InsecureDependencyTest implements TestSig {
+  string getARelevantTag() { result = "result" }
 
-  override string getARelevantTag() { result = "result" }
-
-  override predicate hasActualResult(Location location, string element, string tag, string value) {
+  predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "result" and
     value = "BAD" and
     exists(Expr e |
@@ -18,6 +16,8 @@ class InsecureDependencyTest extends InlineExpectationsTest {
   }
 }
 
+import MakeTest<InsecureDependencyTest>
+
 from Expr url, string msg
 where insecureDependencyUrl(url, msg)
 select url, msg

Original file line number	Diff line number	Diff line change
`@@ -2,17 +2,15 @@ import codeql.ruby.AST`
`2`	`2`	`import codeql.ruby.Concepts`
`3`	`3`	`import TestUtilities.InlineExpectationsTest`
`4`	`4`
`5`		`-class CryptographicOperationTest extends InlineExpectationsTest {`
`6`		`- CryptographicOperationTest() { this = "CryptographicOperationTest" }`
`7`		`-`
`8`		`- override string getARelevantTag() {`
	`5`	`+module CryptographicOperationTest implements TestSig {`
	`6`	`+ string getARelevantTag() {`
`9`	`7`	`result in [`
`10`	`8`	`"CryptographicOperation", "CryptographicOperationInput", "CryptographicOperationAlgorithm",`
`11`	`9`	`"CryptographicOperationBlockMode"`
`12`	`10`	`]`
`13`	`11`	`}`
`14`	`12`
`15`		`- override predicate hasActualResult(Location location, string element, string tag, string value) {`
	`13`	`+ predicate hasActualResult(Location location, string element, string tag, string value) {`
`16`	`14`	`exists(Cryptography::CryptographicOperation cryptoOperation \|`
`17`	`15`	`location = cryptoOperation.getLocation() and`
`18`	`16`	`(`
`@@ -31,3 +29,5 @@ class CryptographicOperationTest extends InlineExpectationsTest {`
`31`	`29`	`)`
`32`	`30`	`}`
`33`	`31`	`}`
	`32`	`+`
	`33`	`+import MakeTest<CryptographicOperationTest>`
Original file line number	Diff line number	Diff line change
`@@ -17,10 +17,8 @@ class CustomEntryPointUse extends API::EntryPoint {`
`17`	`17`	`}`
`18`	`18`	`}`
`19`	`19`
`20`		`-class ApiUseTest extends InlineExpectationsTest {`
`21`		`- ApiUseTest() { this = "ApiUseTest" }`
`22`		`-`
`23`		`- override string getARelevantTag() { result = ["use", "def", "call"] }`
	`20`	`+module ApiUseTest implements TestSig {`
	`21`	`+ string getARelevantTag() { result = ["use", "def", "call"] }`
`24`	`22`
`25`	`23`	`private predicate relevantNode(API::Node a, DataFlow::Node n, Location l, string tag) {`
`26`	`24`	`l = n.getLocation() and`
`@@ -36,13 +34,13 @@ class ApiUseTest extends InlineExpectationsTest {`
`36`	`34`	`)`
`37`	`35`	`}`
`38`	`36`
`39`		`- override predicate hasActualResult(Location location, string element, string tag, string value) {`
	`37`	`+ predicate hasActualResult(Location location, string element, string tag, string value) {`
`40`	`38`	`tag = "use" and // def tags are always optional`
`41`		`- exists(DataFlow::Node n \| this.relevantNode(_, n, location, tag) \|`
	`39`	`+ exists(DataFlow::Node n \| relevantNode(_, n, location, tag) \|`
`42`	`40`	`// Only report the longest path on this line:`
`43`	`41`	`value =`
`44`	`42`	`max(API::Node a2, Location l2, DataFlow::Node n2 \|`
`45`		`- this.relevantNode(a2, n2, l2, tag) and`
	`43`	`+ relevantNode(a2, n2, l2, tag) and`
`46`	`44`	`l2.getFile() = location.getFile() and`
`47`	`45`	`l2.getEndLine() = location.getEndLine()`
`48`	`46`	`\|`
`@@ -56,14 +54,16 @@ class ApiUseTest extends InlineExpectationsTest {`
`56`	`54`
`57`	`55`	`// We also permit optional annotations for any other path on the line.`
`58`	`56`	`// This is used to test subclass paths, which typically have a shorter canonical path.`
`59`		`- override predicate hasOptionalResult(Location location, string element, string tag, string value) {`
`60`		`- exists(API::Node a, DataFlow::Node n \| this.relevantNode(a, n, location, tag) \|`
	`57`	`+ predicate hasOptionalResult(Location location, string element, string tag, string value) {`
	`58`	`+ exists(API::Node a, DataFlow::Node n \| relevantNode(a, n, location, tag) \|`
`61`	`59`	`element = n.toString() and`
`62`	`60`	`value = getAPath(a, _)`
`63`	`61`	`)`
`64`	`62`	`}`
`65`	`63`	`}`
`66`	`64`
	`65`	`+import MakeTest<ApiUseTest>`
	`66`	`+`
`67`	`67`	`private int size(AstNode n) { not n instanceof StmtSequence and result = count(n.getAChild*()) }`
`68`	`68`
`69`	`69`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -24,12 +24,10 @@ query predicate controls(CfgNode condition, BasicBlock bb, SuccessorTypes::Condi`
`24`	`24`	`)`
`25`	`25`	`}`
`26`	`26`
`27`		`-class BarrierGuardTest extends InlineExpectationsTest {`
`28`		`- BarrierGuardTest() { this = "BarrierGuardTest" }`
	`27`	`+module BarrierGuardTest implements TestSig {`
	`28`	`+ string getARelevantTag() { result = "guarded" }`
`29`	`29`
`30`		`- override string getARelevantTag() { result = "guarded" }`
`31`		`-`
`32`		`- override predicate hasActualResult(Location location, string element, string tag, string value) {`
	`30`	`+ predicate hasActualResult(Location location, string element, string tag, string value) {`
`33`	`31`	`tag = "guarded" and`
`34`	`32`	`exists(DataFlow::Node n \|`
`35`	`33`	`newStyleBarrierGuards(n) and`
`@@ -39,3 +37,5 @@ class BarrierGuardTest extends InlineExpectationsTest {`
`39`	`37`	`)`
`40`	`38`	`}`
`41`	`39`	`}`
	`40`	`+`
	`41`	`+import MakeTest<BarrierGuardTest>`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,5 @@`
`1`	`1`	`failures`
	`2`	`+testFailures`
`2`	`3`	`\| improper_memoization.rb:100:1:104:3 \| m14 \| Unexpected result: result=BAD \|`
`3`	`4`	`#select`
`4`	`5`	`\| improper_memoization.rb:50:1:55:3 \| m7 \| improper_memoization.rb:50:8:50:10 \| arg \| improper_memoization.rb:51:3:53:5 \| ... \\|\\|= ... \|`