From f7f8b47f12e1e664d0674a0e840ca18211e2f259 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 09:40:38 +0100
Subject: [PATCH 01/23] Java: Add initial version of empty method query

---
 java/ql/src/Language Abuse/EmptyMethod.md     |  39 +++
 java/ql/src/Language Abuse/EmptyMethod.ql     |  99 +++++++
 .../EmptyMethod/EmptyMethod.expected          |   1 +
 .../query-tests/EmptyMethod/EmptyMethod.qlref |   1 +
 .../ql/test/query-tests/EmptyMethod/Test.java |  29 ++
 java/ql/test/query-tests/EmptyMethod/options  |   1 +
 java/ql/test/stubs/aspectj/LICENSE.txt        | 279 ++++++++++++++++++
 .../org/aspectj/lang/annotation/Pointcut.java |  27 ++
 8 files changed, 476 insertions(+)
 create mode 100644 java/ql/src/Language Abuse/EmptyMethod.md
 create mode 100644 java/ql/src/Language Abuse/EmptyMethod.ql
 create mode 100644 java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
 create mode 100644 java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
 create mode 100644 java/ql/test/query-tests/EmptyMethod/Test.java
 create mode 100644 java/ql/test/query-tests/EmptyMethod/options
 create mode 100644 java/ql/test/stubs/aspectj/LICENSE.txt
 create mode 100644 java/ql/test/stubs/aspectj/org/aspectj/lang/annotation/Pointcut.java

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
new file mode 100644
index 000000000000..6692251fe2bd
--- /dev/null
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -0,0 +1,39 @@
+# J-D-002: An empty method serves no purpose and may indicate programmer error
+
+An empty method serves no purpose and may indicate programmer error.
+
+## Overview
+
+An empty method may indicate that an implmentation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implentation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
+
+## Recommendation
+
+If a method is intended to be left empty, do one of the following to indicate that it is intentionally empty: 1) mark it abstract in an abstract class, 2) place it in an interface (then it can be implicitly abstract), 3) place a comment in that method that lets others know that the implementation was intentionally omitted, or 4) throw an `UnsupportedOperationException` in the method (like is done in `java.util.Collections.add`).
+
+## Example
+
+```java
+public class Test {
+    public void f1() { // COMPLIANT
+        // intentionally empty
+    }
+
+    public void f2() {} // NON_COMPLIANT
+
+    public void f3(){ throw new UnsupportedOperationException(); } // COMPLIANT
+
+    public abstract class TestInner {
+
+        public abstract void f(); // COMPLIANT - intentionally empty
+    }
+
+}
+```
+
+## Implementation Notes
+
+The rule excludes reporting methods annotated with `org.springframework.aop.Pointcut` or `org.aspectj.lang.annotation.Pointcut`. Such annotations are commonly paired with empty methods and are considered an acceptable use case for an empty method.
+
+## References
+- [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
+- [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)
\ No newline at end of file
diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
new file mode 100644
index 000000000000..4f77b8d4679b
--- /dev/null
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -0,0 +1,99 @@
+/**
+ * @id java/empty-method
+ * @name J-D-002: An empty method serves no purpose and may indicate programmer error
+ * @description An empty method serves no purpose and makes code less readable. An empty method may
+ *              indicate an error on the part of the developer.
+ * @kind problem
+ * @precision very-high
+ * @problem.severity warning
+ * @tags correctness
+ *       maintainability
+ *       readability
+ */
+
+import java
+
+/**
+ * Represents a likely a test method, which is either a method that is already
+ * recognized as a `TestMethod` or something that is likely a JUnit test or
+ * something in the expected test path for Java tests.
+ */
+class LikelyTestMethod extends Method {
+  LikelyTestMethod() {
+    this.getDeclaringType() instanceof TestClass
+    or
+    this instanceof TestMethod
+    or
+    this instanceof LikelyJunitTest
+    or
+    //standard Maven/Gradle test file discovery filepath
+    this.getFile().getAbsolutePath().matches("%src/test/java%")
+    or
+    this.getDeclaringType() instanceof SurefireTest
+  }
+}
+
+/**
+ * Classes that are likely part of junit tests (more general than `TestMethod` from `UnitTest.qll`)
+ * A `Method` that is public, has no parameters,
+ * has a "void" return type, AND either has a name that starts with "test" OR
+ * has an annotation that ends with "Test"
+ */
+class LikelyJunitTest extends Method {
+  LikelyJunitTest() {
+    this.isPublic() and
+    this.getReturnType().hasName("void") and
+    this.hasNoParameters() and
+    (
+      this.getName().matches("JUnit%") or
+      this.getName().matches("test%") or
+      this.getAnAnnotation().toString().matches("%Test")
+    )
+  }
+}
+
+/**
+ * Maven surefire patterns to consider which files are testcases:
+ * https://maven.apache.org/surefire/maven-surefire-plugin/examples/inclusion-exclusion.html
+ */
+class SurefireTest extends Class {
+  SurefireTest() {
+    this.getFile().getAbsolutePath().matches("%src/test/java%") and
+    this.getFile()
+        .getBaseName()
+        .matches(["Test%.java", "%Test.java", "%Tests.java", "%TestCase.java"])
+  }
+}
+
+/**
+ * Frameworks that provide `PointCuts`
+ * which commonly intentionally annotate empty methods
+ */
+class PointCutAnnotation extends Annotation {
+  PointCutAnnotation() {
+    this.getType().hasQualifiedName("org.aspectj.lang.annotation", "Pointcut")
+  }
+}
+
+/**
+ * A `Method` from source that is not abstract
+ */
+class NonAbstractSource extends Method {
+  NonAbstractSource() {
+    this.fromSource() and
+    not this.isAbstract() and
+    not this instanceof LikelyTestMethod
+  }
+}
+
+from NonAbstractSource m
+where
+  //empty
+  not exists(m.getBody().getAChild()) and
+  //permit comment lines explaining why this is empty
+  m.getNumberOfCommentLines() = 0 and
+  //permit a javadoc above as well as sufficient reason to leave empty
+  not exists(Javadoc jd | m.getDoc().getJavadoc() = jd) and
+  //methods annotated this way are usually intentionally empty
+  not exists(PointCutAnnotation a | a = m.getAnAnnotation())
+select m, "Empty method found."
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
new file mode 100644
index 000000000000..0bf9111a91e7
--- /dev/null
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -0,0 +1 @@
+| Test.java:16:17:16:18 | f2 | Empty method found. |
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
new file mode 100644
index 000000000000..130ebedd8d93
--- /dev/null
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
@@ -0,0 +1 @@
+Language Abuse/EmptyMethod.ql
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
new file mode 100644
index 000000000000..35bdecd00a4d
--- /dev/null
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -0,0 +1,29 @@
+import org.aspectj.lang.annotation.Pointcut;
+
+public class Test {
+
+    // COMPLIANT
+    public void f() {
+        int i = 0;
+    }
+
+    // COMPLIANT
+    public void f1() {
+        // intentionally empty
+    }
+
+    // NON_COMPLIANT
+    public void f2() {}
+
+    // COMPLIANT - exception
+    @Pointcut()
+    public void f4() {}
+
+    public abstract class TestInner {
+
+        public abstract void f(); // COMPLIANT - intentionally empty
+    }
+
+}
+
+
diff --git a/java/ql/test/query-tests/EmptyMethod/options b/java/ql/test/query-tests/EmptyMethod/options
new file mode 100644
index 000000000000..2103c1f2f26e
--- /dev/null
+++ b/java/ql/test/query-tests/EmptyMethod/options
@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../stubs/aspectj
diff --git a/java/ql/test/stubs/aspectj/LICENSE.txt b/java/ql/test/stubs/aspectj/LICENSE.txt
new file mode 100644
index 000000000000..384e2af26f7e
--- /dev/null
+++ b/java/ql/test/stubs/aspectj/LICENSE.txt
@@ -0,0 +1,279 @@
+Per: https://www.eclipse.org/org/documents/epl-2.0/EPL-2.0.txt
+
+Eclipse Public License - v 2.0
+
+    THE ACCOMPANYING PROGRAM IS PROVIDED UNDER THE TERMS OF THIS ECLIPSE
+    PUBLIC LICENSE ("AGREEMENT"). ANY USE, REPRODUCTION OR DISTRIBUTION
+    OF THE PROGRAM CONSTITUTES RECIPIENT'S ACCEPTANCE OF THIS AGREEMENT.
+
+1. DEFINITIONS
+
+"Contribution" means:
+
+  a) in the case of the initial Contributor, the initial content
+     Distributed under this Agreement, and
+
+  b) in the case of each subsequent Contributor:
+     i) changes to the Program, and
+     ii) additions to the Program;
+  where such changes and/or additions to the Program originate from
+  and are Distributed by that particular Contributor. A Contribution
+  "originates" from a Contributor if it was added to the Program by
+  such Contributor itself or anyone acting on such Contributor's behalf.
+  Contributions do not include changes or additions to the Program that
+  are not Modified Works.
+
+"Contributor" means any person or entity that Distributes the Program.
+
+"Licensed Patents" mean patent claims licensable by a Contributor which
+are necessarily infringed by the use or sale of its Contribution alone
+or when combined with the Program.
+
+"Program" means the Contributions Distributed in accordance with this
+Agreement.
+
+"Recipient" means anyone who receives the Program under this Agreement
+or any Secondary License (as applicable), including Contributors.
+
+"Derivative Works" shall mean any work, whether in Source Code or other
+form, that is based on (or derived from) the Program and for which the
+editorial revisions, annotations, elaborations, or other modifications
+represent, as a whole, an original work of authorship.
+
+"Modified Works" shall mean any work in Source Code or other form that
+results from an addition to, deletion from, or modification of the
+contents of the Program, including, for purposes of clarity any new file
+in Source Code form that contains any contents of the Program. Modified
+Works shall not include works that contain only declarations,
+interfaces, types, classes, structures, or files of the Program solely
+in each case in order to link to, bind by name, or subclass the Program
+or Modified Works thereof.
+
+"Distribute" means the acts of a) distributing or b) making available
+in any manner that enables the transfer of a copy.
+
+"Source Code" means the form of a Program preferred for making
+modifications, including but not limited to software source code,
+documentation source, and configuration files.
+
+"Secondary License" means either the GNU General Public License,
+Version 2.0, or any later versions of that license, including any
+exceptions or additional permissions as identified by the initial
+Contributor.
+
+2. GRANT OF RIGHTS
+
+  a) Subject to the terms of this Agreement, each Contributor hereby
+  grants Recipient a non-exclusive, worldwide, royalty-free copyright
+  license to reproduce, prepare Derivative Works of, publicly display,
+  publicly perform, Distribute and sublicense the Contribution of such
+  Contributor, if any, and such Derivative Works.
+
+  b) Subject to the terms of this Agreement, each Contributor hereby
+  grants Recipient a non-exclusive, worldwide, royalty-free patent
+  license under Licensed Patents to make, use, sell, offer to sell,
+  import and otherwise transfer the Contribution of such Contributor,
+  if any, in Source Code or other form. This patent license shall
+  apply to the combination of the Contribution and the Program if, at
+  the time the Contribution is added by the Contributor, such addition
+  of the Contribution causes such combination to be covered by the
+  Licensed Patents. The patent license shall not apply to any other
+  combinations which include the Contribution. No hardware per se is
+  licensed hereunder.
+
+  c) Recipient understands that although each Contributor grants the
+  licenses to its Contributions set forth herein, no assurances are
+  provided by any Contributor that the Program does not infringe the
+  patent or other intellectual property rights of any other entity.
+  Each Contributor disclaims any liability to Recipient for claims
+  brought by any other entity based on infringement of intellectual
+  property rights or otherwise. As a condition to exercising the
+  rights and licenses granted hereunder, each Recipient hereby
+  assumes sole responsibility to secure any other intellectual
+  property rights needed, if any. For example, if a third party
+  patent license is required to allow Recipient to Distribute the
+  Program, it is Recipient's responsibility to acquire that license
+  before distributing the Program.
+
+  d) Each Contributor represents that to its knowledge it has
+  sufficient copyright rights in its Contribution, if any, to grant
+  the copyright license set forth in this Agreement.
+
+  e) Notwithstanding the terms of any Secondary License, no
+  Contributor makes additional grants to any Recipient (other than
+  those set forth in this Agreement) as a result of such Recipient's
+  receipt of the Program under the terms of a Secondary License
+  (if permitted under the terms of Section 3).
+
+3. REQUIREMENTS
+
+3.1 If a Contributor Distributes the Program in any form, then:
+
+  a) the Program must also be made available as Source Code, in
+  accordance with section 3.2, and the Contributor must accompany
+  the Program with a statement that the Source Code for the Program
+  is available under this Agreement, and informs Recipients how to
+  obtain it in a reasonable manner on or through a medium customarily
+  used for software exchange; and
+
+  b) the Contributor may Distribute the Program under a license
+  different than this Agreement, provided that such license:
+     i) effectively disclaims on behalf of all other Contributors all
+     warranties and conditions, express and implied, including
+     warranties or conditions of title and non-infringement, and
+     implied warranties or conditions of merchantability and fitness
+     for a particular purpose;
+
+     ii) effectively excludes on behalf of all other Contributors all
+     liability for damages, including direct, indirect, special,
+     incidental and consequential damages, such as lost profits;
+
+     iii) does not attempt to limit or alter the recipients' rights
+     in the Source Code under section 3.2; and
+
+     iv) requires any subsequent distribution of the Program by any
+     party to be under a license that satisfies the requirements
+     of this section 3.
+
+3.2 When the Program is Distributed as Source Code:
+
+  a) it must be made available under this Agreement, or if the
+  Program (i) is combined with other material in a separate file or
+  files made available under a Secondary License, and (ii) the initial
+  Contributor attached to the Source Code the notice described in
+  Exhibit A of this Agreement, then the Program may be made available
+  under the terms of such Secondary Licenses, and
+
+  b) a copy of this Agreement must be included with each copy of
+  the Program.
+
+3.3 Contributors may not remove or alter any copyright, patent,
+trademark, attribution notices, disclaimers of warranty, or limitations
+of liability ("notices") contained within the Program from any copy of
+the Program which they Distribute, provided that Contributors may add
+their own appropriate notices.
+
+4. COMMERCIAL DISTRIBUTION
+
+Commercial distributors of software may accept certain responsibilities
+with respect to end users, business partners and the like. While this
+license is intended to facilitate the commercial use of the Program,
+the Contributor who includes the Program in a commercial product
+offering should do so in a manner which does not create potential
+liability for other Contributors. Therefore, if a Contributor includes
+the Program in a commercial product offering, such Contributor
+("Commercial Contributor") hereby agrees to defend and indemnify every
+other Contributor ("Indemnified Contributor") against any losses,
+damages and costs (collectively "Losses") arising from claims, lawsuits
+and other legal actions brought by a third party against the Indemnified
+Contributor to the extent caused by the acts or omissions of such
+Commercial Contributor in connection with its distribution of the Program
+in a commercial product offering. The obligations in this section do not
+apply to any claims or Losses relating to any actual or alleged
+intellectual property infringement. In order to qualify, an Indemnified
+Contributor must: a) promptly notify the Commercial Contributor in
+writing of such claim, and b) allow the Commercial Contributor to control,
+and cooperate with the Commercial Contributor in, the defense and any
+related settlement negotiations. The Indemnified Contributor may
+participate in any such claim at its own expense.
+
+For example, a Contributor might include the Program in a commercial
+product offering, Product X. That Contributor is then a Commercial
+Contributor. If that Commercial Contributor then makes performance
+claims, or offers warranties related to Product X, those performance
+claims and warranties are such Commercial Contributor's responsibility
+alone. Under this section, the Commercial Contributor would have to
+defend claims against the other Contributors related to those performance
+claims and warranties, and if a court requires any other Contributor to
+pay any damages as a result, the Commercial Contributor must pay
+those damages.
+
+5. NO WARRANTY
+
+EXCEPT AS EXPRESSLY SET FORTH IN THIS AGREEMENT, AND TO THE EXTENT
+PERMITTED BY APPLICABLE LAW, THE PROGRAM IS PROVIDED ON AN "AS IS"
+BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, EITHER EXPRESS OR
+IMPLIED INCLUDING, WITHOUT LIMITATION, ANY WARRANTIES OR CONDITIONS OF
+TITLE, NON-INFRINGEMENT, MERCHANTABILITY OR FITNESS FOR A PARTICULAR
+PURPOSE. Each Recipient is solely responsible for determining the
+appropriateness of using and distributing the Program and assumes all
+risks associated with its exercise of rights under this Agreement,
+including but not limited to the risks and costs of program errors,
+compliance with applicable laws, damage to or loss of data, programs
+or equipment, and unavailability or interruption of operations.
+
+6. DISCLAIMER OF LIABILITY
+
+EXCEPT AS EXPRESSLY SET FORTH IN THIS AGREEMENT, AND TO THE EXTENT
+PERMITTED BY APPLICABLE LAW, NEITHER RECIPIENT NOR ANY CONTRIBUTORS
+SHALL HAVE ANY LIABILITY FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING WITHOUT LIMITATION LOST
+PROFITS), HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ARISING IN ANY WAY OUT OF THE USE OR DISTRIBUTION OF THE PROGRAM OR THE
+EXERCISE OF ANY RIGHTS GRANTED HEREUNDER, EVEN IF ADVISED OF THE
+POSSIBILITY OF SUCH DAMAGES.
+
+7. GENERAL
+
+If any provision of this Agreement is invalid or unenforceable under
+applicable law, it shall not affect the validity or enforceability of
+the remainder of the terms of this Agreement, and without further
+action by the parties hereto, such provision shall be reformed to the
+minimum extent necessary to make such provision valid and enforceable.
+
+If Recipient institutes patent litigation against any entity
+(including a cross-claim or counterclaim in a lawsuit) alleging that the
+Program itself (excluding combinations of the Program with other software
+or hardware) infringes such Recipient's patent(s), then such Recipient's
+rights granted under Section 2(b) shall terminate as of the date such
+litigation is filed.
+
+All Recipient's rights under this Agreement shall terminate if it
+fails to comply with any of the material terms or conditions of this
+Agreement and does not cure such failure in a reasonable period of
+time after becoming aware of such noncompliance. If all Recipient's
+rights under this Agreement terminate, Recipient agrees to cease use
+and distribution of the Program as soon as reasonably practicable.
+However, Recipient's obligations under this Agreement and any licenses
+granted by Recipient relating to the Program shall continue and survive.
+
+Everyone is permitted to copy and distribute copies of this Agreement,
+but in order to avoid inconsistency the Agreement is copyrighted and
+may only be modified in the following manner. The Agreement Steward
+reserves the right to publish new versions (including revisions) of
+this Agreement from time to time. No one other than the Agreement
+Steward has the right to modify this Agreement. The Eclipse Foundation
+is the initial Agreement Steward. The Eclipse Foundation may assign the
+responsibility to serve as the Agreement Steward to a suitable separate
+entity. Each new version of the Agreement will be given a distinguishing
+version number. The Program (including Contributions) may always be
+Distributed subject to the version of the Agreement under which it was
+received. In addition, after a new version of the Agreement is published,
+Contributor may elect to Distribute the Program (including its
+Contributions) under the new version.
+
+Except as expressly stated in Sections 2(a) and 2(b) above, Recipient
+receives no rights or licenses to the intellectual property of any
+Contributor under this Agreement, whether expressly, by implication,
+estoppel or otherwise. All rights in the Program not expressly granted
+under this Agreement are reserved. Nothing in this Agreement is intended
+to be enforceable by any entity that is not a Contributor or Recipient.
+No third-party beneficiary rights are created under this Agreement.
+
+Exhibit A - Form of Secondary Licenses Notice
+
+"This Source Code may also be made available under the following 
+Secondary Licenses when the conditions for such availability set forth 
+in the Eclipse Public License, v. 2.0 are satisfied: {name license(s),
+version(s), and exceptions or additional permissions here}."
+
+  Simply including a copy of this Agreement, including this Exhibit A
+  is not sufficient to license the Source Code under Secondary Licenses.
+
+  If it is not possible or desirable to put the notice in a particular
+  file, then You may include the notice in a location (such as a LICENSE
+  file in a relevant directory) where a recipient would be likely to
+  look for such a notice.
+
+  You may add additional accurate notices of copyright ownership.
\ No newline at end of file
diff --git a/java/ql/test/stubs/aspectj/org/aspectj/lang/annotation/Pointcut.java b/java/ql/test/stubs/aspectj/org/aspectj/lang/annotation/Pointcut.java
new file mode 100644
index 000000000000..8afcd092ce5b
--- /dev/null
+++ b/java/ql/test/stubs/aspectj/org/aspectj/lang/annotation/Pointcut.java
@@ -0,0 +1,27 @@
+/*******************************************************************************
+ * Copyright (c) 2005 Contributors.
+ * All rights reserved.
+ * This program and the accompanying materials are made available
+ * under the terms of the Eclipse Public License v 2.0
+ * which accompanies this distribution and is available at
+ * https://www.eclipse.org/org/documents/epl-2.0/EPL-2.0.txt
+ *
+ * Contributors:
+ * initial implementation              Alexandre Vasseur
+ *******************************************************************************/
+package org.aspectj.lang.annotation;
+
+import java.lang.annotation.Target;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+
+/**
+ * Pointcut declaration
+ *
+ * @author Alexandre Vasseur
+ */
+@Retention(RetentionPolicy.RUNTIME)
+@Target(ElementType.METHOD)
+public @interface Pointcut {}
+

