Fix Java PSI resolution crash in analysis#12
Merged
ClankerGuru merged 1 commit intomainfrom Apr 13, 2026
Merged
Conversation
annotation.qualifiedName triggers PsiJavaCodeReferenceElement.resolve() which goes through JavaPsiFacade → Kotlin JavaElementFinder → CliKotlinAsJavaSupport → CliTraceHolder.module — a lateinit property that is only set during compilation, not in srcx's parsing-only environment. This crashes every project containing Java source files. Use nameReferenceElement.referenceName instead, which reads the annotation name text without triggering type resolution. Also wrap parseJavaFile in runCatching so any remaining resolution-triggering operation logs a warning and skips the file instead of failing the entire project analysis.
📝 WalkthroughWalkthroughUpdated Java annotation extraction logic in source file metadata parsing by changing from qualified name extraction to reference element name retrieval. Added centralized error handling and logging for Java file parsing operations using Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt (1)
231-231: Add a Java-annotation regression test for this fix.
SourceFileMetadataTestcurrently exercises only the unannotated Java happy path, so this resolution-avoidance change can regress silently. A small annotated Java fixture would pin both the simple-name behavior and the no-resolution requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt` at line 231, Add a regression test to SourceFileMetadataTest that includes a small Java source fixture with at least one annotation (e.g., `@MyAnno`) to verify the SourceFileMetadata path that uses annotations = cls.annotations.mapNotNull { it.nameReferenceElement?.referenceName } preserves simple-name values and does not trigger PSI resolution; the test should load the Java file, invoke the same metadata extraction used by SourceFileMetadata (or call the helper that builds annotations from cls), and assert the resulting annotations list contains the simple annotation name and that no resolution-dependent behavior occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt`:
- Around line 244-279: Move the LightVirtualFile creation and
psiManager.findFile(...) calls inside a try block within parseJavaFile and
replace the runCatching usage with an explicit try { ... } catch (e: Exception)
{ ... } so I/O or charset errors are handled by the fallback and fatal
Throwables are not swallowed; apply the same change to parseKotlinFile (ensure
you catch Exception, log via javaParseLogger.warn with the exception, and return
null on failure).
---
Nitpick comments:
In `@src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt`:
- Line 231: Add a regression test to SourceFileMetadataTest that includes a
small Java source fixture with at least one annotation (e.g., `@MyAnno`) to verify
the SourceFileMetadata path that uses annotations = cls.annotations.mapNotNull {
it.nameReferenceElement?.referenceName } preserves simple-name values and does
not trigger PSI resolution; the test should load the Java file, invoke the same
metadata extraction used by SourceFileMetadata (or call the helper that builds
annotations from cls), and assert the resulting annotations list contains the
simple annotation name and that no resolution-dependent behavior occurs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 447b097c-63e9-4ff5-ae2d-d2d29e983539
📒 Files selected for processing (1)
src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt
Comment on lines
244
to
+279
| private fun parseJavaFile(file: File, psiManager: PsiManager): SourceFileMetadata? { | ||
| val vf = LightVirtualFile(file.name, JavaFileType.INSTANCE, file.readText()) | ||
| val javaFile = psiManager.findFile(vf) as? PsiJavaFile ?: return null | ||
|
|
||
| val packageName = javaFile.packageName | ||
| val imports = | ||
| javaFile.importList | ||
| ?.importStatements | ||
| ?.mapNotNull { it.qualifiedName } | ||
| ?.filter { fqn -> PLATFORM_PREFIXES.none { fqn.startsWith(it) } } | ||
| ?: emptyList() | ||
| return runCatching { | ||
| val packageName = javaFile.packageName | ||
| val imports = | ||
| javaFile.importList | ||
| ?.importStatements | ||
| ?.mapNotNull { it.qualifiedName } | ||
| ?.filter { fqn -> PLATFORM_PREFIXES.none { fqn.startsWith(it) } } | ||
| ?: emptyList() | ||
|
|
||
| val info = resolveJavaTypeInfo(javaFile, file.nameWithoutExtension) | ||
| val qualifiedName = if (packageName.isNotEmpty()) "$packageName.${info.className}" else info.className | ||
| val info = resolveJavaTypeInfo(javaFile, file.nameWithoutExtension) | ||
| val qualifiedName = if (packageName.isNotEmpty()) "$packageName.${info.className}" else info.className | ||
|
|
||
| return SourceFileMetadata( | ||
| file = file, | ||
| packageName = packageName, | ||
| qualifiedName = qualifiedName, | ||
| simpleName = info.className, | ||
| imports = imports, | ||
| annotations = info.annotations, | ||
| supertypes = info.supertypes, | ||
| isInterface = info.isInterface, | ||
| isAbstract = info.isAbstract, | ||
| isObject = false, | ||
| isDataClass = false, | ||
| language = SourceFileMetadata.Language.JAVA, | ||
| lineCount = countCodeLines(file.readLines()), | ||
| methods = info.methods, | ||
| declarationLine = info.declarationLine, | ||
| ) | ||
| SourceFileMetadata( | ||
| file = file, | ||
| packageName = packageName, | ||
| qualifiedName = qualifiedName, | ||
| simpleName = info.className, | ||
| imports = imports, | ||
| annotations = info.annotations, | ||
| supertypes = info.supertypes, | ||
| isInterface = info.isInterface, | ||
| isAbstract = info.isAbstract, | ||
| isObject = false, | ||
| isDataClass = false, | ||
| language = SourceFileMetadata.Language.JAVA, | ||
| lineCount = countCodeLines(file.readLines()), | ||
| methods = info.methods, | ||
| declarationLine = info.declarationLine, | ||
| ) | ||
| }.onFailure { e -> | ||
| javaParseLogger.warn("srcx: Failed to parse Java file '${file.name}': ${e.message}", e) | ||
| }.getOrNull() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and inspect the file
cd /tmp/repo 2>/dev/null || cd / || true
find . -name "SourceFileMetadata.kt" -type f 2>/dev/null | head -5Repository: ClankerGuru/srcx
Length of output: 151
🏁 Script executed:
# Let's check the actual code at the specified lines
if [ -f "src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt" ]; then
wc -l "src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt"
sed -n '244,279p' "src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt"
fiRepository: ClankerGuru/srcx
Length of output: 1681
🌐 Web query:
Kotlin stdlib runCatching catches Throwable or Exception documentation
💡 Result:
Kotlin stdlib's runCatching catches Throwable, not just Exception. It calls the specified function block and returns its encapsulated result if successful, catching any Throwable thrown from the block and encapsulating it as a Result.Failure. This has been the case since Kotlin 1.3 and remains current in the latest documentation.
Citations:
- 1: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin/run-catching.html
- 2: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/run-catching.html
🏁 Script executed:
# Check for similar parsing patterns and usage context
rg "parseJavaFile|parseKotlinFile|runCatching" src/main/kotlin/zone/clanker/gradle/srcx/analysis/ -B 2 -A 2Repository: ClankerGuru/srcx
Length of output: 2957
🏁 Script executed:
# Check if there's a corresponding parseKotlinFile to understand the pattern
grep -n "fun parse" src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt | head -10Repository: ClankerGuru/srcx
Length of output: 380
Move file setup inside the try block and use catch (Exception) instead of runCatching.
The file.readText() and psiManager.findFile() calls at lines 245–246 are outside the error handler, so I/O or charset failures will crash the analysis before the fallback runs. Additionally, runCatching catches Throwable, which suppresses fatal JVM errors (OutOfMemoryError, StackOverflowError) that should never be caught.
Proposed fix
private fun parseJavaFile(file: File, psiManager: PsiManager): SourceFileMetadata? {
- val vf = LightVirtualFile(file.name, JavaFileType.INSTANCE, file.readText())
- val javaFile = psiManager.findFile(vf) as? PsiJavaFile ?: return null
-
- return runCatching {
+ return try {
+ val vf = LightVirtualFile(file.name, JavaFileType.INSTANCE, file.readText())
+ val javaFile = psiManager.findFile(vf) as? PsiJavaFile ?: return null- }.onFailure { e ->
- javaParseLogger.warn("srcx: Failed to parse Java file '${file.name}': ${e.message}", e)
- }.getOrNull()
+ } catch (e: Exception) {
+ javaParseLogger.warn("srcx: Failed to parse Java file '${file.path}': ${e.message}", e)
+ null
+ }
}Apply the same fix to parseKotlinFile (line 158).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/zone/clanker/gradle/srcx/analysis/SourceFileMetadata.kt`
around lines 244 - 279, Move the LightVirtualFile creation and
psiManager.findFile(...) calls inside a try block within parseJavaFile and
replace the runCatching usage with an explicit try { ... } catch (e: Exception)
{ ... } so I/O or charset errors are handled by the fallback and fatal
Throwables are not swallowed; apply the same change to parseKotlinFile (ensure
you catch Exception, log via javaParseLogger.warn with the exception, and return
null on failure).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
annotation.qualifiedNameinresolveJavaTypeInfotriggersPsiJavaCodeReferenceElement.resolve()→JavaPsiFacade.findClass()→ KotlinJavaElementFinder→CliKotlinAsJavaSupport→CliTraceHolder.getModule()— alateinitproperty that is only initialized during compilation, not in srcx's parsing-onlyKotlinCoreEnvironment. This crashes every project that contains Java source files.nameReferenceElement?.referenceNameinstead ofqualifiedName— reads the annotation name text without triggering type resolution.parseJavaFileinrunCatchingso any remaining resolution-triggering operation logs a warning with full stack trace and skips the file instead of failing the entire project analysis.Context
The previous PR (#11) fixed a real but separate thread-safety issue. This PR fixes the actual root cause seen in repos with Java source files.
Test plan
./gradlew buildpasses (detekt, ktlint, tests, kover)srcx-contexton a repo with Java source files to confirm nolateiniterrorsSummary by CodeRabbit
Bug Fixes
Improvements