Skip to content

Commit 02b17ec

Browse files
committed
test: fail the build when TablesNamesFinder misses a visitor dispatch method
TablesNamesFinder dispatches through 8 visitor interfaces. Abstract dispatch methods are compiler-enforced, but the interfaces progressively gain default ones (e.g. SelectVisitor#visit(PivotQuery, context)), and for a default declared by a single interface a missing implementation compiles silently and loses the tables of that node type without any test failing. Pin the complete post-#2479/#2517 state (214 methods). The walk also guards itself against becoming vacuous: it fails when a declared interface exposes no dispatch method at all (filters too narrow) or when the finder stops declaring one of the 8 families. Signed-off-by: 付典 <fudianchn@gmail.com>
1 parent 8397d3a commit 02b17ec

1 file changed

Lines changed: 82 additions & 0 deletions

File tree

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package net.sf.jsqlparser.util;
2+
3+
import static org.junit.jupiter.api.Assertions.assertTrue;
4+
5+
import java.lang.reflect.Method;
6+
import java.util.ArrayList;
7+
import java.util.List;
8+
import java.util.Set;
9+
10+
import org.junit.jupiter.api.Test;
11+
12+
/**
13+
* TablesNamesFinder collects tables by dispatching through the visitor interfaces it declares. Most
14+
* dispatch methods are abstract, so a missing implementation fails the build already. But the
15+
* interfaces progressively gain default methods (e.g. SelectVisitor#visit(PivotQuery, context)
16+
* returns null), and for those a missing TablesNamesFinder implementation compiles silently and
17+
* loses the tables of that node type without any test failing.
18+
*
19+
* This test fails the build when a declared visitor interface has a dispatch method visit(Node,
20+
* context) for an AST node that is not implemented in the TablesNamesFinder class hierarchy (it
21+
* resolves to an interface default instead). It does not catch empty-shell implementations (an
22+
* override that visits nothing) nor visitor families the finder does not declare at all: both
23+
* remain manual review duties.
24+
*/
25+
public class TablesNamesFinderCompletenessTest {
26+
27+
// GroupByElement: ExpressionVisitor's default forwards the group-by expressions and the
28+
// grouping sets to the visitor, TablesNamesFinder relies on it from visit(PlainSelect).
29+
private static final Set<String> INHERITED_DISPATCH_NODES =
30+
Set.of("net.sf.jsqlparser.statement.select.GroupByElement");
31+
32+
@Test
33+
void testImplementsEveryDispatchMethodOfEveryDeclaredVisitorInterface()
34+
throws NoSuchMethodException {
35+
Class<?>[] visitorInterfaces = TablesNamesFinder.class.getInterfaces();
36+
assertTrue(visitorInterfaces.length >= 8, "TablesNamesFinder must keep declaring the"
37+
+ " visitor families it dispatches through (currently 8), dropping one shrinks"
38+
+ " the completeness walk silently.");
39+
40+
List<String> gaps = new ArrayList<>();
41+
List<String> interfacesWithoutDispatch = new ArrayList<>();
42+
for (Class<?> visitorInterface : visitorInterfaces) {
43+
int dispatchMethods = 0;
44+
for (Method dispatch : visitorInterface.getMethods()) {
45+
if (!dispatch.getName().equals("visit") || dispatch.getParameterCount() != 2) {
46+
continue;
47+
}
48+
Class<?> nodeType = dispatch.getParameterTypes()[0];
49+
// skips bulk dispatch helpers carrying a java.util collection
50+
if (!nodeType.getName().startsWith("net.sf.jsqlparser.")) {
51+
continue;
52+
}
53+
if (INHERITED_DISPATCH_NODES.contains(nodeType.getName())) {
54+
continue;
55+
}
56+
dispatchMethods++;
57+
Method implementation = TablesNamesFinder.class.getMethod(dispatch.getName(),
58+
dispatch.getParameterTypes());
59+
if (implementation.getDeclaringClass().isInterface()) {
60+
gaps.add(visitorInterface.getSimpleName() + "#" + dispatch.getName() + "("
61+
+ nodeType.getSimpleName() + ", context)");
62+
}
63+
}
64+
if (dispatchMethods == 0) {
65+
interfacesWithoutDispatch.add(visitorInterface.getSimpleName());
66+
}
67+
}
68+
69+
assertTrue(interfacesWithoutDispatch.isEmpty(),
70+
() -> "No guarded dispatch method found for: "
71+
+ String.join(", ", interfacesWithoutDispatch) + ". Either the reflection"
72+
+ " walk lost the dispatch methods (filters too narrow, the completeness"
73+
+ " check would pass vacuously) or the interface is not a dispatch"
74+
+ " interface and must not be declared by TablesNamesFinder.");
75+
assertTrue(gaps.isEmpty(), () -> "TablesNamesFinder must implement every dispatch"
76+
+ " method of the visitor interfaces it declares, otherwise the tables of that"
77+
+ " node type are silently not collected. Missing:\n" + String.join("\n", gaps)
78+
+ "\nImplement visit(Node, context) in TablesNamesFinder, or - when the"
79+
+ " inherited default already forwards every child - document the node in"
80+
+ " INHERITED_DISPATCH_NODES.");
81+
}
82+
}

0 commit comments

Comments
 (0)