Kotlin rules in java avoid multiple concat statements#694
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Bump pmd.version from 7.16.0 to 7.21.0 - Add KotlinAstUtil with 6 static helpers for Java-based Kotlin rules: getIdentifierText, typeContainsName, isDirectChildOfFunctionBody, getLhsVarName, findParamNamesOfType, findFunctionNamesReturningType - Refactor AvoidMultipleConcatStatementsRule to use KotlinAstUtil, removing the two private findString* methods and all inline repetition Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…leConcatStatements' into kotlin-rules-in-java-AvoidMultipleConcatStatements # Conflicts: # src/main/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtil.java
…leConcatStatements for mutableMap
|
jborgers
left a comment
jborgers
left a comment
There was a problem hiding this comment.
Added some review comments. Fix Sonar warnings, and can do a code review with coPilot, including best practices and performance first?
| // --- .toRegex() calls --- | ||
| for (KotlinParser.KtNavigationSuffix navSuffix : | ||
| node.descendants(KotlinParser.KtNavigationSuffix.class) | ||
| .filter(ns -> KotlinAstUtil.isDirectDescendantOfFunctionDeclaration(ns, node)) |
There was a problem hiding this comment.
This performs multiple full AST scans per function.
PMD visitors run on every function in every file on the entire project, so this compounds significantly.
Solution: Collect each AST node type once:
List<KtNavigationSuffix> navSuffixes = collectNavSuffixes(node);
List<KtPostfixUnaryExpression> postfixExprs = collectPostfixUnaryExpressions(node);
Precollect each node type once per function and reuse them across checks.
This reduces traversal cost from O(N × 4–8) → O(N).
(also for all other occurences in this file)
There was a problem hiding this comment.
Interesting view, let me have a look on what overhead is and how to make it better.
| * <li>Condition 6: arg contains string concatenation (+) with a class var field</li> | ||
| * </ul> | ||
| */ | ||
| private static boolean isRegexArgDynamic(KotlinParser.KtPostfixUnaryExpression pue, |
There was a problem hiding this comment.
Several methods (e.g. isRegexArgDynamic, isToRegexDynamic) do:
AST navigation
identifier extraction
“business” rule logic
set membership checks
multiple nested thematic checks (call suffix, string template, additive expr, etc.)
This mixing is a big maintainability smell.
Fix:
Split helpers into micro-checks:
isDynamicBecauseOfParam(...)
isDynamicBecauseOfLocalVar(...)
isDynamicBecauseOfMethodCall(...)
isDynamicBecauseOfTemplate(...)
2 participants
No description provided.