fix(entrypoints): the Jakarta finder marks lifecycle methods, not any method with a servlet parameter (#263) (#264)

isEntrypointMethod now requires a servlet/filter/listener/WebSocket
lifecycle name (or an @OnMessage-family annotation) on a type the
class rule already recognises; Filter implementors count as such a
type. A helper such as DayTrader's requestDispatch(ctx, req, resp,
page) is ordinary code again, so the interprocedural tiers bind its
parameters from its callers: daytrader8 view_dispatches 37 -> 54,
entrypoint marks 135 -> 196 (21 helpers dropped, 82 init/destroy/
doFilter/onMessage lifecycle methods gained).

Author: rahlk

src/main/java/com/ibm/cldk/javaee/jakarta/JakartaEntrypointFinder.java

@@ -5,8 +5,10 @@
 import com.github.javaparser.ast.body.ClassOrInterfaceDeclaration;
 import com.github.javaparser.ast.body.Parameter;
 import com.github.javaparser.ast.body.TypeDeclaration;
+import com.github.javaparser.ast.expr.AnnotationExpr;
 import com.github.javaparser.ast.type.ClassOrInterfaceType;
 import com.ibm.cldk.javaee.utils.interfaces.AbstractEntrypointFinder;
+import java.util.Set;
 
 @SuppressWarnings({"unchecked", "rawtypes"})
 public class JakartaEntrypointFinder extends AbstractEntrypointFinder {
@@ -38,16 +40,42 @@ public boolean isEntrypointClass(TypeDeclaration typeDecl) {
                 .anyMatch(n -> n.contains("HttpServlet") || n.contains("GenericServlet"))
                 || classDecl.getImplementedTypes().stream().map(
                 ClassOrInterfaceType::asString).anyMatch(
-                n -> n.contains("ServletContextListener")
+                n -> n.equals("Filter") || n.endsWith(".Filter")
+                        || n.contains("ServletContextListener")
                         || n.contains("HttpSessionListener")
                         || n.contains("ServletRequestListener")
                         || n.contains("MessageListener"));
     }
 
+    /**
+     * The servlet, filter, listener and WebSocket lifecycle methods the container itself invokes.
+     * Anything else on an entrypoint type is ordinary code, however servlet-typed its parameters.
+     */
+    private static final Set<String> LIFECYCLE = Set.of(
+            "service", "doGet", "doPost", "doPut", "doDelete", "doHead", "doOptions", "doTrace",
+            "doFilter", "init", "destroy",
+            "contextInitialized", "contextDestroyed", "sessionCreated", "sessionDestroyed",
+            "requestInitialized", "requestDestroyed", "onMessage");
+
+    private static final Set<String> CALLBACK_ANNOTATIONS =
+            Set.of("OnMessage", "OnOpen", "OnClose", "OnError");
+
+    /**
+     * A container-invoked method of an entrypoint type: a lifecycle method by name, or a WebSocket
+     * callback by annotation (#263). Before this, any method with an {@code HttpServletRequest} /
+     * {@code HttpServletResponse} parameter was marked — which made a plain helper such as
+     * DayTrader's {@code requestDispatch(ctx, req, resp, page)} an entrypoint, and every
+     * interprocedural tier then refused to bind its parameters from the call sites that do bind
+     * them. The container binds a lifecycle method's parameters; it never calls a helper.
+     */
     @Override
     public boolean isEntrypointMethod(CallableDeclaration callableDecl) {
-        return ((NodeList<Parameter>) callableDecl.getParameters()).stream()
-                .anyMatch(parameter -> parameter.getType().asString().contains("HttpServletRequest") ||
-                        parameter.getType().asString().contains("HttpServletResponse"));
+        boolean callback = ((NodeList<AnnotationExpr>) callableDecl.getAnnotations()).stream()
+                .anyMatch(a -> CALLBACK_ANNOTATIONS.contains(a.getNameAsString()));
+        if (!callback && !LIFECYCLE.contains(callableDecl.getNameAsString())) {
+            return false;
+        }
+        java.util.Optional<TypeDeclaration> owner = callableDecl.findAncestor(TypeDeclaration.class);
+        return owner.isPresent() && isEntrypointClass(owner.get());
     }
 }

src/test/java/com/ibm/cldk/artifacts/ViewDispatchDataflowTierTest.java

@@ -110,13 +110,13 @@ void aTracedLiteralThatNamesNoFileIsNoSuchArtifact() throws Exception {
         assertEquals(List.of("literal", "dataflow"), u.getProv());
     }
 
-    // The helper takes no servlet-typed parameter on purpose: the Jakarta finder marks any method
-    // with an HttpServletRequest parameter as an entrypoint, and the interprocedural tier rightly
-    // refuses to bind an entrypoint's parameter from a call site (the container binds it).
+    // The helper takes servlet-typed parameters like DayTrader's requestDispatch does: since #263
+    // the Jakarta finder marks only lifecycle methods, so the interprocedural tier binds `page`
+    // from the call site instead of refusing it as container-bound.
     private static final String PARAMETER = HEAD
-            + "  void doGet(HttpServletRequest req, HttpServletResponse res) { show(\"/y.jsp\"); }\n"
-            + "  void show(String page) {\n"
-            + "    getServletContext().getRequestDispatcher(page).forward(null, null);\n"
+            + "  void doGet(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/y.jsp\"); }\n"
+            + "  void show(HttpServletRequest req, HttpServletResponse res, String page) {\n"
+            + "    req.getRequestDispatcher(page).forward(req, res);\n"
             + "  }\n}\n";
 
     @Test