From a8063e1cd2814f8ed4915bb6a54280dfc54c60e1 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 10:29:24 +0100
Subject: [PATCH 02/23] Adjust query name

---
 java/ql/src/Language Abuse/EmptyMethod.ql | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index 4f77b8d4679b..ef8e342f4c37 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -1,6 +1,6 @@
 /**
  * @id java/empty-method
- * @name J-D-002: An empty method serves no purpose and may indicate programmer error
+ * @name Empty method
  * @description An empty method serves no purpose and makes code less readable. An empty method may
  *              indicate an error on the part of the developer.
  * @kind problem

From 614bee9e2005a0541cf0213a5186b316b283de6a Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 10:29:45 +0100
Subject: [PATCH 03/23] Use inline test expectations

---
 .../EmptyMethod/EmptyMethod.expected          |  2 +-
 .../query-tests/EmptyMethod/EmptyMethod.qlref |  3 +-
 .../ql/test/query-tests/EmptyMethod/Test.java | 31 ++++++++-----------
 3 files changed, 16 insertions(+), 20 deletions(-)

diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index 0bf9111a91e7..b817a89594f3 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1 +1 @@
-| Test.java:16:17:16:18 | f2 | Empty method found. |
+| Test.java:13:15:13:16 | f2 | Empty method found. |
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
index 130ebedd8d93..f99a8f2e7550 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.qlref
@@ -1 +1,2 @@
-Language Abuse/EmptyMethod.ql
+query: Language Abuse/EmptyMethod.ql
+postprocess: utils/test/InlineExpectationsTestQuery.ql
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index 35bdecd00a4d..537c857101f8 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -2,28 +2,23 @@
 
 public class Test {
 
-    // COMPLIANT
-    public void f() {
-        int i = 0;
-    }
+  public void f() {
+    int i = 0;
+  }
 
-    // COMPLIANT
-    public void f1() {
-        // intentionally empty
-    }
+  public void f1() {
+    // intentionally empty
+  }
 
-    // NON_COMPLIANT
-    public void f2() {}
+  public void f2() { } // $ Alert
 
-    // COMPLIANT - exception
-    @Pointcut()
-    public void f4() {}
+  @Pointcut()
+  public void f4() {
+  }
 
-    public abstract class TestInner {
+  public abstract class TestInner {
 
-        public abstract void f(); // COMPLIANT - intentionally empty
-    }
+    public abstract void f();
+  }
 
 }
-
-

From 4bf26afca06c8fed6b3f90a18c1fcc80a288d93d Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 10:33:20 +0100
Subject: [PATCH 04/23] Add more test cases

---
 .../test/query-tests/EmptyMethod/EmptyMethod.expected |  2 ++
 java/ql/test/query-tests/EmptyMethod/Test.java        | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index b817a89594f3..85cf3e91d257 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1 +1,3 @@
 | Test.java:13:15:13:16 | f2 | Empty method found. |
+| Test.java:27:17:27:17 | f | Empty method found. |
+| Test.java:32:18:32:23 | method | Empty method found. |
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index 537c857101f8..d7b88b5debab 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -21,4 +21,15 @@ public abstract class TestInner {
     public abstract void f();
   }
 
+  public class Derived extends TestInner {
+
+    @Override
+    public void f() { } // $ Alert
+  }
+
+  public interface TestInterface {
+
+    default void method() { } // $ Alert
+  }
+
 }

From 349f48982a2eb98e52838ee60c32de8cc9c15114 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 10:56:17 +0100
Subject: [PATCH 05/23] Make query more accepting

---
 java/ql/src/Language Abuse/EmptyMethod.ql      | 18 ++++++------------
 .../EmptyMethod/EmptyMethod.expected           |  2 --
 java/ql/test/query-tests/EmptyMethod/Test.java |  8 ++++++--
 3 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index ef8e342f4c37..5cc745c42e15 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -65,16 +65,6 @@ class SurefireTest extends Class {
   }
 }
 
-/**
- * Frameworks that provide `PointCuts`
- * which commonly intentionally annotate empty methods
- */
-class PointCutAnnotation extends Annotation {
-  PointCutAnnotation() {
-    this.getType().hasQualifiedName("org.aspectj.lang.annotation", "Pointcut")
-  }
-}
-
 /**
  * A `Method` from source that is not abstract
  */
