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/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/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/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/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..4def70ee --- /dev/null +++ b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/components/transformations/structural/ReorderClassMethodsTransformation.kt @@ -0,0 +1,206 @@ +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 + } + // 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/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/TransformationService.kt b/src/main/kotlin/com/github/pderakhshanfar/codecocoonplugin/services/TransformationService.kt index 1caa7ade..7ef18266 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 @@ -133,13 +135,17 @@ class TransformationService { // 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 +161,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 +176,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 +194,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(", ")}") } }