Merge pull request #13032 from kaspersv/kaspersv/ruby-enable-implicit-this-warnings

Ruby: Enable implicit this warnings

Author: kaspersv

ruby/ql/lib/codeql/ruby/frameworks/core/Gem.qll

@@ -57,36 +57,40 @@ module Gem {
     }
 
     /** Gets the name of the gem */
-    string getName() { result = getSpecProperty("name").getConstantValue().getString() }
+    string getName() { result = this.getSpecProperty("name").getConstantValue().getString() }
 
     /** Gets a path that is loaded when the gem is required */
     private string getARequirePath() {
-      result = getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()
+      result =
+        this.getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()
       or
-      not exists(getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()) and
+      not exists(
+        this.getSpecProperty(["require_paths", "require_path"]).getConstantValue().getString()
+      ) and
       result = "lib" // the default is "lib"
     }
 
     /** Gets a file that could be loaded when the gem is required. */
     private File getAPossiblyRequiredFile() {
-      result = File.super.getParentContainer().getFolder(getARequirePath()).getAChildContainer*()
+      result =
+        File.super.getParentContainer().getFolder(this.getARequirePath()).getAChildContainer*()
     }
 
     /** Gets a class/module that is exported by this gem. */
     private ModuleBase getAPublicModule() {
-      result.(Toplevel).getLocation().getFile() = getAPossiblyRequiredFile()
+      result.(Toplevel).getLocation().getFile() = this.getAPossiblyRequiredFile()
       or
-      result = getAPublicModule().getAModule()
+      result = this.getAPublicModule().getAModule()
       or
-      result = getAPublicModule().getAClass()
+      result = this.getAPublicModule().getAClass()
       or
-      result = getAPublicModule().getStmt(_).(SingletonClass)
+      result = this.getAPublicModule().getStmt(_).(SingletonClass)
     }
 
     /** Gets a parameter from an exported method, which is an input to this gem. */
     DataFlow::ParameterNode getAnInputParameter() {
       exists(MethodBase method |
-        method = getAPublicModule().getAMethod() and
+        method = this.getAPublicModule().getAMethod() and
         result.getParameter() = method.getAParameter()
       |
         method.isPublic()

ruby/ql/lib/qlpack.yml

@@ -12,3 +12,4 @@ dependencies:
   codeql/util: ${workspace}
 dataExtensions:
   - codeql/ruby/frameworks/**/model.yml
+warnOnImplicitThis: true

ruby/ql/src/qlpack.yml

@@ -9,3 +9,4 @@ dependencies:
   codeql/ruby-all: ${workspace}
   codeql/suite-helpers: ${workspace}
   codeql/util: ${workspace}
+warnOnImplicitThis: true

ruby/ql/src/queries/meta/internal/TaintMetrics.qll

@@ -11,7 +11,7 @@ private import codeql.ruby.security.UrlRedirectCustomizations
 private import codeql.ruby.security.SqlInjectionCustomizations
 
 class RelevantFile extends File {
-  RelevantFile() { not getRelativePath().regexpMatch(".*/test(case)?s?/.*") }
+  RelevantFile() { not this.getRelativePath().regexpMatch(".*/test(case)?s?/.*") }
 }
 
 RemoteFlowSource relevantTaintSource(string kind) {

ruby/ql/test/library-tests/dataflow/api-graphs/use.ql

@@ -38,11 +38,11 @@ class ApiUseTest extends InlineExpectationsTest {
 
   override predicate hasActualResult(Location location, string element, string tag, string value) {
     tag = "use" and // def tags are always optional
-    exists(DataFlow::Node n | relevantNode(_, n, location, tag) |
+    exists(DataFlow::Node n | this.relevantNode(_, n, location, tag) |
       // Only report the longest path on this line:
       value =
         max(API::Node a2, Location l2, DataFlow::Node n2 |
-          relevantNode(a2, n2, l2, tag) and
+          this.relevantNode(a2, n2, l2, tag) and
           l2.getFile() = location.getFile() and
           l2.getEndLine() = location.getEndLine()
         |
@@ -57,7 +57,7 @@ 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 | relevantNode(a, n, location, tag) |
+    exists(API::Node a, DataFlow::Node n | this.relevantNode(a, n, location, tag) |
       element = n.toString() and
       value = getAPath(a, _)
     )

ruby/ql/test/qlpack.yml

@@ -6,3 +6,4 @@ dependencies:
   codeql/ruby-all: ${workspace}
 extractor: ruby
 tests: .
+warnOnImplicitThis: true