@@ -94,6 +84,10 @@ where
   m.getNumberOfCommentLines() = 0 and
   //permit a javadoc above as well as sufficient reason to leave empty
   not exists(Javadoc jd | m.getDoc().getJavadoc() = jd) and
-  //methods annotated this way are usually intentionally empty
-  not exists(PointCutAnnotation a | a = m.getAnAnnotation())
+  //annotated methods are considered compliant
+  not exists(m.getAnAnnotation()) and
+  //default methods are not abstract, but are considered compliant
+  not m.isDefault() and
+  //native methods have no body
+  not m.isNative()
 select m, "Empty method found."
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index 85cf3e91d257..b817a89594f3 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1,3 +1 @@
 | Test.java:13:15:13:16 | f2 | Empty method found. |
-| Test.java:27:17:27:17 | f | Empty method found. |
-| Test.java:32:18:32:23 | method | Empty method found. |
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index d7b88b5debab..73cbe15fc6d9 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -24,12 +24,16 @@ public abstract class TestInner {
   public class Derived extends TestInner {
 
     @Override
-    public void f() { } // $ Alert
+    public void f() {
+    }
+
+    public native int nativeMethod();
   }
 
   public interface TestInterface {
 
-    default void method() { } // $ Alert
+    default void method() {
+    }
   }
 
 }