@@ -136,10 +136,10 @@ void aParameterClosesOverTheCallGraphAtLevelFour() throws Exception {
     @Test
     void twoCallersWithDifferentPagesStayNonLiteral() throws Exception {
         ViewDispatches.Result r = run(HEAD
-                + "  void a() { show(\"/y.jsp\"); }\n"
-                + "  void b() { show(\"/pages/x.jsp\"); }\n"
-                + "  void show(String page) {\n"
-                + "    getServletContext().getRequestDispatcher(page).forward(null, null);\n"
+                + "  void a(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/y.jsp\"); }\n"
+                + "  void b(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/pages/x.jsp\"); }\n"
+                + "  void show(HttpServletRequest req, HttpServletResponse res, String page) {\n"
+                + "    req.getRequestDispatcher(page).forward(req, res);\n"
                 + "  }\n}\n", 4);
         assertTrue(r.dispatches.isEmpty());
         assertEquals("non-literal", only(r).getReason());

src/test/java/com/ibm/cldk/javaee/JakartaEntrypointMethodsTest.java

@@ -0,0 +1,104 @@
+package com.ibm.cldk.javaee;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import com.ibm.cldk.schema.JCallable;
+import com.ibm.cldk.schema.JEntrypointReport;
+import com.ibm.cldk.schema.JModule;
+import com.ibm.cldk.schema.JType;
+import com.ibm.cldk.syntactic_analysis.L1Extractor;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.TreeMap;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+/**
+ * Which methods the Jakarta finder marks (#263): the servlet / filter / listener lifecycle methods
+ * of an entrypoint type, not any method that happens to take a servlet-typed parameter. A helper
+ * such as DayTrader's {@code requestDispatch(ctx, req, resp, page)} is ordinary code whose
+ * parameters are bound by its callers — marking it an entrypoint made every interprocedural tier
+ * refuse to bind them.
+ */
+class JakartaEntrypointMethodsTest {
+
+    @TempDir
+    Path root;
+
+    private Map<String, List<String>> marks(String source) throws Exception {
+        Path src = root.resolve("src/main/java/demo");
+        Files.createDirectories(src);
+        Files.writeString(src.resolve("Web.java"), source, StandardCharsets.UTF_8);
+        Map<String, JModule> modules = L1Extractor.extractAll(
+                root, "app", null, new LinkedHashMap<>(), 1, 3, "ast", new JEntrypointReport());
+        Map<String, List<String>> out = new TreeMap<>();
+        for (JModule m : modules.values()) {
+            for (Map.Entry<String, JType> e : m.getTypes().entrySet()) {
+                for (JCallable c : e.getValue().getCallables().values()) {
+                    out.put(e.getKey() + "." + c.getSignature().substring(0, c.getSignature().indexOf('(')),
+                            c.getEntrypointFrameworks());
+                }
+            }
+        }
+        return out;
+    }
+
+    @Test
+    void lifecycleMethodsOfAServletAreMarkedAndItsHelpersAreNot() throws Exception {
+        Map<String, List<String>> m = marks(
+                "package demo;\n"
+                        + "import javax.servlet.http.*;\n"
+                        + "public class Front extends HttpServlet {\n"
+                        + "  protected void doGet(HttpServletRequest req, HttpServletResponse res) { show(req, res, \"/x.jsp\"); }\n"
+                        + "  public void init() {}\n"
+                        + "  void show(HttpServletRequest req, HttpServletResponse res, String page) {}\n"
+                        + "  static String render(HttpServletRequest req) { return \"\"; }\n"
+                        + "}\n");
+        assertEquals(List.of("jakarta"), m.get("Front.doGet"));
+        assertEquals(List.of("jakarta"), m.get("Front.init"));
+        assertEquals(List.of(), m.get("Front.show"), "a helper taking req/res is not an entrypoint");
+        assertEquals(List.of(), m.get("Front.render"));
+    }
+
+    @Test
+    void aLifecycleNameOutsideAnEntrypointTypeIsNotMarked() throws Exception {
+        Map<String, List<String>> m = marks(
+                "package demo;\n"
+                        + "import javax.servlet.http.*;\n"
+                        + "public class Plain {\n"
+                        + "  void doGet(HttpServletRequest req, HttpServletResponse res) {}\n"
+                        + "  public void init() {}\n"
+                        + "}\n");
+        assertEquals(List.of(), m.get("Plain.doGet"));
+        assertEquals(List.of(), m.get("Plain.init"));
+    }
+
+    @Test
+    void filterAndListenerAndEndpointCallbacksAreMarked() throws Exception {
+        Map<String, List<String>> m = marks(
+                "package demo;\n"
+                        + "import javax.servlet.*;\n"
+                        + "@WebFilter(\"/*\")\n"
+                        + "class Gate implements Filter {\n"
+                        + "  public void doFilter(ServletRequest q, ServletResponse s, FilterChain c) {}\n"
+                        + "  void log(ServletRequest q) {}\n"
+                        + "}\n"
+                        + "class Boot implements ServletContextListener {\n"
+                        + "  public void contextInitialized(ServletContextEvent e) {}\n"
+                        + "}\n"
+                        + "@ServerEndpoint(\"/ws\")\n"
+                        + "class Sock {\n"
+                        + "  @OnMessage public void onText(String msg) {}\n"
+                        + "  void send(String msg) {}\n"
+                        + "}\n");
+        assertEquals(List.of("jakarta"), m.get("Gate.doFilter"));
+        assertEquals(List.of(), m.get("Gate.log"));
+        assertEquals(List.of("jakarta"), m.get("Boot.contextInitialized"));
+        assertEquals(List.of("jakarta"), m.get("Sock.onText"));
+        assertEquals(List.of(), m.get("Sock.send"));
+    }
+}