Merge pull request github#5272 from Marcono1234/marcono1234/simplify-own-member-access-checks

Java: Simplify own member access checks

Author: aschackmull

java/ql/src/Frameworks/Spring/Architecture/Refactoring Opportunities/UnusedBean.ql

@@ -21,13 +21,7 @@ class InstanceFieldWrite extends FieldWrite {
     not getEnclosingCallable().isStatic() and
     // Must be declared in this type or a supertype.
     getEnclosingCallable().getDeclaringType().inherits(getField()) and
-    (
-      // There must either be no qualifier - implied "this"
-      not exists(getQualifier()) or
-      // Or the qualifier implies we are accessing this or the super type
-      getQualifier() instanceof ThisAccess or
-      getQualifier() instanceof SuperAccess
-    )
+    isOwnFieldAccess()
   }
 }
 

java/ql/src/Likely Bugs/Statements/Chaining.qll

@@ -47,10 +47,7 @@ private predicate nonChainingReturn(Method m, ReturnStmt ret) {
     not hasSubtype*(m.getReturnType(), delegate.getReturnType())
     or
     // A method on the wrong object is called.
-    not (
-      delegateCall.getQualifier() instanceof ThisAccess or
-      not exists(delegateCall.getQualifier())
-    )
+    not delegateCall.isOwnMethodAccess()
     or
     nonChaining(delegate)
   )

java/ql/src/Likely Bugs/Termination/SpinOnField.ql

@@ -50,17 +50,10 @@ class EmptyLoop extends Stmt {
   }
 }
 
-/** An access to a field in this object. */
-class FieldAccessInThis extends VarAccess {
-  FieldAccessInThis() {
-    this.getVariable() instanceof Field and
-    (not this.hasQualifier() or this.getQualifier() instanceof ThisAccess)
-  }
-}
-
-from EmptyLoop loop, FieldAccessInThis access, Field field, ComparisonOrEqTestExpr expr
+from EmptyLoop loop, FieldAccess access, Field field, ComparisonOrEqTestExpr expr
 where
   loop.getCondition() = expr and
+  access.isOwnFieldAccess() and
   access.getParent() = expr and
   field = access.getVariable() and
   field.isStatic() and

java/ql/src/Violations of Best Practice/Dead Code/DeadLocals.qll

@@ -113,12 +113,8 @@ private predicate constructorHasEffect(Constructor c) {
   or
   exists(Assignment a | a.getEnclosingCallable() = c |
     not exists(VarAccess va | va = a.getDest() |
-      va.getVariable() instanceof LocalVariableDecl
-      or
-      exists(Field f | f = va.getVariable() |
-        va.getQualifier() instanceof ThisAccess or
-        not exists(va.getQualifier())
-      )
+      va.getVariable() instanceof LocalVariableDecl or
+      va.(FieldAccess).isOwnFieldAccess()
     )
   )
 }

java/ql/src/Violations of Best Practice/Implementation Hiding/GetClassGetResource.ql

@@ -12,22 +12,15 @@
 
 import java
 
-/** Access to a method in `this` object. */
-class MethodAccessInThis extends MethodAccess {
-  MethodAccessInThis() {
-    not this.hasQualifier() or
-    this.getQualifier() instanceof ThisAccess
-  }
-}
-
-from Class c, MethodAccess getResource, MethodAccessInThis getClass
+from Class c, MethodAccess getResource, MethodAccess getClass
 where
   getResource.getNumArgument() = 1 and
   (
     getResource.getMethod().hasName("getResource") or
     getResource.getMethod().hasName("getResourceAsStream")
   ) and
   getResource.getQualifier() = getClass and
+  getClass.isOwnMethodAccess() and
   getClass.getNumArgument() = 0 and
   getClass.getMethod().hasName("getClass") and
   getResource.getEnclosingCallable().getDeclaringType() = c and

java/ql/src/Violations of Best Practice/Undesirable Calls/NextFromIterator.ql

@@ -16,10 +16,7 @@ from MethodAccess m
 where
   m.getMethod().hasName("next") and
   m.getMethod().getNumberOfParameters() = 0 and
-  (
-    not m.hasQualifier() or
-    m.getQualifier() instanceof ThisAccess
-  ) and
+  m.isOwnMethodAccess() and
   exists(Interface i, Method hasNext |
     i.getSourceDeclaration().hasQualifiedName("java.util", "Iterator") and
     m.getEnclosingCallable() = hasNext and