Skip to content

Commit e8de6e9

Browse files
committed
test: split the coverage walk to settle Codacy's PMD findings
`visit` and `checkRange` exceeded PMD's NPath threshold of 200, so the container walk and the parent check move into methods of their own. `isHeld` compares with `==` on purpose: a getter that answers a new object on every call computes its child rather than holding it.
1 parent 927c724 commit e8de6e9

1 file changed

Lines changed: 30 additions & 21 deletions

File tree

‎src/test/java/net/sf/jsqlparser/parser/ASTNodeCoverageTest.java‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -207,27 +207,31 @@ private void visit(Object value, int[] enclosing) {
207207
checkRange(value, range, enclosing);
208208
}
209209
int[] inner = range != null ? range : enclosing;
210+
visitElements(value, inner);
211+
if (isModel(value)) {
212+
for (Method getter : value.getClass().getMethods()) {
213+
if (isChildGetter(getter)) {
214+
visitChild(value, getter, inner);
215+
}
216+
}
217+
}
218+
}
219+
220+
private void visitElements(Object value, int[] enclosing) {
210221
if (value instanceof Collection) {
211222
for (Object element : (Collection<?>) value) {
212-
visit(element, inner);
223+
visit(element, enclosing);
213224
}
214225
} else if (value instanceof Map) {
215226
for (Map.Entry<?, ?> entry : ((Map<?, ?>) value).entrySet()) {
216-
visit(entry.getKey(), inner);
217-
visit(entry.getValue(), inner);
227+
visit(entry.getKey(), enclosing);
228+
visit(entry.getValue(), enclosing);
218229
}
219230
} else if (value instanceof Map.Entry) {
220-
visit(((Map.Entry<?, ?>) value).getKey(), inner);
221-
visit(((Map.Entry<?, ?>) value).getValue(), inner);
231+
visit(((Map.Entry<?, ?>) value).getKey(), enclosing);
232+
visit(((Map.Entry<?, ?>) value).getValue(), enclosing);
222233
} else if (value instanceof Optional) {
223-
((Optional<?>) value).ifPresent(present -> visit(present, inner));
224-
}
225-
if (isModel(value)) {
226-
for (Method getter : value.getClass().getMethods()) {
227-
if (isChildGetter(getter)) {
228-
visitChild(value, getter, inner);
229-
}
230-
}
234+
((Optional<?>) value).ifPresent(present -> visit(present, enclosing));
231235
}
232236
}
233237

@@ -271,14 +275,7 @@ private void checkRange(Object value, int[] range, int[] enclosing) {
271275
if (enclosing != null && (range[0] < enclosing[0] || range[1] > enclosing[1])) {
272276
report("lies outside the object that holds it", value);
273277
}
274-
ASTNodeAccess parent = value instanceof ASTNodeAccessImpl
275-
? ((ASTNodeAccessImpl) value).getParent()
276-
: null;
277-
Node parentNode = parent == null ? null : parent.getASTNode();
278-
if (parentNode != null && (parentNode.jjtGetFirstToken().absoluteBegin - 1 > range[0]
279-
|| parentNode.jjtGetLastToken().absoluteEnd - 1 < range[1])) {
280-
report("lies outside its parent " + parent.getClass().getSimpleName(), value);
281-
}
278+
checkInsideParent(value, range);
282279
ranges.add(new Object[] {range, value});
283280
String text = sql.substring(range[0], range[1]);
284281
// PRIOR is an operator only inside CONNECT BY
@@ -290,6 +287,17 @@ private void checkRange(Object value, int[] range, int[] enclosing) {
290287
}
291288
}
292289

290+
private void checkInsideParent(Object value, int[] range) {
291+
ASTNodeAccess parent = value instanceof ASTNodeAccessImpl
292+
? ((ASTNodeAccessImpl) value).getParent()
293+
: null;
294+
Node parentNode = parent == null ? null : parent.getASTNode();
295+
if (parentNode != null && (parentNode.jjtGetFirstToken().absoluteBegin - 1 > range[0]
296+
|| parentNode.jjtGetLastToken().absoluteEnd - 1 < range[1])) {
297+
report("lies outside its parent " + parent.getClass().getSimpleName(), value);
298+
}
299+
}
300+
293301
private void checkReparses(Object value, String text, Parse parse) {
294302
Object again;
295303
try {
@@ -341,6 +349,7 @@ private static String normalized(String text) {
341349
}
342350

343351
// a getter that builds new objects on each call computes them rather than holding them
352+
@SuppressWarnings("PMD.CompareObjectsWithEquals")
344353
private static boolean isHeld(Object child, Object again) {
345354
if (child instanceof ASTNodeAccess) {
346355
return child == again;

0 commit comments

Comments
 (0)