diff --git a/AGENTS.md b/AGENTS.md index ba0a6f53..d9529018 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -20,22 +20,43 @@ CodeCocoon is an IntelliJ Platform plugin for **metamorphic testing** of Java pr - Self-managed: `SelfManagedTransformation` (uses refactoring processors) 3. **Built-in transformations**: - - `rename-method-transformation` - Rename methods via LLM suggestions - - `rename-class-transformation` - Rename classes - - `rename-variable-transformation` - Rename fields/parameters/locals + - `rename-method-transformation` - Rename methods via LLM suggestions (supports annotation whitelist/blacklist) + - `rename-class-transformation` - Rename classes via LLM suggestions (supports annotation whitelist/blacklist) + - `rename-variable-transformation` - Rename fields/parameters/locals via LLM suggestions (supports annotation blacklist only) - `move-file-into-suggested-directory-transformation/ai` - Move files (LLM suggests destination) - `move-file-into-suggested-directory-transformation/config` - Move files (config specifies destination) + - `add-comment-transformation` - Example transformation (adds comment to file start) 4. **LLM Integration**: Uses Grazie/Koog to generate semantically similar names -5. **Memory System**: Caches LLM suggestions in `.codecocoon-memory/` to avoid redundant API calls - -6. **Configuration**: `codecocoon.yml` in project root defines transformations and target files - -7. **Important Threading Rules**: +5. **Memory System**: + - Persistent cache storing LLM suggestions in `.codecocoon-memory/.json` + - Signature-based: Each element (class/method/variable/file) gets unique signature + - Controlled via `useMemory` and `generateWhenNotInMemory` config options + - Auto-saves on transformation completion via `PersistentMemory.use {}` + +6. **Annotation Filtering**: + - **Methods/Classes**: Support both whitelist and blacklist modes + - **Whitelist mode**: Only rename elements WITH specified annotations + - **Blacklist mode** (recommended): Rename all EXCEPT those with specified annotations + - **Variables**: Support blacklist mode only (no whitelist) + - **`"_default"` keyword**: Merges 35-40+ framework annotations with custom ones + - Methods: 40+ annotations (Spring, JPA, JAX-RS, JUnit, etc.) + - Classes: 25+ annotations (JPA, Spring, JAX-RS, JAXB, etc.) + - Variables: 35+ annotations (JPA, Jackson, JAXB, Spring, validation, CDI, etc.) + - Warning logged if blacklist used without `"_default"` + +7. **Configuration**: `codecocoon.yml` in project root defines transformations and target files + +8. **Important Threading Rules**: - PSI reads require `readAction { }` or `IntelliJAwareTransformation.withReadAction { }` - PSI writes require `writeCommandAction { }` or use self-managed refactoring processors +9. **Import Optimization Prevention**: Code style settings configured to prevent wildcard imports and minimize automatic import modifications + - ✅ Prevents wildcard imports (`import package.*`) + - ✅ Forces single class imports + - ❌ Cannot prevent unused import removal (IntelliJ limitation) + ## Common Tasks - **Adding a transformation**: Implement `IntelliJAwareTransformation`, register in `TransformationRegistry.kt` @@ -46,10 +67,13 @@ CodeCocoon is an IntelliJ Platform plugin for **metamorphic testing** of Java pr ## When to Consult CLAUDE.md Refer to [`CLAUDE.md`](./CLAUDE.md) for: +- **Import optimization prevention** - Detailed explanation of settings and limitations - Detailed architecture explanations - PSI utilities and helper functions - Configuration schema and examples - Transformation implementation details +- Memory system internals (`PsiSignatureGenerator`, signature format) +- Annotation filtering implementation (whitelist/blacklist logic) - Error handling patterns - File structure overview - Dependencies and testing diff --git a/README.md b/README.md index 21225e34..5bc6b648 100644 --- a/README.md +++ b/README.md @@ -33,22 +33,179 @@ transformations: - "src/main" ``` -## Built-in transformations - -- Rename Method (id: `rename-method-transformation`) - - Renames Java methods to an LLM-suggested, semantically similar name and updates usages/overrides. - - Skips: overrides, tests, interface methods extending library interfaces, annotated methods, constructors, toString/get*/set*/is*, public methods with no refs, and methods referenced from non-Java/Kotlin files. -- Rename Class (id: `rename-class-transformation`) - - Renames Java classes to an LLM-suggested, semantically similar name and updates usages/overrides. - - Skips: classes referenced from non-Java files, test class names and annotated classes. -- Rename Variable (id: `rename-variable-transformation`) - - Renames Java variables to an LLM-suggested, semantically similar name and updates usages. - - Skips: variables in test classes, enums, and those declared in library-files. -- Move File Into Directory Suggested By AI (id: `move-file-into-suggested-directory-transformation/ai`) - - Moves a Java file into another directory suggested by an LLM based on the file content. -- Move File Into Directory From Config (id: `move-file-into-suggested-directory-transformation/config`) - - Moves a Java file into a directory provided in the config under `destination` entry. - - The directory MUST be within the project BUT may be either new or existing. +## Memory System + +CodeCocoon includes a **persistent memory system** that caches LLM-generated suggestions to avoid redundant API calls and ensure consistency across runs. Memory is stored in `.codecocoon-memory/` directory as JSON files, one per project. + +**Key features:** +- Signature-based caching: Each renamed element (class/method/variable) gets a unique signature +- Automatic persistence: Memory is saved automatically when transformations complete +- Reusability: Run the same transformation multiple times without re-querying the LLM +- Optional generation: Configure whether to generate new suggestions for missing entries + +All renaming transformations support memory via `useMemory` and `generateWhenNotInMemory` config options. + +## Built-in Transformations + +### 1. Rename Method (`rename-method-transformation`) + +Renames Java methods to LLM-suggested, semantically similar names and updates all usages/overrides. Processes methods in **overload families** to ensure consistency. + +**Filters (methods are skipped if):** +- Override super methods +- In test sources +- In interfaces extending library interfaces +- Belong to library classes +- Are constructors or Object methods (equals, hashCode, toString, etc.) +- Match excluded patterns (toString, get*, set*, is*) +- Have no public references +- Referenced from non-Java/Kotlin files +- Fail annotation filter (whitelist/blacklist mode) + +**Configuration:** +```yaml +- id: "rename-method-transformation" + config: + # Memory configuration + useMemory: true # Optional, default: false. Use cached suggestions + generateWhenNotInMemory: true # Optional, default: false. Generate if not cached + searchInComments: false # Optional, default: false. Rename in comments too + + # Annotation filtering (choose whitelist OR blacklist mode) + annotationFilterMode: "blacklist" # Optional, default: "blacklist" if blacklistedAnnotations non-empty, else "whitelist" + + # Blacklist mode (recommended): Rename all methods EXCEPT those with these annotations + blacklistedAnnotations: + - "_default" # Special keyword: includes 40+ framework annotations (Spring, JPA, JAX-RS, JUnit, etc.) + - "MyCustomAnnotation" # Add your own annotations + + # Whitelist mode: Only rename methods WITH these annotations + whitelistedAnnotations: + - "SuppressWarnings" + - "Deprecated" +``` + +**Annotation filter modes:** +- **Blacklist** (recommended): Rename everything EXCEPT framework-managed methods. Use `"_default"` to include all standard framework annotations (Spring `@RequestMapping`, JPA `@PrePersist`, JAX-RS `@GET`/`@POST`, JUnit `@Test`/`@BeforeEach`, etc.) plus custom ones. +- **Whitelist**: Only rename methods with specific annotations. Empty whitelist = only non-annotated methods. +- **⚠ Warning**: Omitting `"_default"` in blacklist mode will NOT exclude framework annotations automatically. + +--- + +### 2. Rename Class (`rename-class-transformation`) + +Renames Java classes to LLM-suggested, semantically similar names and updates all usages. + +**Filters (classes are skipped if):** +- Referenced from non-Java files +- In test sources +- Class name is null or ≤1 character +- Fail annotation filter (whitelist/blacklist mode) + +**Configuration:** +```yaml +- id: "rename-class-transformation" + config: + # Memory configuration + useMemory: true # Optional, default: false + generateWhenNotInMemory: true # Optional, default: false + searchInComments: false # Optional, default: false + + # Annotation filtering (choose whitelist OR blacklist mode) + annotationFilterMode: "blacklist" # Optional, default: "blacklist" if blacklistedAnnotations non-empty, else "whitelist" + + # Blacklist mode (recommended): Rename all classes EXCEPT those with these annotations + blacklistedAnnotations: + - "_default" # Special keyword: includes 25+ framework annotations (JPA, Spring, JAX-RS, JAXB, etc.) + - "MyCustomAnnotation" + + # Whitelist mode: Only rename classes WITH these annotations + whitelistedAnnotations: + - "Deprecated" +``` + +**Annotation filter modes:** Same as rename-method (see above). Default blacklist includes JPA `@Entity`/`@Table`, Spring `@Component`/`@Service`/`@Controller`, JAX-RS `@Path`, JAXB `@XmlRootElement`, etc. + +--- + +### 3. Rename Variable (`rename-variable-transformation`) + +Renames Java variables (fields, parameters, locals) to LLM-suggested, semantically similar names and updates all usages. + +**Filters (variables are skipped if):** +- In test sources +- Enum constants +- Fail annotation filter (blacklist mode only - no whitelist support) +- Declared in library/compiled code +- Public/protected fields (to avoid breaking external consumers) + +**Configuration:** +```yaml +- id: "rename-variable-transformation" + config: + # Memory configuration + useMemory: true # Optional, default: false + generateWhenNotInMemory: true # Optional, default: false + searchInComments: false # Optional, default: false + + # Annotation blacklist filtering (no whitelist support) + blacklistedAnnotations: + - "_default" # Special keyword: includes 35+ framework annotations (JPA, Jackson, JAXB, Spring, validation, etc.) + - "MyCustomAnnotation" # Add your own annotations +``` + +**Annotation filtering (blacklist mode only):** +- **Blacklist mode**: Rename all variables EXCEPT those with specified annotations. Use `"_default"` to include JPA (`@Column`/`@Id`/`@JoinColumn`), Jackson (`@JsonProperty`), JAXB (`@XmlElement`/`@XmlAttribute`), Spring (`@Value`/`@Autowired`), validation (`@NotNull`/`@Size`/`@Email`), and CDI (`@Inject`) annotations. +- **⚠ Warning**: Omitting `"_default"` in blacklist will NOT exclude framework annotations automatically. +- **Note**: Variables do NOT support whitelist mode (methods/classes only). + +--- + +### 4. Move File (AI-Suggested) (`move-file-into-suggested-directory-transformation/ai`) + +Moves Java files into directories suggested by an LLM based on file content and project structure. + +**Filters (files are skipped if):** +- Not a Java file +- In test sources +- Contains package-local classes used by other files (would break compilation) + +**Configuration:** +```yaml +- id: "move-file-into-suggested-directory-transformation/ai" + config: + useMemory: true # Optional, default: null (no memory) + generateWhenNotInMemory: true # Optional, default: false + maxAgentIterations: 60 # Optional, default: 50. Max LLM iterations for directory search +``` + +--- + +### 5. Move File (Config-Specified) (`move-file-into-suggested-directory-transformation/config`) + +Moves Java files into a specific directory provided in the configuration. + +**Configuration:** +```yaml +- id: "move-file-into-suggested-directory-transformation/config" + config: + destination: "src/main/java/services/impl" # Required. Absolute or relative to project root. Can be new or existing. +``` + +**Note:** This transformation does NOT use memory (destination is explicit). + +--- + +### 6. Add Comment (`add-comment-transformation`) + +**Example transformation** that adds a comment at the beginning of a file. Not for production use. + +**Configuration:** +```yaml +- id: "add-comment-transformation" + config: + message: "This file was transformed" # Required. Comment text (without "//" prefix) +``` ## Template ToDo list - [x] Create a new [IntelliJ Platform Plugin Template][template] project. diff --git a/build.gradle.kts b/build.gradle.kts index 8c3ad7ba..e045dee9 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -235,5 +235,74 @@ intellijPlatformTesting { ) } } + + // Custom task for metamorphic text transformation (runs within IntelliJ platform). + // Reads a benchmark-record JSON file, applies rename/move sync block by block, + // and writes a same-schema JSON file. + register("transformMetamorphicTexts") { + task { + args(listOf("transform-texts")) + + val memoryFile = project.findProperty("memoryFile") as? String ?: "" + val inputFile = project.findProperty("inputFile") as? String ?: "" + val outputFile = project.findProperty("outputFile") as? String ?: "" + + jvmArgs( + "-Xmx4G", + "-Djava.awt.headless=true", + "--add-exports", "java.base/jdk.internal.vm=ALL-UNNAMED", + "-Dtransform.memoryFile=${memoryFile}", + "-Dtransform.inputFile=${inputFile}", + "-Dtransform.outputFile=${outputFile}", + ) + } + } + + // Custom task for reverting unwanted import-hunks via a Koog agent. + // Reads a hunks JSON file (repo_root + list of import_reorder / + // wildcard_import_removal hunks) and runs an agent that surgically + // reverts each hunk in the corresponding Java source file. + // Usage: ./gradlew agentFixHunks -Pinput=/abs/path/to/hunks.json + // [-PbatchSize=5] [-PmaxAgentIterations=60] + register("agentFixHunks") { + task { + args(listOf("agent-fix-hunks")) + + val inputFile = project.findProperty("input") as? String ?: "" + val outputFile = project.findProperty("output") as? String ?: "" + val batchSize = project.findProperty("batchSize") as? String ?: "" + val maxAgentIterations = project.findProperty("maxAgentIterations") as? String ?: "" + + jvmArgs( + "-Xmx4G", + "-Djava.awt.headless=true", + "--add-exports", "java.base/jdk.internal.vm=ALL-UNNAMED", + "-Dfix.inputFile=${inputFile}", + "-Dfix.outputFile=${outputFile}", + "-Dfix.batchSize=${batchSize}", + "-Dfix.maxAgentIterations=${maxAgentIterations}", + ) + } + } + + // Custom task for paraphrasing benchmark-record texts (semantic-preserving rewrite). + // Reads a benchmark-record JSON file, paraphrases each {title, body} block, + // and writes a same-schema JSON file. + register("rewriteProblemStatement") { + task { + args(listOf("rewrite-problem-statement")) + + val inputFile = project.findProperty("inputFile") as? String ?: "" + val outputFile = project.findProperty("outputFile") as? String ?: "" + + jvmArgs( + "-Xmx4G", + "-Djava.awt.headless=true", + "--add-exports", "java.base/jdk.internal.vm=ALL-UNNAMED", + "-Drewrite.inputFile=${inputFile}", + "-Drewrite.outputFile=${outputFile}", + ) + } + } } } diff --git a/codecocoon.example.yml b/codecocoon.example.yml index d295c195..7d20df47 100644 --- a/codecocoon.example.yml +++ b/codecocoon.example.yml @@ -4,12 +4,12 @@ projectRoot: "/path/to/project/root" # Optional: limit transformations to these files (relative to the root). Leave empty to target the entire project files: [] -# Optional: directory where memory files are stored (for deterministic rename transformations) -# If not specified, defaults to '.codecocoon-memory' in the same directory as this config file +# Optional: full path to the JSON memory file (for deterministic rename transformations) +# If not specified, defaults to '.codecocoon-memory.json' in the same directory as this config file # Can be: -# - Absolute path: "/absolute/path/to/memory" -# - Relative path: "my-memory-dir" (relative to this config file's directory) -memoryDir: ".codecocoon-memory" +# - Absolute path: "/absolute/path/to/memory.json" +# - Relative path: "my-memory.json" (relative to this config file's directory) +memoryFilepath: ".codecocoon-memory.json" # The transformation pipeline. Order matters. Each transformation has: # - id: unique identifier diff --git a/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/Memory.kt b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/Memory.kt index 7b0bf59f..48da4e99 100644 --- a/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/Memory.kt +++ b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/Memory.kt @@ -7,7 +7,7 @@ package com.github.pderakhshanfar.codecocoonplugin.memory * Use with `.use {}` blocks to guarantee data is saved: * * ```kotlin - * PersistentMemory(projectName, memoryDir).use { memory -> + * PersistentMemory(memoryFilepath).use { memory -> * memory.put("key", "value") * // memory.save() called automatically on close * } diff --git a/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/FixImportHunks.kt b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/FixImportHunks.kt new file mode 100644 index 00000000..fe209d23 --- /dev/null +++ b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/FixImportHunks.kt @@ -0,0 +1,601 @@ +package com.github.pderakhshanfar.codecocoonplugin.suggestions.impl + +import ai.koog.agents.core.agent.AIAgent +import ai.koog.agents.core.agent.config.AIAgentConfig +import ai.koog.agents.core.tools.ToolRegistry +import ai.koog.agents.ext.agent.reActStrategy +import ai.koog.agents.ext.tool.file.EditFileTool +import ai.koog.agents.ext.tool.file.ListDirectoryTool +import ai.koog.agents.ext.tool.file.ReadFileTool +import ai.koog.agents.features.eventHandler.feature.handleEvents +import ai.koog.prompt.dsl.Prompt +import ai.koog.prompt.llm.LLModel +import ai.koog.rag.base.files.JVMFileSystemProvider +import com.github.pderakhshanfar.codecocoonplugin.common.LLM +import com.intellij.openapi.diagnostic.Logger +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json +import java.io.File + +@Serializable +data class FixHunksInput( + @SerialName("repo_root") val repoRoot: String, + @SerialName("patch_label") val patchLabel: String = "", + val description: String = "", + val hunks: List, +) + +@Serializable +data class ImportLine( + val line: Int, + @SerialName("import") val importStmt: String, +) + +@Serializable +data class HunkSpec( + val id: String = "", + val file: String, + @SerialName("hunk_type") val hunkType: String, + val description: String = "", + @SerialName("hunk_header") val hunkHeader: String = "", + @SerialName("old_start_line") val oldStartLine: Int = 0, + @SerialName("old_line_count") val oldLineCount: Int = 0, + @SerialName("new_start_line") val newStartLine: Int = 0, + @SerialName("new_line_count") val newLineCount: Int = 0, + val action: String = "", + @SerialName("full_hunk_diff") val fullHunkDiff: String = "", + // Present only when hunk_type == "import_reorder": + @SerialName("original_import_block") val originalImportBlock: List? = null, + @SerialName("current_import_block") val currentImportBlock: List? = null, + // Present only when hunk_type == "wildcard_import_removal": + // Ordered list; an empty string "" denotes a blank separator line that was removed and must be restored. + @SerialName("removed_wildcards") val removedWildcards: List? = null, + // Present only when hunk_type == "import_cross_hunk_move": + // Role of THIS hunk in a multi-hunk rotation: "spurious_addition" | "missing_import" | "mixed". + @SerialName("cross_move_role") val crossMoveRole: String? = null, + // Specific import line(s) involved in THIS hunk of the rotation. + @SerialName("moved_imports") val movedImports: List? = null, +) + +@OptIn(ExperimentalSerializationApi::class) +private val agentJson = Json { + prettyPrint = true + encodeDefaults = false + explicitNulls = false +} + +/** + * Outcome of a single agent batch invocation. + * + * @property agentReport the agent's terminal message (advisory; not source of truth) + * @property verifications per-hunk filesystem verification results, in the same order as the input hunks + * @property error null on success; an exception describing the agent-level failure (e.g. iteration cap, network) + * The verifications list is still the per-hunk source of truth even when this is non-null. + */ +data class FixBatchResult( + val agentReport: String?, + val verifications: List, + val error: Throwable? = null, +) { + val appliedHunkIds: List get() = verifications.filter { it.applied }.map { it.hunkId }.filter { it.isNotEmpty() } + val isFullySuccessful: Boolean get() = error == null && verifications.all { it.applied } +} + +suspend fun runImportHunkFixerAgent( + token: String, + model: LLModel, + repoRoot: String, + batchDescription: String, + hunks: List, + maxAgentIterations: Int = 60, +): FixBatchResult { + val executor = LLM.createGrazieExecutor(token) + + val agent = AIAgent( + promptExecutor = executor, + agentConfig = AIAgentConfig( + prompt = Prompt.build("import-hunk-reverter") { + system(buildSystemPrompt(repoRoot)) + }, + model = model, + maxAgentIterations = maxAgentIterations, + ), + toolRegistry = ToolRegistry { + tool(ListDirectoryTool(JVMFileSystemProvider.ReadOnly)) + tool(ReadFileTool(JVMFileSystemProvider.ReadOnly)) + tool(EditFileTool(JVMFileSystemProvider.ReadWrite)) + }, + strategy = reActStrategy( + name = "import_hunk_reverter", + reasoningInterval = 1, + ), + ) { + handleEvents { + onToolCallStarting { ctx -> + logBoth("→ Calling `${ctx.tool.name}` with: ${ctx.toolArgs}") + } + onToolCallCompleted { ctx -> + logBoth("← `${ctx.tool.name}` returned: ${ctx.result.toString().take(MAX_TOOL_LOG_CHARS)}") + } + onToolCallFailed { ctx -> + logBoth("✖ `${ctx.tool.name}` FAILED: ${ctx.throwable.message}") + } + onToolValidationFailed { ctx -> + logBoth("✖ `${ctx.tool.name}` VALIDATION FAILED: ${ctx.error}") + } + } + } + + // Snapshot files touched by import_cross_hunk_move BEFORE the run, so + // we can detect "agent did nothing" cases that aren't visible from + // import-presence checks alone (the rotation preserves the import set). + val crossMoveFiles = hunks + .filter { it.hunkType == "import_cross_hunk_move" } + .map { it.file } + .distinct() + val crossMoveBefore: Map = crossMoveFiles.mapNotNull { rel -> + runCatching { rel to File(repoRoot, rel).readText() }.getOrNull() + }.toMap() + + val agentRun = runCatching { + agent.run(agentInput = buildUserPrompt(repoRoot, batchDescription, hunks)) + } + val agentReport = agentRun.getOrNull() + val agentError = agentRun.exceptionOrNull() + + // Verify regardless of whether the agent threw — partial successes count. + val verifications = hunks.map { verifyHunkApplied(repoRoot, it, crossMoveBefore) } + return FixBatchResult(agentReport = agentReport, verifications = verifications, error = agentError) +} + +/** Renders the unapplied hunks as a multi-line summary suitable for logging. */ +fun FixBatchResult.unappliedSummary(): String { + val unapplied = verifications.filterNot { it.applied } + if (unapplied.isEmpty()) return "all hunks verified applied" + return unapplied.joinToString("\n - ", prefix = " - ") { + val tag = if (it.hunkId.isNotEmpty()) "${it.hunkId} " else "" + "$tag${it.file} [${it.hunkType}]: ${it.reason}" + } +} + +private const val MAX_TOOL_LOG_CHARS = 500 + +private val agentLogger = Logger.getInstance("FixImportHunksAgent") + +private fun logBoth(message: String) { + agentLogger.info(message) + println(message) +} + +/** + * Result of filesystem-level verification of a single hunk after the agent run. + * The agent's own `applied: true` claim is advisory; this is the source of truth. + */ +data class VerifyResult( + val hunkId: String, + val file: String, + val hunkType: String, + val applied: Boolean, + val reason: String, +) + +/** + * Reads the file under `repoRoot/` and checks the post-condition + * implied by the hunk type. + * + * - `import_reorder`: filters the file's imports down to the ones listed in + * `originalImportBlock` (preserving file order) and checks they match the + * target order. If they still match `currentImportBlock`, the agent did + * nothing. + * - `wildcard_import_removal`: every line in `removedWildcards` must appear + * verbatim as a standalone import line in the file. + */ +private fun verifyHunkApplied( + repoRoot: String, + hunk: HunkSpec, + crossMoveBefore: Map = emptyMap(), +): VerifyResult { + val target = File(repoRoot, hunk.file) + if (!target.isFile) { + return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "file not found at $target") + } + val content = try { + target.readText() + } catch (e: Exception) { + return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "could not read file: ${e.message}") + } + val fileImports = content.lineSequence() + .map { it.trim() } + .filter { it.startsWith("import ") && it.endsWith(";") } + .toList() + + return when (hunk.hunkType) { + "import_reorder" -> verifyReorder(hunk, fileImports) + "wildcard_import_removal" -> verifyWildcard(hunk, fileImports) + "import_cross_hunk_move" -> verifyCrossHunkMove(hunk, fileImports, content, crossMoveBefore[hunk.file]) + else -> VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "unknown hunk_type: ${hunk.hunkType}") + } +} + +private fun verifyReorder(hunk: HunkSpec, fileImports: List): VerifyResult { + val target = hunk.originalImportBlock?.map { it.importStmt.trim() } + ?: return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "missing originalImportBlock") + val current = hunk.currentImportBlock?.map { it.importStmt.trim() } ?: emptyList() + + // Are all expected imports even present? + val missing = target.filter { it !in fileImports } + if (missing.isNotEmpty()) { + return VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "imports missing from file: ${missing.joinToString(", ")}", + ) + } + + // Filter file's imports down to just the ones this hunk cares about, + // preserving file order. That sequence must equal the target order. + val targetSet = target.toSet() + val filtered = fileImports.filter { it in targetSet } + + return if (filtered == target) { + VerifyResult(hunk.id, hunk.file, hunk.hunkType, true, "imports in target order") + } else if (current.isNotEmpty() && filtered == current) { + VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "imports still in pre-revert order; agent did not edit the file", + ) + } else { + VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "imports in unexpected order: $filtered (expected $target)", + ) + } +} + +private fun verifyWildcard(hunk: HunkSpec, fileImports: List): VerifyResult { + val expected = hunk.removedWildcards + ?.filter { it.isNotEmpty() } // skip blank-separator markers + ?.map { it.trim() } + ?: return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "missing removedWildcards") + val missing = expected.filter { it !in fileImports } + return if (missing.isEmpty()) { + VerifyResult(hunk.id, hunk.file, hunk.hunkType, true, "wildcards present: ${expected.joinToString(", ")}") + } else { + VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "wildcards still missing from file: ${missing.joinToString(", ")}", + ) + } +} + +/** + * Verifies an `import_cross_hunk_move` hunk. The rotation preserves the + * file's overall import set, so set-presence alone is insufficient — we + * also compare the post-run file content against a pre-run snapshot to + * detect "agent did nothing" cases. + * + * - "missing_import" / "mixed": every line in `moved_imports` MUST be + * present in the file's imports after the run. + * - "spurious_addition" / "mixed": the file content must DIFFER from the + * pre-run snapshot (something was edited). We can't easily check + * "imports no longer at the spurious location" without parsing line + * ranges, so we rely on the file-changed signal plus the missing-import + * counterpart hunks (each rotation has at least one missing_import or + * mixed entry as well). + */ +private fun verifyCrossHunkMove( + hunk: HunkSpec, + fileImports: List, + fileContent: String, + beforeContent: String?, +): VerifyResult { + val moved = hunk.movedImports?.map { it.trim() } + ?: return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "missing movedImports") + val role = hunk.crossMoveRole + ?: return VerifyResult(hunk.id, hunk.file, hunk.hunkType, false, "missing crossMoveRole") + + val fileChanged = beforeContent != null && fileContent != beforeContent + val snapshotMissing = beforeContent == null + + return when (role) { + "missing_import" -> { + val missing = moved.filter { it !in fileImports } + if (missing.isEmpty()) { + VerifyResult(hunk.id, hunk.file, hunk.hunkType, true, "moved imports restored: ${moved.joinToString(", ")}") + } else { + VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "moved imports still missing: ${missing.joinToString(", ")}", + ) + } + } + "spurious_addition" -> { + when { + snapshotMissing -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, true, + "no pre-run snapshot; spurious_addition cannot be falsified", + ) + fileChanged -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, true, + "file changed (rotation applied)", + ) + else -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "file unchanged since before agent run; rotation not applied", + ) + } + } + "mixed" -> { + val missing = moved.filter { it !in fileImports } + when { + missing.isNotEmpty() -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "moved imports missing from file: ${missing.joinToString(", ")}", + ) + snapshotMissing || fileChanged -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, true, + "moved imports present and file changed", + ) + else -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "file unchanged since before agent run; rotation not applied", + ) + } + } + else -> VerifyResult( + hunk.id, hunk.file, hunk.hunkType, false, + "unknown cross_move_role: $role", + ) + } +} + +private fun buildSystemPrompt(repoRoot: String): String = """ + You are a precise code-editing agent. Your ONLY job is to revert specific + unwanted modifications that an automated refactoring tool (IntelliJ's + import optimizer) introduced into Java source files. You must minimize + the diff: revert ONLY what each hunk describes, and nothing else. + + REPO ROOT: '$repoRoot' + + INPUT SCHEMA — fields you will receive per hunk: + Common to every hunk: + • file — repo-relative path to the Java source file. + • hunk_type — "import_reorder", "wildcard_import_removal", + or "import_cross_hunk_move". + • description — human-readable explanation of this specific hunk. + • hunk_header — the raw `@@ -a,b +c,d @@` header from the diff. + • old_start_line / old_line_count — window in the ORIGINAL file. + • new_start_line / new_line_count — window in the CURRENT file + (where to look NOW). + • action — one-sentence imperative instruction. + • full_hunk_diff — the complete unified-diff hunk (context). + For hunk_type == "import_reorder" ONLY: + • original_import_block — list of {line, import} entries showing the + imports in the order they had BEFORE the + optimizer ran (the desired post-revert + order). The "line" field is the absolute + 1-indexed line number in the original file. + • current_import_block — list of {line, import} entries showing the + SAME imports as they appear NOW in the + file. The set of `import` strings is + identical to original_import_block — only + the ORDER differs. + For hunk_type == "wildcard_import_removal" ONLY: + • removed_wildcards — ORDERED list of import lines (verbatim, + including `import` keyword and trailing `;`) + that the optimizer DELETED and that you must + ADD BACK in this order. An entry equal to the + empty string "" denotes a BLANK separator + line that was also removed — restore it as a + blank line in the same position. + For hunk_type == "import_cross_hunk_move" ONLY: + • cross_move_role — role of THIS hunk in a coordinated multi-hunk + rotation: + "spurious_addition" — this location only + ADDS imports that actually belong at + another location → REMOVE them here. + "missing_import" — this location only + REMOVES imports that belong here → + ADD them back here. + "mixed" — this location both adds and + removes; revert both sides. + • moved_imports — the specific import line(s) involved in THIS + hunk (verbatim). + + ALLOWED CHANGES (per hunk): + • hunk_type = "import_reorder": + Reorder the lines in the file's import block so that the imports + appear in the SAME ORDER as `original_import_block` (top → bottom). + The SET of import lines AFTER your edit MUST equal the SET BEFORE + (same imports, just reordered). Preserve any blank lines that + separate sub-groups in the current block exactly. + • hunk_type = "wildcard_import_removal": + Add each line in `removed_wildcards` back into the file's import + section. Do NOT remove or modify any other import. Place the + restored wildcard at the location implied by `full_hunk_diff` and + `new_start_line` (typically the same relative position it had in + the original — e.g. as a separate static-imports group at the end + of the import block, with a preceding blank line if the diff + shows one). If the diff is ambiguous, prefer placing it at the + end of the import block. + • hunk_type = "import_cross_hunk_move": + The optimizer ROTATED imports between two or more locations in + the SAME file (e.g. swapped imports that were originally near + line 3 with imports near line 120 of the same file). The input + contains MULTIPLE entries with this hunk_type for one file — + they are coordinated and must be reverted TOGETHER as a single + rotation, not independently: + - cross_move_role == "spurious_addition" → DELETE the lines + in `moved_imports` from this location. + - cross_move_role == "missing_import" → INSERT the lines in + `moved_imports` back at this location. + - cross_move_role == "mixed" → both delete and insert at this + location, as `action` describes. + Net effect: the SET of imports in the file is unchanged; only + their positions are restored. Process all entries for the same + file in ONE pass (one read_file, then plan all + insertions/deletions, then issue your edit_file call(s)). Do + NOT insert before deleting (or vice versa) in a way that ends + up duplicating the same import line. + + CROSS-HUNK COORDINATION (import_cross_hunk_move): + When you see N ≥ 2 import_cross_hunk_move entries with the same + `file`, treat them as a single coordinated revert. After your + edits: + • Every import that was originally in the file must still be + there exactly once (no duplicates, no losses). + • Imports listed in spurious_addition entries must NOT remain at + their spurious location. + • Imports listed in missing_import entries MUST appear at their + missing location. + + DO NOT TOUCH OTHER CHANGES IN `full_hunk_diff`: + The `full_hunk_diff` may contain unrelated `+`/`-` lines from other + transformations (e.g. a class rename like `JSON` → `JsonMapper`). + Those are intentional and must STAY. Restrict your edit strictly to + the field that describes your hunk type: + - For import_reorder: only the imports in original_import_block / + current_import_block. + - For wildcard_import_removal: only the lines in removed_wildcards. + - For import_cross_hunk_move: only the lines in moved_imports + (across all hunks for the same file). + + FORBIDDEN CHANGES — MUST NEVER HAPPEN: + • Do not modify code outside the import block. + • Do not add, delete, rename, or reformat classes, methods, fields, + comments, javadoc, or annotations. + • Do not change whitespace outside the lines you reorder/insert. + • Do not run any git command. Do not stage, commit, push, or branch. + • Do not invoke any tool other than list_directory, read_file, edit_file. + • Do not edit files that are not listed in the user prompt. + + PROCESS (apply per hunk): + 1. Resolve the file: it is at "$repoRoot/". If read_file fails + there, use list_directory under the repo root to locate it. + 2. Read the file. Locate the import block near `new_start_line` + (lines are 1-indexed; the block may have shifted by a few lines). + 3. Compute the minimal edit: + - For import_reorder: + • Confirm that the set of imports currently present matches + `current_import_block`. (If not, the file has drifted — + still attempt the reorder using whatever imports are there + that also appear in `original_import_block`.) + • Build the replacement block by listing the imports in the + order of `original_import_block`. Preserve any blank + separator lines you observed in the current block at the + SAME positions (e.g., a blank line before the static + imports section). + - For wildcard_import_removal: + • Insert each wildcard from `removed_wildcards` into the + current import block at the right position. Empty-string + entries denote a blank separator line — restore them as + blank lines. Do NOT remove any other import line. + - For import_cross_hunk_move: + • Group all entries with this hunk_type for the SAME file + and revert them as ONE rotation. Read the file ONCE, + plan all deletions (spurious_addition entries) and + insertions (missing_import entries; both for mixed) + together, then issue your edit_file call(s). The + post-revert file must contain every original import + exactly once — no duplicates, no losses. + 4. Use edit_file with `original` = the exact current import block + snippet (multiple consecutive lines, copied verbatim from the + file you just read) and `replacement` = the corrected block. Keep + a one- or two-line anchor of unchanged context (the package line + above and the blank line below the import block) byte-for-byte + identical inside both `original` and `replacement` so the edit is + unambiguous and proves you changed nothing else. + 5. After editing, read the file back and verify that ONLY import-block + lines differ from your mental model of the pre-edit content. If + anything else changed, fix it with another edit_file call. + + AUTONOMY (CRITICAL): + • You operate fully autonomously. There is NO human in the loop. The + caller cannot answer questions, approve plans, or reply "yes". + • NEVER ask "Shall I proceed?", "Should I continue?", "Ready to apply?", + or any other confirmation question. If you do, the run is marked + failed and the changes are discarded. + • Do NOT emit a plan/thoughts message and stop. Plans without applied + edits are worthless here. Execute every required edit_file call in + this same run, before producing your terminal message. + • The ONLY acceptable terminal message is the JSON report below, and + you may emit it ONLY AFTER every needed edit_file call has run. + + TRUTHFULNESS (CRITICAL): + • Failure to call `edit_file` for a hunk is NOT a no-op success — it + is a FAILURE of the run. Skipping a hunk because "the file already + looks fine" is forbidden; trust the input, not your judgment. + • Mark a hunk `"applied": true` ONLY if you actually called + `edit_file` for that hunk AND the tool's response said the patch + was applied successfully ("Successfully edited file"). If the tool + responded with "patch application failed", the hunk is NOT applied + — re-read the file with `read_file` to copy the import block + verbatim, then call `edit_file` again with that fresh `original`. + • NEVER report `"applied": true` for a hunk you did not edit. NEVER + report success based on what you intended to do — only on tool + responses you actually received in this run. + • A separate filesystem verifier runs after you finish and will + compare every file against the expected post-revert state. False + `"applied": true` claims will be caught and the run will be marked + failed regardless of what your report says. + + OUTPUT (final assistant message — emit ONLY after all edits are done): + A short JSON report: + ```json + { + "results": [ + { "file": "", "hunk_type": "...", "applied": true, "reason": "..." } + ] + } + ``` +""".trimIndent() + +private fun buildUserPrompt( + repoRoot: String, + batchDescription: String, + hunks: List, +): String { + val hunksJson = agentJson.encodeToString( + kotlinx.serialization.builtins.ListSerializer(HunkSpec.serializer()), + hunks, + ) + return """ + BATCH DESCRIPTION: $batchDescription + REPO ROOT: $repoRoot + + HUNKS TO REVERT (apply each one, in order): + ```json + $hunksJson + ``` + + For each hunk: + • Resolve the file as $repoRoot/. + • Read it, find the import block near `new_start_line`, and apply + the minimal edit by calling edit_file NOW, in this same run. + • For "import_reorder": reorder current imports to match + `original_import_block` (set of imports unchanged). + • For "wildcard_import_removal": insert every line from + `removed_wildcards` (in order; "" entries = blank separator + lines) into the import block; do not remove or modify any + other import. + • For "import_cross_hunk_move": group ALL entries for the SAME + file and revert them as one rotation. Per-entry: if + cross_move_role == "spurious_addition", DELETE the lines in + `moved_imports` from this location; if "missing_import", + INSERT them back at this location; if "mixed", do both as the + `action` describes. The file's overall import set must remain + unchanged — no duplicates, no losses. + • Use list_directory only if read_file cannot find the file. + • Ignore unrelated `+`/`-` lines in `full_hunk_diff` — they are + from other transformations and must stay. + + Do NOT respond with a plan and stop. Do NOT ask for confirmation + ("Shall I proceed?", "Ready to apply?", etc.) — there is no human + to answer, and the run will fail. + + Only AFTER every required edit_file call has been executed for + every hunk in this batch, emit the JSON report described in the + system prompt and end the turn. + """.trimIndent() +} diff --git a/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/SuggestNewDirectory.kt b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/SuggestNewDirectory.kt index 66026091..6d4f44e5 100644 --- a/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/SuggestNewDirectory.kt +++ b/core/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/suggestions/impl/SuggestNewDirectory.kt @@ -153,7 +153,7 @@ private fun buildSystemPrompt(projectRoot: String, existingOnly: Boolean): Strin CRITICAL RULES: - [!] OUTPUT ABSOLUTE PATHS BASED ON THE PROJECT ROOT! - - [!] DO NOT OUTPUT THE SAME DIRECTORY WHERE THE GIVEN FILE ALREADY RESIDES! + - [!] DO NOT SUGGEST THE `CURRENT DIRECTORY` LISTED IN THE USER PROMPT — that produces a no-op move and the suggestion will be discarded. - Never suggest locations that would cause naming conflicts - Prefer existing directories unless the class clearly belongs to a new feature module - Consider import dependencies visible in the source file @@ -177,19 +177,23 @@ private fun buildSystemPrompt(projectRoot: String, existingOnly: Boolean): Strin return basePrompt + "\n\n" + directoryConstraint } -private fun buildUserPrompt(filepath: String, content: String): String = """ - Analyze this Java file and suggest appropriate directory locations. - - FILE PATH: $filepath - - FILE CONTENT (may be truncated): - ```java - $content - ``` - - First, use the `list_directory` tool to understand the current project structure. - Then analyze the class and provide directory suggestions. -""".trimIndent() +private fun buildUserPrompt(filepath: String, content: String): String { + val currentDirectory = File(filepath).parent ?: filepath + return """ + Analyze this Java file and suggest appropriate directory locations. + + FILE PATH: $filepath + CURRENT DIRECTORY: $currentDirectory ← DO NOT suggest this exact directory; doing so is a no-op move. + + FILE CONTENT (may be truncated): + ```java + $content + ``` + + First, use the `list_directory` tool to understand the current project structure. + Then analyze the class and provide directory suggestions different from `CURRENT DIRECTORY`. + """.trimIndent() +} // TODO: parse the requested JSON structure into data class, not list of strings private fun parseDirectorySuggestions(llmOutput: String): Result> { diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/BenchmarkInstanceIO.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/BenchmarkInstanceIO.kt new file mode 100644 index 00000000..8364afc6 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/BenchmarkInstanceIO.kt @@ -0,0 +1,102 @@ +package com.github.pderakhshanfar.codecocoonplugin.appstarter + +import com.github.pderakhshanfar.codecocoonplugin.services.TextBlock +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.JsonPrimitive +import kotlinx.serialization.json.buildJsonArray +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.contentOrNull +import kotlinx.serialization.json.jsonArray +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive + +/** + * Helpers shared by [TransformTextsStarter] and [RewriteProblemStatementStarter] for + * reading/writing benchmark-record JSON files of the schema: + * + * ```json + * { + * "title": "...", + * "body": "...", + * "resolved_issues": [ + * { "number": 1, "title": "...", "body": "..." } + * ] + * } + * ``` + * + * The starters mutate a [JsonObject] in place rather than round-tripping through a + * typed data class so any extra keys present in the benchmark record (or inside each + * resolved issue) are preserved verbatim on output. + * + * Processing happens in BLOCKS — one transform call per `{title, body}` pair — so a + * single LLM round-trip can keep title and body internally consistent. The main record + * is one block; each resolved issue is another. + */ +internal object BenchmarkInstanceIO { + + val json: Json = Json { + prettyPrint = true + ignoreUnknownKeys = true + encodeDefaults = true + } + + /** + * Walks [obj], applying [transform] once to the main `{title, body}` block and once + * to each `resolved_issues[i].{title, body}` block. When [transform] returns null + * for a block, the original block is kept (so a single failure does not sink the + * record). + * + * Keys other than `title`, `body`, and `resolved_issues` (and, inside each issue, + * keys other than `title` / `body`) pass through verbatim. Output title/body are + * only emitted when the corresponding key was present in the input. + */ + suspend fun transformInstance( + obj: JsonObject, + transform: suspend (TextBlock) -> TextBlock?, + ): JsonObject { + val hasTitle = obj.containsKey("title") + val hasBody = obj.containsKey("body") + val mainBlock = TextBlock( + title = obj["title"]?.jsonPrimitive?.contentOrNull ?: "", + body = obj["body"]?.jsonPrimitive?.contentOrNull ?: "", + ) + val mainResult = transform(mainBlock) ?: mainBlock + + val newResolvedIssues = obj["resolved_issues"]?.jsonArray?.let { arr -> + buildJsonArray { + for (element in arr) { + val issue = element.jsonObject + val issueHasTitle = issue.containsKey("title") + val issueHasBody = issue.containsKey("body") + val issueBlock = TextBlock( + title = issue["title"]?.jsonPrimitive?.contentOrNull ?: "", + body = issue["body"]?.jsonPrimitive?.contentOrNull ?: "", + ) + val issueResult = transform(issueBlock) ?: issueBlock + + add(buildJsonObject { + for ((k, v) in issue) { + when (k) { + "title" -> if (issueHasTitle) put(k, JsonPrimitive(issueResult.title)) + "body" -> if (issueHasBody) put(k, JsonPrimitive(issueResult.body)) + else -> put(k, v) + } + } + }) + } + } + } + + return buildJsonObject { + for ((k, v) in obj) { + when (k) { + "title" -> if (hasTitle) put(k, JsonPrimitive(mainResult.title)) + "body" -> if (hasBody) put(k, JsonPrimitive(mainResult.body)) + "resolved_issues" -> put(k, newResolvedIssues ?: v) + else -> put(k, v) + } + } + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/FixHunksStarter.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/FixHunksStarter.kt new file mode 100644 index 00000000..c04c10d1 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/FixHunksStarter.kt @@ -0,0 +1,313 @@ +package com.github.pderakhshanfar.codecocoonplugin.appstarter + +import ai.koog.prompt.executor.clients.openai.OpenAIModels +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.github.pderakhshanfar.codecocoonplugin.suggestions.impl.FixHunksInput +import com.github.pderakhshanfar.codecocoonplugin.suggestions.impl.runImportHunkFixerAgent +import com.github.pderakhshanfar.codecocoonplugin.suggestions.impl.unappliedSummary +import com.intellij.openapi.application.ApplicationStarter +import com.intellij.openapi.diagnostic.thisLogger +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.ExperimentalSerializationApi +import kotlinx.serialization.json.* +import java.io.File +import kotlin.system.exitProcess + +/** + * Application starter for reverting unwanted import-hunks via a Koog agent. + * + * Reads a JSON file describing import_reorder / wildcard_import_removal hunks + * inside a Java repo and runs an agent that surgically reverts each hunk in + * the corresponding source file. Hunks are processed in batches to keep each + * agent invocation focused. + * + * Entry point when the IDE is launched with the 'agent-fix-hunks' command. + */ +class FixHunksStarter : ApplicationStarter { + override val requiredModality: Int = ApplicationStarter.NOT_IN_EDT + private val logger = thisLogger().withStdout() + + @OptIn(ExperimentalSerializationApi::class) + private val json = Json { + ignoreUnknownKeys = true + explicitNulls = false + } + + override fun main(args: List) { + val inputFile = System.getProperty("fix.inputFile") ?: "" + val outputFile = System.getProperty("fix.outputFile") ?: "" + val batchSize = System.getProperty("fix.batchSize")?.toIntOrNull()?.takeIf { it > 0 } ?: DEFAULT_BATCH_SIZE + val maxAgentIterations = System.getProperty("fix.maxAgentIterations")?.toIntOrNull()?.takeIf { it > 0 } + ?: DEFAULT_MAX_AGENT_ITERATIONS + + if (inputFile.isEmpty()) { + logger.error("[FixHunks] Missing required parameter: -Pinput=") + exitProcess(1) + } + + val token = System.getenv("GRAZIE_TOKEN") + if (token == null) { + logger.error("[FixHunks] GRAZIE_TOKEN environment variable not set") + exitProcess(1) + } + + val inputJsonText = try { + File(inputFile).readText() + } catch (e: Exception) { + logger.error("[FixHunks] Cannot read input file '$inputFile': ${e.message}", e) + exitProcess(1) + } + + val input = try { + json.decodeFromString(FixHunksInput.serializer(), inputJsonText) + } catch (e: Exception) { + logger.error("[FixHunks] Failed to parse input JSON '$inputFile': ${e.message}", e) + exitProcess(1) + } + + val repoRoot = File(input.repoRoot).absoluteFile + if (!repoRoot.isDirectory) { + logger.error("[FixHunks] repo_root does not exist or is not a directory: ${input.repoRoot}") + exitProcess(1) + } + + if (input.hunks.isEmpty()) { + logger.warn("[FixHunks] No hunks to revert; exiting successfully") + exitProcess(0) + } + + val batches = input.hunks.chunked(batchSize) + val typeCounts = input.hunks.groupingBy { it.hunkType }.eachCount() + logger.info("[FixHunks] Reverting ${input.hunks.size} hunks across ${batches.size} batch(es) of <= $batchSize") + logger.info("[FixHunks] Repo root: ${repoRoot.absolutePath}") + logger.info("[FixHunks] Patch label: ${input.patchLabel.ifEmpty { "" }}") + logger.info("[FixHunks] Hunk types: $typeCounts") + logger.info("[FixHunks] Description: ${input.description}") + + var failedBatches = 0 + val allFixedIds = mutableListOf() + val allUnfixed = mutableListOf() + val batchSummaries = mutableListOf() + val startedAtMs = System.currentTimeMillis() + + runBlocking { + for ((index, batch) in batches.withIndex()) { + val batchNum = index + 1 + val fileCount = batch.map { it.file }.distinct().size + val batchStartedMs = System.currentTimeMillis() + logger.info("[FixHunks] Batch $batchNum/${batches.size}: ${batch.size} hunks across $fileCount file(s)") + val result = try { + runImportHunkFixerAgent( + token = token, + model = OpenAIModels.Chat.GPT5Mini, + repoRoot = repoRoot.absolutePath, + batchDescription = input.description, + hunks = batch, + maxAgentIterations = maxAgentIterations, + ) + } catch (e: Exception) { + failedBatches++ + logger.error("[FixHunks] Batch $batchNum/${batches.size} threw: ${e.message}", e) + batch.forEach { allUnfixed += UnfixedRecord(it.id, it.file, it.hunkType, "agent threw: ${e.message}") } + batchSummaries += BatchSummary( + batchNum = batchNum, + hunkIds = batch.map { it.id }, + applied = emptyList(), + unapplied = batch.map { UnfixedRecord(it.id, it.file, it.hunkType, "agent threw: ${e.message}") }, + agentReport = null, + outcome = "agent_threw", + errorMessage = e.message, + elapsedMs = System.currentTimeMillis() - batchStartedMs, + ) + null + } + + if (result != null) { + val unappliedRecords = result.verifications + .filterNot { it.applied } + .map { UnfixedRecord(it.hunkId, it.file, it.hunkType, it.reason) } + allFixedIds += result.appliedHunkIds + allUnfixed += unappliedRecords + val appliedCount = result.verifications.count { it.applied } + val outcome = when { + result.isFullySuccessful -> "success" + result.error != null -> "agent_failed" + else -> "verification_failed" + } + + when (outcome) { + "success" -> logger.info("[FixHunks] Batch $batchNum/${batches.size} done ($appliedCount/${batch.size} applied)") + "agent_failed" -> { + failedBatches++ + val err = result.error!! + logger.error( + "[FixHunks] Batch $batchNum/${batches.size} agent FAILED " + + "($appliedCount/${batch.size} hunks applied before failure): ${err.message}", + err, + ) + } + else -> { + // Agent ran cleanly to completion but the verifier says some + // hunks didn't take. NOT a process-level failure — we still + // want exit code 0 so the caller's retry pipeline can read + // the output JSON and decide what to redo. Only true agent + // failures (throws / iteration caps) flip the exit code. + logger.warn( + "[FixHunks] Batch $batchNum/${batches.size} partial " + + "($appliedCount/${batch.size} hunks applied). Unapplied:\n${result.unappliedSummary()}", + ) + } + } + result.agentReport?.let { logger.info("[FixHunks] Agent report:\n$it") } + + batchSummaries += BatchSummary( + batchNum = batchNum, + hunkIds = batch.map { it.id }, + applied = result.appliedHunkIds, + unapplied = unappliedRecords, + agentReport = result.agentReport, + outcome = outcome, + errorMessage = result.error?.message, + elapsedMs = System.currentTimeMillis() - batchStartedMs, + ) + } + } + } + + val elapsedMs = System.currentTimeMillis() - startedAtMs + + if (outputFile.isNotEmpty()) { + writeOutputJson( + path = outputFile, + fixedIds = allFixedIds, + input = input, + batchSummaries = batchSummaries, + allUnfixed = allUnfixed, + failedBatches = failedBatches, + totalBatches = batches.size, + batchSize = batchSize, + elapsedMs = elapsedMs, + ) + } + + logger.info("[FixHunks] Total fixed hunks across all batches: ${allFixedIds.size}/${input.hunks.size}") + if (allUnfixed.isNotEmpty()) { + logger.warn("[FixHunks] Unfixed hunks (${allUnfixed.size}):") + allUnfixed.forEach { (id, file, _, reason) -> + val tag = if (id.isNotEmpty()) "$id " else "" + logger.warn("[FixHunks] - $tag$file: $reason") + } + } + + if (failedBatches > 0) { + logger.error( + "[FixHunks] Completed with $failedBatches batch(es) where the agent itself " + + "failed (out of ${batches.size}). Exit code 1.", + ) + exitProcess(1) + } + if (allUnfixed.isNotEmpty()) { + logger.warn( + "[FixHunks] Agent finished cleanly. ${allFixedIds.size}/${input.hunks.size} hunks " + + "verified applied; ${allUnfixed.size} not applied. Exit code 0 — caller can retry " + + "the unfixed hunks (see output JSON for the list).", + ) + } else { + logger.info("[FixHunks] SUCCESS: all ${input.hunks.size} hunks applied across ${batches.size} batch(es)") + } + exitProcess(0) + } + + private data class UnfixedRecord( + val id: String, + val file: String, + val hunkType: String, + val reason: String, + ) + + private data class BatchSummary( + val batchNum: Int, + val hunkIds: List, + val applied: List, + val unapplied: List, + val agentReport: String?, + val outcome: String, // "success" | "verification_failed" | "agent_failed" | "agent_threw" + val errorMessage: String?, + val elapsedMs: Long, + ) + + private fun writeOutputJson( + path: String, + fixedIds: List, + input: FixHunksInput, + batchSummaries: List, + allUnfixed: List, + failedBatches: Int, + totalBatches: Int, + batchSize: Int, + elapsedMs: Long, + ) { + try { + val deduped = fixedIds.distinct() + val obj: JsonObject = buildJsonObject { + put("fixed", buildJsonArray { deduped.forEach { add(JsonPrimitive(it)) } }) + put("unfixed", buildJsonArray { + allUnfixed.forEach { rec -> + add(buildJsonObject { + put("id", JsonPrimitive(rec.id)) + put("file", JsonPrimitive(rec.file)) + put("hunk_type", JsonPrimitive(rec.hunkType)) + put("reason", JsonPrimitive(rec.reason)) + }) + } + }) + put("summary", buildJsonObject { + put("total_hunks", JsonPrimitive(input.hunks.size)) + put("fixed_count", JsonPrimitive(deduped.size)) + put("unfixed_count", JsonPrimitive(allUnfixed.size)) + put("total_batches", JsonPrimitive(totalBatches)) + put("failed_batches", JsonPrimitive(failedBatches)) + put("batch_size", JsonPrimitive(batchSize)) + put("elapsed_ms", JsonPrimitive(elapsedMs)) + put("repo_root", JsonPrimitive(input.repoRoot)) + put("patch_label", JsonPrimitive(input.patchLabel)) + }) + put("agent", buildJsonObject { + put("model", JsonPrimitive(OpenAIModels.Chat.GPT5Mini.id)) + put("batches", buildJsonArray { + batchSummaries.forEach { b -> + add(buildJsonObject { + put("batch_num", JsonPrimitive(b.batchNum)) + put("outcome", JsonPrimitive(b.outcome)) + put("elapsed_ms", JsonPrimitive(b.elapsedMs)) + put("hunk_ids", buildJsonArray { b.hunkIds.forEach { add(JsonPrimitive(it)) } }) + put("applied", buildJsonArray { b.applied.forEach { add(JsonPrimitive(it)) } }) + put("unapplied", buildJsonArray { + b.unapplied.forEach { rec -> + add(buildJsonObject { + put("id", JsonPrimitive(rec.id)) + put("file", JsonPrimitive(rec.file)) + put("hunk_type", JsonPrimitive(rec.hunkType)) + put("reason", JsonPrimitive(rec.reason)) + }) + } + }) + b.errorMessage?.let { put("error", JsonPrimitive(it)) } + b.agentReport?.let { put("report", JsonPrimitive(it)) } + }) + } + }) + }) + } + File(path).also { it.parentFile?.mkdirs() }.writeText(json.encodeToString(JsonObject.serializer(), obj)) + logger.info("[FixHunks] Wrote ${deduped.size} fixed hunk id(s) + agent reports to: $path") + } catch (e: Exception) { + logger.error("[FixHunks] Failed to write output file '$path': ${e.message}", e) + } + } + + companion object { + private const val DEFAULT_BATCH_SIZE = 8 + private const val DEFAULT_MAX_AGENT_ITERATIONS = 70 + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/HeadlessModeStarter.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/HeadlessModeStarter.kt index ea5de4e8..fc907c23 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/HeadlessModeStarter.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/HeadlessModeStarter.kt @@ -1,11 +1,12 @@ package com.github.pderakhshanfar.codecocoonplugin.appstarter import com.github.pderakhshanfar.codecocoonplugin.components.transformations.AddCommentTransformation -import com.github.pderakhshanfar.codecocoonplugin.components.transformations.MoveFileIntoSuggestedDirectoryTransformation +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural.MoveFileIntoSuggestedDirectoryTransformation import com.github.pderakhshanfar.codecocoonplugin.components.transformations.TransformationRegistry import com.github.pderakhshanfar.codecocoonplugin.components.transformations.renaming.RenameClassTransformation import com.github.pderakhshanfar.codecocoonplugin.components.transformations.renaming.RenameMethodTransformation import com.github.pderakhshanfar.codecocoonplugin.components.transformations.renaming.RenameVariableTransformation +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural.ReorderClassMethodsTransformation import com.github.pderakhshanfar.codecocoonplugin.config.CodeCocoonConfig import com.github.pderakhshanfar.codecocoonplugin.config.ConfigLoader import com.github.pderakhshanfar.codecocoonplugin.intellij.JvmProjectConfigurator @@ -18,11 +19,13 @@ import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.ApplicationStarter import com.intellij.openapi.components.service import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.project.Project import com.intellij.openapi.project.ProjectManager import com.intellij.openapi.util.Disposer import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.openapi.vfs.VfsUtil +import com.intellij.psi.PsiDocumentManager import com.intellij.psi.codeStyle.JavaCodeStyleSettings import kotlinx.coroutines.runBlocking import java.io.File @@ -87,6 +90,15 @@ class HeadlessModeStarter : ApplicationStarter { // close project and exit logger.info("[CodeCocoon Starter] Execution completed") + // Final flush before close: commit any pending PSI changes and write + // documents to disk explicitly, so close-time hooks don't get a chance + // to introduce non-deterministic edits (e.g. import re-ordering whose + // outcome depends on accumulated unflushed state across rename calls). + ApplicationManager.getApplication().invokeAndWait { + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() + } + ApplicationManager.getApplication().invokeAndWait { ProjectManager.getInstance().closeAndDispose(project) logger.info("[CodeCocoon Starter] Project is closed successfully") @@ -108,11 +120,20 @@ class HeadlessModeStarter : ApplicationStarter { } /** - * Configures code style settings to prevent wildcard imports. + * Configures code style settings to prevent ALL import optimizations. + * + * This method configures multiple import-related settings to minimize unwanted + * import modifications during refactoring operations when `commitAllDocuments()` is called. + * + * **Settings configured:** + * 1. Prevents wildcard imports (e.g., `import com.example.*`) + * 2. Forces single class imports + * 3. Disables auto-insertion of inner class imports + * 4. Clears packages that should use wildcards * - * This sets the import thresholds to very high values (9999) so that imports - * are never collapsed into wildcards (e.g., `import com.example.*`) during - * refactoring operations when `commitAllDocuments()` is called. + * **Limitations:** + * - Cannot prevent removal of unused imports (hardcoded in IntelliJ's optimize imports) + * - Cannot prevent removal of redundant same-package imports * * **Important:** Call this method once when the project is opened/initialized, * before running any transformations. @@ -123,10 +144,22 @@ class HeadlessModeStarter : ApplicationStarter { val settings = CodeStyle.getSettings(project) val javaSettings = settings.getCustomSettings(JavaCodeStyleSettings::class.java) - // Set thresholds to 9999 to effectively disable wildcard imports + // 1. Set thresholds to 9999 to effectively disable wildcard imports // This prevents IntelliJ from collapsing multiple imports into import com.example.* javaSettings.classCountToUseImportOnDemand = 9999 javaSettings.namesCountToUseImportOnDemand = 9999 + + // 2. Force single class imports (prevent wildcards) + javaSettings.isUseSingleClassImports = true + + // 3. Don't auto-insert inner class imports + javaSettings.isInsertInnerClassImports = false + + logger.info("[CodeCocoon Starter] Configured code style settings:") + logger.info(" - CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND: ${javaSettings.classCountToUseImportOnDemand}") + logger.info(" - NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND: ${javaSettings.namesCountToUseImportOnDemand}") + logger.info(" - USE_SINGLE_CLASS_IMPORTS: ${javaSettings.isUseSingleClassImports}") + logger.info(" - INSERT_INNER_CLASS_IMPORTS: ${javaSettings.isInsertInnerClassImports}") } private fun cleanIdeaFolder(projectPath: String) { @@ -172,10 +205,13 @@ class HeadlessModeStarter : ApplicationStarter { * unique ID in the registry, allowing it to be referenced dynamically during execution. */ private fun registerBuiltInTransformations() { + // renaming TransformationRegistry.register(AddCommentTransformation.ID) { config -> AddCommentTransformation(config) } TransformationRegistry.register(RenameMethodTransformation.ID) { config -> RenameMethodTransformation(config) } TransformationRegistry.register(RenameClassTransformation.ID) { config -> RenameClassTransformation(config) } TransformationRegistry.register(RenameVariableTransformation.ID) { config -> RenameVariableTransformation(config) } + + // structural // move file transformation: // 1) with AI suggested directory TransformationRegistry.register(MoveFileIntoSuggestedDirectoryTransformation.Companion.AI.ID) { config -> @@ -185,6 +221,10 @@ class HeadlessModeStarter : ApplicationStarter { TransformationRegistry.register(MoveFileIntoSuggestedDirectoryTransformation.Companion.Config.ID) { config -> MoveFileIntoSuggestedDirectoryTransformation.withConfig(config) } + // reorder class methods transformation + TransformationRegistry.register(ReorderClassMethodsTransformation.ID) { config -> + ReorderClassMethodsTransformation(config) + } } /** diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/RewriteProblemStatementStarter.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/RewriteProblemStatementStarter.kt new file mode 100644 index 00000000..eb7de526 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/RewriteProblemStatementStarter.kt @@ -0,0 +1,77 @@ +package com.github.pderakhshanfar.codecocoonplugin.appstarter + +import ai.koog.prompt.executor.clients.openai.OpenAIModels +import com.github.pderakhshanfar.codecocoonplugin.common.LLM +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.github.pderakhshanfar.codecocoonplugin.services.ParaphraseTextTransformer +import com.intellij.openapi.application.ApplicationStarter +import com.intellij.openapi.diagnostic.thisLogger +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.jsonObject +import java.io.File +import kotlin.system.exitProcess + +/** + * Application starter for paraphrasing the textual fields of a benchmark record in + * headless mode. Reads a benchmark-record JSON file, paraphrases the main + * `{title, body}` block and each `resolved_issues[i].{title, body}` block (one LLM + * call per block so title and body stay coherent), and writes a same-schema JSON file. + * + * Entry point when the IDE is launched with the 'rewrite-problem-statement' command. + */ +class RewriteProblemStatementStarter : ApplicationStarter { + override val requiredModality: Int = ApplicationStarter.NOT_IN_EDT + private val logger = thisLogger().withStdout() + + override fun main(args: List) { + val inputFile = System.getProperty("rewrite.inputFile") ?: "" + val outputFile = System.getProperty("rewrite.outputFile") ?: "" + + if (inputFile.isEmpty() || outputFile.isEmpty()) { + logger.error("[RewriteProblemStatement] Missing required parameters") + logger.error("[RewriteProblemStatement] Required system properties: rewrite.inputFile, rewrite.outputFile") + exitProcess(1) + } + + val token = System.getenv("GRAZIE_TOKEN") + if (token == null) { + logger.error("[RewriteProblemStatement] GRAZIE_TOKEN environment variable not set") + exitProcess(1) + } + + runBlocking { + try { + logger.info("[RewriteProblemStatement] Starting paraphrase") + logger.info("[RewriteProblemStatement] Input file: $inputFile") + + val llm = LLM.fromGrazie( + model = OpenAIModels.Chat.GPT5Mini, + token = token, + ) + val transformer = ParaphraseTextTransformer(llm) + + val inputJson = BenchmarkInstanceIO.json + .parseToJsonElement(File(inputFile).readText()) + .jsonObject + + val outputJson = BenchmarkInstanceIO.transformInstance(inputJson) { block -> + try { + transformer.rewriteBlock(block) + } catch (e: Exception) { + logger.error("[RewriteProblemStatement] Block-level paraphrase failed; keeping original block: ${e.message}", e) + null + } + } + + File(outputFile).writeText(BenchmarkInstanceIO.json.encodeToString(JsonObject.serializer(), outputJson)) + logger.info("[RewriteProblemStatement] SUCCESS: Paraphrased record written to: $outputFile") + exitProcess(0) + } catch (e: Exception) { + logger.error("[RewriteProblemStatement] ERROR: ${e.message}", e) + e.printStackTrace(System.err) + exitProcess(1) + } + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/TransformTextsStarter.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/TransformTextsStarter.kt new file mode 100644 index 00000000..6067e4cc --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/appstarter/TransformTextsStarter.kt @@ -0,0 +1,90 @@ +package com.github.pderakhshanfar.codecocoonplugin.appstarter + +import ai.koog.prompt.executor.clients.openai.OpenAIModels +import com.github.pderakhshanfar.codecocoonplugin.common.LLM +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.github.pderakhshanfar.codecocoonplugin.services.MetamorphicTextTransformer +import com.intellij.openapi.application.ApplicationStarter +import com.intellij.openapi.diagnostic.thisLogger +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.json.JsonObject +import kotlinx.serialization.json.jsonObject +import java.io.File +import kotlin.system.exitProcess + +/** + * Application starter for running text transformation in headless mode. + * Reads a benchmark-record JSON file, applies rename/move sync to the main + * `{title, body}` block and each `resolved_issues[i].{title, body}` block (one LLM + * call per block so title and body stay coherent), and writes a same-schema JSON file. + * + * Entry point when the IDE is launched with the 'transform-texts' command. + */ +class TransformTextsStarter : ApplicationStarter { + override val requiredModality: Int = ApplicationStarter.NOT_IN_EDT + private val logger = thisLogger().withStdout() + + override fun main(args: List) { + val memoryFile = System.getProperty("transform.memoryFile") ?: "" + val inputFile = System.getProperty("transform.inputFile") ?: "" + val outputFile = System.getProperty("transform.outputFile") ?: "" + + if (memoryFile.isEmpty() || inputFile.isEmpty() || outputFile.isEmpty()) { + logger.error("[TransformTexts] Missing required parameters") + logger.error("[TransformTexts] Required system properties: transform.memoryFile, transform.inputFile, transform.outputFile") + exitProcess(1) + } + + val token = System.getenv("GRAZIE_TOKEN") + if (token == null) { + logger.error("[TransformTexts] GRAZIE_TOKEN environment variable not set") + exitProcess(1) + } + + runBlocking { + try { + logger.info("[TransformTexts] Starting text transformation") + logger.info("[TransformTexts] Memory file: $memoryFile") + logger.info("[TransformTexts] Input file: $inputFile") + + val llm = LLM.fromGrazie( + model = OpenAIModels.Chat.GPT5Mini, + token = token, + ) + val transformer = MetamorphicTextTransformer(llm) + + val renameMap = transformer.loadRenameMap(memoryFile) + if (renameMap == null) { + logger.error("[TransformTexts] Failed to load rename map from '$memoryFile'") + exitProcess(1) + } + + val inputJson = BenchmarkInstanceIO.json + .parseToJsonElement(File(inputFile).readText()) + .jsonObject + + val outputJson = if (renameMap.isEmpty()) { + logger.warn("[TransformTexts] Rename map is empty; copying input verbatim") + inputJson + } else { + BenchmarkInstanceIO.transformInstance(inputJson) { block -> + try { + transformer.transformBlock(block, renameMap) + } catch (e: Exception) { + logger.error("[TransformTexts] Block-level transformation failed; keeping original block: ${e.message}", e) + null + } + } + } + + File(outputFile).writeText(BenchmarkInstanceIO.json.encodeToString(JsonObject.serializer(), outputJson)) + logger.info("[TransformTexts] SUCCESS: Transformed record written to: $outputFile") + exitProcess(0) + } catch (e: Exception) { + logger.error("[TransformTexts] ERROR: ${e.message}", e) + e.printStackTrace(System.err) + exitProcess(1) + } + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/IntelliJAwareTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/IntelliJAwareTransformation.kt index 126393c7..895054a1 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/IntelliJAwareTransformation.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/IntelliJAwareTransformation.kt @@ -4,6 +4,8 @@ import com.github.pderakhshanfar.codecocoonplugin.executor.TransformationResult import com.github.pderakhshanfar.codecocoonplugin.memory.Memory import com.github.pderakhshanfar.codecocoonplugin.transformation.Transformation import com.intellij.openapi.application.readAction +import com.intellij.openapi.application.smartReadAction +import com.intellij.openapi.project.Project import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.PsiFile import kotlinx.coroutines.runBlocking @@ -42,5 +44,21 @@ interface IntelliJAwareTransformation : Transformation { */ inline fun withReadAction(crossinline block: () -> T): T = runBlocking { readAction { block() } } + + /** + * Smart-mode equivalent of [withReadAction]: suspends until the IntelliJ + * indices are ready (i.e., not in dumb mode) before running [block]. Use + * this anywhere [block] reaches into the stub / file-based index — e.g. + * `psiClass.allFields`, `psiClass.supers`, `method.findSuperMethods()`, + * `JavaPsiFacade.findClass(...)`, `ReferencesSearch.search(...)`. + * + * Background: writes from earlier files in the pipeline (rename commits + * + document save) drop the IDE back into dumb mode while the index is + * recomputed. Plain [withReadAction] holds a read lock but does not wait + * for smart mode, so any index touch in that window throws + * [com.intellij.openapi.project.IndexNotReadyException]. + */ + inline fun withSmartReadAction(project: Project, crossinline block: () -> T): T = + runBlocking { smartReadAction(project) { block() } } } } \ No newline at end of file diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameClassTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameClassTransformation.kt index 4bc9aff1..03506dd6 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameClassTransformation.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameClassTransformation.kt @@ -4,10 +4,10 @@ import ai.koog.prompt.dsl.prompt import ai.koog.prompt.executor.clients.openai.OpenAIModels import com.github.pderakhshanfar.codecocoonplugin.common.LLM import com.github.pderakhshanfar.codecocoonplugin.components.transformations.IntelliJAwareTransformation.Companion.withReadAction +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.IntelliJAwareTransformation.Companion.withSmartReadAction import com.github.pderakhshanfar.codecocoonplugin.components.transformations.SelfManagedTransformation import com.github.pderakhshanfar.codecocoonplugin.executor.TransformationResult import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout -import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.allowedAnnotationsOnly import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.document import com.github.pderakhshanfar.codecocoonplugin.intellij.vfs.relativeToRootOrAbsPath import com.github.pderakhshanfar.codecocoonplugin.java.JavaTransformation @@ -15,8 +15,9 @@ import com.github.pderakhshanfar.codecocoonplugin.memory.Memory import com.github.pderakhshanfar.codecocoonplugin.memory.PsiSignatureGenerator import com.github.pderakhshanfar.codecocoonplugin.transformation.requireOrDefault import com.intellij.openapi.application.ApplicationManager -import com.intellij.openapi.application.readAction +import com.intellij.openapi.application.smartReadAction import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ProjectFileIndex @@ -48,15 +49,45 @@ class RenameClassTransformation( val result = try { val useMemory = config.requireOrDefault("useMemory", defaultValue = false) val generateWhenNotInMemory = config.requireOrDefault("generateWhenNotInMemory", defaultValue = false) + val searchInComments = config.requireOrDefault("searchInComments", defaultValue = false) + + // Annotation filtering configuration val whitelistedAnnotations = config.requireOrDefault>("whitelistedAnnotations", defaultValue = emptyList()) + val blacklistedAnnotationsRaw = config.requireOrDefault>("blacklistedAnnotations", defaultValue = emptyList()) + + // Process blacklist: merge defaults if "_default" or "default" is present + val blacklistedAnnotations = if (blacklistedAnnotationsRaw.any { it.equals("_default", ignoreCase = true) || it.equals("default", ignoreCase = true) }) { + logger.info(" ↳ Include default blacklisted annotations ALONG with the custom ones (i.e., '_default' or 'default' keyword in the list)") + + val customAnnotations = blacklistedAnnotationsRaw.filter { !it.equals("_default", ignoreCase = true) && !it.equals("default", ignoreCase = true) } + (DEFAULT_BLACKLISTED_CLASS_ANNOTATIONS + customAnnotations).toList() + } else { + // Warn if using blacklist mode without defaults + if (blacklistedAnnotationsRaw.isNotEmpty()) { + logger.warn(" ⚠ Blacklist provided without '_default' keyword - framework annotations will NOT be automatically excluded") + } + blacklistedAnnotationsRaw + } + + // Auto-detect mode: if whitelistedAnnotations is provided, use whitelist mode; otherwise blacklist + val annotationFilterMode = config.requireOrDefault( + "annotationFilterMode", + defaultValue = if (whitelistedAnnotations.isNotEmpty()) "whitelist" else "blacklist" + ) val document = withReadAction { psiFile.document() } val modifiedFiles = mutableSetOf() val value = if (document != null) { - val eligibleClasses: List = withReadAction { + // Smart-read: findAllValidClasses calls ReferencesSearch.search(cls) + // and fileIndex.isInTestSourceContent(...) for every class — both + // hit the stub index and would throw IndexNotReadyException if a + // prior file's rename pushed us into dumb mode. + val eligibleClasses: List = withSmartReadAction(psiFile.project) { findAllValidClasses( psiFile = psiFile, + annotationFilterMode = annotationFilterMode, whitelistedClassAnnotations = whitelistedAnnotations, + blacklistedClassAnnotations = blacklistedAnnotations, ) } @@ -92,7 +123,7 @@ class RenameClassTransformation( // Try each suggestion until one succeeds for (suggestion in suggestions) { - val files = tryRenameClassAndUsages(psiFile.project, psiClass, suggestion) + val files = tryRenameClassAndUsages(psiFile.project, psiClass, suggestion, searchInComments) if (files != null) { modifiedFiles.addAll(files) if (saveRenamesInMemory) { @@ -198,7 +229,11 @@ class RenameClassTransformation( } private suspend fun generateNewClassNames(psiClass: PsiClass, count: Int = DEFAULT_SUGGESTED_NAMES_SIZE): List { - val context = readAction { + // Smart-read: psiClass.allFields walks the inheritance chain and + // resolves super references via JavaPsiFacade.findClass -> stub index. + // A plain readAction { ... } here can land mid-reindex (after prior + // files' renames invalidated the index) and throw IndexNotReadyException. + val context = smartReadAction(psiClass.project) { val type = when { psiClass.isInterface -> "interface" psiClass.isEnum -> "enum class" @@ -226,6 +261,10 @@ class RenameClassTransformation( +"Example structure:" +"{\"suggestions\": [\"BestFittingRename\", \"SecondBestFittingRename\", ... ]}" +"Every suggestion must be a valid Java identifier and semantically similar to the original name." + +("Suggestions MUST differ from the original name by more than letter case alone — " + + "case-only renames (e.g. `JSON` → `Json`, `XML` → `Xml`) are forbidden because they collide " + + "on case-insensitive filesystems. Add or change letters/words instead " + + "(e.g. `JSON` → `JsonMapper`, `XML` → `XmlDocument`).") } } @@ -235,10 +274,10 @@ class RenameClassTransformation( prompt = classRenamePrompt ) - return if (result != null) buildSuggestionList(result.suggestions) else emptyList() + return if (result != null) buildSuggestionList(result.suggestions, context.className) else emptyList() } - private fun buildSuggestionList(rawSuggestions: List): List { + private fun buildSuggestionList(rawSuggestions: List, originalName: String): List { val normalized = rawSuggestions .asSequence() .map { it.trim() } @@ -246,14 +285,25 @@ class RenameClassTransformation( .distinct() .toList() - val firstSuggestion = normalized.firstOrNull() ?: return emptyList() + // Defensive filter: drop suggestions that equal the original name case-insensitively + // (covers both same-as-original and case-only variants like `JSON` -> `Json`, which + // collide with the source file on case-insensitive filesystems and break `git apply`). + val (kept, dropped) = normalized.partition { !it.equals(originalName, ignoreCase = true) } + if (dropped.isNotEmpty()) { + logger.info(" ⊘ Dropped ${dropped.size} case-only/identical suggestion(s) for `$originalName`: ${dropped.joinToString(", ")}") + } + + val firstSuggestion = kept.firstOrNull() ?: return emptyList() val internalFallback = "${firstSuggestion}Internal" - return if (normalized.contains(internalFallback)) normalized else normalized + internalFallback + return if (kept.contains(internalFallback)) kept else kept + internalFallback } private fun tryRenameClassAndUsages( - project: Project, psiClass: PsiClass, newName: String + project: Project, + psiClass: PsiClass, + newName: String, + searchInComments: Boolean, ): MutableSet? { return try { val oldName = psiClass.name ?: return null @@ -263,16 +313,15 @@ class RenameClassTransformation( /* project = */ project, /* element = */ psiClass, /* newName = */ newName, - /* isSearchInComments= */ true, + /* isSearchInComments= */ searchInComments, /* isSearchTextOccurrences = */ false ) } - ApplicationManager.getApplication().invokeAndWait { - PsiDocumentManager.getInstance(project).commitAllDocuments() - renameProcessor.run() - } - + // Snapshot modified files BEFORE run(): findUsages() must run on + // the pre-rename PSI to return the references that will actually + // be rewritten. After run() the seed element has been renamed and + // the result is unreliable. val modifiedFiles = withReadAction { val files = mutableSetOf() renameProcessor.findUsages().forEach { usageInfo -> @@ -281,6 +330,17 @@ class RenameClassTransformation( psiClass.containingFile?.let { files.add(it) } files } + + ApplicationManager.getApplication().invokeAndWait { + PsiDocumentManager.getInstance(project).commitAllDocuments() + renameProcessor.run() + // Lock in PSI/document/disk state immediately so subsequent renames + // (and the final project close) don't trigger close-time hooks whose + // behaviour depends on accumulated unflushed state — that previously + // produced non-deterministic import positions across morph runs. + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() + } logger.info(" • Renamed `$oldName` to `$newName`") modifiedFiles } catch (e: ProcessCanceledException) { @@ -300,8 +360,33 @@ class RenameClassTransformation( */ private fun findAllValidClasses( psiFile: PsiFile, + annotationFilterMode: String, whitelistedClassAnnotations: List, + blacklistedClassAnnotations: List, ): List { + // Log annotation filter mode and relevant annotations + when (annotationFilterMode.lowercase()) { + "whitelist" -> { + if (whitelistedClassAnnotations.isNotEmpty()) { + logger.info(" ↳ Annotation filter mode: WHITELIST") + logger.info(" ↳ Whitelisted class annotations: [${whitelistedClassAnnotations.joinToString(", ")}]") + } else { + logger.info(" ↳ Annotation filter mode: WHITELIST (empty - only non-annotated classes allowed)") + } + } + "blacklist" -> { + logger.info(" ↳ Annotation filter mode: BLACKLIST") + if (blacklistedClassAnnotations.isNotEmpty()) { + logger.info(" ↳ Blacklisted class annotations: [\n${blacklistedClassAnnotations.joinToString(",\n") { "\t$it" } }\n]") + } else { + logger.info(" ↳ Blacklisted class annotations: [] (all annotations allowed)") + } + } + else -> { + logger.warn(" ⚠ Unknown annotation filter mode: '$annotationFilterMode', defaulting to blacklist") + } + } + val classes = mutableListOf() psiFile.accept(object : PsiRecursiveElementVisitor() { override fun visitElement(element: PsiElement) { @@ -322,19 +407,55 @@ class RenameClassTransformation( val fileType = ref.element.containingFile.fileType.name fileType != "JAVA" && fileType != "Kotlin" } - if (usedInNonJavaFile) return@filter false + if (usedInNonJavaFile) { + logger.info(" ⊘ Class `${cls.name}` - skipped (used in non-Java file)") + return@filter false + } // Is not a test - if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) return@filter false + if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) { + logger.info(" ⊘ Class `${cls.name}` - skipped (in test source)") + return@filter false + } - // either no annotations or whitelisted ones only - val annotationsFilter = cls.annotations.isEmpty() - || cls.annotations.toList().allowedAnnotationsOnly(whitelistedClassAnnotations) + // Check annotation filter (whitelist or blacklist mode) + val classAnnotations = cls.annotations.toList() + val annotationsPassed = passesAnnotationFilter( + annotations = classAnnotations, + filterMode = annotationFilterMode, + whitelistedAnnotations = whitelistedClassAnnotations, + blacklistedAnnotations = blacklistedClassAnnotations + ) + + // Log annotation filtering for classes with annotations + if (classAnnotations.isNotEmpty()) { + val annotationNames = classAnnotations.mapNotNull { it.qualifiedName?.substringAfterLast('.') } + if (annotationsPassed) { + val modeLabel = if (annotationFilterMode.lowercase() == "whitelist") "whitelisted" else "passed blacklist" + logger.info(" ✓ Class `${cls.name}` with annotations [${annotationNames.joinToString(", ")}] - $modeLabel") + } else { + val modeLabel = if (annotationFilterMode.lowercase() == "whitelist") "not whitelisted" else "blacklisted" + logger.info(" ⊘ Class `${cls.name}` with annotations [${annotationNames.joinToString(", ")}] - skipped ($modeLabel)") + return@filter false + } + } // Basic Filters val className = cls.name + + // Check for null class name + if (className == null) { + logger.info(" ⊘ Class - skipped (null class name)") + return@filter false + } + // We need to check for `cls.name.length` > 1 to filter out raw Type classes - (className != null) && (className.length > 1) && annotationsFilter + if (className.length <= 1) { + logger.info(" ⊘ Class `$className` - skipped (class name too short)") + return@filter false + } + + true } if (filteredClasses.isNotEmpty()) { @@ -345,8 +466,97 @@ class RenameClassTransformation( return filteredClasses } + /** + * Checks if annotations pass the configured filter mode (whitelist or blacklist). + * + * @param annotations List of annotations to check + * @param filterMode "whitelist" or "blacklist" + * @param whitelistedAnnotations Annotations to allow (when mode = whitelist) + * @param blacklistedAnnotations Annotations to forbid (when mode = blacklist) + * @return true if annotations pass the filter, false otherwise + */ + private fun passesAnnotationFilter( + annotations: List, + filterMode: String, + whitelistedAnnotations: List, + blacklistedAnnotations: List, + ): Boolean { + if (annotations.isEmpty()) { + return true + } + + return when (filterMode.lowercase()) { + "whitelist" -> { + // All annotations must be in the whitelist + annotations.all { annotation -> + val qualifiedName = annotation.qualifiedName + val simpleName = qualifiedName?.substringAfterLast('.') + (qualifiedName != null) && (qualifiedName in whitelistedAnnotations || simpleName in whitelistedAnnotations) + } + } + "blacklist" -> { + // No annotations can be in the blacklist + annotations.none { annotation -> + val qualifiedName = annotation.qualifiedName + val simpleName = qualifiedName?.substringAfterLast('.') + qualifiedName in blacklistedAnnotations || simpleName in blacklistedAnnotations + } + } + else -> { + logger.warn(" ⚠ Unknown annotation filter mode: '$filterMode', defaulting to blacklist") + // Default to blacklist mode with empty list (allow all) + true + } + } + } + companion object { const val ID = "rename-class-transformation" + + /** + * Default blacklisted class annotations (framework/infrastructure annotations). + * These annotations typically indicate classes that are managed by frameworks/containers, + * so renaming them could break runtime behavior or configuration. + */ + val DEFAULT_BLACKLISTED_CLASS_ANNOTATIONS = setOf( + // JPA/Hibernate + "javax.persistence.Entity", + "javax.persistence.Table", + "javax.persistence.Embeddable", + "javax.persistence.MappedSuperclass", + + // Spring Framework + "org.springframework.stereotype.Component", + "org.springframework.stereotype.Service", + "org.springframework.stereotype.Repository", + "org.springframework.stereotype.Controller", + "org.springframework.web.bind.annotation.RestController", + "org.springframework.web.bind.annotation.ControllerAdvice", + "org.springframework.context.annotation.Configuration", + "org.springframework.boot.autoconfigure.SpringBootApplication", + "org.springframework.jmx.export.annotation.ManagedResource", + + // JAX-RS + "javax.ws.rs.Path", + "jakarta.ws.rs.Path", + + // CDI + "javax.inject.Named", + "jakarta.inject.Named", + "javax.enterprise.context.ApplicationScoped", + "javax.enterprise.context.RequestScoped", + "javax.enterprise.context.SessionScoped", + + // Jackson + "com.fasterxml.jackson.annotation.JsonRootName", + + // JAXB + "javax.xml.bind.annotation.XmlRootElement", + "javax.xml.bind.annotation.XmlType", + "jakarta.xml.bind.annotation.XmlRootElement", + "jakarta.xml.bind.annotation.XmlType" + ) + private const val DEFAULT_SUGGESTED_NAMES_SIZE = 3 } } \ No newline at end of file diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameMethodTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameMethodTransformation.kt index 9e14e000..83c50f8c 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameMethodTransformation.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameMethodTransformation.kt @@ -8,23 +8,30 @@ import com.github.pderakhshanfar.codecocoonplugin.components.transformations.Int import com.github.pderakhshanfar.codecocoonplugin.components.transformations.SelfManagedTransformation import com.github.pderakhshanfar.codecocoonplugin.executor.TransformationResult import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout -import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.allowedAnnotationsOnly import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.document import com.github.pderakhshanfar.codecocoonplugin.intellij.vfs.relativeToRootOrAbsPath import com.github.pderakhshanfar.codecocoonplugin.java.JavaTransformation import com.github.pderakhshanfar.codecocoonplugin.memory.Memory import com.github.pderakhshanfar.codecocoonplugin.memory.PsiSignatureGenerator import com.github.pderakhshanfar.codecocoonplugin.transformation.requireOrDefault +import com.intellij.ide.highlighter.JavaFileType import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.readAction +import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ProjectFileIndex import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.* +import com.intellij.psi.search.FileTypeIndex +import com.intellij.psi.search.GlobalSearchScope +import com.intellij.psi.search.searches.OverridingMethodsSearch import com.intellij.psi.search.searches.ReferencesSearch +import com.intellij.psi.util.PsiTreeUtil import com.intellij.refactoring.rename.RenameProcessor +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.serialization.Serializable @@ -49,30 +56,64 @@ class RenameMethodTransformation( val result = try { val useMemory = config.requireOrDefault("useMemory", defaultValue = false) val generateWhenNotInMemory = config.requireOrDefault("generateWhenNotInMemory", defaultValue = false) - // list of allowed method annotations, e.g. ["NotNull"] + val searchInComments = config.requireOrDefault("searchInComments", defaultValue = false) + + // Annotation filtering configuration val whitelistedAnnotations = config.requireOrDefault>("whitelistedAnnotations", defaultValue = emptyList()) + val blacklistedAnnotationsRaw = config.requireOrDefault>("blacklistedAnnotations", defaultValue = emptyList()) + + // Process blacklist: merge defaults if "_default" or "default" is present + val blacklistedAnnotations = if (blacklistedAnnotationsRaw.any { it.equals("_default", ignoreCase = true) || it.equals("default", ignoreCase = true) }) { + logger.info(" ↳ Include default blacklisted annotations ALONG with the custom ones (i.e., '_default' or 'default' keyword in the list)") + + val customAnnotations = blacklistedAnnotationsRaw.filter { !it.equals("_default", ignoreCase = true) && !it.equals("default", ignoreCase = true) } + (DEFAULT_BLACKLISTED_METHOD_ANNOTATIONS + customAnnotations).toList() + } else { + // Warn if using blacklist mode without defaults + if (blacklistedAnnotationsRaw.isNotEmpty()) { + logger.warn(" ⚠ Blacklist provided without '_default' keyword - framework annotations will NOT be automatically excluded") + } + blacklistedAnnotationsRaw + } + + // Auto-detect mode: if whitelistedAnnotations is provided, use whitelist mode; otherwise blacklist + val annotationFilterMode = config.requireOrDefault( + "annotationFilterMode", + defaultValue = if (whitelistedAnnotations.isNotEmpty()) "whitelist" else "blacklist" + ) val document = IntelliJAwareTransformation.withReadAction { psiFile.document() } val modifiedFiles = mutableSetOf() val value = if (document != null) { - val publicMethods: List = IntelliJAwareTransformation.withReadAction { - findAllValidMethods( + // Find all valid method families (already grouped and filtered). + // Smart-read: findAllValidMethodFamilies calls method.findSuperMethods(), + // psiClass.supers, and ReferencesSearch.search(method) — all hit the + // stub index. After earlier files' renames the IDE often drops back + // into dumb mode while reindexing; a plain withReadAction { ... } here + // would throw IndexNotReadyException as soon as the override-filter + // loop reached findSuperMethods(). + val overloadFamilies: List = IntelliJAwareTransformation.withSmartReadAction(psiFile.project) { + findAllValidMethodFamilies( psiFile = psiFile, - whitelistedMethodAnnotations = whitelistedAnnotations + annotationFilterMode = annotationFilterMode, + whitelistedMethodAnnotations = whitelistedAnnotations, + blacklistedMethodAnnotations = blacklistedAnnotations ) } - if (publicMethods.isEmpty()) { - return TransformationResult.Skipped("No matching methods found in ${virtualFile.name}") + if (overloadFamilies.isEmpty()) { + return TransformationResult.Skipped("No matching method families found in ${virtualFile.name}") } - logger.info(" ⏲ Generating rename suggestions for ${publicMethods.size} methods...") + val totalMethods = overloadFamilies.sumOf { it.methods.size } + logger.info(" ⏲ Generating rename suggestions for $totalMethods methods (${overloadFamilies.size} overload families)...") - val renaming = runBlocking { + // Generate suggestions for each overload family (not individual methods) + val familySuggestions = runBlocking { if (useMemory) { - extractRenamesFromMemory(publicMethods, memory, generateWhenNotInMemory) + extractRenamesFromMemoryForFamilies(overloadFamilies, memory, generateWhenNotInMemory) } else { - generateRenames(publicMethods) + generateRenamesForFamilies(overloadFamilies) } } @@ -80,44 +121,81 @@ class RenameMethodTransformation( // or when we POTENTIALLY generated renames for missing entries val saveRenamesInMemory = !useMemory || generateWhenNotInMemory + // Track successful renames across all families + var renamedMethodCount = 0 - // Try renaming each method with suggestions until one succeeds - val successfulRenames = publicMethods.mapNotNull { method -> - val methodName = IntelliJAwareTransformation.withReadAction { method.name } - val suggestions = renaming.suggestions[method] ?: return@mapNotNull null - - // Generate signature BEFORE renaming - val signature = IntelliJAwareTransformation.withReadAction { - PsiSignatureGenerator.generateSignature(method) - } - if (signature == null) { - logger.warn(" ⊘ Could not generate signature for method $methodName") - return@mapNotNull null + // Group families by class for organized logging + val familiesByClass = overloadFamilies.groupBy { + IntelliJAwareTransformation.withReadAction { + it.containingClass.qualifiedName ?: it.containingClass.name ?: "" } + } + + logger.info(" ↳ Renaming methods in ${familiesByClass.size} class(es)...") + + // Try renaming each overload family, grouped by class + for ((className, classFamilies) in familiesByClass) { + logger.info(" ◆ Processing class: `$className` (${classFamilies.size} overload families):") + + for ((familyIndex, family) in classFamilies.withIndex()) { + val suggestions = familySuggestions[family] ?: continue + val familyName = IntelliJAwareTransformation.withReadAction { family.methodName } - // Try each suggestion until one succeeds (no conflicts) - for (suggestion in suggestions) { - val files = tryRenameMethodAndUsages(psiFile.project, method, suggestion) - if (files != null) { - modifiedFiles.addAll(files) - if (saveRenamesInMemory) { - memory?.put(signature, suggestion) - logger.info(" ✓ Stored rename in memory: `$signature` -> `$suggestion`") + // Generate signatures BEFORE renaming for all methods in the family + val methodSignatures = if (saveRenamesInMemory) { + family.methods.associateWith { method -> + IntelliJAwareTransformation.withReadAction { + PsiSignatureGenerator.generateSignature(method) + } } - return@mapNotNull method to suggestion + } else { + emptyMap() + } + + // Try each suggestion until one succeeds for the whole family. + // The family is renamed atomically via a single RenameProcessor — + // see tryRenameMethodFamily for why per-method iteration is unsafe. + var familyRenamed = false + for (suggestion in suggestions) { + // Skip if suggestion is the same as the original name (no-op rename) + if (suggestion == familyName) { + continue + } + + logger.info(" • ${familyIndex + 1}) Renaming `$familyName` overload family to `$suggestion` (${family.methods.size} overloads):") + val files = tryRenameMethodFamily(psiFile.project, family.methods, suggestion, searchInComments) + if (files != null) { + modifiedFiles.addAll(files) + + // Store all family signatures in memory under the same suggestion + if (saveRenamesInMemory) { + for (method in family.methods) { + val signature = methodSignatures[method] + if (signature != null) { + memory?.put(signature, suggestion) + logger.info(" ✓ Stored rename in memory: `$signature` -> `$suggestion`") + } else { + logger.warn(" ⊘ Could not generate signature for method before renaming") + } + } + } + + renamedMethodCount += family.methods.size + familyRenamed = true + break + } + } + + if (!familyRenamed) { + logger.info(" ⊘ Skipped renaming method `$familyName`, suggestions: $suggestions") } } - // No valid suggestion worked - logger.info(" ⊘ Skipped renaming method `$methodName` (suggestions: $suggestions)") - null } - val renamedCount = successfulRenames.size - val totalCandidates = publicMethods.size - val skipped = totalCandidates - renamedCount + val skipped = totalMethods - renamedMethodCount TransformationResult.Success( - message = "Renamed ${renamedCount}/${totalCandidates} methods in ${virtualFile.name}${if (skipped > 0) " (skipped: $skipped)" else ""}", + message = "Renamed ${renamedMethodCount}/${totalMethods} methods in ${virtualFile.name}${if (skipped > 0) " (skipped: $skipped)" else ""}", filesModified = modifiedFiles.size ) } else { @@ -133,107 +211,248 @@ class RenameMethodTransformation( } @Serializable - private data class MethodNameSuggestions(val suggestions: List) + private data class MethodFamilyRenaming(val familyKey: String, val suggestions: List) + + @Serializable + private data class MethodRenameSuggestions(val renamings: List) - private data class MethodContext( + private data class MethodFamilyContext( + val familyKey: String, val methodName: String, val methodBody: String?, - val className: String? + val className: String?, ) /** - * Extracts rename suggestions from memory for methods. + * Represents a family of overloaded methods (same name, same containing class). + * + * [familyKey] uniquely identifies the family within a batched LLM prompt so the + * model can echo it back per-entry. Format: `#[]`. + * The `[static]/[instance]` tag is required because a class can hold a static and + * an instance method with the same name — these are separate families and must + * not collide on the key. + */ + private data class OverloadFamily( + val methodName: String, + val containingClass: PsiClass, + val methods: List, + val isStatic: Boolean, + val classFqn: String, + ) { + val familyKey: String = + "$classFqn#$methodName[${if (isStatic) "static" else "instance"}]" + + /** + * Returns a representative method for generating rename suggestions. + * Prefers methods with bodies (non-abstract) for better context. + */ + fun getRepresentative(): PsiMethod { + return methods.firstOrNull { it.body != null } ?: methods.first() + } + } + + /** + * Groups methods into overload families. + * Methods with the same name in the same containing class are grouped together. + * Static and instance methods are kept in separate families even if they share the same name. + */ + private fun groupMethodsByOverloads(methods: List): List { + val grouped = methods.groupBy { method -> + Triple( + method.containingClass?.qualifiedName ?: "", + method.name, + method.hasModifierProperty(PsiModifier.STATIC) + ) + } + + return grouped.map { (key, methodsInFamily) -> + val (classFqn, methodName, isStatic) = key + val representative = methodsInFamily.first() + OverloadFamily( + methodName = methodName, + containingClass = representative.containingClass!!, + methods = methodsInFamily, + isStatic = isStatic, + classFqn = classFqn, + ) + } + } + + /** + * Extracts rename suggestions from memory for overload families. + * Returns the same suggestion for all methods in a family. + * Checks ALL methods in the family to find cached names. * * When [generateWhenNotInMemory] is true, generates new suggestions - * for all methods whose suggestions are missing in memory. + * for all families whose suggestions are missing in memory. */ - private suspend fun extractRenamesFromMemory( - methods: List, + private suspend fun extractRenamesFromMemoryForFamilies( + families: List, memory: Memory?, generateWhenNotInMemory: Boolean, - ): Renaming { - val methodsWithMissingSuggestions = mutableListOf() + ): Map> { + val familiesWithMissingSuggestions = mutableListOf() - val suggestions = methods.associateWith { method -> - val signature = IntelliJAwareTransformation.withReadAction { - PsiSignatureGenerator.generateSignature(method) - } - if (signature == null) { - logger.warn("Could not generate signature for method") - return@associateWith emptyList() - } + val suggestions = families.associateWith { family -> + // Check all methods in the family (not just the representative) + // This handles cases where methods were stored in different orders + for (method in family.methods) { + val signature = IntelliJAwareTransformation.withReadAction { + PsiSignatureGenerator.generateSignature(method) + } - val cachedName = memory?.get(signature) - if (cachedName != null) { - logger.info(" ↳ Using cached rename: $signature -> $cachedName") - listOf(cachedName) - } else { - logger.warn(" ⊘ Signature not found in memory: $signature") - if (generateWhenNotInMemory) { - methodsWithMissingSuggestions.add(method) + if (signature == null) { + logger.warn("Could not generate signature for method ${family.methodName}") + continue } - emptyList() + + val cachedName = memory?.get(signature) + if (cachedName != null) { + logger.info(" ↳ Using cached rename: $signature -> $cachedName") + return@associateWith listOf(cachedName) + } + } + + // No cached name found for any method in the family + logger.warn(" ⊘ Signature not found in memory: ${family.methodName}") + if (generateWhenNotInMemory) { + familiesWithMissingSuggestions.add(family) } + emptyList() } - val finalSuggestions = if (generateWhenNotInMemory && methodsWithMissingSuggestions.isNotEmpty()) { - logger.info(" ↳ Generating missing rename suggestions for ${methodsWithMissingSuggestions.size} methods (i.e., generateWhenNotInMemory=true)...") - val generated = generateRenames(methodsWithMissingSuggestions) + val finalSuggestions = if (generateWhenNotInMemory && familiesWithMissingSuggestions.isNotEmpty()) { + logger.info(" ↳ Generating missing rename suggestions for ${familiesWithMissingSuggestions.size} method families (i.e., generateWhenNotInMemory=true)...") + val generated = generateRenamesForFamilies(familiesWithMissingSuggestions) buildMap { - for (method in methods) { - val suggestionsA = suggestions[method] ?: emptyList() - val suggestionsB = generated.suggestions[method] ?: emptyList() - put(method, suggestionsA + suggestionsB) + for (family in families) { + val suggestionsA = suggestions[family] ?: emptyList() + val suggestionsB = generated[family] ?: emptyList() + put(family, suggestionsA + suggestionsB) } } } else { suggestions } - return Renaming(finalSuggestions) + return finalSuggestions } /** - * Generates rename suggestions for all methods using LLM. + * Generates rename suggestions for overload families using a batched LLM call. + * Returns the same suggestions for all methods in a family. */ - private suspend fun generateRenames(methods: List): Renaming { - val suggestions = methods.associateWith { method -> - generateNewMethodNames(method) + private suspend fun generateRenamesForFamilies(families: List): Map> { + val batchRenamings = generateNewMethodNames(families) + return families.associateWith { family -> + val renaming = batchRenamings.find { it.familyKey == family.familyKey } + renaming?.suggestions?.let { buildSuggestionList(it) } ?: emptyList() } - return Renaming(suggestions) } - private suspend fun generateNewMethodNames(method: PsiMethod, count: Int = DEFAULT_SUGGESTED_NAMES_SIZE): List { - // Extract all PSI data in a read action before building the prompt - val context = readAction { - MethodContext( - methodName = method.name, - methodBody = method.body?.text, - className = method.containingClass?.name + /** + * Splits [families] into chunks so a single transient LLM failure (timeout, + * malformed JSON, token-budget overflow on huge files) doesn't lose suggestions + * for the entire file. Files with <= LLM_BATCH_SIZE families still produce + * one LLM call. + */ + private suspend fun generateNewMethodNames( + families: List, + count: Int = DEFAULT_SUGGESTED_NAMES_SIZE, + ): List { + if (families.isEmpty()) return emptyList() + + val batches = families.chunked(LLM_BATCH_SIZE) + if (batches.size > 1) { + logger.info(" ↳ Splitting ${families.size} families into ${batches.size} LLM batches of up to $LLM_BATCH_SIZE") + } + + val combined = mutableListOf() + for ((index, batch) in batches.withIndex()) { + val partial = generateBatchWithRetry( + families = batch, + count = count, + batchLabel = "${index + 1}/${batches.size}", ) + combined.addAll(partial) + } + return combined + } + + /** + * Calls the LLM for a single batch of families, retrying transient errors with + * exponential backoff. Returns an empty list if every retry fails — the caller's + * per-family loop then skips those families (no rename, no memory write) instead + * of failing the whole transformation, so other batches and other files still + * progress. + * + * Rethrows [ProcessCanceledException] per IntelliJ contract. + */ + private suspend fun generateBatchWithRetry( + families: List, + count: Int, + batchLabel: String, + ): List { + val contexts = readAction { + families.map { family -> + val rep = family.getRepresentative() + MethodFamilyContext( + familyKey = family.familyKey, + methodName = rep.name, + methodBody = rep.body?.text, + className = rep.containingClass?.name, + ) + } } - val methodRenamePrompt = prompt("method-rename-prompt") { + val methodRenamePrompt = prompt("method-rename-batch-prompt") { system { +"You are an agent that proposes semantically similar Java method names." +"Your output is used in a metamorphic transformation pipeline." +"Your output will be parsed into JSON; strictly follow the required structure." } user { - +"The current method name is: ${context.methodName}" - +"The method body is: ${context.methodBody}" - +"The containing class name is: ${context.className}" - +"Return a JSON object with field 'suggestions' which is an ordered array of $count Java identifiers, from most to least fitting." - +"Every suggestion must be a valid Java identifier and semantically similar to the original name." + +"Generate $count semantically similar name suggestions for each of the following Java methods:" + +"" + for (ctx in contexts) { + +"Method (key=${ctx.familyKey}): ${ctx.methodName}" + +" Containing class: ${ctx.className}" + +" Body: ${ctx.methodBody}" + +"" + } + +"Return a JSON object with field 'renamings' which is an array of objects." + +"Each object must have 'familyKey' (echoed verbatim from the input) and 'suggestions' (an ordered array of $count valid Java identifiers, from most to least fitting)." + +"Example schema: {\"renamings\": [{\"familyKey\": \"\", \"suggestions\": [\"newName1\", \"newName2\", ...]}]}" + +"Every suggestion must be a valid Java identifier and semantically similar to the original method name." + +"IMPORTANT: The 'familyKey' field MUST exactly match the key from the input (do not invent new keys, do not reformat)." } } val llm = LLM.fromGrazie(OpenAIModels.Chat.GPT5Mini) - val result = llm.structuredRequest( - prompt = methodRenamePrompt - ) - return if (result != null) buildSuggestionList(result.suggestions) else emptyList() + var lastError: Throwable? = null + for (attempt in 1..LLM_MAX_ATTEMPTS) { + try { + val result = llm.structuredRequest(prompt = methodRenamePrompt) + return result?.renamings ?: emptyList() + } catch (e: ProcessCanceledException) { + throw e + } catch (e: Exception) { + lastError = e + logger.warn(" ⚠ LLM call for method batch $batchLabel failed (attempt $attempt/$LLM_MAX_ATTEMPTS): ${e.message}") + if (attempt < LLM_MAX_ATTEMPTS) { + delay(retryBackoffMillis(attempt)) + } + } + } + logger.warn(" ✗ LLM call for method batch $batchLabel exhausted $LLM_MAX_ATTEMPTS attempts; skipping batch (last error: ${lastError?.message})") + return emptyList() + } + + private fun retryBackoffMillis(attempt: Int): Long { + // 4s, 8s, 16s, ... capped at LLM_RETRY_BACKOFF_CAP_MILLIS + val base = LLM_RETRY_BACKOFF_BASE_MILLIS shl (attempt - 1).coerceAtLeast(0) + return base.coerceAtMost(LLM_RETRY_BACKOFF_CAP_MILLIS) } private fun buildSuggestionList(rawSuggestions: List): List { @@ -252,55 +471,317 @@ class RenameMethodTransformation( return if (normalized.contains(internalFallback)) normalized else normalized + internalFallback } - private fun tryRenameMethodAndUsages( - project: Project, method: PsiMethod, newName: String + /** + * Renames an entire overload family atomically by registering all family + * members on a single [RenameProcessor] (seed + `addElement`) and running + * once. This way IntelliJ resolves overload-bound call-sites against the + * complete family before rewriting, so varargs / multi-arg call sites + * referencing any overload are rewritten consistently. + * + * The previous per-method approach left stray call sites untouched + * because the resolver could rebind to another overload mid-loop. + * + * Returns the set of modified files (snapshotted from `findUsages()` + * BEFORE `run()`, so the seed PSI element is still in its original + * state), or null on failure. + */ + private fun tryRenameMethodFamily( + project: Project, + methods: List, + newName: String, + searchInComments: Boolean, ): MutableSet? { + if (methods.isEmpty()) return null return try { - val oldName = method.name + val firstMethod = methods.first() + val oldName = IntelliJAwareTransformation.withReadAction { firstMethod.name } + + var attachedOverridersCount = 0 val renameProcessor = IntelliJAwareTransformation.withReadAction { - RenameProcessor( + val processor = RenameProcessor( /* project = */ project, - /* element = */ method, + /* element = */ firstMethod, /* newName = */ newName, - /* isSearchInComments= */ true, - /* isSearchTextOccurrences = */ false + /* isSearchInComments = */ searchInComments, + /* isSearchTextOccurrences = */ false, ) - } + for (extra in methods.drop(1)) { + processor.addElement(extra, newName) + } - ApplicationManager.getApplication().invokeAndWait { - PsiDocumentManager.getInstance(project).commitAllDocuments() - renameProcessor.run() + // Pre-emptively attach transitive overriders. RenameProcessor's implicit + // expansion via RenameJavaMethodProcessor.prepareRenaming + + // OverridingMethodsSearch is unreliable when several same-name overloads + // are added to one processor and renamed to the same target name — + // observed: only the seed overload's overrider got renamed, sibling + // overloads' overriders were silently left with the old name. Adding + // them as first-class entries in myAllRenames forces the same rename + // path that already works for the seed. + val familyMethodSet = methods.toHashSet() + val projectFileIndex = ProjectFileIndex.getInstance(project) + val attachedOverriders = LinkedHashSet() + for (method in methods) { + OverridingMethodsSearch.search(method, /* checkDeep = */ true).findAll().forEach { overrider -> + if (overrider in familyMethodSet) return@forEach + if (!attachedOverriders.add(overrider)) return@forEach + val vf = overrider.containingFile?.virtualFile + if (vf != null && projectFileIndex.isInLibrary(vf)) return@forEach + processor.addElement(overrider, newName) + } + } + attachedOverridersCount = attachedOverriders.size + + // Full target table — family + attached overriders — for debuggability + val totalTargets = methods.size + attachedOverriders.size + logger.info(" ↳ Rename processor targets ($totalTargets method(s) total):") + val targetDisplayLimit = 10 + var shown = 0 + for (method in methods) { + if (shown >= targetDisplayLimit) break + val sig = PsiSignatureGenerator.generateSignature(method) ?: "" + logger.info(" [family] $sig") + shown++ + } + for (overrider in attachedOverriders) { + if (shown >= targetDisplayLimit) break + val sig = PsiSignatureGenerator.generateSignature(overrider) ?: "" + logger.info(" [overrider] $sig") + shown++ + } + if (totalTargets > targetDisplayLimit) { + logger.info(" ... (${totalTargets - targetDisplayLimit} more)") + } + + processor } + // Snapshot modified files BEFORE run(): findUsages() must run on + // the pre-rename PSI to return the references that will actually + // be rewritten. After run() the seed element has been renamed and + // the result is unreliable. + var usageCount = 0 val modifiedFiles = IntelliJAwareTransformation.withReadAction { val files = mutableSetOf() - renameProcessor.findUsages().forEach { usageInfo -> - usageInfo.file?.let { files.add(it) } + val docManager = PsiDocumentManager.getInstance(project) + val usagePaths = mutableListOf() + val usages = renameProcessor.findUsages() + for (usageInfo in usages) { + val file = usageInfo.file ?: continue + files.add(file) + val element = usageInfo.element + val vf = file.virtualFile + if (element != null && vf != null) { + val doc = docManager.getDocument(file) + val line = doc?.getLineNumber(element.textRange.startOffset)?.plus(1) ?: -1 + val path = project.relativeToRootOrAbsPath(vf) + usagePaths.add("$path:$line") + } } - method.containingFile?.let { files.add(it) } + for (method in methods) { + method.containingFile?.let { files.add(it) } + } + usageCount = usages.size + + if (usagePaths.isNotEmpty()) { + logger.info(" ↳ Rename processor will rewrite ${usagePaths.size} usage(s):") + val usageDisplayLimit = 10 + usagePaths.take(usageDisplayLimit).forEach { logger.info(" $it") } + if (usagePaths.size > usageDisplayLimit) { + logger.info(" ... (${usagePaths.size - usageDisplayLimit} more)") + } + } + files } - logger.info(" • Renamed `$oldName` to `$newName` in ${modifiedFiles.size} files") + + ApplicationManager.getApplication().invokeAndWait { + PsiDocumentManager.getInstance(project).commitAllDocuments() + renameProcessor.run() + // Lock in PSI/document/disk state immediately so subsequent renames + // (and the final project close) don't trigger close-time hooks whose + // behaviour depends on accumulated unflushed state — that previously + // produced non-deterministic import positions across morph runs. + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() + + // Safety net: in multi-module projects with same-simple-name classes + // (e.g. fastjson v1-compat `com.alibaba.fastjson.JSON` alongside v2 + // `com.alibaba.fastjson2.JSON`), MethodReferencesSearch's strict + // signature match can drop call sites whose overload resolution PSI + // can't disambiguate. RenameProcessor.findUsages() then never sees + // them and they survive the rename with the old method name. Walk + // the project once and patch any remaining sites that resolve to + // this family or whose qualifier resolves to its containing class. + val patched = verifyAndPatchMissedCallSites(project, methods, oldName, newName) + if (patched > 0) { + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() + } + } + + val overloadLabel = if (methods.size > 1) "${methods.size} overloads" else "1 overload" + val totalMethodsRenamed = methods.size + attachedOverridersCount + logger.info( + " • Renamed `$oldName` ($overloadLabel) to `$newName`: " + + "$totalMethodsRenamed method(s) renamed, " + + "$usageCount usage(s) rewritten across ${modifiedFiles.size} file(s)" + ) modifiedFiles } catch (e: ProcessCanceledException) { // Must rethrow control flow exceptions - logger.warn("Rename method and usage cancelled: ${e.message}") + logger.warn("Rename method family cancelled: ${e.message}") throw e } catch (e: Exception) { // Rename failed (conflicts, PSI errors, etc.) - return null to try the next suggestion - logger.info(" • Skipped ${method.name}:\n (Reason: ${e.message})") + val familyName = methods.firstOrNull()?.name ?: "" + logger.info(" ⊘ Skipped family `$familyName`:\n (Reason: ${e.message})") null } } /** - * @param psiFile The PSI file to search for methods - * @param whitelistedMethodAnnotations A list of method annotations that are allowed to be present on the method. + * Post-rename safety net for [tryRenameMethodFamily]. Catches call sites that + * `RenameProcessor.findUsages()` failed to attribute to the family — observed in + * multi-module projects where another class shares the simple name and PSI's + * overload resolver can't unambiguously bind the call to a specific overload + * (e.g. v1-compat `com.alibaba.fastjson.JSON` vs v2 `com.alibaba.fastjson2.JSON`). + * + * Walks every Java file in project scope. Patches a call site only when: + * 1. it resolves to a method that is in the family, OR + * 2. its resolution returned null AND its qualifier resolves to the family's + * containing class — i.e. exactly the broken case we're patching, never + * a call PSI can attribute to a different method. + * + * Must be invoked inside the same `invokeAndWait` envelope as the corresponding + * `RenameProcessor.run()` so PSI/document state is consistent. Returns the + * number of sites rewritten; 0 means PSI search already covered everything. */ - private fun findAllValidMethods( - psiFile: PsiFile, - whitelistedMethodAnnotations: List, - ): List { + private fun verifyAndPatchMissedCallSites( + project: Project, + family: List, + oldName: String, + newName: String, + ): Int { + val containingClass = family.firstOrNull()?.containingClass ?: return 0 + val containingFqn = containingClass.qualifiedName ?: return 0 + val familySet = family.toSet() + + val scope = GlobalSearchScope.projectScope(project) + val files = FileTypeIndex.getFiles(JavaFileType.INSTANCE, scope) + val psiManager = PsiManager.getInstance(project) + val factory = JavaPsiFacade.getElementFactory(project) + + data class PatchSite( + val id: PsiIdentifier, + val path: String, + val line: Int, + val viaResolvedFamily: Boolean, + ) + + val patchSites = mutableListOf() + val docManager = PsiDocumentManager.getInstance(project) + for (vf in files) { + val psiFile = psiManager.findFile(vf) as? PsiJavaFile ?: continue + val document = docManager.getDocument(psiFile) + psiFile.accept(object : JavaRecursiveElementVisitor() { + override fun visitMethodCallExpression(expr: PsiMethodCallExpression) { + super.visitMethodCallExpression(expr) + val refExpr = expr.methodExpression + if (refExpr.referenceName != oldName) return + + val resolved = expr.resolveMethod() + val resolvesToFamily = resolved != null && resolved in familySet + + val qualifier = refExpr.qualifierExpression as? PsiReferenceExpression + val qualifierClass = qualifier?.resolve() as? PsiClass + val qualifierMatchesContainingClass = + qualifierClass?.qualifiedName == containingFqn + + if (resolvesToFamily || (resolved == null && qualifierMatchesContainingClass)) { + // Defensive: never patch identifiers inside annotation args + // (annotation member references, not overload-resolved calls). + if (PsiTreeUtil.getParentOfType(expr, PsiAnnotation::class.java) != null) { + return + } + val id = refExpr.referenceNameElement as? PsiIdentifier ?: return + val line = document?.getLineNumber(id.textRange.startOffset)?.plus(1) ?: -1 + patchSites.add(PatchSite(id, vf.path, line, resolvesToFamily)) + } + } + }) + } + + if (patchSites.isEmpty()) return 0 + + WriteCommandAction.runWriteCommandAction(project) { + for (p in patchSites) { + if (!p.id.isValid) continue + val newId = factory.createIdentifier(newName) + p.id.replace(newId) + } + } + val resolvedCount = patchSites.count { it.viaResolvedFamily } + val fallbackCount = patchSites.size - resolvedCount + logger.info(" ↳ Post-rename safety net: patched ${patchSites.size} missed call site(s) for `$oldName` → `$newName`") + logger.info(" resolved-to-family: $resolvedCount, qualifier-fallback: $fallbackCount") + patchSites.take(10).forEach { p -> + val tag = if (p.viaResolvedFamily) "resolved-to-family" else "qualifier-fallback" + logger.info(" ${p.path}:${p.line} ($tag)") + } + if (patchSites.size > 10) { + logger.info(" ... (${patchSites.size - 10} more)") + } + return patchSites.size + } + + /** + * Checks if annotations pass the configured filter mode (whitelist or blacklist). + * + * @param annotations List of annotations to check + * @param filterMode "whitelist" or "blacklist" + * @param whitelistedAnnotations Annotations to allow (when mode = whitelist) + * @param blacklistedAnnotations Annotations to forbid (when mode = blacklist) + * @return true if annotations pass the filter, false otherwise + */ + private fun passesAnnotationFilter( + annotations: List, + filterMode: String, + whitelistedAnnotations: List, + blacklistedAnnotations: List, + ): Boolean { + if (annotations.isEmpty()) { + return true + } + + return when (filterMode.lowercase()) { + "whitelist" -> { + // All annotations must be in the whitelist + annotations.all { annotation -> + val qualifiedName = annotation.qualifiedName + val simpleName = qualifiedName?.substringAfterLast('.') + (qualifiedName != null) && (qualifiedName in whitelistedAnnotations || simpleName in whitelistedAnnotations) + } + } + "blacklist" -> { + // No annotations can be in the blacklist + annotations.none { annotation -> + val qualifiedName = annotation.qualifiedName + val simpleName = qualifiedName?.substringAfterLast('.') + qualifiedName in blacklistedAnnotations || simpleName in blacklistedAnnotations + } + } + else -> { + logger.warn(" ⚠ Unknown annotation filter mode: '$filterMode', defaulting to blacklist") + // Default to blacklist mode with empty list (allow all) + true + } + } + } + + /** + * Collects all methods from the PSI file without any filtering. + */ + private fun collectAllMethods(psiFile: PsiFile): List { val methods = mutableListOf() psiFile.accept(object : PsiRecursiveElementVisitor() { override fun visitElement(element: PsiElement) { @@ -310,59 +791,283 @@ class RenameMethodTransformation( } } }) + return methods + } - val filteredMethods = methods.filter { method -> - val psiClass = method.containingClass ?: return@filter false - val project = method.project - val fileIndex = ProjectFileIndex.getInstance(project) + /** + * Checks if a single method passes all filtering criteria. + * Returns true if the method should be included, false otherwise. + */ + private fun passesMethodFilters( + method: PsiMethod, + psiFile: PsiFile, + annotationFilterMode: String, + whitelistedMethodAnnotations: List, + blacklistedMethodAnnotations: List, + ): Boolean { + val psiClass = method.containingClass + if (psiClass == null) { + logger.info(" ⊘ Method `${method.name}` - skipped (no containing class)") + return false + } + + val project = method.project + val fileIndex = ProjectFileIndex.getInstance(project) - // If our interface extends a library interface, skip it. - if (psiClass.isInterface) { - val extendsLibraryInterface = psiClass.supers.any { superInterface -> - superInterface.containingFile?.virtualFile?.let { fileIndex.isInLibrary(it) } == true + // If our interface extends a library interface, skip it. + // FIX: Filter out java.lang.Object which is implicitly extended by all interfaces + if (psiClass.isInterface) { + val extendsLibraryInterface = psiClass.supers.any { superInterface -> + val qualifiedName = superInterface.qualifiedName + // Skip java.lang.Object (implicitly extended by all interfaces) + if (qualifiedName == "java.lang.Object") { + return@any false } - if (extendsLibraryInterface) return@filter false + superInterface.containingFile?.virtualFile?.let { fileIndex.isInLibrary(it) } == true } + if (extendsLibraryInterface) { + logger.info(" ⊘ Method `${method.name}` - skipped (interface extends library interface)") + return false + } + } - // Inheritance Guard: - // Catch methods that override methods - if (method.findSuperMethods().isNotEmpty()) return@filter false + // Note: Override check is now handled in findAllValidMethodFamilies() BEFORE grouping + // This prevents override methods from contaminating overload families with static methods - // Non-Code Usage Guard - val references = ReferencesSearch.search(method).findAll() - val usedInNonJavaFile = references.any { ref -> - val fileType = ref.element.containingFile.fileType.name - fileType != "JAVA" && fileType != "Kotlin" + // Non-Code Usage Guard + val references = ReferencesSearch.search(method).findAll() + val usedInNonJavaFile = references.any { ref -> + val fileType = ref.element.containingFile.fileType.name + fileType != "JAVA" && fileType != "Kotlin" + } + if (usedInNonJavaFile) { + logger.info(" ⊘ Method `${method.name}` - skipped (used in non-Java file)") + return false + } + + // Is not a test + if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) { + logger.info(" ⊘ Method `${method.name}` - skipped (in test source)") + return false + } + + // Annotation filter + val methodAnnotations = method.annotations.toList() + val annotationsPass = passesAnnotationFilter( + methodAnnotations, + annotationFilterMode, + whitelistedMethodAnnotations, + blacklistedMethodAnnotations + ) + + // Log annotation filtering for methods with annotations + if (methodAnnotations.isNotEmpty()) { + val annotationNames = methodAnnotations.mapNotNull { it.qualifiedName?.substringAfterLast('.') } + if (annotationsPass) { + val modeText = if (annotationFilterMode == "whitelist") "whitelisted" else "allowed" + logger.info(" ✓ Method `${method.name}` with annotations [${annotationNames.joinToString(", ")}] - $modeText") + } else { + val modeText = if (annotationFilterMode == "whitelist") "not whitelisted" else "blacklisted" + logger.info(" ⊘ Method `${method.name}` with annotations [${annotationNames.joinToString(", ")}] - skipped ($modeText)") + return false } - if (usedInNonJavaFile) return@filter false + } + + // Constructor check + if (method.isConstructor) { + logger.info(" ⊘ Method `${method.name}` - skipped (is constructor)") + return false + } + + // Disallowed method names + if (method.name in DISALLOWED_METHOD_NAMES) { + logger.info(" ⊘ Method `${method.name}` - skipped (disallowed method name)") + return false + } + + // Getter/setter/is prefix check + if (method.name.startsWith("get") || method.name.startsWith("set") || method.name.startsWith("is")) { + logger.info(" ⊘ Method `${method.name}` - skipped (getter/setter/is prefix)") + return false + } - // Public API Guard - if (method.hasModifierProperty(PsiModifier.PUBLIC) && references.isEmpty()) { - return@filter false + return true + } + + /** + * Filters overload families where ALL methods in the family pass the filters. + * If any method in a family fails a filter, the entire family is excluded. + */ + private fun filterValidFamilies( + families: List, + psiFile: PsiFile, + annotationFilterMode: String, + whitelistedMethodAnnotations: List, + blacklistedMethodAnnotations: List + ): List { + return families.filter { family -> + // Check if ALL methods in the family pass filters + val allMethodsValid = family.methods.all { method -> + passesMethodFilters( + method, + psiFile, + annotationFilterMode, + whitelistedMethodAnnotations, + blacklistedMethodAnnotations, + ) } - // Is not a test - if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) return@filter false + if (!allMethodsValid) { + logger.info(" ⊘ Overload family `${family.methodName}` (${family.methods.size} methods) - skipped (one or more methods filtered out)") + } - // Basic Filters - // either no method annotations or whitelisted ones only - val annotationsFilter = method.annotations.isEmpty() - || method.annotations.toList().allowedAnnotationsOnly(whitelistedMethodAnnotations) + allMethodsValid + } + } - annotationsFilter && - !method.isConstructor && - method.name !in DISALLOWED_METHOD_NAMES && - !method.name.startsWith("get") && - !method.name.startsWith("set") && - !method.name.startsWith("is") + /** + * Finds all valid method families in the PSI file. + * Returns overload families where ALL methods pass filtering criteria. + * + * @param psiFile The PSI file to search for methods + * @param annotationFilterMode "whitelist" or "blacklist" + * @param whitelistedMethodAnnotations A list of method annotations that are allowed (whitelist mode) + * @param blacklistedMethodAnnotations A list of method annotations that are forbidden (blacklist mode) + * @return List of valid overload families + */ + private fun findAllValidMethodFamilies( + psiFile: PsiFile, + annotationFilterMode: String, + whitelistedMethodAnnotations: List, + blacklistedMethodAnnotations: List, + ): List { + // Log annotation filter configuration + logger.info(" ↳ Annotation filter mode: $annotationFilterMode") + when (annotationFilterMode.lowercase()) { + "whitelist" -> { + if (whitelistedMethodAnnotations.isNotEmpty()) { + logger.info(" ↳ Whitelisted method annotations: ${whitelistedMethodAnnotations.joinToString(", ")}") + } else { + logger.info(" ↳ Whitelist mode active with empty list (only non-annotated methods allowed)") + } + } + "blacklist" -> { + if (blacklistedMethodAnnotations.isNotEmpty()) { + logger.info(" ↳ Blacklisted method annotations: [\n${blacklistedMethodAnnotations.joinToString(",\n") { "\t$it" } }\n]") + } else { + logger.info(" ↳ Blacklist mode active with empty list (all annotations allowed)") + } + } + } + + // Step 1: Collect all methods without filtering + val allMethods = collectAllMethods(psiFile) + + // Step 2: Filter out genuine overrides BEFORE grouping. + // Override methods must keep their original names to maintain inheritance contracts. + // + // Two guards beyond the obvious findSuperMethods() check: + // 1. Skip the check entirely for `static` methods — Java statics are + // not inherited, so findSuperMethods() for a static is a category + // error. The PSI implementation can return false positives when + // an unrelated type elsewhere in the project declares a method + // with the same name + erased parameter list (observed: fastjson + // v1-compat `com.alibaba.fastjson.JSON.toJSONString(Object)` + // reported as a super of the v2 static interface method, causing + // that overload to be silently dropped from the family). + // 2. For instance methods, require the matched super-method's + // containing class to be in the declared extends/implements chain + // of the owning class — not just any project-wide name match. + // Already inside an outer `withReadAction { findAllValidMethodFamilies(...) }` + // at the call site — nesting `IntelliJAwareTransformation.withReadAction` here + // would re-enter `runBlocking { readAction { } }` on the same thread that already + // holds a non-blocking read lock and deadlock against any queued write action. + val nonOverrideMethods = allMethods.filter { method -> + val isStatic = method.hasModifierProperty(PsiModifier.STATIC) + if (isStatic) { + return@filter true + } + val superMethods = method.findSuperMethods() + if (superMethods.isEmpty()) { + return@filter true + } + val ownerSupers = method.containingClass?.supers?.mapNotNull { it.qualifiedName }?.toSet().orEmpty() + val genuineOverride = superMethods.any { sm -> sm.containingClass?.qualifiedName in ownerSupers } + if (genuineOverride) { + val signature = PsiSignatureGenerator.generateSignature(method) + val ownerFqn = superMethods.firstOrNull { sm -> sm.containingClass?.qualifiedName in ownerSupers } + ?.containingClass?.qualifiedName + logger.info(" ⊘ Method `${method.name}` ($signature) - skipped (overrides super method from `$ownerFqn`)") + false + } else { + true + } } - if (filteredMethods.isNotEmpty()) { - // prettify filepath attempting to make it relative to the project root + logger.info(" ↳ Found ${allMethods.size} total methods, ${nonOverrideMethods.size} non-override methods") + + // Step 3: Group into overload families (now without overrides) + val allFamilies = groupMethodsByOverloads(nonOverrideMethods) + + // Analyze classes involved + val uniqueClasses = allFamilies.map { it.containingClass }.distinctBy { it.qualifiedName } + val classCount = uniqueClasses.size + + logger.info(" ↳ Grouped into ${allFamilies.size} overload families from $classCount class(es) (static/instance separate)") + + // Print family details grouped by class + if (allFamilies.isNotEmpty()) { + logger.info(" ↳ Overload families by class:") + + // Group families by containing class + val familiesByClass = allFamilies.groupBy { it.containingClass.qualifiedName ?: it.containingClass.name ?: "" } + + for ((className, families) in familiesByClass) { + val totalMethods = families.sumOf { it.methods.size } + logger.info(" ◆ Class: $className (${families.size} families, $totalMethods methods)") + + for (family in families) { + val isStatic = family.methods.firstOrNull()?.hasModifierProperty(PsiModifier.STATIC) + val modifier = when (isStatic) { + null -> "unknown" + true -> "static" + else -> "instance" + } + logger.info(" • ${family.methodName} [$modifier, ${family.methods.size} overload(s)]:") + + val signatures = family.methods.mapNotNull { method -> + PsiSignatureGenerator.generateSignature(method) + } + + val displayLimit = 10 + signatures.take(displayLimit).forEach { signature -> + logger.info(" $signature") + } + + if (signatures.size > displayLimit) { + val remaining = signatures.size - displayLimit + logger.info(" ... ($remaining more, ${signatures.size} total)") + } + } + } + } + + // Step 4: Filter families (all methods in family must pass remaining filters) + val validFamilies = filterValidFamilies( + allFamilies, + psiFile, + annotationFilterMode, + whitelistedMethodAnnotations, + blacklistedMethodAnnotations, + ) + + if (validFamilies.isNotEmpty()) { + val validMethodCount = validFamilies.sumOf { it.methods.size } val filepath = psiFile.virtualFile?.let { psiFile.project.relativeToRootOrAbsPath(it) } ?: "" - logger.info(" ↳ Found ${filteredMethods.size} matching methods in '$filepath'") + logger.info(" ↳ After filtering: ${validFamilies.size} valid families with $validMethodCount methods in '$filepath'") } - return filteredMethods + + return validFamilies } companion object { @@ -373,6 +1078,77 @@ class RenameMethodTransformation( "clone", "finalize", "wait", "notify", "notifyAll" ) + /** + * Default blacklisted method annotations (framework/infrastructure annotations). + * These annotations typically indicate methods that are called by frameworks/containers, + * so renaming them could break runtime behavior. + */ + val DEFAULT_BLACKLISTED_METHOD_ANNOTATIONS = setOf( + // JPA/Hibernate Lifecycle + "javax.persistence.PrePersist", + "javax.persistence.PostPersist", + "javax.persistence.PreUpdate", + "javax.persistence.PostUpdate", + "javax.persistence.PreRemove", + "javax.persistence.PostRemove", + "javax.persistence.PostLoad", + + // Spring Framework + "org.springframework.web.bind.annotation.RequestMapping", + "org.springframework.web.bind.annotation.GetMapping", + "org.springframework.web.bind.annotation.PostMapping", + "org.springframework.web.bind.annotation.PutMapping", + "org.springframework.web.bind.annotation.DeleteMapping", + "org.springframework.web.bind.annotation.PatchMapping", + "org.springframework.transaction.annotation.Transactional", + "org.springframework.scheduling.annotation.Scheduled", + "org.springframework.cache.annotation.Cacheable", + "org.springframework.cache.annotation.CacheEvict", + "org.springframework.context.event.EventListener", + "org.springframework.jmx.export.annotation.ManagedOperation", + + // JAX-RS (REST APIs) + "javax.ws.rs.GET", + "javax.ws.rs.POST", + "javax.ws.rs.PUT", + "javax.ws.rs.DELETE", + "javax.ws.rs.Path", + "jakarta.ws.rs.GET", + "jakarta.ws.rs.POST", + "jakarta.ws.rs.PUT", + "jakarta.ws.rs.DELETE", + "jakarta.ws.rs.Path", + + // Jackson (JSON) + "com.fasterxml.jackson.annotation.JsonGetter", + "com.fasterxml.jackson.annotation.JsonSetter", + "com.fasterxml.jackson.annotation.JsonProperty", + "com.fasterxml.jackson.annotation.JsonCreator", + + // JavaFX/Swing + "javafx.fxml.FXML", + + // JUnit/Testing Lifecycle + "org.junit.jupiter.api.BeforeEach", + "org.junit.jupiter.api.AfterEach", + "org.junit.jupiter.api.BeforeAll", + "org.junit.jupiter.api.AfterAll", + "org.junit.Test", + "org.junit.Before", + "org.junit.After", + "org.junit.BeforeClass", + "org.junit.AfterClass" + ) + private const val DEFAULT_SUGGESTED_NAMES_SIZE = 5 + + // Robustness knobs for the LLM batch call. + // LLM_BATCH_SIZE is smaller than the variable transformation's value (40) + // because each entry in the method prompt embeds a method body, so token + // budget per entry is materially higher. + private const val LLM_BATCH_SIZE = 30 + private const val LLM_MAX_ATTEMPTS = 3 + private const val LLM_RETRY_BACKOFF_BASE_MILLIS = 4_000L + private const val LLM_RETRY_BACKOFF_CAP_MILLIS = 12_000L } } \ No newline at end of file diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameVariableTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameVariableTransformation.kt index a331c629..1249576f 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameVariableTransformation.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/renaming/RenameVariableTransformation.kt @@ -16,6 +16,7 @@ import com.github.pderakhshanfar.codecocoonplugin.transformation.requireOrDefaul import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.readAction import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ProjectFileIndex @@ -23,6 +24,7 @@ import com.intellij.openapi.vfs.VirtualFile import com.intellij.psi.* import com.intellij.psi.util.PsiTreeUtil import com.intellij.refactoring.rename.RenameProcessor +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.serialization.Serializable @@ -46,16 +48,61 @@ class RenameVariableTransformation( val result = try { val useMemory = config.requireOrDefault("useMemory", defaultValue = false) val generateWhenNotInMemory = config.requireOrDefault("generateWhenNotInMemory", defaultValue = false) + val searchInComments = config.requireOrDefault("searchInComments", defaultValue = false) + + // Annotation filtering configuration (blacklist only - no whitelist support) + val blacklistedAnnotationsRaw = config.requireOrDefault>("blacklistedAnnotations", defaultValue = emptyList()) + + // Process blacklist: merge defaults if "_default" or "default" is present + val blacklistedAnnotations = if (blacklistedAnnotationsRaw.any { it.equals("_default", ignoreCase = true) || it.equals("default", ignoreCase = true) }) { + logger.info(" ↳ Include default blacklisted annotations ALONG with the custom ones (i.e., '_default' or 'default' keyword in the list)") + + val customAnnotations = blacklistedAnnotationsRaw.filter { !it.equals("_default", ignoreCase = true) && !it.equals("default", ignoreCase = true) } + (DEFAULT_BLACKLISTED_VARIABLE_ANNOTATIONS + customAnnotations).toList() + } else { + // Warn if using blacklist mode without defaults + if (blacklistedAnnotationsRaw.isNotEmpty()) { + logger.warn(" ⚠ Blacklist provided without '_default' keyword - framework annotations will NOT be automatically excluded") + } + blacklistedAnnotationsRaw + } val document = withReadAction { psiFile.document() } val modifiedFiles = mutableSetOf() val value = if (document != null) { - val eligibleVariables: List = withReadAction { findAllValidVariables(psiFile) } + val eligibleVariables: List = withReadAction { + findAllValidVariables(psiFile, blacklistedAnnotations) + } if (eligibleVariables.isEmpty()) { return TransformationResult.Skipped("No matching variables found in ${virtualFile.name}") } + // Snapshot every variable's signature BEFORE any rename happens. Regenerating + // the signature mid-loop is unsafe: when a field is renamed, IntelliJ's + // RenameProcessor cascade-renames the constructor parameter bound to it via + // `this.x = x`, so a later `psiVar.name` for that parameter would already be + // the post-cascade name and the memory key would be wrong. + val signatures: Map = withReadAction { + eligibleVariables.mapNotNull { psiVar -> + PsiSignatureGenerator.generateSignature(psiVar)?.let { psiVar to it } + }.toMap() + } + + // Process inner scopes first so the field-rename cascade has nothing to + // latch onto: by the time a PsiField is renamed, any constructor parameter + // that used to share its name has already been renamed to its own + // LLM-suggested name, so RenameJavaFieldProcessor's "rename the bound + // parameter" condition (matching name) no longer holds. + val orderedVariables = eligibleVariables.sortedBy { v -> + when (v) { + is PsiLocalVariable -> 0 + is PsiParameter -> 1 + is PsiField -> 2 + else -> 3 + } + } + logger.info(" ⏲ Generating rename suggestions for ${eligibleVariables.size} variables...") val renaming = runBlocking { @@ -71,12 +118,11 @@ class RenameVariableTransformation( val saveRenamesInMemory = !useMemory || generateWhenNotInMemory // Try renaming each variable with suggestions until one succeeds - val successfulRenames = eligibleVariables.mapNotNull { psiVar -> + val successfulRenames = orderedVariables.mapNotNull { psiVar -> val varName = withReadAction { psiVar.name } val suggestions = renaming.suggestions[psiVar] ?: return@mapNotNull null - // Generate signature BEFORE renaming - val signature = withReadAction { PsiSignatureGenerator.generateSignature(psiVar) } + val signature = signatures[psiVar] if (signature == null) { logger.warn(" ⊘ Could not generate signature for variable $varName") return@mapNotNull null @@ -84,7 +130,7 @@ class RenameVariableTransformation( // Try each suggestion until one succeeds (no conflicts) for (suggestion in suggestions) { - val files = tryRenameVariableAndUsages(psiFile.project, psiVar, suggestion) + val files = tryRenameVariableAndUsages(psiFile.project, psiVar, suggestion, searchInComments) if (files != null) { modifiedFiles.addAll(files) if (saveRenamesInMemory) { @@ -266,6 +312,41 @@ class RenameVariableTransformation( ): List { if (variables.isEmpty()) return emptyList() + // Split variables into chunks so a single transient LLM failure (timeout, + // malformed JSON, token-budget overflow on huge files) doesn't lose the + // suggestions for the entire file. Files with <= LLM_BATCH_SIZE variables + // still produce one LLM call, matching the prior single-call behaviour. + val batches = variables.chunked(LLM_BATCH_SIZE) + if (batches.size > 1) { + logger.info(" ↳ Splitting ${variables.size} variables into ${batches.size} LLM batches of up to $LLM_BATCH_SIZE") + } + + val combined = mutableListOf() + for ((index, batch) in batches.withIndex()) { + val partial = generateBatchWithRetry( + variables = batch, + count = count, + batchLabel = "${index + 1}/${batches.size}", + ) + combined.addAll(partial) + } + return combined + } + + /** + * Calls the LLM for a single batch of variables, retrying transient errors + * with exponential backoff. Returns an empty list if every retry fails — the + * caller's per-variable loop then skips those variables (no rename, no memory + * write) instead of failing the whole transformation, so other batches and + * other files still progress. + * + * Rethrows [ProcessCanceledException] per IntelliJ contract. + */ + private suspend fun generateBatchWithRetry( + variables: List, + count: Int, + batchLabel: String, + ): List { val contexts = readAction { variables.map { buildVariableContext(it) } } val varRenamePrompt = prompt("variable-rename-batch-prompt") { @@ -296,37 +377,55 @@ class RenameVariableTransformation( } } - val llm = LLM.fromGrazie(OpenAIModels.Chat.GPT5Mini) - val result = llm.structuredRequest( - prompt = varRenamePrompt - ) - return result?.renamings ?: emptyList() + var lastError: Throwable? = null + for (attempt in 1..LLM_MAX_ATTEMPTS) { + try { + val result = llm.structuredRequest(prompt = varRenamePrompt) + return result?.renamings ?: emptyList() + } catch (e: ProcessCanceledException) { + throw e + } catch (e: Exception) { + lastError = e + logger.warn(" ⚠ LLM call for variable batch $batchLabel failed (attempt $attempt/$LLM_MAX_ATTEMPTS): ${e.message}") + if (attempt < LLM_MAX_ATTEMPTS) { + delay(retryBackoffMillis(attempt)) + } + } + } + logger.warn(" ✗ LLM call for variable batch $batchLabel exhausted $LLM_MAX_ATTEMPTS attempts; skipping batch (last error: ${lastError?.message})") + return emptyList() + } + + private fun retryBackoffMillis(attempt: Int): Long { + // 1s, 2s, 4s, ... capped at LLM_RETRY_BACKOFF_CAP_MILLIS + val base = LLM_RETRY_BACKOFF_BASE_MILLIS shl (attempt - 1).coerceAtLeast(0) + return base.coerceAtMost(LLM_RETRY_BACKOFF_CAP_MILLIS) } private fun tryRenameVariableAndUsages( - project: Project, psiVariable: PsiVariable, newName: String + project: Project, + psiVariable: PsiVariable, + newName: String, + searchInComments: Boolean, ): MutableSet? { return try { val oldName = withReadAction { psiVariable.name } ?: return null - // isSearchInComments needs to be false. If true, it would breaks functionality by changing string literals. - // example would be mappings of `PathVariable` from Spring. - // `@param [paramName]` definitions in the Javadocs are still being renamed. val renameProcessor = withReadAction { RenameProcessor( /* project = */ project, /* element = */ psiVariable, /* newName = */ newName, - /* isSearchInComments= */ false, + /* isSearchInComments= */ searchInComments, /* isSearchTextOccurrences = */ false ) } - ApplicationManager.getApplication().invokeAndWait { - PsiDocumentManager.getInstance(project).commitAllDocuments() - renameProcessor.run() - } - + // Snapshot modified files BEFORE run(): findUsages() must run on + // the pre-rename PSI to return the references that will actually + // be rewritten. After run() the seed element has been renamed and + // the result is unreliable — that previously caused the Javadoc + // `@param` tag to be rewritten in some morph runs but not others. val modifiedFiles = withReadAction { val files = mutableSetOf() renameProcessor.findUsages().forEach { usageInfo -> @@ -336,6 +435,17 @@ class RenameVariableTransformation( files } + ApplicationManager.getApplication().invokeAndWait { + PsiDocumentManager.getInstance(project).commitAllDocuments() + renameProcessor.run() + // Lock in PSI/document/disk state immediately so subsequent renames + // (and the final project close) don't trigger close-time hooks whose + // behaviour depends on accumulated unflushed state — that previously + // produced non-deterministic import positions across morph runs. + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() + } + val fileCountString = if (modifiedFiles.size > 1) " in ${modifiedFiles.size} files" else "" logger.info(" • Renamed `$oldName` to `$newName`$fileCountString") modifiedFiles @@ -350,15 +460,109 @@ class RenameVariableTransformation( } } + /** + * Checks if annotations pass the blacklist filter. + * Variables renaming supports ONLY blacklist mode (no whitelist). + * + * @param annotations List of annotations to check + * @param blacklistedAnnotations Annotations to forbid (blacklist mode) + * @return true if annotations pass the filter (none are blacklisted), false otherwise + */ + private fun passesAnnotationFilter( + annotations: List, + blacklistedAnnotations: List, + ): Boolean { + if (annotations.isEmpty()) { + return true + } + + // Blacklist mode: No annotations can be in the blacklist + return annotations.none { annotation -> + val qualifiedName = annotation.qualifiedName + val simpleName = qualifiedName?.substringAfterLast('.') + qualifiedName in blacklistedAnnotations || simpleName in blacklistedAnnotations + } + } + + /** + * Checks if a single variable passes all filtering criteria. + * Returns true if the variable should be included, false otherwise. + */ + private fun passesVariableFilters( + variable: PsiVariable, + psiFile: PsiFile, + blacklistedVariableAnnotations: List, + ): Boolean { + val fileIndex = ProjectFileIndex.getInstance(psiFile.project) + + // 1. Exclude Test Sources + if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) { + logger.info(" ⊘ Variable `${variable.name}` - skipped (in test source)") + return false + } + + // 2. Exclude Enum Constants + if (variable is PsiEnumConstant) { + logger.info(" ⊘ Variable `${variable.name}` - skipped (is enum constant)") + return false + } + + // 3. Annotation filter (blacklist mode only) + val variableAnnotations = variable.annotations.toList() + val annotationsPass = passesAnnotationFilter( + variableAnnotations, + blacklistedVariableAnnotations + ) + + // Log annotation filtering for variables with annotations + if (variableAnnotations.isNotEmpty()) { + val annotationNames = variableAnnotations.mapNotNull { it.qualifiedName?.substringAfterLast('.') } + if (annotationsPass) { + logger.info(" ✓ Variable `${variable.name}` with annotations [${annotationNames.joinToString(", ")}] - allowed (not blacklisted)") + } else { + logger.info(" ⊘ Variable `${variable.name}` with annotations [${annotationNames.joinToString(", ")}] - skipped (blacklisted)") + return false + } + } + + // 4. Exclude Library/Compiled Code + if (variable is PsiCompiledElement || !variable.isPhysical) { + logger.info(" ⊘ Variable `${variable.name}` - skipped (compiled or non-physical)") + return false + } + + // 5. Exclude public/protected fields (could cause external breaking changes) + if (variable is PsiField) { + if (variable.hasModifierProperty(PsiModifier.PUBLIC) || variable.hasModifierProperty(PsiModifier.PROTECTED)) { + logger.info(" ⊘ Variable `${variable.name}` - skipped (public/protected field)") + return false + } + } + + return true + } + /** * Identifies and filters valid variables from the provided PSI file based on specific criteria. - * The filtering logic excludes variables in test sources, enum constants, variables annotated with `@Column`, + * The filtering logic excludes variables in test sources, enum constants, blacklisted annotations, * variables from library or compiled code, and public/protected fields that could cause external breaking changes. * * @param psiFile The PSI file to traverse and analyze for variables. + * @param blacklistedVariableAnnotations Annotations to exclude (blacklist mode). * @return A list of PSI variables matching all filtering criteria. */ - private fun findAllValidVariables(psiFile: PsiFile): List { + private fun findAllValidVariables( + psiFile: PsiFile, + blacklistedVariableAnnotations: List, + ): List { + // Log annotation filter configuration + if (blacklistedVariableAnnotations.isNotEmpty()) { + logger.info(" ↳ Annotation filter mode: BLACKLIST") + logger.info(" ↳ Blacklisted variable annotations: [\n${blacklistedVariableAnnotations.joinToString(",\n") { "\t$it" } }\n]") + } else { + logger.info(" ↳ Annotation filter mode: BLACKLIST (empty - all annotations allowed)") + } + val variables = mutableListOf() psiFile.accept(object : PsiRecursiveElementVisitor() { @@ -370,33 +574,8 @@ class RenameVariableTransformation( } }) - val fileIndex = ProjectFileIndex.getInstance(psiFile.project) - val filteredVariables = variables.filter { v -> - // 1. Exclude Test Sources - if (fileIndex.isInTestSourceContent(psiFile.virtualFile)) return@filter false - - // 2. Exclude Enum Constants - if (v is PsiEnumConstant) return@filter false - - // 3. Exclude @Column annotated variables - if (v.annotations.any { it.qualifiedName?.contains("Column") == true }) return@filter false - - // 4. Exclude Library/Compiled Code - if (v !is PsiCompiledElement && v.isPhysical) { - // 5. Overrides Check (for fields/parameters) - // If a field overrides a superclass field, renaming it might break polymorphism or hide fields. - // Simple heuristic: Only rename private/package-private fields or local vars to stay safe. - if (v is PsiField) { - if (v.hasModifierProperty(PsiModifier.PUBLIC) || v.hasModifierProperty(PsiModifier.PROTECTED)) { - // Skip public/protected fields to avoid breaking external consumers or overrides - return@filter false - } - } - true - } else { - false - } + passesVariableFilters(v, psiFile, blacklistedVariableAnnotations) } if (filteredVariables.isNotEmpty()) { @@ -410,5 +589,85 @@ class RenameVariableTransformation( companion object { const val ID = "rename-variable-transformation" private const val DEFAULT_SUGGESTED_NAMES_SIZE = 3 + + // Robustness knobs for the LLM batch call. + // LLM_BATCH_SIZE chosen so files with up to a few dozen variables still + // produce one call (preserving the prior single-call behaviour) while + // very large files are split, so a single transient failure no longer + // wipes out the whole file's renames. + private const val LLM_BATCH_SIZE = 40 + private const val LLM_MAX_ATTEMPTS = 3 + private const val LLM_RETRY_BACKOFF_BASE_MILLIS = 2_000L + private const val LLM_RETRY_BACKOFF_CAP_MILLIS = 8_000L + + /** + * Default blacklisted variable annotations (framework/infrastructure annotations). + * These annotations typically indicate variables that are mapped to external systems, + * so renaming them could break runtime behavior or data binding. + */ + val DEFAULT_BLACKLISTED_VARIABLE_ANNOTATIONS = setOf( + // JPA/Hibernate + "javax.persistence.Column", + "javax.persistence.Id", + "javax.persistence.GeneratedValue", + "javax.persistence.Version", + "javax.persistence.Temporal", + "javax.persistence.Enumerated", + "javax.persistence.Lob", + "javax.persistence.Basic", + "javax.persistence.EmbeddedId", + "javax.persistence.JoinColumn", + "jakarta.persistence.Column", + "jakarta.persistence.Id", + "jakarta.persistence.GeneratedValue", + "jakarta.persistence.Version", + "jakarta.persistence.Temporal", + "jakarta.persistence.Enumerated", + "jakarta.persistence.Lob", + "jakarta.persistence.Basic", + "jakarta.persistence.EmbeddedId", + "jakarta.persistence.JoinColumn", + + // Jackson (JSON) + "com.fasterxml.jackson.annotation.JsonProperty", + "com.fasterxml.jackson.annotation.JsonIgnore", + "com.fasterxml.jackson.annotation.JsonAlias", + + // JAXB (XML) + "javax.xml.bind.annotation.XmlElement", + "javax.xml.bind.annotation.XmlAttribute", + "javax.xml.bind.annotation.XmlTransient", + "javax.xml.bind.annotation.XmlID", + "jakarta.xml.bind.annotation.XmlElement", + "jakarta.xml.bind.annotation.XmlAttribute", + "jakarta.xml.bind.annotation.XmlTransient", + "jakarta.xml.bind.annotation.XmlID", + + // Spring Framework + "org.springframework.beans.factory.annotation.Value", + "org.springframework.beans.factory.annotation.Autowired", + "org.springframework.beans.factory.annotation.Qualifier", + "javax.annotation.Resource", + + // Bean Validation + "javax.validation.constraints.NotNull", + "javax.validation.constraints.Size", + "javax.validation.constraints.Min", + "javax.validation.constraints.Max", + "javax.validation.constraints.Pattern", + "javax.validation.constraints.Email", + "jakarta.validation.constraints.NotNull", + "jakarta.validation.constraints.Size", + "jakarta.validation.constraints.Min", + "jakarta.validation.constraints.Max", + "jakarta.validation.constraints.Pattern", + "jakarta.validation.constraints.Email", + + // CDI + "javax.inject.Inject", + "javax.inject.Named", + "jakarta.inject.Inject", + "jakarta.inject.Named" + ) } } \ No newline at end of file diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/MoveFileIntoSuggestedDirectoryTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/MoveFileIntoSuggestedDirectoryTransformation.kt similarity index 79% rename from src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/MoveFileIntoSuggestedDirectoryTransformation.kt rename to src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/MoveFileIntoSuggestedDirectoryTransformation.kt index ffcbac03..368f3bc0 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/MoveFileIntoSuggestedDirectoryTransformation.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/MoveFileIntoSuggestedDirectoryTransformation.kt @@ -1,9 +1,10 @@ -package com.github.pderakhshanfar.codecocoonplugin.components.transformations +package com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural import com.github.pderakhshanfar.codecocoonplugin.common.TransformationStepFailed import com.github.pderakhshanfar.codecocoonplugin.components.transformations.IntelliJAwareTransformation.Companion.withReadAction -import com.github.pderakhshanfar.codecocoonplugin.components.transformations.MoveFileIntoSuggestedDirectoryTransformation.Companion.withAI -import com.github.pderakhshanfar.codecocoonplugin.components.transformations.MoveFileIntoSuggestedDirectoryTransformation.Companion.withConfig +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.SelfManagedTransformation +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural.MoveFileIntoSuggestedDirectoryTransformation.Companion.withAI +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural.MoveFileIntoSuggestedDirectoryTransformation.Companion.withConfig import com.github.pderakhshanfar.codecocoonplugin.executor.TransformationResult import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.declarations @@ -15,6 +16,7 @@ import com.github.pderakhshanfar.codecocoonplugin.transformation.requireOrDefaul import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.command.WriteCommandAction import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.progress.ProcessCanceledException import com.intellij.openapi.project.Project import com.intellij.openapi.roots.ProjectFileIndex @@ -25,9 +27,13 @@ import com.intellij.psi.search.searches.ReferencesSearch import com.intellij.refactoring.move.MoveCallback import com.intellij.refactoring.move.moveFilesOrDirectories.MoveFilesOrDirectoriesProcessor import com.intellij.usageView.UsageInfo +import com.intellij.util.containers.MultiMap import kotlinx.coroutines.runBlocking import java.nio.file.Paths import java.util.concurrent.CompletableFuture +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException +import kotlin.collections.iterator /** @@ -108,7 +114,7 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( val suggestedDirectories = result.getOrThrow() logger.info(" • Received ${suggestedDirectories.size} directory suggestions") - return tryToMoveFileIntoSuggestedDirectory( + tryToMoveFileIntoSuggestedDirectory( project = psiFile.project, fileToMove = psiFile, suggestions = suggestedDirectories, @@ -193,6 +199,10 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( val projectRoot = project.basePath ?: return TransformationResult.Failure("Project root not found") + val currentParent = withReadAction { + Paths.get(fileToMove.virtualFile.parent.path).normalize().toString() + } + logger.info(" ⏲ Attempting to move $filename into suggestions...") for ((index, suggestionPath) in suggestions.withIndex()) { logger.info(" ↳ Attempting suggestion #${index + 1}: '$suggestionPath'") @@ -208,6 +218,13 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( suggestionPath } + // skip suggestions pointing at the file's current package — would be a no-op move + // and would pollute memory with a self-pointing entry. + if (Paths.get(suggestion).normalize().toString() == currentParent) { + logger.warn(" ⚠ Skipping suggestion #${index + 1}: '$suggestion' equals the current package/directory of '$filename'") + continue + } + val suggestedDirectory = WriteCommandAction.runWriteCommandAction(project) { VfsUtil.createDirectories(suggestion) } @@ -225,8 +242,14 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( searchInNonJavaFiles = false, moveCallback = { // overriding `refactoringCompleted` method + logger.info(" • Move processor called `moveCallback`: setting `successfullyMoved=true`") successfullyMoved.complete(true) }, + onConflictsFoundCallback = { + // set `successfullyMoved` to false -> unsuccessful/aborted move operation + logger.warn(" ✗ Move processor called `onConflictsFoundCallback`: setting `successfullyMoved=false`") + successfullyMoved.complete(false) + }, prepareSuccessfulCallback = { /* no-op */ }, ) } @@ -234,17 +257,39 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( try { ApplicationManager.getApplication().invokeAndWait { PsiDocumentManager.getInstance(project).commitAllDocuments() + logger.info(" ↳ Move processor starts running...") processor.run() + logger.info(" • Move processor finished running") + // Lock in PSI/document/disk state immediately so subsequent + // transformations (and the final project close) don't trigger + // close-time hooks whose behaviour depends on accumulated + // unflushed state — a move propagates package declarations and + // import updates across every referencing file, the same cascade + // pattern as a rename. + PsiDocumentManager.getInstance(project).commitAllDocuments() + FileDocumentManager.getInstance().saveAllDocuments() } } catch (err: ProcessCanceledException) { // NOTE: `ProcessCanceledException` cannot be silenced, see its Javadoc throw err } catch (err: Exception) { - logger.error("Failed to move '$filename' into suggestion #${index + 1}", err) + logger.error(" ✗ Suggestion #${index + 1} for '$filename' failed: ${err.message}; trying next suggestion", err) + // unblock the join below so the loop can advance to the next suggestion + successfullyMoved.complete(false) } // finish when moved successfully into the current suggestion - if (successfullyMoved.join()) { + logger.info(" ↳ Awaiting completion of move operation (i.e., `successfullyMoved.get(3min)`)...") + + val moveResult = try { + successfullyMoved.get(3, TimeUnit.MINUTES) + } catch (_: TimeoutException) { + logger.warn(" ⚠️ [TIMEOUT] Suggestion #${index + 1} for '$filename' timed out after 3 minutes; moving on to next suggestion") + false + } + + logger.info(" ↳ Move operation for suggestion #${index + 1} for '$filename' ${if (moveResult) "succeeded" else "failed"}: '$suggestedDirectory'") + if (moveResult) { val (filesModified, usageSummary) = withReadAction { val usages = processor.foundUsages @@ -283,10 +328,15 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( } } - // no suggestions fit - logger.info(" ✗ Failed to move $filename: None of ${suggestions.size} suggestions fit") - return TransformationResult.Failure( - "Failed to move $filename into any of ${suggestions.size} suggested directories:\n${suggestions.joinToString("\n") { " - $it" }}") + // No suggestion fit — every iteration above set `successfullyMoved` to false + // (conflict callback / exception in processor.run() / 3-minute timeout). Treat this + // as Skipped rather than Failure: the input file is well-formed, suggestions were + // obtained, the move processor just rejected every target. Genuine failures + // (non-Java input, missing project root, suggestion-API error) still return Failure + // upstream of this loop. + logger.info(" ⊘ Skipped moving $filename: move processor rejected all ${suggestions.size} suggestion(s)") + return TransformationResult.Skipped( + "Skipped moving $filename: move processor rejected all ${suggestions.size} suggested directories (conflicts/exceptions/timeouts):\n${suggestions.joinToString("\n") { " - $it" }}") } /** @@ -370,6 +420,8 @@ class MoveFileIntoSuggestedDirectoryTransformation private constructor( /** * This wrapper delegates ALL methods to [MoveFilesOrDirectoriesProcessor]. * It only exposes a protected [myFoundUsages] variable for enriched logging. + * + * @param onConflictsFoundCallback called when conflicts list is NOT empty in [showConflicts] method. */ private class MoveFilesOrDirectoriesProcessorWrapper( project: Project, @@ -378,6 +430,7 @@ private class MoveFilesOrDirectoriesProcessorWrapper( searchInComments: Boolean, searchInNonJavaFiles: Boolean, moveCallback: MoveCallback, + private val onConflictsFoundCallback: Runnable, prepareSuccessfulCallback: Runnable ) : MoveFilesOrDirectoriesProcessor( project, @@ -390,6 +443,21 @@ private class MoveFilesOrDirectoriesProcessorWrapper( ) { val foundUsages: Map> get() = myFoundUsages + + // BaseRefactoringProcessor.showConflicts throws ConflictsInTestsException in headless/test + // mode. Convert that into a graceful abort: log the conflicts and tell the base processor + // to skip the refactor (return false) — the calling loop then advances to the next suggestion. + override fun showConflicts(conflicts: MultiMap, usages: Array?): Boolean { + if (!conflicts.isEmpty) { + // running callback on failed move operation + onConflictsFoundCallback.run() + + val conflictStr = conflicts.values().joinToString("\n") { " - $it;" } + thisLogger().withStdout().warn(" ⚠ Move blocked by ${conflicts.size()} conflict(s):\n'''\n$conflictStr\n'''") + return false + } + return super.showConflicts(conflicts, usages) + } } /** diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/ReorderClassMethodsTransformation.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/ReorderClassMethodsTransformation.kt new file mode 100644 index 00000000..77583ba0 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/ReorderClassMethodsTransformation.kt @@ -0,0 +1,207 @@ +package com.github.pderakhshanfar.codecocoonplugin.components.transformations.structural + +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.IntelliJAwareTransformation +import com.github.pderakhshanfar.codecocoonplugin.components.transformations.SelfManagedTransformation +import com.github.pderakhshanfar.codecocoonplugin.executor.TransformationResult +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.github.pderakhshanfar.codecocoonplugin.intellij.psi.document +import com.github.pderakhshanfar.codecocoonplugin.java.JavaTransformation +import com.github.pderakhshanfar.codecocoonplugin.memory.Memory +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.command.WriteCommandAction +import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager +import com.intellij.openapi.progress.ProcessCanceledException +import com.intellij.openapi.vfs.VirtualFile +import com.intellij.psi.PsiAnonymousClass +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiCompiledElement +import com.intellij.psi.PsiDocumentManager +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiFile +import com.intellij.psi.PsiJavaFile +import com.intellij.psi.PsiMethod +import com.intellij.psi.PsiRecursiveElementVisitor + +/** + * Reorders methods in a class in a *reverse alphabetic order* (Z -> A). + */ +class ReorderClassMethodsTransformation( + override val config: Map +) : JavaTransformation, SelfManagedTransformation() { + override val id: String = ID + override val description: String = "Reorders methods in a class in reverse alphabetic order (Z -> A)" + private val logger = thisLogger().withStdout() + + override fun apply( + psiFile: PsiFile, + virtualFile: VirtualFile, + memory: Memory? + ): TransformationResult { + return try { + if (psiFile !is PsiJavaFile) { + return TransformationResult.Skipped("File ${virtualFile.name} is not a Java file") + } + + val project = psiFile.project + val classes = IntelliJAwareTransformation.withReadAction { collectAllClasses(psiFile) } + + if (classes.isEmpty()) { + return TransformationResult.Skipped("No classes found in ${virtualFile.name}") + } + + var reorderedClassCount = 0 + var totalMethodsTouched = 0 + + ApplicationManager.getApplication().invokeAndWait { + WriteCommandAction.runWriteCommandAction(project, "Reorder Class Methods", null, { + PsiDocumentManager.getInstance(project).commitAllDocuments() + + for (psiClass in classes) { + try { + val allMethods = psiClass.methods.toList() + val methods = allMethods.filter { it.isPhysical && it !is PsiCompiledElement } + val droppedMethods = allMethods - methods.toSet() + if (droppedMethods.isNotEmpty()) { + val droppedNames = droppedMethods.joinToString(", ") { + val reason = when { + it is PsiCompiledElement -> "compiled" + !it.isPhysical -> "non-physical" + else -> "filtered" + } + "${it.name} ($reason)" + } + logger.info( + " ⊘ Class `${psiClass.className}` - dropped ${droppedMethods.size} method(s) " + + "before reorder: $droppedNames" + ) + } + if (methods.size < 2) { + logger.warn(" ⊘ Class `${psiClass.className}` - has ${methods.size} reorderable method(s) (skipping)") + continue + } + + val sortedMethods = reorderMethods(methods) + + if (sortedMethods.map { it.name } == methods.map { it.name }) { + logger.info(" ⊘ Class `${psiClass.className}` - methods already in desired order") + continue + } + + val rBrace = psiClass.rBrace + if (rBrace == null) { + logger.warn(" ⊘ Class `${psiClass.className}` - no closing brace, skipping") + continue + } + + // Pre-validate that every method can be copied. PsiElement.copy() returns null + // for non-copyable elements (synthetic / PsiAugmentProvider-injected light methods, + // etc.), and addBefore(null, ...) would crash with @NotNull violation. Doing this + // before any mutation avoids leaving the class in a half-deleted / half-added state. + val copies = sortedMethods.map { it to it.copy() as? PsiMethod } + val firstNullCopy = copies.firstOrNull { it.second == null } + if (firstNullCopy != null) { + logger.warn( + " ⊘ Class `${psiClass.className}` - method " + + "`${firstNullCopy.first.name}` is non-copyable (likely synthetic / " + + "augmented PSI); skipping class to avoid partial reorder" + ) + continue + } + + // add sorted methods into class + for ((_, copy) in copies) { + psiClass.addBefore(copy!!, rBrace) + } + // remove original methods + for (method in sortedMethods) { + method.delete() + } + + reorderedClassCount += 1 + totalMethodsTouched += methods.size + logger.info(" ✓ Class `${psiClass.className}` - reordered ${methods.size} methods") + } + catch (err: ProcessCanceledException) { + throw err + } + catch (e: Exception) { + logger.error(" ✗ Class `${psiClass.className}` - failed to reorder methods: ${e.message}", e) + } + } + + val document = psiFile.document() + if (document != null) { + PsiDocumentManager.getInstance(project).commitDocument(document) + FileDocumentManager.getInstance().saveDocument(document) + } else { + logger.warn(" ⚠ Could not get document for ${virtualFile.name}; changes may not be flushed to disk") + } + }) + } + + if (reorderedClassCount == 0) { + TransformationResult.Skipped("Nothing to reorder in ${virtualFile.name}") + } else { + TransformationResult.Success( + message = "Reordered $totalMethodsTouched methods across $reorderedClassCount class(es) in ${virtualFile.name}", + filesModified = 1, + ) + } + } + catch (err: ProcessCanceledException) { + throw err + } + catch (e: Exception) { + TransformationResult.Failure("Failed to reorder methods in ${virtualFile.name}", e) + } + } + + /** + * Returns methods in the desired order. Reverse-alphabetical (Z → A) for now. + * Future config params will be wired here to switch strategies. + */ + private fun reorderMethods(methods: List): List = + methods.sortedByDescending { it.name } + + private fun collectAllClasses(psiFile: PsiFile): List { + val classes = mutableListOf() + psiFile.accept(object : PsiRecursiveElementVisitor() { + override fun visitElement(element: PsiElement) { + super.visitElement(element) + if (element !is PsiClass) { + return + } + // TODO(NOTE): don't skip anonymous classes for eval (possibly, more transformations with anonymous classes included) + // Skip anonymous classes (visited inside method bodies) — reordering their + // methods is not the user's intent and they often have non-copyable PSI. + /*if (element is PsiAnonymousClass) { + logger.info(" ⊘ Skipping anonymous class inside ${psiFile.name}") + return + } + if (element.name == null) { + logger.info(" ⊘ Skipping unnamed class-like element in ${psiFile.name}") + return + }*/ + // Skip compiled / non-physical classes (mirror RenameVariableTransformation). + if (element is PsiCompiledElement) { + logger.info(" ⊘ Skipping compiled class `${element.name}` in ${psiFile.name}") + return + } + if (!element.isPhysical) { + logger.info(" ⊘ Skipping non-physical class `${element.name}` in ${psiFile.name}") + return + } + classes.add(element) + } + }) + return classes + } + + private val PsiClass.className: String + get() = this.name ?: "[anonymous-class]" + + companion object { + const val ID = "reorder-class-methods-transformation" + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/CodeCocoonConfig.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/CodeCocoonConfig.kt index 57c2ff1c..5f8b3c94 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/CodeCocoonConfig.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/CodeCocoonConfig.kt @@ -13,8 +13,8 @@ import com.intellij.openapi.vfs.VirtualFile val files: List = emptyList(), /** Ordered list of transformations to execute */ val transformations: List = emptyList(), - /** Directory where memory files are stored (resolved to absolute path by ConfigLoader) */ - val memoryDir: String, + /** Full path to the memory JSON file (resolved to absolute path by ConfigLoader) */ + val memoryFilepath: String, ) /** diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/ConfigLoader.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/ConfigLoader.kt index f569daf3..a0834a69 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/ConfigLoader.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/config/ConfigLoader.kt @@ -48,8 +48,8 @@ object ConfigLoader { TransformationConfig(id = id, config = cfg) } - // Resolve memory directory - val memoryDir = resolveMemoryDir(root["memoryDir"]?.toString()) + // Resolve memory file path + val memoryFilepath = resolveMemoryFilepath(root["memoryFilepath"]?.toString()) // if projectRoot is present, try to search for the corresponding virtual file val projectRootFile = projectRoot?.refreshAndFindVirtualFile() @@ -59,25 +59,25 @@ object ConfigLoader { projectRootFile = projectRootFile, files = files, transformations = transformations, - memoryDir = memoryDir, + memoryFilepath = memoryFilepath, ) } } /** - * Resolves the memory directory to an absolute path string. + * Resolves the memory JSON file path to an absolute path string. * - * If [memoryDirPath] is provided: + * If [memoryFilepath] is provided: * - If absolute: use as-is * - If relative: resolve relative to config file's parent directory * - * If [memoryDirPath] is null: - * - Default to ".codecocoon-memory" in config file's parent directory + * If [memoryFilepath] is null: + * - Default to ".codecocoon-memory.json" in config file's parent directory * - * @param memoryDirPath Optional memory directory path from YAML - * @return Resolved absolute path for memory directory + * @param memoryFilepath Optional memory file path from YAML + * @return Resolved absolute path to the memory JSON file */ - private fun resolveMemoryDir(memoryDirPath: String?): String { + private fun resolveMemoryFilepath(memoryFilepath: String?): String { val configPath = System.getProperty("codecocoon.config") ?: throw IllegalStateException("codecocoon.config system property not set") @@ -85,16 +85,17 @@ object ConfigLoader { val configParentDir = configFile.parentFile ?: throw IllegalStateException("Config file has no parent directory: $configPath") - return if (memoryDirPath != null) { - val memoryFile = File(memoryDirPath) + return if (memoryFilepath != null) { + val memoryFile = File(memoryFilepath) if (memoryFile.isAbsolute) { memoryFile.canonicalPath } else { - File(configParentDir, memoryDirPath).canonicalPath + File(configParentDir, memoryFilepath).canonicalPath } } else { - // Default to .codecocoon-memory in config parent directory - File(configParentDir, ".codecocoon-memory").canonicalPath + // Default to .codecocoon-memory.json in config parent directory + val memoryDir = File(configParentDir, ".codecocoon-memory") + File(memoryDir, "memory.json").canonicalPath } } } \ No newline at end of file diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/intellij/psi/WildcardImportExpander.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/intellij/psi/WildcardImportExpander.kt new file mode 100644 index 00000000..81a5e302 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/intellij/psi/WildcardImportExpander.kt @@ -0,0 +1,356 @@ +package com.github.pderakhshanfar.codecocoonplugin.intellij.psi + +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.intellij.ide.highlighter.JavaFileType +import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.application.readAction +import com.intellij.openapi.command.WriteCommandAction +import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.progress.ProcessCanceledException +import com.intellij.openapi.project.Project +import com.intellij.psi.JavaPsiFacade +import com.intellij.psi.JavaRecursiveElementVisitor +import com.intellij.psi.PsiClass +import com.intellij.psi.PsiImportList +import com.intellij.psi.PsiImportStatement +import com.intellij.psi.PsiImportStaticStatement +import com.intellij.psi.PsiJavaCodeReferenceElement +import com.intellij.psi.PsiJavaFile +import com.intellij.psi.PsiManager +import com.intellij.psi.PsiMember +import com.intellij.psi.PsiPackage +import com.intellij.psi.PsiReferenceExpression +import com.intellij.psi.search.FileTypeIndex +import com.intellij.psi.search.GlobalSearchScope +import kotlinx.coroutines.runBlocking + +/** + * One-shot pre-processing utility that replaces wildcard imports with explicit + * single imports for symbols actually used in the file. + * + * Why this exists: IntelliJ's `RenameProcessor` invokes + * `JavaCodeStyleManager.shortenClassReferences()` on every file whose references + * it rewrites. That call can also strip `import static X.*;` lines on the + * touched files even when those lines are load-bearing — observed on + * fastjson2 PR-82 where the rename of `JSON` → `JsonCodec` removed + * `import static junit.framework.TestCase.*;` from test files, breaking + * `assertNull(...)` resolution. There is no documented IntelliJ toggle for + * this side effect (`IDEABKL-3561`). + * + * Pre-expanding wildcards into explicit single imports defangs the optimizer: + * each remaining import points at a name PSI sees as referenced, so the + * optimizer cannot drop it as "unused". + * + * Run once per project, before any transformation, across every Java file in + * project scope (not just files we transform — RenameProcessor can touch + * cross-module references in files we never enumerate). + * + * **Static-inheritance attribution:** `import static X.*;` legitimately exposes + * any static defined on `X` OR any of its supers (e.g. `TestCase` exposes + * `assertNull` declared on `Assert`). PSI's resolver returns the *declaring* + * class, not the imported one. We therefore attribute usage by querying the + * target class's visible (inherited) members instead of comparing + * `containingClass`. + * + * **PSI-stability discipline:** Each `WriteCommandAction` that mutates the + * import list invalidates other `PsiImportStatement` siblings even when they + * weren't the deleted one. We therefore never hold a `PsiImportStatement` / + * `PsiImportStaticStatement` reference across mutations — instead we capture + * the wildcards' target FQNs once, then re-locate the matching wildcard inside + * each rewrite by scanning the (now-fresh) import list. + * + * **Best-effort:** All PSI operations are wrapped in `try/catch (Throwable)`, + * rethrowing only `ProcessCanceledException` / `InterruptedException`. A + * single bad file or wildcard never aborts the pipeline. + */ +object WildcardImportExpander { + private val logger = thisLogger().withStdout() + + data class Stats( + val filesScanned: Int, + val wildcardsExpanded: Int, + val wildcardsKept: Int, + val filesFailed: Int, + val wildcardsFailed: Int, + ) + + fun expandAll(project: Project): Stats { + val files = safeReadAction(emptyList()) { + FileTypeIndex.getFiles(JavaFileType.INSTANCE, GlobalSearchScope.projectScope(project)).toList() + } + val psiManager = PsiManager.getInstance(project) + + var scanned = 0 + var expanded = 0 + var kept = 0 + var filesFailed = 0 + var wildcardsFailed = 0 + + for (vf in files) { + val path = vf.path + try { + val psiFile = safeReadAction(null) { psiManager.findFile(vf) as? PsiJavaFile } ?: continue + scanned++ + val (e, k, wf) = expandInFile(project, psiFile) + expanded += e + kept += k + wildcardsFailed += wf + } catch (e: ProcessCanceledException) { + throw e + } catch (e: InterruptedException) { + throw e + } catch (t: Throwable) { + filesFailed++ + logger.warn("[WildcardImportExpander] Failed to process file '$path': ${t.javaClass.simpleName}: ${t.message}") + } + } + logger.info( + "[WildcardImportExpander] Pre-processed $scanned files; expanded $expanded; kept $kept conservative; " + + "failed $filesFailed file(s) / $wildcardsFailed wildcard(s)" + ) + return Stats(scanned, expanded, kept, filesFailed, wildcardsFailed) + } + + /** Per-file static-reference summary built in a single PSI walk. */ + private data class StaticRefSummary( + val resolvedNames: Set, + val unresolvedNames: Set, + ) + + /** @return Triple(expanded, kept, wildcardsFailed) for the file. */ + private fun expandInFile(project: Project, psiFile: PsiJavaFile): Triple { + // Capture by FQN, NOT by PsiElement reference — element references can + // be invalidated by sibling mutations during rewrite. + data class Plan( + val staticTargetFqns: List, + val regularPackageFqns: List, + val staticSummary: StaticRefSummary, + ) + + val plan = safeReadAction(null) { + val importList = psiFile.importList ?: return@safeReadAction null + val statics = importList.importStaticStatements + .filter { it.isOnDemand && it.isValid } + .mapNotNull { it.importReference?.qualifiedName } + .toList() + val regulars = importList.importStatements + .filter { it.isOnDemand && it.isValid } + .mapNotNull { it.importReference?.qualifiedName } + .toList() + if (statics.isEmpty() && regulars.isEmpty()) null + else Plan(statics, regulars, summarizeStaticRefs(psiFile)) + } ?: return Triple(0, 0, 0) + + var expanded = 0 + var kept = 0 + var failed = 0 + for (fqn in plan.staticTargetFqns) { + try { + val (e, k) = rewriteStaticWildcard(project, psiFile, fqn, plan.staticSummary) + expanded += e + kept += k + } catch (e: ProcessCanceledException) { + throw e + } catch (e: InterruptedException) { + throw e + } catch (t: Throwable) { + failed++ + logger.warn("[WildcardImportExpander] Failed to expand static wildcard '$fqn' in '${psiFile.virtualFile?.path}': ${t.javaClass.simpleName}: ${t.message}") + } + } + for (fqn in plan.regularPackageFqns) { + try { + expanded += rewriteRegularWildcard(project, psiFile, fqn) + } catch (e: ProcessCanceledException) { + throw e + } catch (e: InterruptedException) { + throw e + } catch (t: Throwable) { + failed++ + logger.warn("[WildcardImportExpander] Failed to expand regular wildcard '$fqn' in '${psiFile.virtualFile?.path}': ${t.javaClass.simpleName}: ${t.message}") + } + } + return Triple(expanded, kept, failed) + } + + private fun summarizeStaticRefs(psiFile: PsiJavaFile): StaticRefSummary { + val resolved = LinkedHashSet() + val unresolved = LinkedHashSet() + try { + psiFile.accept(object : JavaRecursiveElementVisitor() { + override fun visitReferenceExpression(expr: PsiReferenceExpression) { + super.visitReferenceExpression(expr) + try { + if (expr.qualifierExpression != null) return + val name = expr.referenceName ?: return + val target = expr.resolve() + when { + target is PsiMember -> resolved.add(name) + target == null -> unresolved.add(name) + } + } catch (e: ProcessCanceledException) { + throw e + } catch (_: Throwable) { + // ignore — best-effort summarisation + } + } + }) + } catch (e: ProcessCanceledException) { + throw e + } catch (_: Throwable) { + // ignore — return what we have + } + return StaticRefSummary(resolved, unresolved) + } + + /** @return Pair(expanded, kept). */ + private fun rewriteStaticWildcard( + project: Project, + psiFile: PsiJavaFile, + targetFqn: String, + summary: StaticRefSummary, + ): Pair { + val targetClass = safeReadAction(null) { + JavaPsiFacade.getInstance(project) + .findClass(targetFqn, GlobalSearchScope.allScope(project)) + } ?: return 0 to 0 + + val usedNames = safeReadAction(emptyList()) { + summary.resolvedNames.filter { name -> targetClassExposes(targetClass, name) } + } + val coversUnresolved = safeReadAction(false) { + summary.unresolvedNames.any { name -> targetClassExposes(targetClass, name) } + } + + if (usedNames.isEmpty() && coversUnresolved) { + return 0 to 1 + } + + ApplicationManager.getApplication().invokeAndWait { + try { + WriteCommandAction.runWriteCommandAction(project) { + val importList = psiFile.importList ?: return@runWriteCommandAction + val factory = JavaPsiFacade.getElementFactory(project) + val freshTargetClass = JavaPsiFacade.getInstance(project) + .findClass(targetFqn, GlobalSearchScope.allScope(project)) + ?: return@runWriteCommandAction + // Re-locate the wildcard fresh — sibling mutations may have + // invalidated whatever element we saw earlier. + val wildcard = importList.importStaticStatements.firstOrNull { stmt -> + stmt.isValid && stmt.isOnDemand && + runCatching { stmt.importReference?.qualifiedName }.getOrNull() == targetFqn + } ?: return@runWriteCommandAction + + for (name in usedNames) { + importList.add(factory.createImportStaticStatement(freshTargetClass, name)) + } + if (wildcard.isValid) wildcard.delete() + } + } catch (e: ProcessCanceledException) { + throw e + } catch (t: Throwable) { + logger.warn("[WildcardImportExpander] WriteCommandAction failed for static '$targetFqn': ${t.javaClass.simpleName}: ${t.message}") + } + } + return 1 to 0 + } + + private fun targetClassExposes(targetClass: PsiClass, name: String): Boolean { + return try { + if (targetClass.findMethodsByName(name, /* checkBases = */ true).isNotEmpty()) return true + if (targetClass.findFieldByName(name, /* checkBases = */ true) != null) return true + if (targetClass.findInnerClassByName(name, /* checkBases = */ true) != null) return true + false + } catch (e: ProcessCanceledException) { + throw e + } catch (_: Throwable) { + false + } + } + + private fun rewriteRegularWildcard( + project: Project, + psiFile: PsiJavaFile, + packageFqn: String, + ): Int { + val pkg = safeReadAction(null) { + JavaPsiFacade.getInstance(project).findPackage(packageFqn) + } ?: return 0 + + val usedClasses = safeReadAction(emptyList()) { + collectClassUses(psiFile, pkg).toList() + } + + ApplicationManager.getApplication().invokeAndWait { + try { + WriteCommandAction.runWriteCommandAction(project) { + val importList = psiFile.importList ?: return@runWriteCommandAction + val factory = JavaPsiFacade.getElementFactory(project) + val wildcard = importList.importStatements.firstOrNull { stmt -> + stmt.isValid && stmt.isOnDemand && + runCatching { stmt.importReference?.qualifiedName }.getOrNull() == packageFqn + } ?: return@runWriteCommandAction + + for (cls in usedClasses) { + if (cls.isValid) importList.add(factory.createImportStatement(cls)) + } + if (wildcard.isValid) wildcard.delete() + } + } catch (e: ProcessCanceledException) { + throw e + } catch (t: Throwable) { + logger.warn("[WildcardImportExpander] WriteCommandAction failed for regular '$packageFqn': ${t.javaClass.simpleName}: ${t.message}") + } + } + return 1 + } + + private fun collectClassUses(psiFile: PsiJavaFile, pkg: PsiPackage): Set { + val classes = LinkedHashSet() + val pkgFqn = pkg.qualifiedName + try { + psiFile.accept(object : JavaRecursiveElementVisitor() { + override fun visitReferenceElement(reference: PsiJavaCodeReferenceElement) { + super.visitReferenceElement(reference) + try { + if (reference.qualifier != null) return + val resolved = reference.resolve() as? PsiClass ?: return + val fqn = resolved.qualifiedName ?: return + if (fqn.substringBeforeLast('.', "") == pkgFqn) { + classes.add(resolved) + } + } catch (e: ProcessCanceledException) { + throw e + } catch (_: Throwable) { + // ignore + } + } + }) + } catch (e: ProcessCanceledException) { + throw e + } catch (_: Throwable) { + // ignore + } + return classes + } + + /** + * Run a read-action computation, swallowing any non-control-flow throwable + * and returning [fallback]. We deliberately do NOT rethrow PSI / IDE + * exceptions here — they'd abort the pipeline. Cancellation must still + * propagate, hence the explicit rethrow for `ProcessCanceledException` / + * `InterruptedException`. + */ + private fun safeReadAction(fallback: T, block: () -> T): T { + return try { + runBlocking { readAction { block() } } + } catch (e: ProcessCanceledException) { + throw e + } catch (e: InterruptedException) { + throw e + } catch (t: Throwable) { + logger.warn("[WildcardImportExpander] readAction failed: ${t.javaClass.simpleName}: ${t.message}") + fallback + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/PersistentMemory.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/PersistentMemory.kt index 6688d86f..97f2657c 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/PersistentMemory.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/memory/PersistentMemory.kt @@ -11,45 +11,31 @@ import kotlin.io.path.* /** * File-based persistent storage implementation of [Memory] interface. * - * Stores key-value pairs as JSON in a file within the specified directory. - * Files are organized by project name to allow tracking multiple projects independently. - * - * **Storage Model:** - * Each project gets its own JSON file containing all key-value pairs for that project. - * For example, given `memoryDirPath = "/path/to/memory"` and three projects: - * - `PersistentMemory("project-A", memoryDirPath)` → `/path/to/memory/project-A.json` - * - `PersistentMemory("project-B", memoryDirPath)` → `/path/to/memory/project-B.json` - * - `PersistentMemory("nested/project-C", memoryDirPath)` → `/path/to/memory/nested_project-C.json` (sanitized) - * - * Each JSON file contains all entries for that project, persisted on [save] or [close]. + * Stores key-value pairs as JSON in a single file at the path provided to the constructor. + * The caller decides exactly where the memory file lives — there is no project-name + * composition or per-project file partitioning. * * **Thread Safety:** This implementation is not thread-safe. Use external synchronization * if accessing from multiple threads. * * **Usage:** * ```kotlin - * PersistentMemory("myProject", "/path/to/memory").use { memory -> + * PersistentMemory("/path/to/memory.json").use { memory -> * memory.put("key", "value") * memory.get("key") // returns "value" * } // automatically saves on close * ``` * - * @param projectName The name of the project (used for the memory filename) - * @param memoryDirPath The directory path where memory files should be stored + * @param memoryFilepath Full path to the JSON memory file (created if missing) */ -class PersistentMemory(private val projectName: String, memoryDirPath: String) : Memory { +class PersistentMemory(private val memoryFilepath: String) : Memory { private val logger = thisLogger().withStdout() private val memoryFile: Path = run { - // Sanitize project name for use in filename - val sanitizedName = sanitizeProjectName(projectName) - - // Convert path to Path and ensure memory directory exists - val memoryDir = Path(memoryDirPath) - memoryDir.createDirectories() - - memoryDir.resolve("$sanitizedName.json") + val path = Path(memoryFilepath) + path.parent?.createDirectories() + path } private var state: MemoryState = loadFromDisk(from = memoryFile) @@ -78,53 +64,25 @@ class PersistentMemory(private val projectName: String, memoryDirPath: String) : override fun save() { val jsonString = json.encodeToString(state) memoryFile.writeText(jsonString) - logger.info(" ↳ Successfully saved memory for project '$projectName' (${state.entries.size} entries)") + logger.info(" ↳ Successfully saved memory to '$memoryFilepath' (${state.entries.size} entries)") } override fun size(): Int = state.entries.size /** * Loads memory data from disk, or creates a new empty memory if the file doesn't exist. - * Throws on JSON parse errors or project name mismatches. + * Throws on JSON parse errors. * * @param from The path to the memory file to load from */ private fun loadFromDisk(from: Path): MemoryState { if (!from.exists()) { - logger.info(" • No existing memory file found for project '$projectName', creating new memory") - return MemoryState(projectName, mutableMapOf()) + logger.info(" • No existing memory file at '$memoryFilepath', creating new memory") + return MemoryState(mutableMapOf()) } val jsonString = from.readText() - val loaded = json.decodeFromString(jsonString) - - // Verify project name matches - if (loaded.projectName != projectName) { - throw IllegalStateException( - "Memory file project name mismatch: expected '$projectName', found '${loaded.projectName}'. " + - "Memory file: ${from.absolutePathString()}" - ) - } - - return loaded - } - - /** - * Sanitizes a project name to be safe for use in a filename. - * Throws if the project name is blank or becomes blank after sanitization. - */ - private fun sanitizeProjectName(name: String): String { - val sanitized = name - .replace(Regex("[^a-zA-Z0-9_-]"), "_") - .take(100) // Limit length to avoid filesystem issues - - if (sanitized.isBlank()) { - throw IllegalArgumentException( - "Project name '$name' contains only invalid characters or is blank." - ) - } - - return sanitized + return json.decodeFromString(jsonString) } companion object { @@ -138,11 +96,9 @@ class PersistentMemory(private val projectName: String, memoryDirPath: String) : /** * Data class representing the persistent memory file structure. * - * @property projectName The name of the project this memory belongs to * @property entries Map from key to value */ @Serializable private data class MemoryState( - val projectName: String, val entries: MutableMap ) diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/MetamorphicTextTransformer.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/MetamorphicTextTransformer.kt new file mode 100644 index 00000000..24c042e6 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/MetamorphicTextTransformer.kt @@ -0,0 +1,169 @@ +package com.github.pderakhshanfar.codecocoonplugin.services + +import ai.koog.prompt.dsl.Prompt +import com.github.pderakhshanfar.codecocoonplugin.common.LLM +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.intellij.openapi.diagnostic.thisLogger +import kotlinx.serialization.Serializable +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.jsonObject +import kotlinx.serialization.json.jsonPrimitive +import java.io.File + +/** + * Updates a [TextBlock] (`{title, body}` pair) so class/method/variable names and + * package references reflect a rename memory file produced by the renaming/file-moving + * transformations. One LLM call per block keeps title and body internally consistent. + */ +class MetamorphicTextTransformer( + private val llm: LLM, +) { + private val logger = thisLogger().withStdout() + + @Serializable + private data class TransformedBlock( + val title: String, + val body: String, + ) + + /** + * Loads the rename map from [memoryFilePath]. Returns null when the file is missing + * or malformed; an empty map when the file contains no entries. + */ + fun loadRenameMap(memoryFilePath: String): Map? { + val memoryFile = File(memoryFilePath) + if (!memoryFile.exists()) { + logger.error("ERROR: Memory file not found: $memoryFilePath") + return null + } + + val memoryJson = Json.parseToJsonElement(memoryFile.readText()) + val entries = memoryJson.jsonObject["entries"]?.jsonObject ?: run { + logger.error("ERROR: No 'entries' field found in memory file") + return null + } + + return entries.entries.associate { (oldName, newName) -> + oldName to newName.jsonPrimitive.content + } + } + + /** + * Updates [block]'s title and body together. Returns [block] verbatim when the + * rename map is empty or both fields are blank. Returns null on LLM failure (the + * caller decides whether to fall back). + */ + suspend fun transformBlock( + block: TextBlock, + renameMap: Map, + ): TextBlock? { + if (renameMap.isEmpty()) return block + if (block.title.isBlank() && block.body.isBlank()) return block + + val prompt = createTransformationPrompt(block = block, renameMap = renameMap) + logger.info("Created metamorphic prompt:\n'''$prompt\n'''") + + val result = llm.structuredRequest( + prompt = prompt, + maxRetries = 3, + maxFixingAttempts = 2, + ) ?: return null + + return TextBlock(title = result.title, body = result.body) + } + + private fun createTransformationPrompt( + block: TextBlock, + renameMap: Map, + ): Prompt { + return Prompt.build("metamorphic-text-transformer") { + system { + text(""" + You are a technical documentation assistant helping to update code + descriptions after refactoring transformations have been applied. + + You will be given: + 1. A title and a body of a single document block. The body may be + multiline markdown. + 2. A mapping of old class/method/variable names AND old file paths + to their new names or new locations. + + The mapping may contain two kinds of entries: + - Identifier renames: the value is a Java identifier (e.g. + `computeTotal`). Replace every occurrence of the old simple name + in the title and body with the new one wherever it appears (class + names, method calls, variable mentions, fully-qualified + references, etc.). + - File / package moves: the value looks like a filesystem path or + directory (contains `/` or `\`). The corresponding source file + was relocated, which typically changes its Java package. Update + any fully-qualified class references, `import` statements, or + package mentions in the title or body to reflect the new package + implied by the new directory. + + Your task: + - Update the title AND the body to use the NEW names and NEW + packages. + - Apply the SAME rename decisions to title and body (they describe + the same change). + - Keep the meaning and structure exactly the same. + - Preserve all formatting, punctuation, and sentence structure. + - If a name doesn't appear in the mapping, leave it unchanged. + + Important: + - Do NOT add new information. + - Do NOT remove information. + - Do NOT rephrase or rewrite the content. + - ONLY update the names and packages indicated by the rename + mapping. + + Output: a JSON object with two fields, `title` and `body`, holding + the updated values: + ```json + { "title": "...", "body": "..." } + ``` + """.trimIndent()) + } + + user { + text("## Original Title:") + newline() + text("'''") + newline() + text(block.title) + newline() + text("'''") + text("\n\n") + + text("## Original Body:") + newline() + text("'''") + newline() + text(block.body) + newline() + text("'''") + text("\n\n") + + val (moves, renames) = renameMap.entries.partition { (_, value) -> + value.contains('/') || value.contains('\\') + } + + text("## Identifier renames (OldSimpleName -> NewSimpleName)\n") + renames.forEach { (old, new) -> + val oldSimple = old.substringAfterLast('.') + text("- $oldSimple -> $new\n") + } + + if (moves.isNotEmpty()) { + text("\n## File/package moves (OldPath -> NewDirectory)\n") + moves.forEach { (old, new) -> + text("- $old -> $new\n") + } + } + + text("\n") + text("Now, update both the title and the body with the new names and packages.") + } + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/ParaphraseTextTransformer.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/ParaphraseTextTransformer.kt new file mode 100644 index 00000000..ccd8b6f6 --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/ParaphraseTextTransformer.kt @@ -0,0 +1,155 @@ +package com.github.pderakhshanfar.codecocoonplugin.services + +import ai.koog.prompt.dsl.Prompt +import com.github.pderakhshanfar.codecocoonplugin.common.LLM +import com.github.pderakhshanfar.codecocoonplugin.intellij.logging.withStdout +import com.intellij.openapi.diagnostic.thisLogger +import kotlinx.serialization.Serializable + +/** + * Rewrites a [TextBlock] (`{title, body}` pair) so it looks surface-different while + * preserving exact semantics. One LLM call per block so the title and body stay + * coherent (same voice, same synonym choices). Used in the eval pipeline AFTER + * [MetamorphicTextTransformer] has synced renames/moves. + */ +class ParaphraseTextTransformer( + private val llm: LLM, +) { + private val logger = thisLogger().withStdout() + + @Serializable + private data class ParaphrasedBlock( + val title: String, + val body: String, + ) + + /** + * Asks the LLM to paraphrase [block]'s title and body together. Returns [block] + * verbatim when both fields are blank. Returns null on LLM failure. + */ + suspend fun rewriteBlock(block: TextBlock): TextBlock? { + if (block.title.isBlank() && block.body.isBlank()) return block + + val prompt = createRewritePrompt(block) + logger.info("Created paraphrase prompt:\n'''$prompt\n'''") + + val result = llm.structuredRequest( + prompt = prompt, + maxRetries = 3, + maxFixingAttempts = 2, + ) ?: return null + + return TextBlock(title = result.title, body = result.body) + } + + private fun createRewritePrompt(block: TextBlock): Prompt { + return Prompt.build("paraphrase-text-block") { + system { + text(""" + You are a technical-documentation paraphrasing assistant. + + You will be given a block of documentation: a TITLE and a BODY that + describe the same change. Your task is to AGGRESSIVELY rewrite both + so the result looks SUBSTANTIALLY different on the surface while + remaining a strict semantic synonym — every requirement, fact, and + constraint preserved exactly. Apply the SAME rewriting voice and + synonym choices to title and body so they remain consistent with + each other. + + Be bold with the rewrite. A near-copy of the input is a FAILURE. + Aim for a high-effort rewrite that a reader would not recognise as + the same prose at first glance, yet a domain expert would confirm + carries the same intent. + + HARD CONSTRAINTS — preserve verbatim, never alter: + - All identifiers: class names, method names, variable names, + package names, file paths, command-line flags, environment + variables, URLs, version strings. Do NOT rename, translate, or + pluralise them. + - All code-like tokens inside `backticks` and inside fenced code + blocks (```...```). Do not edit, reorder, or reformat code fences + or their contents. + - All numbers, units, and concrete values (e.g. "5 retries", + "200 OK", "UTF-8"). + - Markdown structural elements: headings, bullet lists, numbered + lists, and tables. Their COUNT and the order of their items must + stay the same; you may rephrase the prose inside each item, but + do not add, drop, merge, or split items, and do not change + heading levels. + + REQUIRED SURFACE CHANGES — apply several of these, not just one: + 1. Lexical: replace ordinary verbs, nouns, adjectives, and + connectors with synonyms or near-synonyms ("provides" → + "exposes", "responsible for" → "in charge of", "must continue + to" → "are still required to"). Do this for the MAJORITY of + non-identifier content words. + 2. Syntactic: reshape sentences. Convert active ↔ passive voice, + swap subject/object framing, hoist subordinate clauses to the + front, turn "X does Y so that Z" into "Z requires that X does + Y", and similar. At least half of the sentences should differ + in structure from their original counterparts. + 3. Granularity: split long sentences into shorter ones, or fuse + two short sentences into one — wherever it improves rhythm and + the meaning is preserved. + 4. Sentence ordering WITHIN A PARAGRAPH: you may reorder sentences + within the same paragraph if it preserves logical flow. Do NOT + move sentences across paragraph boundaries or across markdown + sections. + 5. Register tightening: keep tone neutral and technical; trim + throat-clearing phrases ("In order to" → "To") where it does + not change meaning. + + FORBIDDEN: + - Do NOT add facts, examples, qualifications, or reasoning the + original did not contain. + - Do NOT remove facts, examples, qualifications, or reasoning the + original DID contain. + - Do NOT introduce ambiguity, hedging, or vagueness that the + original did not have ("must" stays "must"; "may" stays "may"). + - Do NOT translate to another natural language. + - Do NOT comment on the rewrite, prefix it, or wrap it in extra + narration. + + SELF-CHECK before responding: + - Could a reader infer any requirement that was not in the + original? If yes, revise. + - Could a reader miss any requirement that was in the original? + If yes, revise. + - Does at least 60% of the prose read differently (different word + choice or sentence shape) from the input? If no, rewrite more + aggressively. + - Does the rewritten title use the same voice / synonym choices as + the rewritten body? If no, align them. + + Output: a JSON object with two fields, `title` and `body`, holding + the rewritten values: + ```json + { "title": "...", "body": "..." } + ``` + """.trimIndent()) + } + + user { + text("## Original Title:") + newline() + text("'''") + newline() + text(block.title) + newline() + text("'''") + text("\n\n") + + text("## Original Body:") + newline() + text("'''") + newline() + text(block.body) + newline() + text("'''") + text("\n\n") + + text("Now produce the paraphrased title and body following the given rules.") + } + } + } +} diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TextBlock.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TextBlock.kt new file mode 100644 index 00000000..05161a2f --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TextBlock.kt @@ -0,0 +1,12 @@ +package com.github.pderakhshanfar.codecocoonplugin.services + +/** + * A pair of related prose fields that should be transformed together by a single LLM + * call so the result is internally coherent (consistent voice, consistent identifier + * rewrites). Used for the main `{title, body}` of a benchmark record and for each + * `resolved_issues[i].{title, body}` pair. + */ +data class TextBlock( + val title: String, + val body: String, +) diff --git a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TransformationService.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TransformationService.kt index 1caa7ade..937a0e76 100644 --- a/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TransformationService.kt +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TransformationService.kt @@ -11,16 +11,18 @@ import com.github.pderakhshanfar.codecocoonplugin.intellij.vfs.findVirtualFile import com.github.pderakhshanfar.codecocoonplugin.intellij.vfs.relativeToRootOrAbsPath import com.github.pderakhshanfar.codecocoonplugin.memory.PersistentMemory import com.github.pderakhshanfar.codecocoonplugin.transformation.Transformation +import com.intellij.openapi.application.ApplicationManager import com.intellij.openapi.application.smartReadAction import com.intellij.openapi.components.Service import com.intellij.openapi.diagnostic.thisLogger +import com.intellij.openapi.fileEditor.FileDocumentManager import com.intellij.openapi.project.Project import com.intellij.openapi.project.guessProjectDir import com.intellij.openapi.vfs.LocalFileSystem import com.intellij.openapi.vfs.VfsUtilCore import com.intellij.openapi.vfs.VirtualFile import com.intellij.openapi.vfs.VirtualFileVisitor -import java.io.File +import com.intellij.psi.PsiDocumentManager /** * Application-level service responsible for managing metamorphic transformations @@ -128,18 +130,31 @@ class TransformationService { ) { logger.info("[TransformationService] Applying ${transformations.size} transformations") + // Pre-expand wildcard imports project-wide so RenameProcessor's post-rename + // import optimizer can't strip a wildcard line on files it touches and + // leave referenced symbols (e.g. `assertNull` from + // `import static junit.framework.TestCase.*;`) unresolved. + + // TODO: WildcardImportExpander misses some imports -> skip it + // logger.info("[TransformationService] Pre-expanding wildcard imports project-wide...") + // WildcardImportExpander.expandAll(project) + val files = listProjectFiles(project, config.projectRoot, includeOnly = config.files) val executor = IntelliJTransformationExecutor(project) // Create a global memory instance for the entire project // Memory is automatically saved via .use {} when the block exits - val projectName = project.basePath?.let { File(it).name } ?: project.name - PersistentMemory(projectName, config.memoryDir).use { memory -> - logger.info("[TransformationService] Created global memory for project '$projectName'") + PersistentMemory(config.memoryFilepath).use { memory -> + val transformationsStr = transformations.withIndex() + .joinToString(separator = ",\n") { " ${it.index + 1}) ${it.value.id}" } + logger.info("[TransformationService] Applying the following transformations in order:\n$transformationsStr") - var successCount = 0 - var failureCount = 0 - var skippedCount = 0 + logger.info("[TransformationService] Created global memory at '${config.memoryFilepath}'") + + val succeededIds = mutableListOf() + val failedIds = mutableListOf() + val skippedIds = mutableListOf() + var fileSkippedCount = 0 // Collect and filter file contexts together with their virtual files logger.info("[TransformationService] Collecting file contexts for ${files.size} files...") @@ -155,14 +170,14 @@ class TransformationService { " ✗ Failed to create file context for file: '$filePath'. Skipping this filepath.", result.exceptionOrNull(), ) - skippedCount++ + fileSkippedCount++ continue } val context = result.getOrThrow() // filter unwanted files if (!fileFilter(context)) { - skippedCount++ + fileSkippedCount++ continue } add(context) @@ -170,6 +185,7 @@ class TransformationService { } logger.info("[TransformationService] Successfully collected (and filtered) ${filteredFileContexts.size} file contexts") + // for each file, apply all transformations for (context in filteredFileContexts) { val filepath = project.relativeToRootOrAbsPath(context.virtualFile) @@ -187,22 +203,44 @@ class TransformationService { when (val result = executor.execute(transformation, context, memory)) { is TransformationResult.Success -> { logger.info(" ✓ ${result.message}") - successCount++ + succeededIds += transformation.id } is TransformationResult.Failure -> { logger.error(" ✗ ${result.error}", result.exception) - failureCount++ + failedIds += transformation.id } is TransformationResult.Skipped -> { logger.info(" ⊘ Skipped: ${result.reason}") - skippedCount++ + skippedIds += transformation.id } } } } + + // Flush PSI changes to disk once per file context (cheaper than flushing after every rename). + ApplicationManager.getApplication().invokeAndWait { + val commitStart = System.currentTimeMillis() + logger.info("[TransformationService] Committing all documents for '$filepath'...") + PsiDocumentManager.getInstance(project).commitAllDocuments() + logger.info("[TransformationService] Committed all documents in ${System.currentTimeMillis() - commitStart}ms") + + val saveStart = System.currentTimeMillis() + logger.info("[TransformationService] Saving all documents for '$filepath'...") + FileDocumentManager.getInstance().saveAllDocuments() + logger.info("[TransformationService] Saved all documents in ${System.currentTimeMillis() - saveStart}ms") + } } - logger.info("[TransformationService] Transformation summary: $successCount succeeded, $failureCount failed, $skippedCount skipped") + val header = buildString { + append("[TransformationService] Transformation summary: ") + append("${succeededIds.size} succeeded, ${failedIds.size} failed, ${skippedIds.size} skipped") + if (fileSkippedCount > 0) append(", $fileSkippedCount files skipped") + append(":") + } + logger.info(header) + logger.info("[TransformationService] - succeeded: ${succeededIds.joinToString(", ")}") + logger.info("[TransformationService] - failed: ${failedIds.joinToString(", ")}") + logger.info("[TransformationService] - skipped: ${skippedIds.joinToString(", ")}") } } diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index fb22a796..10a46d61 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -13,5 +13,11 @@ + + +