diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000000..6138f83c06 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,48 @@ +# Copilot Instructions for PMD-jPinpoint-rules + +## Build, Test, and Lint Commands + +- **Build and test all rules:** + - `./mvnw clean test` + - `./test` (shortcut for `./mvnw test`) +- **Run a single test class:** + - `./mvnw test -Dtest=YourRuleTest` +- **Merge rule categories into combined rulesets:** + - `./merge` (Java) + - `./merge kotlin` (Kotlin) + +## High-Level Architecture + +- **Rule definitions:** + - Rules are defined as XPath expressions in XML files under `src/main/resources/category/{java,kotlin}/`. + - Each category is registered in `categories.properties`. + - Combined rulesets (`rulesets/java/jpinpoint-rules.xml`, `rulesets/kotlin/jpinpoint-kotlin-rules.xml`) are generated by the merger tool. +- **Tests:** + - Each rule has an empty test class (extends `PmdRuleTst`) and a test data XML file with `` blocks. + - Test classes: `src/test/java/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/{RuleName}Test.java` + - Test data: `src/test/resources/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/xml/{RuleName}.xml` +- **Merger tool:** + - Located in `rulesets-merger/src/main/java/com/jpinpoint/perf/tools/RulesetMerger.java`. + - Merges category XMLs into combined rulesets. Build with `cd rulesets-merger && mvn clean install`. + +## Key Conventions + +- **Rule XML files:** + - Edit category XMLs, not the combined ruleset files. + - Register new categories in `categories.properties`. +- **Test data XML:** + - Use `` starting with `violation:` or `no violation:`. + - Use class names like `Foo`, method names like `bad`/`good`, and `//bad` comments on flagged lines. +- **Kotlin rules:** + - Complex rules can be implemented as Java classes extending `AbstractKotlinRule`. + - Use text comparison for token checks (e.g., `t.getText().equals("+=")`) (this is due to a little bug in pmd-core lib, waiting to be fixed). +- **Indentation:** + - Use spaces only (no tabs). + +## Documentation + +- Rule documentation is in `docs/` (e.g., `JavaCodePerformance.md`, `JavaCodeQuality.md`, `JavaDataAccessPerformance.md`). + +--- + +Let me know if you want to adjust anything or add coverage for areas I may have missed. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000000..62d0a843ab --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,92 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +PMD-jPinpoint-rules is a collection of custom PMD static analysis rules for Java and Kotlin, focused on performance, sustainability, multi-threading, data mix-up, and code quality. Rules are implemented as XPath expressions inside XML rule definition files (no custom Java rule classes). + +## Commands + +### Build and test +```sh +mvn clean test # run all unit tests +./test # shorthand script (runs ./mvnw test) +``` + +### Run a single test class +```sh +mvn test -Dtest=AvoidSpringMVCMemoryLeaksTest +``` + +### Merge category XML files into the combined ruleset +```sh +./merge # merges Java categories → rulesets/java/jpinpoint-rules.xml +./merge kotlin # merges Kotlin categories → rulesets/kotlin/jpinpoint-kotlin-rules.xml +``` +The merge step is required after editing any `src/main/resources/category/**/*.xml` file so that the combined `rulesets/` files stay up to date. + +## Architecture + +### Rule definitions (source of truth) +Rules live in per-category XML files under `src/main/resources/category/`: + +| Language | Category files | +|----------|---------------| +| Java | `java/common.xml`, `java/common_std.xml`, `java/concurrent.xml`, `java/enterprise.xml`, `java/remoting.xml`, `java/spring.xml`, `java/sql.xml` | +| Kotlin | `kotlin/common.xml`, `kotlin/remoting.xml` | + +Each category is registered in `src/main/resources/category/{java,kotlin}/categories.properties`. + +Most rules use `class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"` — rules are XPath 2.0 queries over the PMD AST. Complex rules that are hard to express in XPath can be implemented as Java classes (see below). + +### Combined rulesets (generated) +`rulesets/java/jpinpoint-rules.xml` and `rulesets/kotlin/jpinpoint-kotlin-rules.xml` are **generated** by the merger tool from the category files. Edit the category files, not these. + +### Tests +Each rule has exactly two files: + +1. **Test class** — `src/test/java/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/{RuleName}Test.java` + Always empty except for extending `PmdRuleTst`. The package path encodes the language and category, which PMD uses to locate the rule XML and test data. + +2. **Test data** — `src/test/resources/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/xml/{RuleName}.xml` + Contains `` blocks with: + - `` starting with `violation:` or `no violation:` + - `` (count) + - `` + - `` (CDATA) — use class names `Foo`, method names `bad`/`good`, and `//bad` comments on flagged lines + +### Merger tool +`rulesets-merger/src/main/java/com/jpinpoint/perf/tools/RulesetMerger.java` — standalone Maven module that merges category XMLs into the combined ruleset files. + +## Adding a New Rule + +1. Add the rule definition to the appropriate `src/main/resources/category/{java,kotlin}/{category}.xml` +2. Create the empty test class in `src/test/java/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/{RuleName}Test.java` +3. Create the test data XML in `src/test/resources/com/jpinpoint/perf/lang/{java,kotlin}/ruleset/{category}/xml/{RuleName}.xml` +4. Document the rule in the appropriate `docs/` markdown file +5. Run `./merge` to regenerate the combined ruleset +6. Run `./test` to verify + +For a new category, also add an entry to `categories.properties`. + +### Java-based Kotlin rules + +For complex logic that is hard to express in XPath, rules can be implemented as Java classes: + +- **Location**: `src/main/java/com/jpinpoint/perf/lang/kotlin/rule/{category}/{RuleName}Rule.java` +- **Pattern**: extend `AbstractKotlinRule`, override `buildTargetSelector()` and `buildVisitor()` +- **Visitor**: inner class extending `KotlinVisitorBase`, override `visitXxx()` methods +- **pmd-kotlin dependency**: must be `compile`-scoped (not `test`) so the rule class can import from `net.sourceforge.pmd.lang.kotlin.*` +- **XML registration**: use `class="com.jpinpoint.perf.lang.kotlin.rule.common.MyRule"` instead of `class="net.sourceforge.pmd.lang.rule.xpath.XPathRule"`; keep `language="kotlin"` attribute + +**Key API notes** (PMD 7.x ANTLR-based Kotlin AST): +- `node.descendants(KtFoo.class)` / `node.ancestors(KtFoo.class)` / `node.children(KtFoo.class)` return `NodeStream` +- `NodeStream` methods: `.any(Predicate)`, `.filter(Predicate)`, `.first()`, `.toList()`, `.nonEmpty()`, `.isEmpty()` +- `KotlinTerminalNode.getText()` returns the token image (e.g. `"String"`, `"+="`, `"+"`) +- **Do NOT use** `node.getToken(type, idx)` or `terminalNode.ADD()` / `terminalNode.ADD_ASSIGNMENT()` style methods — `BaseAntlrTerminalNode.getTokenKind()` returns the token stream index, not the token type, causing these to always return null. Use **text comparison** instead: `t.getText().equals("+=")` +- Reference implementation in pmd-kotlin jar: `net.sourceforge.pmd.lang.kotlin.rule.errorprone.OverrideBothEqualsAndHashcodeRule` + +## Code Style + +- Indentation: **spaces only** (no tabs) diff --git a/pom.xml b/pom.xml index 14b6066531..719e327fea 100644 --- a/pom.xml +++ b/pom.xml @@ -15,9 +15,21 @@ UTF-8 6.2.10 - 7.16.0 + 7.21.0 + + + + org.junit + junit-bom + 5.14.2 + pom + import + + + + @@ -27,19 +39,14 @@ 11 11 - - true org.apache.maven.plugins maven-surefire-plugin - 3.5.2 + 3.5.4 - - **/kotlin/** - **/java/** - + @@ -74,6 +81,18 @@ ${pmd.version} test + + net.sourceforge.pmd + pmd-lang-test + ${pmd.version} + test + + + io.kotest + * + + + net.sourceforge.pmd pmd-java @@ -84,12 +103,12 @@ net.sourceforge.pmd pmd-kotlin ${pmd.version} - test + - junit - junit - 4.13.2 + org.hamcrest + hamcrest + 2.2 test @@ -223,12 +242,6 @@ 1.18.34 - - - org.apache.maven.plugins - maven-surefire-plugin - 3.3.1 - io.github.resilience4j @@ -258,14 +271,6 @@ 2.20.0 compile - - - - maven_central - Maven Central - https://repo.maven.apache.org/maven2/ - - + - \ No newline at end of file diff --git a/rulesets/kotlin/jpinpoint-kotlin-rules.xml b/rulesets/kotlin/jpinpoint-kotlin-rules.xml index 005a701615..70c16c3328 100644 --- a/rulesets/kotlin/jpinpoint-kotlin-rules.xml +++ b/rulesets/kotlin/jpinpoint-kotlin-rules.xml @@ -61,7 +61,7 @@ class Foo { @@ -71,80 +71,6 @@ class Foo { 2 - - - class Foo { @@ -218,34 +144,19 @@ class AvoidInMemoryStreamingDefaultConstructor { - + Multiple statements concatenate to the same String. Problem: Each statement with one or more +-operators creates a hidden temporary StringBuilder, a char[] and a new String object, which all have to be garbage collected. Solution: Use StringBuilder.append. (jpinpoint-rules) 2 - - 1] -//Statement[Assignment//(T-ADD_ASSIGNMENT|T-ADD)][position()=last()] - - ]]> - - diff --git a/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidImplicitlyRecompilingRegexRule.java b/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidImplicitlyRecompilingRegexRule.java new file mode 100644 index 0000000000..27d96ea5cc --- /dev/null +++ b/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidImplicitlyRecompilingRegexRule.java @@ -0,0 +1,284 @@ +package com.jpinpoint.perf.lang.kotlin.rule.common; + +import com.jpinpoint.perf.lang.kotlin.util.KotlinAstUtil; +import net.sourceforge.pmd.lang.kotlin.AbstractKotlinRule; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinParser; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinVisitor; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinVisitorBase; +import net.sourceforge.pmd.lang.rule.RuleTargetSelector; +import net.sourceforge.pmd.reporting.RuleContext; + +import java.util.*; + +/** + * Detects implicit regex compilation inside Kotlin function bodies: + *
    + *
  • {@code "pattern".toRegex()} / {@code CONST.toRegex()} / {@code field.toRegex()}
    • + *
  • {@code Regex("pattern")} / {@code Regex(CONST)} / {@code Regex(field)}
    • + *
  • {@code Pattern.matches("pattern", ...)} (requires {@code import java.util.regex.Pattern})
    • + *
  • {@code FileSystems.getDefault().getPathMatcher("pattern")} + * (requires {@code import java.nio.file.FileSystems} or {@code java.nio.file.Path})
    • + *