From 7476f19b096ca47e7a916d250a38030b6565fa83 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 11:00:27 +0100
Subject: [PATCH 06/23] Adjust query help

---
 java/ql/src/Language Abuse/EmptyMethod.md | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index 6692251fe2bd..e2a847153885 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -1,10 +1,6 @@
-# J-D-002: An empty method serves no purpose and may indicate programmer error
-
-An empty method serves no purpose and may indicate programmer error.
-
 ## Overview
 
-An empty method may indicate that an implmentation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implentation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
+An empty method may indicate that an implementation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implentation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
 
 ## Recommendation
 
@@ -32,8 +28,8 @@ public class Test {
 
 ## Implementation Notes
 
-The rule excludes reporting methods annotated with `org.springframework.aop.Pointcut` or `org.aspectj.lang.annotation.Pointcut`. Such annotations are commonly paired with empty methods and are considered an acceptable use case for an empty method.
+The rule excludes reporting methods that are annotated or marked as `default`.
 
 ## References
 - [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
-- [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)
\ No newline at end of file
+- [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)

From 6512ed9429da5490528799fcd5834edee79c0594 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 7 Mar 2025 11:03:53 +0100
Subject: [PATCH 07/23] Adjust alert message

---
 java/ql/src/Language Abuse/EmptyMethod.ql                 | 2 +-
 java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index 5cc745c42e15..18932660ef14 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -90,4 +90,4 @@ where
   not m.isDefault() and
   //native methods have no body
   not m.isNative()
-select m, "Empty method found."
+select m, "This empty method should be completed."
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index b817a89594f3..e3c3e65f7f0b 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1 +1 @@
-| Test.java:13:15:13:16 | f2 | Empty method found. |
+| Test.java:13:15:13:16 | f2 | This empty method should be completed. |

From 3d2a72341b0f942149f86f768b7f642389829bce Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Mon, 10 Mar 2025 11:06:35 +0100
Subject: [PATCH 08/23] Improve ql code quality

---
 java/ql/src/Language Abuse/EmptyMethod.ql | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index 18932660ef14..f8b38ca00dd5 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -47,7 +47,7 @@ class LikelyJunitTest extends Method {
     (
       this.getName().matches("JUnit%") or
       this.getName().matches("test%") or
-      this.getAnAnnotation().toString().matches("%Test")
+      this.getAnAnnotation().getType().getName().matches("%Test")
     )
   }
 }
@@ -83,7 +83,7 @@ where
   //permit comment lines explaining why this is empty
   m.getNumberOfCommentLines() = 0 and
   //permit a javadoc above as well as sufficient reason to leave empty
-  not exists(Javadoc jd | m.getDoc().getJavadoc() = jd) and
+  not exists(m.getDoc().getJavadoc()) and
   //annotated methods are considered compliant
   not exists(m.getAnAnnotation()) and
   //default methods are not abstract, but are considered compliant

From 77400778ea868ea224db3ffa5ef471c16cc2a980 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Mon, 10 Mar 2025 11:21:48 +0100
Subject: [PATCH 09/23] Add change note

---
 java/ql/src/change-notes/2025-03-10-empty-method.md | 4 ++++
 1 file changed, 4 insertions(+)
 create mode 100644 java/ql/src/change-notes/2025-03-10-empty-method.md

diff --git a/java/ql/src/change-notes/2025-03-10-empty-method.md b/java/ql/src/change-notes/2025-03-10-empty-method.md
new file mode 100644
index 000000000000..6b33deffd1a0
--- /dev/null
+++ b/java/ql/src/change-notes/2025-03-10-empty-method.md
@@ -0,0 +1,4 @@
+---
+category: newQuery
+---
+* Added a new quality query, `java/empty-method`, to detect empty methods.

From 3d4fcefe70696d019c379cadf9e6ae5b5596865e Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Wed, 12 Mar 2025 10:37:37 +0100
Subject: [PATCH 10/23] Do not accept empty `default` methods

---
 java/ql/src/Language Abuse/EmptyMethod.md                 | 2 +-
 java/ql/src/Language Abuse/EmptyMethod.ql                 | 2 --
 java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected | 1 +
 java/ql/test/query-tests/EmptyMethod/Test.java            | 3 +--
 4 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index e2a847153885..fd6ee28ca39b 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -28,7 +28,7 @@ public class Test {
 
 ## Implementation Notes
 
-The rule excludes reporting methods that are annotated or marked as `default`.
+The rule excludes reporting methods that are annotated.
 
 ## References
 - [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index f8b38ca00dd5..b35cf5e279d5 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -86,8 +86,6 @@ where
   not exists(m.getDoc().getJavadoc()) and
   //annotated methods are considered compliant
   not exists(m.getAnAnnotation()) and
-  //default methods are not abstract, but are considered compliant
-  not m.isDefault() and
   //native methods have no body
   not m.isNative()
 select m, "This empty method should be completed."
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index e3c3e65f7f0b..edc9df534f83 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1 +1,2 @@
 | Test.java:13:15:13:16 | f2 | This empty method should be completed. |
+| Test.java:35:18:35:23 | method | This empty method should be completed. |
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index 73cbe15fc6d9..b6812e0b1a00 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -32,8 +32,7 @@ public void f() {
 
   public interface TestInterface {
 
-    default void method() {
-    }
+    default void method() { } // $ Alert
   }
 
 }

From 3be7044c6ecdc35020d5680085f2a3257b348975 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Wed, 12 Mar 2025 10:39:46 +0100
Subject: [PATCH 11/23] Fix references in query help file

---
 java/ql/src/Language Abuse/EmptyMethod.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index fd6ee28ca39b..085c0d3cb0cb 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -31,5 +31,5 @@ public class Test {
 The rule excludes reporting methods that are annotated.
 
 ## References
-- [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
-- [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)
+- Java SE Documentation: [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
+- Wikipedia: [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)

From dea081b385987da714de54452971c095e9481abd Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Wed, 12 Mar 2025 10:42:34 +0100
Subject: [PATCH 12/23] Add quality and cwe tag

---
 java/ql/src/Language Abuse/EmptyMethod.ql | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index b35cf5e279d5..88dbad287ac2 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -9,6 +9,8 @@
  * @tags correctness
  *       maintainability
  *       readability
+ *       quality
+ *       external/cwe/cwe-1071
  */
 
 import java

From 17aa3fc4280eb760455a47c41e784c1a4cac3631 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Wed, 12 Mar 2025 10:50:03 +0100
Subject: [PATCH 13/23] Add compliant/non-compliant comments back to the test
 file

---
 .../test/query-tests/EmptyMethod/EmptyMethod.expected  |  4 ++--
 java/ql/test/query-tests/EmptyMethod/Test.java         | 10 +++++++++-
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index edc9df534f83..3b0bba7ea4ee 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1,2 +1,2 @@
-| Test.java:13:15:13:16 | f2 | This empty method should be completed. |
-| Test.java:35:18:35:23 | method | This empty method should be completed. |
+| Test.java:16:15:16:16 | f2 | This empty method should be completed. |
+| Test.java:43:18:43:23 | method | This empty method should be completed. |
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index b6812e0b1a00..0096cc26dd7e 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -2,36 +2,44 @@
 
 public class Test {
 
+  // COMPLIANT
   public void f() {
     int i = 0;
   }
 
+  // COMPLIANT
   public void f1() {
     // intentionally empty
   }
 
+  // NON_COMPLIANT
   public void f2() { } // $ Alert
 
+  // COMPLIANT - exception
   @Pointcut()
   public void f4() {
   }
 
   public abstract class TestInner {
 
-    public abstract void f();
+    public abstract void f(); // COMPLIANT - intentionally empty
+
   }
 
   public class Derived extends TestInner {
 
+    // COMPLIANT: with annotation
     @Override
     public void f() {
     }
 
+    // COMPLIANT: native
     public native int nativeMethod();
   }
 
   public interface TestInterface {
 
+    // NON_COMPLIANT
     default void method() { } // $ Alert
   }
 

From 24f129c12ce6254ff58648ee04ff8ce21ba23b34 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Wed, 12 Mar 2025 11:00:16 +0100
Subject: [PATCH 14/23] Fix typo in QL help

---
 java/ql/src/Language Abuse/EmptyMethod.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index 085c0d3cb0cb..4b351dd0fae0 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -1,6 +1,6 @@
 ## Overview
 
-An empty method may indicate that an implementation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implentation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
+An empty method may indicate that an implementation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implementation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
 
 ## Recommendation
 

From 050ef405c12e067dabd05579f86d02aecf2d6089 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tam=C3=A1s=20Vajk?= <tamasvajk@github.com>
Date: Thu, 13 Mar 2025 10:08:15 +0100
Subject: [PATCH 15/23] Improve query help

Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com>
---
 java/ql/src/Language Abuse/EmptyMethod.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index 4b351dd0fae0..700f9654998b 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -4,7 +4,7 @@ An empty method may indicate that an implementation was intended to be provided
 
 ## Recommendation
 
-If a method is intended to be left empty, do one of the following to indicate that it is intentionally empty: 1) mark it abstract in an abstract class, 2) place it in an interface (then it can be implicitly abstract), 3) place a comment in that method that lets others know that the implementation was intentionally omitted, or 4) throw an `UnsupportedOperationException` in the method (like is done in `java.util.Collections.add`).
+If a method is intended to be left empty, do one of the following to indicate that it is intentionally empty: 1) mark it abstract in an abstract class, 2) place it in an interface (then it can be implicitly abstract), 3) place a comment in that method that lets others know that the implementation was intentionally omitted, or 4) throw an `UnsupportedOperationException` in the method (like is done in `java.util.Collection.add`).
 
 ## Example
 
