Merge pull request #73 from github/tausbn/add-implicit-this-query

Add "implicit `this`" query

ql/src/queries/style/ImplicitThis.ql

@@ -0,0 +1,34 @@
+/**
+ * @name Using implicit `this`
+ * @description Writing member predicate calls with an implicit `this` can be confusing
+ * @kind problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id ql/implicit-this
+ * @tags maintainability
+ */
+
+import ql
+
+MemberCall explicitThisCallInFile(File f) {
+  result.getLocation().getFile() = f and
+  result.getBase() instanceof ThisAccess and
+  // Exclude `this.(Type).whatever(...)`, as some files have that as their only instance of `this`.
+  not result = any(InlineCast c).getBase()
+}
+
+PredicateCall implicitThisCallInFile(File f) {
+  result.getLocation().getFile() = f and
+  exists(result.getTarget().getDeclaringType().getASuperType()) and
+  // Exclude `SomeModule::whatever(...)`
+  not exists(result.getQualifier())
+}
+
+PredicateCall confusingImplicitThisCall(File f) {
+  result = implicitThisCallInFile(f) and
+  exists(explicitThisCallInFile(f))
+}
+
+from PredicateCall c
+where c = confusingImplicitThisCall(_)
+select c, "Use of implicit `this`."

ql/test/queries/style/ImplicitThis/Bad.qll

@@ -0,0 +1,11 @@
+import ql
+
+class Foo extends string {
+  Foo() { this = "hello" }
+
+  string getBar() { result = "bar" }
+
+  string getBarWithThis() { result = this.getBar() }
+
+  string getBarWithoutThis() { result = getBar() }
+}

ql/test/queries/style/ImplicitThis/Good.qll

@@ -0,0 +1,21 @@
+import ql
+
+class Foo extends string {
+  Foo() { this = "hello" }
+
+  string getBar() { result = "bar" }
+
+  string getBarWithThis() { result = this.getBar() }
+
+  /* Okay because not a member predicate. */
+  string getBaz() { result = Baz::baz() }
+
+  /* Okay because not a member predicate. */
+  string getOuterQuux() { result = getQuux() }
+}
+
+string getQuux() { result = "quux" }
+
+module Baz {
+  string baz() { result = "baz" }
+}

ql/test/queries/style/ImplicitThis/ImplicitThis.expected

@@ -0,0 +1 @@
+| Bad.qll:10:41:10:48 | PredicateCall | Use of implicit `this`. |

ql/test/queries/style/ImplicitThis/ImplicitThis.qlref

@@ -0,0 +1 @@
+queries/style/ImplicitThis.ql

ql/test/queries/style/ImplicitThis/Okay.qll

@@ -0,0 +1,13 @@
+import ql
+
+class Foo extends string {
+  Foo() { this = "hello" }
+
+  string getBar() { result = "bar" }
+
+  /* Okay, because we don't write `this.some_method` anywhere */
+  string getBarWithoutThis() { result = getBar() }
+
+  /* Okay, because this is the only way to cast `this`. */
+  string useThisWithInlineCast() { result = this.(string).toUpperCase() }
+}