+ * + *

Regex that are dynamic (derived from function parameters, local variables, or method-call + * results) are not flagged, as they cannot be pre-compiled.

+ */ +public class AvoidImplicitlyRecompilingRegexRule extends AbstractKotlinRule { + + @Override + protected RuleTargetSelector buildTargetSelector() { + return RuleTargetSelector.forTypes(KotlinParser.KtFunctionDeclaration.class); + } + + @Override + public KotlinVisitor buildVisitor() { + return new Visitor(); + } + + private static final class Visitor extends KotlinVisitorBase { + + @Override + public Void visitFunctionDeclaration(KotlinParser.KtFunctionDeclaration node, RuleContext ctx) { + Set paramNames = KotlinAstUtil.collectAllParamNames(node); + + KotlinParser.KtFunctionBody body = node.functionBody(); + Set localVarNames = KotlinAstUtil.collectLocalVarNames(body); + + Set classVarFields = KotlinAstUtil.collectClassVarFieldNames(node); + + boolean hasPatternImport = + KotlinAstUtil.hasImport(node, "java.util.regex.Pattern"); + boolean hasFileImport = + KotlinAstUtil.hasImport(node, "java.nio.file.FileSystems") + || KotlinAstUtil.hasImport(node, "java.nio.file.Path"); + + // --- .toRegex() calls --- + for (KotlinParser.KtNavigationSuffix navSuffix : + node.descendants(KotlinParser.KtNavigationSuffix.class) + .filter(ns -> KotlinAstUtil.isDirectDescendantOfFunctionDeclaration(ns, node)) + .toList()) { + if (!KotlinAstUtil.isToRegexNavSuffix(navSuffix)) continue; + if (isToRegexDynamic(navSuffix, paramNames, localVarNames)) continue; + ctx.addViolation(navSuffix); + } + + // --- Regex(...) constructor calls --- + for (KotlinParser.KtPostfixUnaryExpression pue : + node.descendants(KotlinParser.KtPostfixUnaryExpression.class) + .filter(p -> KotlinAstUtil.isDirectDescendantOfFunctionDeclaration(p, node)) + .toList()) { + if (!KotlinAstUtil.isRegexConstructorCall(pue)) continue; + KotlinParser.KtCallSuffix callSuffix = KotlinAstUtil.getFirstCallSuffix(pue); + if (callSuffix == null) continue; + if (isRegexArgDynamic(pue, callSuffix, paramNames, localVarNames, classVarFields)) continue; + // Report on the SimpleIdentifier "Regex" inside the PrimaryExpression + KotlinParser.KtSimpleIdentifier si = pue.primaryExpression().simpleIdentifier(); + if (si != null) ctx.addViolation(si); + } + + // --- Pattern.matches(...) calls --- + if (hasPatternImport) { + for (KotlinParser.KtPostfixUnaryExpression pue : + node.descendants(KotlinParser.KtPostfixUnaryExpression.class) + .filter(p -> KotlinAstUtil.isDirectDescendantOfFunctionDeclaration(p, node)) + .toList()) { + if (!isPatternMatchesCall(pue)) continue; + if (isPatternMatchesArgDynamic(pue, paramNames)) continue; + KotlinParser.KtSimpleIdentifier si = pue.primaryExpression().simpleIdentifier(); + if (si != null) ctx.addViolation(si); + } + } + + // --- FileSystems.getPathMatcher(...) calls --- + if (hasFileImport) { + Set fileSystemsVarNames = KotlinAstUtil.collectFileSystemsVarNames(node); + for (KotlinParser.KtNavigationSuffix navSuffix : + node.descendants(KotlinParser.KtNavigationSuffix.class) + .filter(ns -> KotlinAstUtil.isDirectDescendantOfFunctionDeclaration(ns, node)) + .toList()) { + if (!KotlinAstUtil.isGetPathMatcherNavSuffix(navSuffix)) continue; + if (!KotlinAstUtil.isOnFileSystemsReceiver(navSuffix, fileSystemsVarNames)) continue; + if (isGetPathMatcherArgDynamic(navSuffix, paramNames)) continue; + ctx.addViolation(navSuffix); + } + } + + return visitChildren(node, ctx); + } + + /** + * Returns true if the .toRegex() call should be treated as dynamic (no violation): + *
    + *
  • Condition 2: first PostfixUnarySuffix of the receiver PostfixUnaryExpression + * is a CallSuffix (meaning the receiver is the result of a method call)
    • + *
  • Condition 1: the PrimaryExpression's identifier matches a function parameter
    • + *
  • Condition 3: the PrimaryExpression's identifier is a local variable
    • + *
+ */ + private static boolean isToRegexDynamic(KotlinParser.KtNavigationSuffix navSuffix, + Set paramNames, + Set localVarNames) { + // Navigate: NavigationSuffix -> PostfixUnarySuffix -> PostfixUnaryExpression + KotlinParser.KtPostfixUnarySuffix suffix = + navSuffix.ancestors(KotlinParser.KtPostfixUnarySuffix.class).first(); + if (suffix == null) return true; + KotlinParser.KtPostfixUnaryExpression pue = + suffix.ancestors(KotlinParser.KtPostfixUnaryExpression.class).first(); + if (pue == null) return true; + + // Condition 2: first PostfixUnarySuffix is a CallSuffix -> receiver is method result + KotlinParser.KtPostfixUnarySuffix firstSuffix = pue.postfixUnarySuffix(0); + if (firstSuffix != null && firstSuffix.callSuffix() != null) return true; + + // Get receiver identifier from PrimaryExpression + String receiverName = KotlinAstUtil.getPrimaryExpressionSimpleIdentifierText( + pue.primaryExpression()); + + // Condition 1: receiver is a function parameter + if (receiverName != null && paramNames.contains(receiverName)) return true; + + // Condition 3: receiver is a local variable + if (receiverName != null && localVarNames.contains(receiverName)) return true; + + return false; + } + + /** + * Returns true if the Regex(...) argument is dynamic and should not be flagged: + *
    + *
  • Condition 1: arg contains a PrimaryExpression with a function parameter name
    • + *
  • Condition 2: arg contains a nested CallSuffix (method call)
    • + *
  • Condition 3: nearest ancestor CallSuffix contains a local-variable reference
    • + *
  • Condition 4: arg contains a string-template reference to a param (e.g. {@code "$param"})
    • + *
  • Condition 5: arg contains a string-template reference to a class var field
    • + *
  • Condition 6: arg contains string concatenation (+) with a class var field
    • + *
+ */ + private static boolean isRegexArgDynamic(KotlinParser.KtPostfixUnaryExpression pue, + KotlinParser.KtCallSuffix callSuffix, + Set paramNames, + Set localVarNames, + Set classVarFields) { + // Condition 1: any PrimaryExpression in the CallSuffix contains a param identifier + if (callSuffix.descendants(KotlinParser.KtPrimaryExpression.class) + .any(pe -> { + String name = KotlinAstUtil.getPrimaryExpressionSimpleIdentifierText(pe); + return paramNames.contains(name) || classVarFields.contains(name); + })) { + return true; + } + + // Condition 2: any nested CallSuffix (method call) inside the argument + if (callSuffix.descendants(KotlinParser.KtCallSuffix.class).nonEmpty()) { + return true; + } + + // Condition 3: nearest ancestor CallSuffix of this Regex PUE has a local-var reference + KotlinParser.KtCallSuffix ancestorCallSuffix = + pue.ancestors(KotlinParser.KtCallSuffix.class).first(); + if (ancestorCallSuffix != null) { + if (ancestorCallSuffix.descendants(KotlinParser.KtPostfixUnaryExpression.class) + .any(inner -> { + String text = KotlinAstUtil.getPrimaryExpressionSimpleIdentifierText( + inner.primaryExpression()); + return text != null && localVarNames.contains(text); + })) { + return true; + } + } + + // Condition 4: string template in the arg contains a reference to a function param + if (KotlinAstUtil.descendantHasStringTemplateRefFromSet(callSuffix, paramNames)) { + return true; + } + + // Condition 5: string template in the arg contains a reference to a class var field + if (KotlinAstUtil.descendantHasStringTemplateRefFromSet(callSuffix, classVarFields)) { + return true; + } + + // Condition 6: additive expression in the arg contains a class var field identifier + // (only when the arg also contains a string literal, matching XPath [//LineStringLiteral]) + if (callSuffix.descendants(KotlinParser.KtStringLiteral.class).nonEmpty()) { + return callSuffix.descendants(KotlinParser.KtAdditiveExpression.class) + .any(ae -> ae.descendants(KotlinParser.KtMultiplicativeExpression.class) + .any(me -> me.descendants(KotlinParser.KtSimpleIdentifier.class) + .any(si -> classVarFields.contains( + KotlinAstUtil.getIdentifierText(si))))); + } + + return false; + } + + /** + * Returns true if the PostfixUnaryExpression is a {@code Pattern.matches(...)} call: + * primary expression is "Pattern" and there is a NavigationSuffix with "matches". + */ + private static boolean isPatternMatchesCall(KotlinParser.KtPostfixUnaryExpression pue) { + KotlinParser.KtPrimaryExpression pe = pue.primaryExpression(); + if (pe == null) return false; + if (!"Pattern".equals(KotlinAstUtil.getIdentifierText(pe.simpleIdentifier()))) return false; + // Check that some PostfixUnarySuffix has NavigationSuffix with text "matches" + return pue.postfixUnarySuffix().stream() + .anyMatch(sus -> { + KotlinParser.KtNavigationSuffix ns = sus.navigationSuffix(); + return ns != null && "matches".equals( + KotlinAstUtil.getIdentifierText(ns.simpleIdentifier())); + }); + } + + /** + * Returns true if the first argument to {@code Pattern.matches(...)} is a function param. + */ + private static boolean isPatternMatchesArgDynamic(KotlinParser.KtPostfixUnaryExpression pue, + Set paramNames) { + // Find the CallSuffix that follows "matches" NavigationSuffix + List suffixes = pue.postfixUnarySuffix(); + for (int i = 0; i < suffixes.size(); i++) { + KotlinParser.KtNavigationSuffix ns = suffixes.get(i).navigationSuffix(); + if (ns != null && "matches".equals(KotlinAstUtil.getIdentifierText(ns.simpleIdentifier()))) { + // The CallSuffix with the arguments should follow + if (i + 1 < suffixes.size()) { + KotlinParser.KtCallSuffix callSuffix = suffixes.get(i + 1).callSuffix(); + if (callSuffix != null) { + return KotlinAstUtil.firstArgIsParam(callSuffix, paramNames); + } + } + } + } + return false; + } + + /** + * Returns true if the getPathMatcher argument is dynamic (function param or string template + * containing a function param). + */ + private static boolean isGetPathMatcherArgDynamic(KotlinParser.KtNavigationSuffix navSuffix, + Set paramNames) { + // Find the CallSuffix that immediately follows this NavigationSuffix's PostfixUnarySuffix + KotlinParser.KtPostfixUnarySuffix suffix = + navSuffix.ancestors(KotlinParser.KtPostfixUnarySuffix.class).first(); + if (suffix == null) return false; + KotlinParser.KtPostfixUnaryExpression pue = + suffix.ancestors(KotlinParser.KtPostfixUnaryExpression.class).first(); + if (pue == null) return false; + + List suffixes = pue.postfixUnarySuffix(); + for (int i = 0; i < suffixes.size(); i++) { + if (suffixes.get(i) == suffix) { + // The next PostfixUnarySuffix should be the CallSuffix + if (i + 1 < suffixes.size()) { + KotlinParser.KtCallSuffix callSuffix = suffixes.get(i + 1).callSuffix(); + if (callSuffix != null) { + if (KotlinAstUtil.firstArgIsParam(callSuffix, paramNames)) return true; + // String template with param + if (KotlinAstUtil.descendantHasStringTemplateRefFromSet(callSuffix, paramNames)) return true; + } + } + break; + } + } + return false; + } + } +} diff --git a/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidMultipleConcatStatementsRule.java b/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidMultipleConcatStatementsRule.java new file mode 100644 index 0000000000..3be846b944 --- /dev/null +++ b/src/main/java/com/jpinpoint/perf/lang/kotlin/rule/common/AvoidMultipleConcatStatementsRule.java @@ -0,0 +1,151 @@ +package com.jpinpoint.perf.lang.kotlin.rule.common; + +import com.jpinpoint.perf.lang.kotlin.util.KotlinAstUtil; +import net.sourceforge.pmd.lang.kotlin.AbstractKotlinRule; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinParser; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinTerminalNode; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinVisitor; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinVisitorBase; +import net.sourceforge.pmd.lang.rule.RuleTargetSelector; +import net.sourceforge.pmd.reporting.RuleContext; + +import java.util.*; + +/** + * Detects when a local String variable in a Kotlin function body is concatenated + * to via 2+ separate assignment statements ({@code +=} or {@code var = var + ...}), + * indicating repeated hidden StringBuilder creation. + * Violation is reported at the last such statement. + */ +public class AvoidMultipleConcatStatementsRule extends AbstractKotlinRule { + + @Override + protected RuleTargetSelector buildTargetSelector() { + return RuleTargetSelector.forTypes(KotlinParser.KtFunctionBody.class); + } + + @Override + public KotlinVisitor buildVisitor() { + return new Visitor(); + } + + private static final class Visitor extends KotlinVisitorBase { + + @Override + public Void visitFunctionBody(KotlinParser.KtFunctionBody node, RuleContext ctx) { + Set stringParams = KotlinAstUtil.findParamNamesOfType(node, "String"); + Set stringFunctions = KotlinAstUtil.findFunctionNamesReturningType(node, "String"); + + // Collect string variable names declared directly in this function body (not in nested lambdas) + Set stringVarNames = new HashSet<>(); + for (KotlinParser.KtPropertyDeclaration propDecl : + node.descendants(KotlinParser.KtPropertyDeclaration.class).toList()) { + if (!KotlinAstUtil.isDirectChildOfFunctionBody(propDecl, node)) continue; + if (!isStringProperty(propDecl, stringParams, stringFunctions)) continue; + KotlinParser.KtVariableDeclaration varDecl = propDecl.variableDeclaration(); + if (varDecl != null) { + String name = KotlinAstUtil.getIdentifierText(varDecl.simpleIdentifier()); + if (name != null) stringVarNames.add(name); + } + } + + if (stringVarNames.isEmpty()) { + return visitChildren(node, ctx); + } + + // Track concat-assignment statements per string variable, in source order + Map> concatStmts = new LinkedHashMap<>(); + for (KotlinParser.KtStatement stmt : + node.descendants(KotlinParser.KtStatement.class) + .filter(s -> KotlinAstUtil.isDirectChildOfFunctionBody(s, node)) + .toList()) { + KotlinParser.KtAssignment assignment = stmt.assignment(); + if (assignment == null) continue; + if (!hasConcatOperator(assignment)) continue; + + String lhsName = KotlinAstUtil.getLhsVarName(assignment); + if (lhsName == null || !stringVarNames.contains(lhsName)) continue; + + concatStmts.computeIfAbsent(lhsName, k -> new ArrayList<>()).add(stmt); + } + + // Report violation at the last concat statement for each variable with >= 2 + for (List stmts : concatStmts.values()) { + if (stmts.size() >= 2) { + ctx.addViolation(stmts.get(stmts.size() - 1)); + } + } + + return visitChildren(node, ctx); + } + + /** + * Determines if a property declaration represents a String variable. + * Uses four heuristics, matching the original XPath rule. + */ + private boolean isStringProperty(KotlinParser.KtPropertyDeclaration propDecl, + Set stringParams, + Set stringFunctions) { + // Skip if initializer contains toMutableList (false positive guard for issue #651) + KotlinParser.KtExpression initExpr = propDecl.expression(); + if (initExpr != null + && initExpr.descendants(KotlinTerminalNode.class) + .any(t -> "toMutableList".equals(t.getText()))) { + return false; + } + + // (a) explicit type annotation: var x: String = ... + KotlinParser.KtVariableDeclaration varDecl = propDecl.variableDeclaration(); + if (varDecl != null && KotlinAstUtil.typeContainsName(varDecl.type(), "String")) { + return true; + } + + if (initExpr == null) return false; + + // (b) initializer is a simple expression (no nested KtExpression) containing a string literal + // matches: var x = "" or var x = "text" + boolean hasNestedExpr = initExpr.descendants(KotlinParser.KtExpression.class).nonEmpty(); + boolean hasStringLiteral = initExpr.descendants(KotlinParser.KtStringLiteral.class).nonEmpty(); + if (!hasNestedExpr && hasStringLiteral) { + return true; + } + + // (c) initializer references a String parameter by name + if (initExpr.descendants(KotlinTerminalNode.class) + .any(t -> stringParams.contains(t.getText()))) { + return true; + } + + // (d) initializer calls a String-returning function at the top level + // (not as an argument inside another call's CallSuffix) + for (KotlinTerminalNode t : initExpr.descendants(KotlinTerminalNode.class) + .filter(tok -> stringFunctions.contains(tok.getText())) + .toList()) { + if (t.ancestors(KotlinParser.KtCallSuffix.class).isEmpty()) { + return true; + } + } + + return false; + } + + /** + * Returns true if the assignment uses += or uses + in the RHS expression. + * Uses text comparison ("+=", "+") because PMD's BaseAntlrTerminalNode.getTokenKind() + * returns the token stream index, not the token type, making getToken(type, idx) + * unreliable for operator checks. + */ + private boolean hasConcatOperator(KotlinParser.KtAssignment assignment) { + // Check for += (compound assignment) + KotlinParser.KtAssignmentAndOperator andOp = assignment.assignmentAndOperator(); + if (andOp != null + && andOp.children(KotlinTerminalNode.class).any(t -> "+=".equals(t.getText()))) { + return true; + } + // Check for + in plain assignment (e.g. x = x + y) + return assignment.descendants(KotlinParser.KtAdditiveOperator.class) + .any(op -> op.children(KotlinTerminalNode.class) + .any(t -> "+".equals(t.getText()))); + } + } +} diff --git a/src/main/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtil.java b/src/main/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtil.java new file mode 100644 index 0000000000..bebea1b7b4 --- /dev/null +++ b/src/main/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtil.java @@ -0,0 +1,378 @@ +package com.jpinpoint.perf.lang.kotlin.util; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinParser; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinTerminalNode; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.regex.Pattern; + +/** + * Static utility methods for navigating the PMD 7 ANTLR-based Kotlin AST. + * + *

Key gotcha: {@code BaseAntlrTerminalNode.getTokenKind()} returns the token-stream + * index, not the token-type constant. Generated accessor methods such as {@code ADD()} or + * {@code ADD_ASSIGNMENT()} therefore always return {@code null}. + * Always use text comparison instead: {@code t.getText().equals("+=")} etc.

+ */ +public final class KotlinAstUtil { + + private KotlinAstUtil() { /* utility class */ } + + /** + * Returns the text of the first terminal-node child of a {@link KotlinParser.KtSimpleIdentifier}, + * or {@code null} if the node itself is {@code null} or has no terminal children. + */ + public static String getIdentifierText(KotlinParser.KtSimpleIdentifier simpleId) { + if (simpleId == null) return null; + KotlinTerminalNode token = simpleId.children(KotlinTerminalNode.class).first(); + return token != null ? token.getText() : null; + } + + /** + * Returns the text of the SimpleIdentifier child of a {@link KotlinParser.KtPrimaryExpression}, + * or {@code null} if none is present. + */ + public static String getPrimaryExpressionSimpleIdentifierText(KotlinParser.KtPrimaryExpression pe) { + if (pe == null) return null; + return getIdentifierText(pe.simpleIdentifier()); + } + + /** + * Returns {@code true} if any terminal-node descendant of {@code type} has text equal to + * {@code typeName}. Useful for checking type annotations like {@code var x: String} or a + * declared return type of {@code String}. + */ + public static boolean typeContainsName(KotlinParser.KtType type, String typeName) { + if (type == null || typeName == null) return false; + return type.descendants(KotlinTerminalNode.class).any(t -> typeName.equals(t.getText())); + } + + /** + * Returns {@code true} if the nearest enclosing {@link KotlinParser.KtFunctionBody} ancestor + * of {@code node} is exactly {@code body}. Used to restrict descendant searches to a single + * function body without crossing into nested lambdas or local functions. + */ + public static boolean isDirectChildOfFunctionBody(Node node, KotlinParser.KtFunctionBody body) { + return node.ancestors(KotlinParser.KtFunctionBody.class).first() == body; + } + + /** + * Returns {@code true} if the nearest enclosing {@link KotlinParser.KtFunctionDeclaration} + * ancestor of {@code node} is exactly {@code funcDecl}. Used to restrict descendant searches + * to a single function declaration without crossing into nested local functions. + * Note: lambdas (KtFunctionLiteral) are not KtFunctionDeclaration so they are transparent to this check. + */ + public static boolean isDirectDescendantOfFunctionDeclaration(Node node, + KotlinParser.KtFunctionDeclaration funcDecl) { + return node.ancestors(KotlinParser.KtFunctionDeclaration.class).first() == funcDecl; + } + + /** + * Extracts the simple variable name from the left-hand side of an assignment. + * Handles both plain assignment ({@code x = ...}) and compound assignment ({@code x += ...}). + * Returns {@code null} if the LHS is not a simple identifier (e.g. property access, index). + */ + public static String getLhsVarName(KotlinParser.KtAssignment assignment) { + // directlyAssignableExpression covers `x = ...` (plain assignment) + KotlinParser.KtDirectlyAssignableExpression dae = assignment.directlyAssignableExpression(); + if (dae != null && dae.simpleIdentifier() != null) { + KotlinTerminalNode token = dae.simpleIdentifier().children(KotlinTerminalNode.class).first(); + if (token != null) return token.getText(); + } + // assignableExpression covers `x += ...` (compound assignment) + KotlinParser.KtAssignableExpression ae = assignment.assignableExpression(); + if (ae != null) { + KotlinTerminalNode token = ae.descendants(KotlinTerminalNode.class).first(); + if (token != null) return token.getText(); + } + return null; + } + + /** + * Returns the names of all parameters in the function declaration (regardless of type). + */ + public static Set collectAllParamNames(KotlinParser.KtFunctionDeclaration funcDecl) { + Set result = new HashSet<>(); + if (funcDecl == null) return result; + KotlinParser.KtFunctionValueParameters params = funcDecl.functionValueParameters(); + if (params == null) return result; + for (KotlinParser.KtFunctionValueParameter param : params.functionValueParameter()) { + KotlinParser.KtParameter p = param.parameter(); + if (p != null) { + String name = getIdentifierText(p.simpleIdentifier()); + if (name != null) result.add(name); + } + } + return result; + } + + /** + * Returns the names of all parameters whose type annotation contains {@code typeName} in the + * function declaration that directly encloses {@code functionBody}. + */ + public static Set findParamNamesOfType(KotlinParser.KtFunctionBody functionBody, + String typeName) { + Set result = new HashSet<>(); + KotlinParser.KtFunctionDeclaration funcDecl = + functionBody.ancestors(KotlinParser.KtFunctionDeclaration.class).first(); + if (funcDecl == null) return result; + + KotlinParser.KtFunctionValueParameters params = funcDecl.functionValueParameters(); + if (params == null) return result; + + for (KotlinParser.KtFunctionValueParameter param : params.functionValueParameter()) { + KotlinParser.KtParameter p = param.parameter(); + if (p != null && typeContainsName(p.type(), typeName)) { + String name = getIdentifierText(p.simpleIdentifier()); + if (name != null) result.add(name); + } + } + return result; + } + + /** + * Returns the names of all local variables (PropertyDeclarations) declared anywhere within + * {@code functionBody}, including inside nested lambdas. This matches the XPath behaviour of + * {@code ancestor::FunctionBody//PropertyDeclaration/VariableDeclaration/...}. + */ + public static Set collectLocalVarNames(KotlinParser.KtFunctionBody functionBody) { + Set result = new HashSet<>(); + if (functionBody == null) return result; + for (KotlinParser.KtPropertyDeclaration propDecl : + functionBody.descendants(KotlinParser.KtPropertyDeclaration.class).toList()) { + KotlinParser.KtVariableDeclaration varDecl = propDecl.variableDeclaration(); + if (varDecl != null) { + String name = getIdentifierText(varDecl.simpleIdentifier()); + if (name != null) result.add(name); + } + } + return result; + } + + /** + * Returns the names of all mutable ({@code var}) class fields declared in the class + * body that encloses {@code node}. This matches the XPath pattern + * {@code ancestor::ClassDeclaration//PropertyDeclaration[T-VAR]//SimpleIdentifier/T-Identifier/@Text}. + */ + public static Set collectClassVarFieldNames(Node node) { + Set result = new HashSet<>(); + KotlinParser.KtClassDeclaration classDecl = + node.ancestors(KotlinParser.KtClassDeclaration.class).first(); + if (classDecl == null) return result; + for (KotlinParser.KtPropertyDeclaration propDecl : + classDecl.descendants(KotlinParser.KtPropertyDeclaration.class).toList()) { + // Check for `var` keyword using text comparison (not VAR() method which may be unreliable) + if (propDecl.children(KotlinTerminalNode.class).any(t -> "var".equals(t.getText()))) { + KotlinParser.KtVariableDeclaration varDecl = propDecl.variableDeclaration(); + if (varDecl != null) { + String name = getIdentifierText(varDecl.simpleIdentifier()); + if (name != null) result.add(name); + } + } + } + return result; + } + + /** + * Returns the names of all class-level functions that explicitly declare {@code typeName} as + * their return type, searching within the class body that encloses {@code functionBody}. + */ + public static Set findFunctionNamesReturningType(KotlinParser.KtFunctionBody functionBody, + String typeName) { + Set result = new HashSet<>(); + KotlinParser.KtClassMemberDeclarations classMembers = + functionBody.ancestors(KotlinParser.KtClassMemberDeclarations.class).first(); + if (classMembers == null) return result; + + for (KotlinParser.KtFunctionDeclaration funcDecl : + classMembers.descendants(KotlinParser.KtFunctionDeclaration.class).toList()) { + if (typeContainsName(funcDecl.type(), typeName)) { + String name = getIdentifierText(funcDecl.simpleIdentifier()); + if (name != null) result.add(name); + } + } + return result; + } + + /** + * Returns {@code true} if the Kotlin file enclosing {@code node} has an import that matches + */ + public static boolean hasImport(Node node, String fullyQualifiedName) { + if (fullyQualifiedName == null || fullyQualifiedName.isEmpty()) return false; + String[] identifier = fullyQualifiedName.split("\\."); + return hasImport(node, identifier); + } + + /** + * Returns {@code true} if the Kotlin file enclosing {@code node} has an import that matches + * all the given identifier parts (or has a wildcard import). For example, to check for + * {@code import java.util.regex.Pattern}, pass {@code "java", "util", "regex", "Pattern"}. + * + *

A wildcard import (containing {@code *}) always matches.

+ */ + public static boolean hasImport(Node node, String... identifiers) { + KotlinParser.KtKotlinFile file = node.getClass().equals(KotlinParser.KtKotlinFile.class) ? (KotlinParser.KtKotlinFile) node : node.ancestors(KotlinParser.KtKotlinFile.class).first(); + if (file == null) return false; + KotlinParser.KtImportList importList = file.importList(); + if (importList == null) return false; + + for (KotlinParser.KtImportHeader importHeader : importList.importHeader()) { + List tokens = importHeader.descendants(KotlinTerminalNode.class).toList(); + + // Build identifier-like token list directly from terminal nodes (keep original order). + List idTokens = new ArrayList<>(); + for (KotlinTerminalNode tn : tokens) { + String txt = tn.getText().trim(); + if (txt.isEmpty()) continue; + if (!".".equals(txt) && !"import".equals(txt)) { + idTokens.add(txt); + } + } + + // If this import has a wildcard (e.g. import x.y.z.*), only match when the prefix before '*' + // equals the leading part of the requested identifiers. For example, import a.b.* matches + // identifiers ["a","b","C"] but should NOT match an unrelated package. + if (!idTokens.isEmpty() && lastTokenIsWildcard(idTokens)) { + int prefixLen = idTokens.size() - 1; + if (identifiers.length >= prefixLen) { + boolean prefixMatches = true; + for (int i = 0; i < prefixLen; i++) { + if (!idTokens.get(i).equals(identifiers[i])) { + prefixMatches = false; + break; + } + } + if (prefixMatches) return true; + } + // wildcard present but prefix doesn't match; continue checking other imports + continue; + } + + // No wildcard: check whether the requested identifiers appear as an adjacent, exact sequence + // in the identifier tokens (must be contiguous and in exact order). + if (identifierMatchAllTokens(identifiers, idTokens)) return true; + } + return false; + } + + private static boolean lastTokenIsWildcard(List idTokens) { + return "*".equals(idTokens.get(idTokens.size() - 1)); + } + + private static boolean identifierMatchAllTokens(String[] identifiers, List idTokens) { + // Require exact same size and exact element equality (contiguous and same length). + int n = idTokens.size(); + int m = identifiers.length; + if (m != n) return false; + for (int i = 0; i < m; i++) { + if (!identifiers[i].equals(idTokens.get(i))) return false; + } + return true; + } + + /** + * Returns {@code true} if any {@link KotlinParser.KtLineStringContent} descendant of + * {@code node} has a {@code LineStrRef} token whose text (including the leading {@code $}) + * equals {@code "$" + name} for some {@code name} in {@code names}. + *

+ * This matches the XPath pattern + * {@code //LineStringContent/T-LineStrRef[@Text = concat('$', name)]}. + */ + public static boolean descendantHasStringTemplateRefFromSet(Node node, Set names) { + if (node == null || names == null || names.isEmpty()) return false; + for (KotlinParser.KtLineStringContent lsc : + node.descendants(KotlinParser.KtLineStringContent.class).toList()) { + KotlinTerminalNode lineStrRef = lsc.children(KotlinTerminalNode.class) + .filter(t -> t.getText().startsWith("$")) + .first(); + if (lineStrRef != null) { + String refText = lineStrRef.getText(); // e.g. "$context1" + if (refText.length() > 1 && names.contains(refText.substring(1))) return true; + } + } + return false; + } + + /** + * Returns true if the NavigationSuffix refers to {@code toRegex}. */ + public static boolean isToRegexNavSuffix(KotlinParser.KtNavigationSuffix navSuffix) { + return "toRegex".equals(getIdentifierText(navSuffix.simpleIdentifier())); + } + + /** Returns true if the PostfixUnaryExpression represents a {@code Regex(...)} constructor call. */ + public static boolean isRegexConstructorCall(KotlinParser.KtPostfixUnaryExpression pue) { + KotlinParser.KtPrimaryExpression pe = pue.primaryExpression(); + if (pe == null) return false; + if (!"Regex".equals(getIdentifierText(pe.simpleIdentifier()))) return false; + KotlinParser.KtPostfixUnarySuffix firstSuffix = pue.postfixUnarySuffix(0); + return firstSuffix != null && firstSuffix.callSuffix() != null; + } + + /** Returns the CallSuffix of the first PostfixUnarySuffix, or null. */ + public static KotlinParser.KtCallSuffix getFirstCallSuffix(KotlinParser.KtPostfixUnaryExpression pue) { + KotlinParser.KtPostfixUnarySuffix firstSuffix = pue.postfixUnarySuffix(0); + return firstSuffix != null ? firstSuffix.callSuffix() : null; + } + + /** Returns true if the NavigationSuffix refers to {@code getPathMatcher}. */ + public static boolean isGetPathMatcherNavSuffix(KotlinParser.KtNavigationSuffix navSuffix) { + return "getPathMatcher".equals(getIdentifierText(navSuffix.simpleIdentifier())); + } + + /** Returns true if the getPathMatcher call is on a FileSystems instance. */ + public static boolean isOnFileSystemsReceiver(KotlinParser.KtNavigationSuffix navSuffix, Set fileSystemsVarNames) { + KotlinParser.KtPostfixUnarySuffix suffix = navSuffix.ancestors(KotlinParser.KtPostfixUnarySuffix.class).first(); + if (suffix == null) return false; + KotlinParser.KtPostfixUnaryExpression pue = suffix.ancestors(KotlinParser.KtPostfixUnaryExpression.class).first(); + if (pue == null) return false; + String receiverName = getPrimaryExpressionSimpleIdentifierText(pue.primaryExpression()); + if (receiverName == null) return false; + if ("FileSystems".equals(receiverName)) return true; + return fileSystemsVarNames.contains(receiverName); + } + + /** + * Collects names of local variables in the function body that were initialized from + * an expression containing {@code FileSystems} (e.g. {@code val fs = FileSystems.getDefault()}). + */ + public static Set collectFileSystemsVarNames(KotlinParser.KtFunctionDeclaration funcDecl) { + Set result = new HashSet<>(); + KotlinParser.KtFunctionBody body = funcDecl.functionBody(); + if (body == null) return result; + for (KotlinParser.KtStatement stmt : body.descendants(KotlinParser.KtStatement.class).toList()) { + KotlinParser.KtDeclaration decl = stmt.declaration(); + if (decl == null) continue; + KotlinParser.KtPropertyDeclaration propDecl = decl.propertyDeclaration(); + if (propDecl == null) continue; + KotlinParser.KtExpression initExpr = propDecl.expression(); + if (initExpr == null) continue; + if (initExpr.descendants(KotlinTerminalNode.class) + .any(t -> "FileSystems".equals(t.getText()))) { + KotlinParser.KtVariableDeclaration varDecl = propDecl.variableDeclaration(); + if (varDecl != null) { + String name = getIdentifierText(varDecl.simpleIdentifier()); + if (name != null) result.add(name); + } + } + } + return result; + } + + /** + * Returns true if the first ValueArgument in the CallSuffix is a simple identifier + * that matches a function parameter name. + */ + public static boolean firstArgIsParam(KotlinParser.KtCallSuffix callSuffix, Set paramNames) { + if (callSuffix.valueArguments() == null) return false; + List args = callSuffix.valueArguments().valueArgument(); + if (args.isEmpty()) return false; + KotlinParser.KtValueArgument firstArg = args.get(0); + return firstArg.descendants(KotlinParser.KtPrimaryExpression.class) + .any(pe -> paramNames.contains(getPrimaryExpressionSimpleIdentifierText(pe))); + } + +} diff --git a/src/main/resources/category/kotlin/common.xml b/src/main/resources/category/kotlin/common.xml index 2e849cdc0e..7a102cdaec 100644 --- a/src/main/resources/category/kotlin/common.xml +++ b/src/main/resources/category/kotlin/common.xml @@ -389,44 +389,19 @@ class Foo { - + Multiple statements concatenate to the same String. Problem: Each statement with one or more +-operators creates a hidden temporary StringBuilder, a char[] and a new String object, which all have to be garbage collected. Solution: Use StringBuilder.append. (jpinpoint-rules) 2 - - 1 -] -//Statement[Assignment//(T-ADD_ASSIGNMENT|T-ADD)][position()=last()] - ]]> - - @@ -600,7 +575,7 @@ class FooGood { @@ -610,80 +585,6 @@ class FooGood { 2 - - - class Foo { diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/util/AddEqualsTest.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/AddEqualsTest.java new file mode 100644 index 0000000000..02eec23b98 --- /dev/null +++ b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/AddEqualsTest.java @@ -0,0 +1,43 @@ +package com.jpinpoint.perf.lang.kotlin.util; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinParser; +import net.sourceforge.pmd.lang.kotlin.ast.KotlinTerminalNode; +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class AddEqualsTest { + + private Node parseResource(String resourcePath) throws Exception { + String code = new String(Files.readAllBytes(Paths.get(resourcePath))); + return KotlinParsingHelper.DEFAULT.parse(code); + } + + @Test + public void testHasAddEquals() throws Exception { + Node node = parseResource("src/test/resources/com/jpinpoint/perf/lang/kotlin/util/AddEquals.kt"); + // find all "+=" nodes + List addEqualsNodes = node.descendantsOrSelf() + .filter(n -> n instanceof KotlinTerminalNode) + .map(n -> (KotlinTerminalNode) n) + .filter(n -> n.getText().equals("+=")) + .toList(); + + assertEquals(2, addEqualsNodes.size()); + + // this one is broken because of the bug https://github.com/pmd/pmd/issues/6471 + List addAssigmentNodes = node.descendantsOrSelf() + .filter(n -> n instanceof KotlinParser.KtAssignmentAndOperator) + .map(n -> (KotlinParser.KtAssignmentAndOperator) n) + .filter(n -> n.ADD_ASSIGNMENT() != null) + .toList(); + + // this is now actually 0, activate after fixing the bug + //assertEquals(2, addAssigmentNodes.size()); + } +} diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtilTest.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtilTest.java new file mode 100644 index 0000000000..6537c7115a --- /dev/null +++ b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinAstUtilTest.java @@ -0,0 +1,48 @@ +package com.jpinpoint.perf.lang.kotlin.util; + +import net.sourceforge.pmd.lang.ast.Node; +import org.junit.jupiter.api.Test; + +import java.nio.file.Files; +import java.nio.file.Paths; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class KotlinAstUtilTest { + + private Node parseResource(String resourcePath) throws Exception { + String code = new String(Files.readAllBytes(Paths.get(resourcePath))); + // KotlinParsingHelper is in the same package and extends BaseParsingHelper + return KotlinParsingHelper.DEFAULT.parse(code); + } + + @Test + public void testHasImportExactMatch() throws Exception { + Node node = parseResource("src/test/resources/com/jpinpoint/perf/lang/kotlin/util/TestImports.kt"); + assertTrue(KotlinAstUtil.hasImport(node, "java.util.regex.Pattern")); + assertTrue(KotlinAstUtil.hasImport(node, "java", "util", "regex", "Pattern")); + } + + @Test + public void testHasImportWildcard() throws Exception { + Node node = parseResource("src/test/resources/com/jpinpoint/perf/lang/kotlin/util/TestImports.kt"); + assertTrue(KotlinAstUtil.hasImport(node, "java.util.regex.Pattern")); + assertTrue(KotlinAstUtil.hasImport(node, "java", "util", "regex", "Pattern")); + } + + @Test + public void testHasImportNegative() throws Exception { + Node node = parseResource("src/test/resources/com/jpinpoint/perf/lang/kotlin/util/TestImports.kt"); + assertFalse(KotlinAstUtil.hasImport(node, "not.present.Import")); + } + + @Test + public void testHasImportMultipleIdentifiers() throws Exception { + Node node = parseResource("src/test/resources/com/jpinpoint/perf/lang/kotlin/util/TestImports.kt"); + assertTrue(KotlinAstUtil.hasImport(node, "java.util.regex.Pattern")); + assertTrue(KotlinAstUtil.hasImport(node, "org.example.Foo")); + assertTrue(KotlinAstUtil.hasImport(node, "java.util.MatchAnything")); + assertFalse(KotlinAstUtil.hasImport(node, "io.test.ListNotPresent")); + } +} diff --git a/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinParsingHelper.java b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinParsingHelper.java new file mode 100644 index 0000000000..bd0ef8d3f6 --- /dev/null +++ b/src/test/java/com/jpinpoint/perf/lang/kotlin/util/KotlinParsingHelper.java @@ -0,0 +1,26 @@ +package com.jpinpoint.perf.lang.kotlin.util; + +import net.sourceforge.pmd.lang.kotlin.ast.KotlinParser; +import org.jetbrains.annotations.NotNull; + +import net.sourceforge.pmd.lang.kotlin.KotlinLanguageModule; +import net.sourceforge.pmd.lang.test.ast.BaseParsingHelper; + +/** + * + */ +public class KotlinParsingHelper extends BaseParsingHelper { + + public static final KotlinParsingHelper DEFAULT = new KotlinParsingHelper(Params.getDefault()); + + + public KotlinParsingHelper(@NotNull Params params) { + super(KotlinLanguageModule.getInstance(), KotlinParser.KtKotlinFile.class, params); + } + + @NotNull + @Override + protected KotlinParsingHelper clone(@NotNull Params params) { + return new KotlinParsingHelper(params); + } +} diff --git a/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml index f2d31ef808..bcb893094b 100644 --- a/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml +++ b/src/test/resources/com/jpinpoint/perf/lang/kotlin/ruleset/common/xml/AvoidImplicitlyRecompilingRegex.xml @@ -6,9 +6,8 @@ violation: Avoid implicit recompiling of regular expressions 29 - 14,15,16, 20,21,22,23, 29,32,33, 40,41,42,44,46,47, 54,55,56, 62,63,64,67, 73, 82,83,84,86, 108 + 13,14,15, 19,20,21,22, 28,31,32, 39,40,41,43,45,46, 53,54,55, 60,62,63,66, 72, 81,82,83,85, 107 1 @@ -124,7 +123,7 @@ class Foo { violation: Avoid implicit recompiling of regular expressions - non toRegex 8 - 16,24,25,26,32,33,34,37 + 16,24,25,26,31,33,34,37