@@ -31,5 +31,5 @@ public class Test {
 The rule excludes reporting methods that are annotated.
 
 ## References
-- Java SE Documentation: [java.util.Collections.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
-- Wikipedia: [Template pattern is a valid empty method use](https://en.wikipedia.org/wiki/Template_method_pattern)
+- Java SE Documentation: [java.util.Collection.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
+- Wikipedia: [Template method pattern](https://en.wikipedia.org/wiki/Template_method_pattern)

From 2538ba82cc671085580741ef8b65379c7ee339a3 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Thu, 13 Mar 2025 10:10:41 +0100
Subject: [PATCH 16/23] Revert message

---
 java/ql/src/Language Abuse/EmptyMethod.ql                 | 2 +-
 java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index 88dbad287ac2..96fb9d222dfb 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -90,4 +90,4 @@ where
   not exists(m.getAnAnnotation()) and
   //native methods have no body
   not m.isNative()
-select m, "This empty method should be completed."
+select m, "Empty method found."
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index 3b0bba7ea4ee..ad677b548e85 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1,2 +1,2 @@
-| Test.java:16:15:16:16 | f2 | This empty method should be completed. |
-| Test.java:43:18:43:23 | method | This empty method should be completed. |
+| Test.java:16:15:16:16 | f2 | Empty method found. |
+| Test.java:43:18:43:23 | method | Empty method found. |

From 30ff68dc715f5b13b04f0561245ede080d2beb36 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tam=C3=A1s=20Vajk?= <tamasvajk@github.com>
Date: Fri, 14 Mar 2025 09:46:18 +0100
Subject: [PATCH 17/23] Update java/ql/src/Language Abuse/EmptyMethod.md

Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com>
---
 java/ql/src/Language Abuse/EmptyMethod.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index 700f9654998b..9e74252af587 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -33,3 +33,4 @@ The rule excludes reporting methods that are annotated.
 ## References
 - Java SE Documentation: [java.util.Collection.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
 - Wikipedia: [Template method pattern](https://en.wikipedia.org/wiki/Template_method_pattern)
+- Common Weakness Enumeration: [CWE-1071](https://cwe.mitre.org/data/definitions/1071.html)

From 05502bc74e99d340d8c489b4942d762b98e4fea7 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 14 Mar 2025 11:13:18 +0100
Subject: [PATCH 18/23] Change severity and precision

---
 java/ql/src/Language Abuse/EmptyMethod.ql | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index 96fb9d222dfb..a9987a8d499a 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -4,8 +4,8 @@
  * @description An empty method serves no purpose and makes code less readable. An empty method may
  *              indicate an error on the part of the developer.
  * @kind problem
- * @precision very-high
- * @problem.severity warning
+ * @precision medium
+ * @problem.severity recommendation
  * @tags correctness
  *       maintainability
  *       readability

From 9662b47464a3b181b6aec304ca2dce78cd5263b3 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 14 Mar 2025 11:31:53 +0100
Subject: [PATCH 19/23] Move likely test method logic to library

---
 java/ql/lib/semmle/code/java/UnitTests.qll    | 44 +++++++++++++++
 java/ql/src/Language Abuse/EmptyMethod.ql     | 54 +------------------
 .../Security/CWE/CWE-489/TestLib.qll          |  7 +--
 3 files changed, 46 insertions(+), 59 deletions(-)

diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll
index 38f37fa4ff01..807c6bfa0742 100644
--- a/java/ql/lib/semmle/code/java/UnitTests.qll
+++ b/java/ql/lib/semmle/code/java/UnitTests.qll
@@ -66,6 +66,19 @@ class TestClass extends Class {
   }
 }
 
+/**
+ * A class that is likely a test class. That is either a definite test class, or
+ * a class whose name, package, or location suggests that it might be a test class.
+ */
+class LikelyTestClass extends Class {
+  LikelyTestClass() {
+    this instanceof TestClass or
+    this.getName().toLowerCase().matches("%test%") or
+    this.getPackage().getName().toLowerCase().matches("%test%") or
+    this.getLocation().getFile().getAbsolutePath().matches("%/src/test/java%")
+  }
+}
+
 /**
  * A test method declared within a JUnit 3.8 test class.
  */
@@ -185,6 +198,37 @@ class TestMethod extends Method {
   }
 }
 
+/**
+ * Any method that is likely a test method.
+ */
+class LikelyTestMethod extends Method {
+  LikelyTestMethod() {
+    this.getDeclaringType() instanceof LikelyTestClass
+    or
+    this instanceof TestMethod
+    or
+    this instanceof LikelyJunitTest
+  }
+}
+
+/**
+ * A `Method` that is public, has no parameters,
+ * has a "void" return type, AND either has a name that starts with "test" OR
+ * has an annotation that ends with "Test"
+ */
+class LikelyJunitTest extends Method {
+  LikelyJunitTest() {
+    this.isPublic() and
+    this.getReturnType().hasName("void") and
+    this.hasNoParameters() and
+    (
+      this.getName().matches("JUnit%") or
+      this.getName().matches("test%") or
+      this.getAnAnnotation().getType().getName().matches("%Test")
+    )
+  }
+}
+
 /**
  * A TestNG annotation used to mark a method that runs "before".
  */
diff --git a/java/ql/src/Language Abuse/EmptyMethod.ql b/java/ql/src/Language Abuse/EmptyMethod.ql
index a9987a8d499a..f8e2d350eec4 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.ql	
+++ b/java/ql/src/Language Abuse/EmptyMethod.ql	
@@ -16,59 +16,7 @@
 import java
 
 /**
- * Represents a likely a test method, which is either a method that is already
- * recognized as a `TestMethod` or something that is likely a JUnit test or
- * something in the expected test path for Java tests.
- */
-class LikelyTestMethod extends Method {
-  LikelyTestMethod() {
-    this.getDeclaringType() instanceof TestClass
-    or
-    this instanceof TestMethod
-    or
-    this instanceof LikelyJunitTest
-    or
-    //standard Maven/Gradle test file discovery filepath
-    this.getFile().getAbsolutePath().matches("%src/test/java%")
-    or
-    this.getDeclaringType() instanceof SurefireTest
-  }
-}
-
-/**
- * Classes that are likely part of junit tests (more general than `TestMethod` from `UnitTest.qll`)
- * A `Method` that is public, has no parameters,
- * has a "void" return type, AND either has a name that starts with "test" OR
- * has an annotation that ends with "Test"
- */
-class LikelyJunitTest extends Method {
-  LikelyJunitTest() {
-    this.isPublic() and
-    this.getReturnType().hasName("void") and
-    this.hasNoParameters() and
-    (
-      this.getName().matches("JUnit%") or
-      this.getName().matches("test%") or
-      this.getAnAnnotation().getType().getName().matches("%Test")
-    )
-  }
-}
-
-/**
- * Maven surefire patterns to consider which files are testcases:
- * https://maven.apache.org/surefire/maven-surefire-plugin/examples/inclusion-exclusion.html
- */
-class SurefireTest extends Class {
-  SurefireTest() {
-    this.getFile().getAbsolutePath().matches("%src/test/java%") and
-    this.getFile()
-        .getBaseName()
-        .matches(["Test%.java", "%Test.java", "%Tests.java", "%TestCase.java"])
-  }
-}
-
-/**
- * A `Method` from source that is not abstract
+ * A `Method` from source that is not abstract, and likely not a test method
  */
 class NonAbstractSource extends Method {
   NonAbstractSource() {
diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/TestLib.qll b/java/ql/src/experimental/Security/CWE/CWE-489/TestLib.qll
index 1d20dc0db1e0..8279d4d823c5 100644
--- a/java/ql/src/experimental/Security/CWE/CWE-489/TestLib.qll
+++ b/java/ql/src/experimental/Security/CWE/CWE-489/TestLib.qll
@@ -10,9 +10,4 @@ import java
  *    c) in a test class whose name has the word `test`
  *    d) in a test class implementing a test framework such as JUnit or TestNG
  */
-predicate isTestMethod(Method m) {
-  m.getDeclaringType().getName().toLowerCase().matches("%test%") or // Simple check to exclude test classes to reduce FPs
-  m.getDeclaringType().getPackage().getName().toLowerCase().matches("%test%") or // Simple check to exclude classes in test packages to reduce FPs
-  exists(m.getLocation().getFile().getAbsolutePath().indexOf("/src/test/java")) or //  Match test directory structure of build tools like maven
-  m instanceof TestMethod // Test method of a test case implementing a test framework such as JUnit or TestNG
-}
+predicate isTestMethod(LikelyTestMethod m) { any() }

From d4955a07477e3e23f80297cd55644b1da1a802e5 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Fri, 14 Mar 2025 13:07:56 +0100
Subject: [PATCH 20/23] Fix failing test and add new test case

---
 .../test/query-tests/EmptyMethod/Class1.java  | 46 +++++++++++++++++++
 .../EmptyMethod/EmptyMethod.expected          |  6 ++-
 2 files changed, 51 insertions(+), 1 deletion(-)
 create mode 100644 java/ql/test/query-tests/EmptyMethod/Class1.java

diff --git a/java/ql/test/query-tests/EmptyMethod/Class1.java b/java/ql/test/query-tests/EmptyMethod/Class1.java
new file mode 100644
index 000000000000..f56c2144d61f
--- /dev/null
+++ b/java/ql/test/query-tests/EmptyMethod/Class1.java
@@ -0,0 +1,46 @@
+import org.aspectj.lang.annotation.Pointcut;
+
+public class Class1 {
+
+  // COMPLIANT
+  public void f() {
+    int i = 0;
+  }
+
+  // COMPLIANT
+  public void f1() {
+    // intentionally empty
+  }
+
+  // NON_COMPLIANT
+  public void f2() { } // $ Alert
+
+  // COMPLIANT - exception
+  @Pointcut()
+  public void f4() {
+  }
+
+  public abstract class TestInner {
+
+    public abstract void f(); // COMPLIANT - intentionally empty
+
+  }
+
+  public class Derived extends TestInner {
+
+    // COMPLIANT: with annotation
+    @Override
+    public void f() {
+    }
+
+    // COMPLIANT: native
+    public native int nativeMethod();
+  }
+
+  public interface TestInterface {
+
+    // NON_COMPLIANT
+    default void method() { } // $ Alert
+  }
+
+}
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index ad677b548e85..dbb8a322604c 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1,2 +1,6 @@
-| Test.java:16:15:16:16 | f2 | Empty method found. |
+#select
+| Class1.java:16:15:16:16 | f2 | Empty method found. |
+| Class1.java:43:18:43:23 | method | Empty method found. |
 | Test.java:43:18:43:23 | method | Empty method found. |
+testFailures
+| Test.java:16:24:16:33 | // $ Alert | Missing result: Alert |

From 246c8276e066951cca7d5f9dce29f4f783895796 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tam=C3=A1s=20Vajk?= <tamasvajk@github.com>
Date: Thu, 20 Mar 2025 09:22:13 +0100
Subject: [PATCH 21/23] Update java/ql/lib/semmle/code/java/UnitTests.qll

Co-authored-by: Jami <57204504+jcogs33@users.noreply.github.com>
---
 java/ql/lib/semmle/code/java/UnitTests.qll | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/java/ql/lib/semmle/code/java/UnitTests.qll b/java/ql/lib/semmle/code/java/UnitTests.qll
index 807c6bfa0742..d0fb6849f422 100644
--- a/java/ql/lib/semmle/code/java/UnitTests.qll
+++ b/java/ql/lib/semmle/code/java/UnitTests.qll
@@ -199,7 +199,7 @@ class TestMethod extends Method {
 }
 
 /**
- * Any method that is likely a test method.
+ * A method that is likely a test method.
  */
 class LikelyTestMethod extends Method {
   LikelyTestMethod() {

From 9bdec217e45d072c609caf71968e285c93ce3f69 Mon Sep 17 00:00:00 2001
From: Tamas Vajk <tamasvajk@github.com>
Date: Thu, 20 Mar 2025 09:32:27 +0100
Subject: [PATCH 22/23] Clean test files and add new test cases

---
 .../test/query-tests/EmptyMethod/Class1.java  |  8 ++++
 .../EmptyMethod/EmptyMethod.expected          |  6 +--
 .../ql/test/query-tests/EmptyMethod/Test.java | 43 +------------------
 3 files changed, 10 insertions(+), 47 deletions(-)

diff --git a/java/ql/test/query-tests/EmptyMethod/Class1.java b/java/ql/test/query-tests/EmptyMethod/Class1.java
index f56c2144d61f..b1d5ac1fbf8b 100644
--- a/java/ql/test/query-tests/EmptyMethod/Class1.java
+++ b/java/ql/test/query-tests/EmptyMethod/Class1.java
@@ -20,6 +20,12 @@ public void f2() { } // $ Alert
   public void f4() {
   }
 
+  /**
+   * COMPLIANT - empty method with javadoc
+   */
+  public void f5() {
+  }
+
   public abstract class TestInner {
 
     public abstract void f(); // COMPLIANT - intentionally empty
@@ -41,6 +47,8 @@ public interface TestInterface {
 
     // NON_COMPLIANT
     default void method() { } // $ Alert
+
+    void method2(); // COMPLIANT
   }
 
 }
diff --git a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
index dbb8a322604c..f8d854c84759 100644
--- a/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
+++ b/java/ql/test/query-tests/EmptyMethod/EmptyMethod.expected
@@ -1,6 +1,2 @@
-#select
 | Class1.java:16:15:16:16 | f2 | Empty method found. |
-| Class1.java:43:18:43:23 | method | Empty method found. |
-| Test.java:43:18:43:23 | method | Empty method found. |
-testFailures
-| Test.java:16:24:16:33 | // $ Alert | Missing result: Alert |
+| Class1.java:49:18:49:23 | method | Empty method found. |
diff --git a/java/ql/test/query-tests/EmptyMethod/Test.java b/java/ql/test/query-tests/EmptyMethod/Test.java
index 0096cc26dd7e..e3cb48f5abcc 100644
--- a/java/ql/test/query-tests/EmptyMethod/Test.java
+++ b/java/ql/test/query-tests/EmptyMethod/Test.java
@@ -1,46 +1,5 @@
-import org.aspectj.lang.annotation.Pointcut;
-
 public class Test {
-
-  // COMPLIANT
+  // COMPLIANT: allow empty method in test class
   public void f() {
-    int i = 0;
   }
-
-  // COMPLIANT
-  public void f1() {
-    // intentionally empty
-  }
-
-  // NON_COMPLIANT
-  public void f2() { } // $ Alert
-
-  // COMPLIANT - exception
-  @Pointcut()
-  public void f4() {
-  }
-
-  public abstract class TestInner {
-
-    public abstract void f(); // COMPLIANT - intentionally empty
-
-  }
-
-  public class Derived extends TestInner {
-
-    // COMPLIANT: with annotation
-    @Override
-    public void f() {
-    }
-
-    // COMPLIANT: native
-    public native int nativeMethod();
-  }
-
-  public interface TestInterface {
-
-    // NON_COMPLIANT
-    default void method() { } // $ Alert
-  }
-
 }

From a5fd2e923a33416a62c336b37252e007a7537f10 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tam=C3=A1s=20Vajk?= <tamasvajk@github.com>
Date: Mon, 24 Mar 2025 11:03:43 +0100
Subject: [PATCH 23/23] Improve query documentation

Co-authored-by: mc <42146119+mchammer01@users.noreply.github.com>
---
 java/ql/src/Language Abuse/EmptyMethod.md | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/java/ql/src/Language Abuse/EmptyMethod.md b/java/ql/src/Language Abuse/EmptyMethod.md
index 9e74252af587..e4042973681c 100644
--- a/java/ql/src/Language Abuse/EmptyMethod.md	
+++ b/java/ql/src/Language Abuse/EmptyMethod.md	
@@ -1,10 +1,14 @@
 ## Overview
 
-An empty method may indicate that an implementation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implementation and with dynamic dispatch, resolving to a blank method may result in unexpected program behaviour.
+An empty method may indicate that an implementation was intended to be provided but was accidentally omitted. When using the method, it will not be clear that it does not provide an implementation and with dynamic dispatch, resolving to a blank method may result in unexpected program behavior.
 
 ## Recommendation
 
-If a method is intended to be left empty, do one of the following to indicate that it is intentionally empty: 1) mark it abstract in an abstract class, 2) place it in an interface (then it can be implicitly abstract), 3) place a comment in that method that lets others know that the implementation was intentionally omitted, or 4) throw an `UnsupportedOperationException` in the method (like is done in `java.util.Collection.add`).
+If a method is intended to be left empty, do one of the following to indicate that it is intentionally empty: 
+1. Mark it abstract in an abstract class
+2. Place it in an interface (then it can be implicitly abstract)
+3. Place a comment in that method that lets others know that the implementation was intentionally omitted
+4. Add `UnsupportedOperationException` to the method (as in `java.util.Collection.add`).
 
 ## Example
 
@@ -31,6 +35,6 @@ public class Test {
 The rule excludes reporting methods that are annotated.
 
 ## References
-- Java SE Documentation: [java.util.Collection.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E))
-- Wikipedia: [Template method pattern](https://en.wikipedia.org/wiki/Template_method_pattern)
-- Common Weakness Enumeration: [CWE-1071](https://cwe.mitre.org/data/definitions/1071.html)
+- Java SE Documentation: [java.util.Collection.add](https://docs.oracle.com/en/java/javase/20/docs/api/java.base/java/util/Collection.html#add(E)).
+- Wikipedia: [Template method pattern](https://en.wikipedia.org/wiki/Template_method_pattern).
+- Common Weakness Enumeration: [CWE-1071](https://cwe.mitre.org/data/definitions/1071.html).