ADFA-4649: Remove com.blankj:utilcodex (AndroidUtilCode) dependency - #1576
Merged
Conversation
…ve equivalents Adds Context.dpToPx()/spToPx() extensions to common's ContextUtils.kt, matching blankj SizeUtils' rounding formula exactly, and swaps all 22 com.blankj.utilcode SizeUtils/ConvertUtils.dp2px call sites across app, editor, uidesigner, xml-inflater, and common-ui to use them. First of several staged commits removing the com.blankj:utilcodex dependency. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…helpers Adds a shared mainThreadHandler to common's TaskExecutor.kt and updates its existing runOnUiThread() to post through it instead of blankj's ThreadUtils. Swaps all com.blankj.utilcode ThreadUtils.runOnUiThread/ getMainHandler/runOnUiThreadDelayed call sites across app, lsp/models, lsp/java, and uidesigner to use it - preserving the same Handler instance for post/removeCallbacks pairs where cancellation depends on it. Second of several staged commits removing com.blankj:utilcodex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds native FileUtils/FileIOUtils objects to common's utils package, matching blankj's method names/signatures (isUtf8, readFile2String, writeFileFromString, listFilesInDirWithFilter, delete, rename, getFileExtension, createOrExistsDir) so call sites only needed an import swap, not a rewrite. Covers app, editor, xml-inflater, and common modules. Third of several staged commits removing com.blankj:utilcodex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ive equivalents Adds copyToClipboard/isNetworkConnected/isSoftInputVisible extensions to common's ContextUtils.kt (the latter using WindowInsetsCompat's IME visibility check instead of blankj's decor-view-height heuristic). Updates the CloneRepositoryViewModelTest mock to stub the new extension via mockkStatic on the facade class instead of blankj's NetworkUtils object. Fourth of several staged commits removing com.blankj:utilcodex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…lents - ThrowableUtils.getFullStackTrace -> Throwable.stackTraceToString() (Kotlin stdlib) - ConvertUtils.byte2MemorySize/MemoryConstants -> inline division - AppUtils.getAppVersionCode -> new Context.getAppVersionCode() extension - DeviceUtils.getManufacturer/getModel/isEmulator/isDeviceRooted -> ported onto the project's own (same-name, same-package) DeviceUtils object - CloseUtils.closeIO -> direct try/close - ArrayUtils.contains -> IntStream.anyMatch - StringUtils.isTrimEmpty -> inline null/trim check - Utils.getApp() -> BaseApplication.baseInstance - ActivityUtils.startActivity -> IDEApplication.instance.startActivity - ActivityUtils.getTopActivity() -> new BaseApplication.foregroundActivity, tracked via a lifecycle-callbacks registration in BaseApplication.onCreate() (common module can't depend on IDEApplication in app); IDEApplication's own more precise Pre/Post-based tracker now overrides it Fifth of several staged commits removing com.blankj:utilcodex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…h native equivalents Adds four new common-module utility objects, each a same-name drop-in so call sites only needed an import swap: - ReflectUtils: a compact fluent reflection helper (reflect/field/ method/newInstance/get) covering exactly the subset used across lsp/java, javac-services, and xml-inflater - field lookup uses the field's declared type when wrapping, matching blankj's behavior bug-for-bug (verified via javap against the actual blankj bytecode) so existing call chains keep working identically. - ImageUtils: isImage (BitmapFactory decode-bounds check), getBitmap, and getImageType (magic-number header sniffing for JPG/PNG/GIF/TIFF/ BMP/ICO/WEBP). - ResourceUtils: copyFileFromAssets (recursive directory-aware asset copy, matching blankj) and readAssets2String, both via BaseApplication.baseInstance.assets. - ZipUtils: unzipFile using java.util.zip.ZipFile, with zip-slip path-traversal protection added as a safe hardening. Sixth of several staged commits removing com.blankj:utilcodex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Drops the utilcodex dependency declaration from all 12 modules that had it (app, common, editor, uidesigner, xml-inflater, actions, lsp/api, lsp/java, lsp/models, lsp/xml, subprojects/javac-services, subprojects/flashbar) and its now-unused entries from gradle/libs.versions.toml. Verified: zero com.blankj.utilcode source references remain, :app:compileV8DebugKotlin/JavaWithJavac builds clean, unit tests pass (190 tests, 0 failures) across app/common/ editor/xml-inflater/lsp-java, a full :app:assembleV8Debug succeeds, and the resulting debug APK contains no blankj/utilcode classes. Final commit removing com.blankj:utilcodex (AndroidUtilCode/blankj) - duplicated AndroidX core, Kotlin stdlib, and coroutines functionality that this app now covers with small native equivalents, shrinking the APK. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
…y tracker Per architecture review: common can't depend on app (IDEApplication), so this base tracker exists for common-module consumers (FlashbarUtils); IDEApplication's own pre-existing Pre/Post-callback tracker overrides it and wins at runtime in the real app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Collaborator
Author
Architecture reviewRan the This is a mechanical dependency-removal — every file swaps a blankj
Summary: 0 violations, 1 non-blocking warning (addressed with a doc comment), rest checked and clean. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request replaces BlankJ UtilCode with AndroidIDE utilities, migrates dependencies and callers across modules, updates file, threading, connectivity, crash, and editor flows, adds utility and connectivity tests, and records refactoring guidance. ChangesAndroidIDE utility migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…code' into fix/ADFA-4649-remove-blankj-utilcode
Collaborator
Code review: two blocking regressions in the blankj replacementsI diffed each new in-house util against the actual blankj 1.31.1 sources. Most substitutions are faithful, but two change behavior in ways that are reachable in normal use. 1.
|
rename() silently overwrote an existing file at the destination since File.renameTo invokes rename(2), which atomically replaces the target. blankj's original guarded against this; restore the check. isUtf8() read and decoded the entire file instead of sampling a small header like blankj's original 24-byte check, turning a cheap classifier call into a full read+decode on hot paths (RecursiveFileSearcher, BaseEditorActivity tab restore, IDELanguageClientImpl). Sample only the header again, and return false for empty files to match prior behavior. Adds FileUtilsTest covering both fixes plus the header-sampling and empty-file edge cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt (1)
75-85: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAllow initialization to retry after user unlock.
_isLoadedis set beforeisCredentialStorageReady(). If the app starts before credential storage is available, later calls return at Line 70 and credential-protected initialization never occurs. Set the loaded flag only after readiness succeeds, ideally withcompareAndSet.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt` around lines 75 - 85, Update the loading flow in the credential-protected application loader so _isLoaded is not set until after isCredentialStorageReady(app) succeeds. Use compareAndSet to atomically claim initialization after readiness, allowing a later call after user unlock to retry; preserve the existing early return when storage is unavailable and set application only for successful initialization.app/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.kt (1)
150-162: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCancel delayed callbacks during teardown. Both callbacks outlive their UI owner and can apply stale state after dismissal or destruction.
app/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.kt#L150-L162: retain the debounce runnable/text watcher and remove the callback inonDestroyView().app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt#L1435-L1444: retain the onboarding runnable and remove it frommainThreadHandlerinpreDestroy().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.kt` around lines 150 - 162, Retain the debounce Runnable and text watcher created in RunTasksDialogFragment’s search listener, and remove the runnable from mainThreadHandler during onDestroyView() before the view is released. In BaseEditorActivity, retain the onboarding runnable around the affected startup flow and remove it from mainThreadHandler in preDestroy().Source: Coding guidelines
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
409-435: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRethrow lifecycle cancellation. Both
lifecycleScope.launchblocks treatCancellationExceptionas a restore failure, causing cancellation to be logged and suppressed. Rethrow it before handling parse failures, and narrow the remaining catch to expected JSON failures.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt#L409-L435: rethrowCancellationExceptionbefore handling cached-file JSON parsing failures.app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt#L445-L472: rethrowCancellationExceptionbefore handling cached-plugin-tab JSON parsing failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt` around lines 409 - 435, Update both lifecycleScope.launch blocks in EditorHandlerActivity.kt at lines 409-435 and 445-472: add a CancellationException catch that immediately rethrows before the existing failure handling, then narrow the remaining catch to expected JSON parsing failures for cached files and cached plugin tabs. Preserve the existing error logging for those parsing failures.Sources: Coding guidelines, Learnings
🧹 Nitpick comments (14)
common/src/main/java/com/itsaky/androidide/utils/ResourceUtils.kt (1)
37-56: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCatch only expected asset and stream failures.
These paths primarily throw
IOException; catchingExceptionalso converts unrelated state/programming defects intofalseor"". Keep the logged fallback, but narrow recovery to expected I/O failures (plus any explicitly intended recoverable security failure).Also applies to: 63-71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/ResourceUtils.kt` around lines 37 - 56, Narrow the exception handling in the asset-copy logic around copyFileFromAssets to catch only expected IOException failures, while preserving the existing warning log and false return fallback. Apply the same restriction to the related fallback path around the method’s lines 63-71, and do not convert unrelated programming or state exceptions into recovery values unless an explicitly intended security failure is handled.Source: Coding guidelines
editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt (1)
378-378: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor: implicit default locale in log format string.
detekt flags
String.format("Executing command '%s' for completion item.", command.title)for using the implicit default locale. Since this is only a debug/info log with a single%s, the practical impact is negligible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt` at line 378, Update the log formatting in the completion command execution flow to avoid relying on the implicit default locale. Replace the String.format usage in the visible log.info call with a locale-independent formatting approach while preserving the existing message and command.title value.Source: Linters/SAST tools
app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt (1)
516-524: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse explicit
LocaleinString.formatfor archive listing.detekt flags all three
String.format(...)calls here for using the implicit default locale. Since these compose the numeric columns (sizes, CRC, compression ratio) shown to the user, output can vary by device locale (e.g. decimal separator in"%.1f%%"), producing inconsistent archive listings across devices.♻️ Proposed fix
builder.appendLine( String.format( + java.util.Locale.ROOT, "%-10s %-10s %-6s %-8s %-20s %s", "Length", "Compressed", "Method", "CRC-32", "Date & Time", "Name", ), )Apply the same
Locale.ROOTargument to the other twoString.formatcalls at lines 549-557 and 569-575.Also applies to: 549-557, 569-575
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt` around lines 516 - 524, Update all three archive-listing String.format calls in CodeEditorView to pass Locale.ROOT explicitly, including the header formatting call and the calls near the numeric size/CRC/compression output. Preserve the existing format strings and arguments so listing output remains locale-independent.Source: Linters/SAST tools
common/src/main/java/com/itsaky/androidide/utils/ContextUtils.kt (1)
42-53: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueSwallowed exceptions without logging.
isAccessibilityEnabled()(catch at line 50) andgetAppVersionCode()(catch at line 122) both discard the caught exception without any logging, so a genuinely unexpected failure mode (e.g., unexpectedSettingNotFoundException, or a package-lookup failure) would be silently invisible in production/GlitchTip.♻️ Suggested fix (getAppVersionCode example)
fun Context.getAppVersionCode(): Int = try { packageManager.getPackageInfo(packageName, 0).versionCode } catch (e: PackageManager.NameNotFoundException) { + LoggerFactory.getLogger("ContextUtils").warn("Unable to resolve own package version code", e) -1 }As per coding guidelines: "Do not swallow exceptions silently; log handled notable failures and report them through the established observability mechanism when appropriate."
Also applies to: 115-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/ContextUtils.kt` around lines 42 - 53, Update isAccessibilityEnabled and getAppVersionCode to log the caught exceptions through the project’s established logging/observability mechanism before returning their existing fallback values. Preserve the current false/fallback behavior and avoid changing exception handling beyond making these failures visible.Sources: Coding guidelines, Linters/SAST tools
editor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.java (1)
354-379: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueHoist the lowercased prefix out of the loops.
prefix.toLowerCase(Locale.ROOT)is recomputed for every one of the ~300 candidates on each keystroke.♻️ Proposed refactor
+ final var lowerPrefix = prefix.toLowerCase(Locale.ROOT); for (String artifact : ANDROIDX_ARTIFACTS) { - if (!artifact.toLowerCase(Locale.ROOT).startsWith(prefix.toLowerCase(Locale.ROOT))) { + if (!artifact.toLowerCase(Locale.ROOT).startsWith(lowerPrefix)) { continue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@editor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.java` around lines 354 - 379, Compute the lowercased prefix once before the ANDROIDX_ARTIFACTS, CONFIGURATIONS, and OTHERS loops, then reuse that value in each startsWith comparison instead of calling prefix.toLowerCase(Locale.ROOT) per candidate.app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt (1)
209-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCallback body is a no-op.
The
executeAsynccompletion block only computesdeletedand returns; it neither surfaces failure to the user nor refreshes anything. Either report the failure or drop the callback.♻️ Suggested simplification
.setPositiveButton(R.string.yes) { _, _ -> onRemoveProjectClick(project) - executeAsync({ FileUtils.delete(project.path) }) { - val deleted = it ?: false - if (!deleted) { - return@executeAsync - } - } + executeAsync({ FileUtils.delete(project.path) }) { deleted -> + if (deleted != true) { + flashError(R.string.delete_failed) + } + } }.create()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt` around lines 209 - 217, Remove the no-op completion callback from the executeAsync call in the setPositiveButton handler, unless a user-facing failure message or project-list refresh is implemented there; avoid computing deleted without taking any action.app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt (1)
182-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winViewModel now depends on the
BaseApplicationsingleton for connectivity.Reaching into
BaseApplication.baseInstancemakes the offline branches ofpush/pulluntestable without a Robolectric application. Consider injecting a small connectivity provider (e.g.() -> Boolean) alongsidecredentialsManager. Same pattern applies at Line 266 inpull.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt` around lines 182 - 188, Replace direct BaseApplication.baseInstance connectivity access in the push and pull flows with an injected connectivity provider, such as a Boolean-returning function, alongside credentialsManager. Use that provider for the offline checks in the relevant ViewModel methods and update construction/call sites to supply it, preserving the existing no_internet_connection error behavior.xml-inflater/src/main/java/com/itsaky/androidide/inflater/vectormaster/VectorMasterDrawable.java (1)
58-62: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCatch I/O failures before
fromXMLFile.
FileIOUtils.readFile2String(file)delegates toFile.readText(Charsets.UTF_8), which throws unrecoverable I/O failures to callers. UpdatefromXMLFileto catch the I/O failure and return/propagate an explicit error state instead of letting callers receive an unexpectedUncheckedIOException/crash.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xml-inflater/src/main/java/com/itsaky/androidide/inflater/vectormaster/VectorMasterDrawable.java` around lines 58 - 62, Update VectorMasterDrawable.fromXMLFile to catch the I/O failure raised by FileIOUtils.readFile2String before invoking fromXML, then return or propagate the established explicit error state rather than allowing an unchecked I/O exception to escape. Preserve the existing XML parsing behavior for successfully read files.Source: Coding guidelines
common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt (2)
106-115: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead elvis branch in
newInstance.
Constructor.newInstancenever returnsnull, so the?: return ReflectUtils(Any::class.java, null)fallback is unreachable and, if it ever were reached, would hand back a wrapper with a lost target.♻️ Suggested simplification
- reflect(constructor.newInstance(*args) ?: return ReflectUtils(Any::class.java, null)) + reflect(constructor.newInstance(*args))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt` around lines 106 - 115, Remove the unreachable Elvis fallback from ReflectUtils.newInstance and pass the result of constructor.newInstance(*args) directly to reflect. Preserve the existing exception handling and constructor accessibility behavior.
117-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd KDoc to the public fluent API.
field,field(name, value),method, andgetare the public surface of a hand-rolled reflection helper; documenting the contract (throwsReflectException,methodreturns the receiver forvoid/nullresults,get()is an unchecked cast) would prevent misuse.As per coding guidelines: "Public classes, functions, and non-obvious logic must have KDoc or Javadoc documenting contracts, rationale, threading, nullability, side effects, or units."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt` around lines 117 - 163, Add KDoc to the public ReflectUtils APIs field(name), field(name, value), method(name, vararg args), and get(). Document their reflection behavior, ReflectException failure contract, method’s receiver return for void or null results, and get()’s unchecked cast semantics without changing implementation behavior.Source: Coding guidelines
app/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.kt (1)
50-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the typed
mockkStaticoverload and add an offline-path test.The string form
"com.itsaky.androidide.utils.ContextUtilsKt"hard-codes the generated file-class name; a file rename or@file:JvmNamebreaks the stub. MockK's function-reference overload is refactor-safe. Also, the newly introduced connectivity precondition has no test for thefalsebranch (R.string.no_internet_connection+canRetry), which is the error path the guidelines ask to cover.As per coding guidelines: "Use unit tests for non-UI logic, cover error and edge paths".
♻️ Suggested change
- mockkStatic("com.itsaky.androidide.utils.ContextUtilsKt") + mockkStatic(Context::isNetworkConnected) every { context.isNetworkConnected() } returns truePlus a new test:
`@Test` fun `clone is rejected when device is offline`() { every { context.isNetworkConnected() } returns false val folder = tempFolder.newFolder("Offline") viewModel.cloneRepository("https://github.com/username/newproject.git", folder.absolutePath) val state = viewModel.uiState.value assertTrue(state is CloneRepoUiState.Error) assertEquals(R.string.no_internet_connection, (state as CloneRepoUiState.Error).errorResId) assertTrue(state.canRetry) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.kt` around lines 50 - 65, The setup in CloneRepositoryViewModelTest uses a brittle generated file-class string and lacks coverage for the offline branch. Replace the string-based mockkStatic call with the typed function-reference overload for isNetworkConnected, then add a test covering cloneRepository when connectivity returns false that asserts CloneRepoUiState.Error, R.string.no_internet_connection, and canRetry true.Source: Coding guidelines
common/src/main/java/com/itsaky/androidide/utils/ZipUtils.kt (1)
30-64: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd unit tests for the new
unzipFile, especially the zip-slip rejection.The traversal guard is the security-relevant part of this replacement and there is no
ZipUtilsTestin this PR (onlyFileUtilsTest). A small JUnit test writing a zip with a../evil.txtentry plus a happy-path/nested-directory case would lock the behavior in.As per coding guidelines: "Use unit tests for non-UI logic, cover error and edge paths, and target at least 50% line and branch coverage for new or changed non-UI code."
Want me to generate
common/src/test/java/com/itsaky/androidide/utils/ZipUtilsTest.kt?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/ZipUtils.kt` around lines 30 - 64, The new unzipFile behavior lacks unit coverage, particularly for path traversal rejection. Add a ZipUtilsTest test class covering a successful extraction with nested directories and a zip containing ../evil.txt that causes unzipFile to throw IOException; use temporary files/directories and assert the expected extracted content and security failure.Source: Coding guidelines
lsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/CompletionProvider.java (1)
253-279: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop reflection from the provider switch.
The switch already maps each
Tree.Kindto a concrete completion provider, and the referenced providers already provide constructors accepting those arguments. Instantiating viaReflectUtils.reflect(klass).newInstance(...)defers a constructor mismatch until runtime and adds a reflection dependency for work that can be checked at compile time by returning the provider directly from eachcase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/CompletionProvider.java` around lines 253 - 279, Replace the reflective provider construction in CompletionProvider with direct instantiation in each Tree.Kind switch case, returning the corresponding concrete provider with file, cursor, compiler, and getSettings() arguments. Remove the klass variable and ReflectUtils usage while preserving the existing default KeywordCompletionProvider mapping and cancellation check.common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt (1)
27-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new public utility contracts.
common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt#L27-L112: add KDoc for the public objects and methods, especially UTF-8 sampling, recursive filtering, deletion, rename, and write-result semantics.common/src/main/java/com/itsaky/androidide/utils/ImageUtils.kt#L25-L60: add KDoc forImageUtilsandImageType, including header-sniffing limitations.As per coding guidelines, “Public classes, functions, and non-obvious logic must have KDoc or Javadoc documenting contracts.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt` around lines 27 - 112, Document the public objects and methods in common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt:27-112 with KDoc, covering UTF-8 header sampling, recursive filtering, deletion, rename behavior, and write-result semantics; document common/src/main/java/com/itsaky/androidide/utils/ImageUtils.kt:25-60 with KDoc for ImageUtils and ImageType, including header-sniffing limitations. Use the existing symbols FileUtils, FileIOUtils, ImageUtils, and ImageType, with no behavioral changes required.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/actions/FileActionManager.kt`:
- Around line 23-37: Narrow exception handling across the specified recovery
paths: in FileActionManager’s coroutine wrapper and the three NewFileAction
UI-action paths, rethrow coroutine cancellation before handling only expected
file, UI/I/O, or parsing failures; replace broad Exception catches and top-level
runCatching accordingly. In CredentialProtectedApplicationLoader’s WorkManager
initialization path, replace runCatching around WorkManager.getInstance with a
catch for the expected recoverable failure, without adding cancellation handling
there.
In `@app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt`:
- Around line 251-267: The rename handler in the positive-button callback
performs project.rename disk I/O on the main thread. Route the rename operation
through the existing executeAsync pattern used by the delete flow, and move
flashSuccess, onFileRenamed, notifyItemChanged, and error handling into the
appropriate completion callback while preserving the existing rename arguments
and user-facing results.
In `@app/src/main/java/com/itsaky/androidide/utils/TemplateRecipeExecutor.kt`:
- Around line 64-71: Update copyAsset to manage the destination stream with its
own use block alongside the openAsset input stream, ensuring both streams are
closed after copying while preserving the existing copy behavior.
In `@app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt`:
- Around line 42-51: Update the case-only rename branch in the renamed
computation to roll back the temporary rename when tempFile.renameTo(destFile)
fails: attempt to rename tempFile back to file before returning failure.
Preserve the existing success path and non-case-only FileUtils.rename behavior.
In `@common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt`:
- Around line 46-47: Replace broad exception handling with IOException-only
catches in FileUtils.kt lines 46-47 and 109-110, preserving false results; log
the IOException at the write/failure site before returning false at lines
109-110. In ImageUtils.kt lines 67-69, catch IOException only and preserve the
TYPE_UNKNOWN result; allow unrelated runtime exceptions to propagate.
In `@common/src/main/java/com/itsaky/androidide/utils/ImageUtils.kt`:
- Line 81: Update the TIFF branch in the image-type detection logic to require
the complete signature: little-endian headers must be II 2A 00 and big-endian
headers must be MM 00 2A, rather than validating only the byte-order markers.
Preserve other format checks and add coverage for both valid TIFF headers and
invalid II/MM-prefixed headers.
In `@common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt`:
- Around line 4-6: Update the new tests in FileUtilsTest to use JUnit Jupiter
annotations and replace the JUnit 4 Rule/TemporaryFolder setup with the Jupiter
temporary-directory extension. Remove the org.junit.Rule and
org.junit.rules.TemporaryFolder imports, use the Jupiter equivalents, and adapt
the test fixture to receive the extension-provided temporary directory.
In
`@editor/src/main/java/com/itsaky/androidide/editor/language/incremental/BaseIncrementalAnalyzeManager.java`:
- Around line 389-392: Update the token-type check in the incremental analysis
loop to use the cached blockTokens field instead of calling getCodeBlockTokens()
and creating an IntStream for each token. Preserve null-safe behavior so a null
blockTokens value is treated as no matching code-block token, and remove the
now-unused IntStream import.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 409-435: Update both lifecycleScope.launch blocks in
EditorHandlerActivity.kt at lines 409-435 and 445-472: add a
CancellationException catch that immediately rethrows before the existing
failure handling, then narrow the remaining catch to expected JSON parsing
failures for cached files and cached plugin tabs. Preserve the existing error
logging for those parsing failures.
In
`@app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt`:
- Around line 75-85: Update the loading flow in the credential-protected
application loader so _isLoaded is not set until after
isCredentialStorageReady(app) succeeds. Use compareAndSet to atomically claim
initialization after readiness, allowing a later call after user unlock to
retry; preserve the existing early return when storage is unavailable and set
application only for successful initialization.
In `@app/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.kt`:
- Around line 150-162: Retain the debounce Runnable and text watcher created in
RunTasksDialogFragment’s search listener, and remove the runnable from
mainThreadHandler during onDestroyView() before the view is released. In
BaseEditorActivity, retain the onboarding runnable around the affected startup
flow and remove it from mainThreadHandler in preDestroy().
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt`:
- Around line 209-217: Remove the no-op completion callback from the
executeAsync call in the setPositiveButton handler, unless a user-facing failure
message or project-list refresh is implemented there; avoid computing deleted
without taking any action.
In `@app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt`:
- Around line 516-524: Update all three archive-listing String.format calls in
CodeEditorView to pass Locale.ROOT explicitly, including the header formatting
call and the calls near the numeric size/CRC/compression output. Preserve the
existing format strings and arguments so listing output remains
locale-independent.
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt`:
- Around line 182-188: Replace direct BaseApplication.baseInstance connectivity
access in the push and pull flows with an injected connectivity provider, such
as a Boolean-returning function, alongside credentialsManager. Use that provider
for the offline checks in the relevant ViewModel methods and update
construction/call sites to supply it, preserving the existing
no_internet_connection error behavior.
In
`@app/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.kt`:
- Around line 50-65: The setup in CloneRepositoryViewModelTest uses a brittle
generated file-class string and lacks coverage for the offline branch. Replace
the string-based mockkStatic call with the typed function-reference overload for
isNetworkConnected, then add a test covering cloneRepository when connectivity
returns false that asserts CloneRepoUiState.Error,
R.string.no_internet_connection, and canRetry true.
In `@common/src/main/java/com/itsaky/androidide/utils/ContextUtils.kt`:
- Around line 42-53: Update isAccessibilityEnabled and getAppVersionCode to log
the caught exceptions through the project’s established logging/observability
mechanism before returning their existing fallback values. Preserve the current
false/fallback behavior and avoid changing exception handling beyond making
these failures visible.
In `@common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt`:
- Around line 27-112: Document the public objects and methods in
common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt:27-112 with KDoc,
covering UTF-8 header sampling, recursive filtering, deletion, rename behavior,
and write-result semantics; document
common/src/main/java/com/itsaky/androidide/utils/ImageUtils.kt:25-60 with KDoc
for ImageUtils and ImageType, including header-sniffing limitations. Use the
existing symbols FileUtils, FileIOUtils, ImageUtils, and ImageType, with no
behavioral changes required.
In `@common/src/main/java/com/itsaky/androidide/utils/ReflectUtils.kt`:
- Around line 106-115: Remove the unreachable Elvis fallback from
ReflectUtils.newInstance and pass the result of constructor.newInstance(*args)
directly to reflect. Preserve the existing exception handling and constructor
accessibility behavior.
- Around line 117-163: Add KDoc to the public ReflectUtils APIs field(name),
field(name, value), method(name, vararg args), and get(). Document their
reflection behavior, ReflectException failure contract, method’s receiver return
for void or null results, and get()’s unchecked cast semantics without changing
implementation behavior.
In `@common/src/main/java/com/itsaky/androidide/utils/ResourceUtils.kt`:
- Around line 37-56: Narrow the exception handling in the asset-copy logic
around copyFileFromAssets to catch only expected IOException failures, while
preserving the existing warning log and false return fallback. Apply the same
restriction to the related fallback path around the method’s lines 63-71, and do
not convert unrelated programming or state exceptions into recovery values
unless an explicitly intended security failure is handled.
In `@common/src/main/java/com/itsaky/androidide/utils/ZipUtils.kt`:
- Around line 30-64: The new unzipFile behavior lacks unit coverage,
particularly for path traversal rejection. Add a ZipUtilsTest test class
covering a successful extraction with nested directories and a zip containing
../evil.txt that causes unzipFile to throw IOException; use temporary
files/directories and assert the expected extracted content and security
failure.
In
`@editor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.java`:
- Around line 354-379: Compute the lowercased prefix once before the
ANDROIDX_ARTIFACTS, CONFIGURATIONS, and OTHERS loops, then reuse that value in
each startsWith comparison instead of calling prefix.toLowerCase(Locale.ROOT)
per candidate.
In `@editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt`:
- Line 378: Update the log formatting in the completion command execution flow
to avoid relying on the implicit default locale. Replace the String.format usage
in the visible log.info call with a locale-independent formatting approach while
preserving the existing message and command.title value.
In
`@lsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/CompletionProvider.java`:
- Around line 253-279: Replace the reflective provider construction in
CompletionProvider with direct instantiation in each Tree.Kind switch case,
returning the corresponding concrete provider with file, cursor, compiler, and
getSettings() arguments. Remove the klass variable and ReflectUtils usage while
preserving the existing default KeywordCompletionProvider mapping and
cancellation check.
In
`@xml-inflater/src/main/java/com/itsaky/androidide/inflater/vectormaster/VectorMasterDrawable.java`:
- Around line 58-62: Update VectorMasterDrawable.fromXMLFile to catch the I/O
failure raised by FileIOUtils.readFile2String before invoking fromXML, then
return or propagate the established explicit error state rather than allowing an
unchecked I/O exception to escape. Preserve the existing XML parsing behavior
for successfully read files.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f893600-589c-4eca-93e5-647e3e3688af
📒 Files selected for processing (97)
CLAUDE.mdactions/build.gradle.ktsapp/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/actions/FileActionManager.ktapp/src/main/java/com/itsaky/androidide/actions/filetree/CopyPathAction.ktapp/src/main/java/com/itsaky/androidide/actions/filetree/DeleteAction.ktapp/src/main/java/com/itsaky/androidide/actions/filetree/NewFileAction.ktapp/src/main/java/com/itsaky/androidide/actions/text/RedoAction.ktapp/src/main/java/com/itsaky/androidide/activities/AboutActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/FullscreenManager.ktapp/src/main/java/com/itsaky/androidide/activities/editor/ProjectHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.ktapp/src/main/java/com/itsaky/androidide/adapters/SearchListAdapter.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/adapters/onboarding/OnboardingPermissionsAdapter.ktapp/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.javaapp/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/app/IDEApplication.ktapp/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/SearchFieldToolbar.ktapp/src/main/java/com/itsaky/androidide/fragments/sheets/OptionsListFragment.javaapp/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.ktapp/src/main/java/com/itsaky/androidide/handlers/CrashEventSubscriber.ktapp/src/main/java/com/itsaky/androidide/handlers/GlitchTipDiagnosticsContext.ktapp/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.javaapp/src/main/java/com/itsaky/androidide/services/builder/GradleBuildService.ktapp/src/main/java/com/itsaky/androidide/tasks/callables/ListDirectoryCallable.javaapp/src/main/java/com/itsaky/androidide/tasks/callables/UnzipCallable.javaapp/src/main/java/com/itsaky/androidide/ui/CodeEditorView.ktapp/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.ktapp/src/main/java/com/itsaky/androidide/utils/BuildInfoUtils.ktapp/src/main/java/com/itsaky/androidide/utils/IntentUtils.ktapp/src/main/java/com/itsaky/androidide/utils/ProjectWriter.javaapp/src/main/java/com/itsaky/androidide/utils/RecursiveFileSearcher.javaapp/src/main/java/com/itsaky/androidide/utils/TemplateRecipeExecutor.ktapp/src/main/java/com/itsaky/androidide/utils/WindowInsetsExtensions.ktapp/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/EditorViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.ktapp/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.ktcommon-ui/src/main/java/com/itsaky/androidide/FabPositionCalculator.ktcommon/build.gradle.ktscommon/src/main/java/com/itsaky/androidide/app/BaseApplication.ktcommon/src/main/java/com/itsaky/androidide/managers/ToolsManager.javacommon/src/main/java/com/itsaky/androidide/tasks/TaskExecutor.ktcommon/src/main/java/com/itsaky/androidide/utils/ContextUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/DeviceUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/Environment.javacommon/src/main/java/com/itsaky/androidide/utils/FileUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/FlashbarUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ImageUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ReflectUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ResourceUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ZipUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/urlManager.ktcommon/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.ktdocs/process/learnings.mddocs/process/retrospective.mdeditor/build.gradle.ktseditor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.javaeditor/src/main/java/com/itsaky/androidide/editor/language/incremental/BaseIncrementalAnalyzeManager.javaeditor/src/main/java/com/itsaky/androidide/editor/ui/BaseEditorWindow.javaeditor/src/main/java/com/itsaky/androidide/editor/ui/EditorActionsMenu.kteditor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.ktgradle/libs.versions.tomllsp/api/build.gradle.ktslsp/java/build.gradle.ktslsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/FieldBasedAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateConstructorAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateSettersAndGettersAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/GenerateToStringMethodAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/actions/generators/OverrideSuperclassMethodsAction.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/compiler/SourceFileManager.javalsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/CompletionProvider.javalsp/models/build.gradle.ktslsp/models/src/main/java/com/itsaky/androidide/lsp/edits/DefaultEditHandler.ktlsp/xml/build.gradle.ktssubprojects/flashbar/build.gradle.ktssubprojects/javac-services/build.gradle.ktssubprojects/javac-services/src/main/java/com/itsaky/androidide/javac/services/visitors/UnEnter.ktuidesigner/build.gradle.ktsuidesigner/src/main/java/com/itsaky/androidide/uidesigner/drawable/UiViewLayeredForeground.ktuidesigner/src/main/java/com/itsaky/androidide/uidesigner/fragments/DesignerWorkspaceFragment.ktuidesigner/src/main/java/com/itsaky/androidide/uidesigner/utils/ValueCompletionProvider.ktuidesigner/src/main/java/com/itsaky/androidide/uidesigner/utils/ViewToXml.ktuidesigner/src/main/java/com/itsaky/androidide/uidesigner/views/LayoutHierarchyView.ktxml-inflater/build.gradle.ktsxml-inflater/src/main/java/com/itsaky/androidide/inflater/drawable/DrawableParserFactory.javaxml-inflater/src/main/java/com/itsaky/androidide/inflater/internal/adapters/GestureOverlayViewAdapter.ktxml-inflater/src/main/java/com/itsaky/androidide/inflater/internal/adapters/ListViewAdapter.ktxml-inflater/src/main/java/com/itsaky/androidide/inflater/internal/adapters/TextViewAdapter.ktxml-inflater/src/main/java/com/itsaky/androidide/inflater/internal/adapters/ToggleButtonAdapter.ktxml-inflater/src/main/java/com/itsaky/androidide/inflater/models/UiWidget.ktxml-inflater/src/main/java/com/itsaky/androidide/inflater/vectormaster/VectorMasterDrawable.java
💤 Files with no reviewable changes (13)
- subprojects/javac-services/build.gradle.kts
- subprojects/flashbar/build.gradle.kts
- uidesigner/build.gradle.kts
- lsp/models/build.gradle.kts
- lsp/java/build.gradle.kts
- lsp/xml/build.gradle.kts
- xml-inflater/build.gradle.kts
- editor/build.gradle.kts
- common/build.gradle.kts
- actions/build.gradle.kts
- lsp/api/build.gradle.kts
- app/build.gradle.kts
- gradle/libs.versions.toml
Correctness fixes: - CredentialProtectedApplicationLoader: don't mark initialization as loaded until credential storage readiness succeeds, so a failed attempt can retry after user unlock (compareAndSet-based claim). - RecentProjectsAdapter: move project rename off the main thread (executeAsyncProvideError, matching the existing delete flow). - TemplateRecipeExecutor: close the destination stream in copyAsset (was leaking a file descriptor per copy). - FileManagerViewModel: roll back a case-only rename to its original name if the second rename step fails, instead of stranding the file under its temporary name. - ImageUtils: fix TIFF detection to require the full 4-byte magic number, not just the 2-byte byte-order marker (was false-positiving on any file starting with "II"/"MM"). - ImageUtils.isUtf8-adjacent FileUtils.isUtf8: decode the UTF-8 sample with endOfInput=false so a multi-byte sequence truncated at the 24-byte sample boundary reports underflow instead of a spurious malformed-input error. - ReflectUtils: remove a dead Elvis fallback in newInstance, and restore blankj's final-field-modifier stripping in the field setter (best-effort; falls back to the normal IllegalAccessException where the runtime blocks it). - CompletionProvider: replace reflective provider construction with direct instantiation per Tree.Kind (drops ReflectUtils from the completion hot path). - VectorMasterDrawable.fromXMLFile: throw a clear IOException instead of passing a null read result into the XML parser. - BaseIncrementalAnalyzeManager: use the already-cached blockTokens field instead of rebuilding an IntStream per token in the analysis loop. Exception handling: - FileActionManager, NewFileAction, EditorHandlerActivity: rethrow CancellationException before narrower catches, so cancellation isn't swallowed by structured concurrency. - FileUtils, ImageUtils, ResourceUtils: narrow broad Exception catches to IOException; FileUtils/ResourceUtils now log on failure. Other fixes: - RunTasksDialogFragment, BaseEditorActivity: retain debounce/ onboarding Runnables as stable fields and remove their callbacks in onDestroyView()/preDestroy(), since Handler.removeCallbacks needs the exact same Runnable instance to actually cancel anything. - GitBottomSheetViewModel: make the connectivity check injectable (default arg preserves production behavior) for testability. - CodeEditorView, IDEEditor, GroovyAutoComplete: locale-independent formatting for archive listing/logging, and hoist a loop-invariant toLowerCase call out of a per-candidate autocomplete loop. - ContextUtils: log the exceptions isAccessibilityEnabled and getAppVersionCode were silently swallowing. Test coverage: - Add regression tests for all of the above where practical. - Add ReflectUtilsTest, ImageUtilsTest, ResourceUtilsTest (new mockk test dependency in :common) covering previously-untested public API surface, and expand ZipUtils/FileUtils test coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt (1)
12-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMigrate these new tests to JUnit Jupiter.
These tests introduce JUnit 4 APIs despite the required Jupiter test stack. This repeats the prior
FileUtilsTestfinding.
common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt#L12-L14: replaceTemporaryFolderwith Jupiter@TempDir.common/src/test/java/com/itsaky/androidide/utils/ImageUtilsTest.kt#L4-L6: replace JUnit 4@TestandTemporaryFolder.common/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.kt#L4-L5: replace JUnit 4@TestandAssert.assertThrows.common/src/test/java/com/itsaky/androidide/utils/ResourceUtilsTest.kt#L10-L14: replace JUnit 4 lifecycle annotations andTemporaryFolder.common/src/test/java/com/itsaky/androidide/utils/ZipUtilsTest.kt#L4-L7: replace JUnit 4@Test,assertThrows, andTemporaryFolder.As per coding guidelines, “Use JUnit Jupiter, Truth, MockK for new tests.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt` around lines 12 - 14, Migrate the listed tests to JUnit Jupiter: in common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt (12-14), replace TemporaryFolder with `@TempDir`; in ImageUtilsTest.kt (4-6), replace JUnit 4 `@Test` and TemporaryFolder; in ReflectUtilsTest.kt (4-5), replace JUnit 4 `@Test` and Assert.assertThrows; in ResourceUtilsTest.kt (10-14), replace JUnit 4 lifecycle annotations and TemporaryFolder; and in ZipUtilsTest.kt (4-7), replace JUnit 4 `@Test`, assertThrows, and TemporaryFolder. Use Jupiter, Truth, and MockK APIs consistently while preserving each test’s behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.kt`:
- Around line 82-89: Update the load method’s compareAndSet claim and
initialization flow so failures or cancellation during WorkManager, EventBus,
theme, or plugin setup do not leave _isLoaded true. Track initialization
separately or reset the claim and clean up partial registrations on failure, and
publish _isLoaded only after all initialization completes successfully; preserve
the existing concurrent-load guard.
In `@common/src/main/java/com/itsaky/androidide/utils/FileUtils.kt`:
- Around line 138-140: The IOException handler’s logger.warn call currently
exposes the full file path and potentially path details from the throwable.
Update the catch block to log only a path-free, redacted error identifier or
message, while preserving the false return behavior.
---
Outside diff comments:
In `@common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt`:
- Around line 12-14: Migrate the listed tests to JUnit Jupiter: in
common/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.kt (12-14),
replace TemporaryFolder with `@TempDir`; in ImageUtilsTest.kt (4-6), replace JUnit
4 `@Test` and TemporaryFolder; in ReflectUtilsTest.kt (4-5), replace JUnit 4 `@Test`
and Assert.assertThrows; in ResourceUtilsTest.kt (10-14), replace JUnit 4
lifecycle annotations and TemporaryFolder; and in ZipUtilsTest.kt (4-7), replace
JUnit 4 `@Test`, assertThrows, and TemporaryFolder. Use Jupiter, Truth, and MockK
APIs consistently while preserving each test’s behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a4a15a41-e76d-46f8-829a-c21d95602d26
📒 Files selected for processing (28)
app/src/main/java/com/itsaky/androidide/actions/FileActionManager.ktapp/src/main/java/com/itsaky/androidide/actions/filetree/NewFileAction.ktapp/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.ktapp/src/main/java/com/itsaky/androidide/app/CredentialProtectedApplicationLoader.ktapp/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.ktapp/src/main/java/com/itsaky/androidide/ui/CodeEditorView.ktapp/src/main/java/com/itsaky/androidide/utils/TemplateRecipeExecutor.ktapp/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.ktapp/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.ktcommon/build.gradle.ktscommon/src/main/java/com/itsaky/androidide/utils/ContextUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/FileUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ImageUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ReflectUtils.ktcommon/src/main/java/com/itsaky/androidide/utils/ResourceUtils.ktcommon/src/test/java/com/itsaky/androidide/utils/FileUtilsTest.ktcommon/src/test/java/com/itsaky/androidide/utils/ImageUtilsTest.ktcommon/src/test/java/com/itsaky/androidide/utils/ReflectUtilsTest.ktcommon/src/test/java/com/itsaky/androidide/utils/ResourceUtilsTest.ktcommon/src/test/java/com/itsaky/androidide/utils/ZipUtilsTest.kteditor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.javaeditor/src/main/java/com/itsaky/androidide/editor/language/incremental/BaseIncrementalAnalyzeManager.javaeditor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.ktlsp/java/src/main/java/com/itsaky/androidide/lsp/java/providers/CompletionProvider.javaxml-inflater/src/main/java/com/itsaky/androidide/inflater/vectormaster/VectorMasterDrawable.java
🚧 Files skipped from review as they are similar to previous changes (14)
- common/src/main/java/com/itsaky/androidide/utils/ResourceUtils.kt
- common/build.gradle.kts
- app/src/main/java/com/itsaky/androidide/fragments/RunTasksDialogFragment.kt
- app/src/main/java/com/itsaky/androidide/viewmodel/FileManagerViewModel.kt
- app/src/main/java/com/itsaky/androidide/utils/TemplateRecipeExecutor.kt
- app/src/test/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModelTest.kt
- editor/src/main/java/com/itsaky/androidide/editor/language/groovy/GroovyAutoComplete.java
- app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
- app/src/main/java/com/itsaky/androidide/actions/filetree/NewFileAction.kt
- common/src/main/java/com/itsaky/androidide/utils/ContextUtils.kt
- app/src/main/java/com/itsaky/androidide/adapters/RecentProjectsAdapter.kt
- app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
- editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt
- app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt
CredentialProtectedApplicationLoader: _isLoaded was claimed via compareAndSet before ~10 subsequent init steps (WorkManager, Environment, FeatureFlags, EventBus, Termux, theme, plugin system, ToolsManager), several of which had no error handling and would leave _isLoaded stuck true on failure/cancellation, permanently blocking any retry. Wrap the init body in try/catch that un-claims _isLoaded before rethrowing, and make EventBus registration idempotent so a retry after a partial failure doesn't immediately fail on double-registration. FileUtils.writeFileFromString: the failure log included the file's full path and the raw exception (whose message often re-embeds the path). Log only the exception's class name instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…devforall/CodeOnTheGo into fix/ADFA-4649-remove-blankj-utilcode
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt (1)
284-290: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winApply word-wrap to the active fragment when clicked.
This handler only changes the preference and button state. It does not call
setWordWrapEnabled()on the activeWrappableOutputFragment, so the output can remain unchanged until another sheet-state update occurs.Proposed fix
binding.wordWrapOutputAction.setOnClickListener { val newState = !EditorPreferences.outputWordWrap EditorPreferences.outputWordWrap = newState + val fragment = + pagerAdapter.getFragmentAtIndex<Fragment>(binding.tabs.selectedTabPosition) + if (fragment is WrappableOutputFragment) { + fragment.setWordWrapEnabled(newState) + } updateWordWrapButtonState(newState) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt` around lines 284 - 290, Update the wordWrapOutputAction click handler to apply the new state immediately to the active WrappableOutputFragment by calling setWordWrapEnabled(newState), while preserving the existing preference and button-state updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt`:
- Around line 284-290: Update the wordWrapOutputAction click handler to apply
the new state immediately to the active WrappableOutputFragment by calling
setWordWrapEnabled(newState), while preserving the existing preference and
button-state updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 51efc43d-3698-447b-bc39-26a8859197da
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/ui/EditorBottomSheet.kt
dara-abijo-adfa
requested changes
Jul 28, 2026
Per review feedback, use the existing ActionData.requireContext() helper instead of data[Context::class.java]!! - same behavior (throws if absent) but matches the idiom already used by every other action in this package. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-blankj-utilcode # Conflicts: # editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt
Fixes Hal Eisen's 5-point review of PR #1576: - isSoftInputVisible() now gates the WindowInsetsCompat IME check on API 30+ and falls back to a decor visible-frame heuristic below that, since the inset bit misreports on API 28-29 (MIN_SDK = 28), same as our own termux KeyboardUtils.isSoftKeyboardVisible() documents. - BaseApplication drops its dead foregroundActivity tracker: IDEApplication is the only subclass that populates it, so the base getter now just returns null instead of registering an unused ActivityLifecycleCallbacks. - ReflectUtils' four public methods now share one wrapErrors() helper instead of repeating the same try/catch wrapping block. - DeviceUtils.getModel() drops a redundant .trim() before a regex that already strips all whitespace. - FileUtils.kt gets a doc pointer to the pre-existing FileUtil.java, noting the differing failure contracts, so the next maintainer knows which to reach for. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dara-abijo-adfa
approved these changes
Jul 29, 2026
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
3 participants
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
com.blankj:utilcodex("AndroidUtilCode") dependency entirely — it duplicated functionality AndroidX core, Kotlin stdlib, and coroutines already provide, per the ticket.SizeUtils/ConvertUtilsdp↔px conversions →Context.dpToPx()/spToPx()extensionsThreadUtils→ a sharedmainThreadHandler+ nativerunOnUiThread()FileUtils/FileIOUtils→ same-name native drop-ins (isUtf8, readFile2String, writeFileFromString, listFilesInDirWithFilter, delete, rename, getFileExtension, createOrExistsDir) so most call sites only needed an import swapClipboardUtils/KeyboardUtils/NetworkUtils→Contextextensions (copyToClipboard,isNetworkConnected,isSoftInputVisibleviaWindowInsetsCompat's IME check instead of blankj's decor-view-height heuristic)DeviceUtils,AppUtils,ThrowableUtils.getFullStackTrace→ Kotlin'sstackTraceToString(),CloseUtils,ArrayUtils,StringUtils,Utils.getApp()/ActivityUtils→BaseApplication)ReflectUtilsport (verified bug-for-bug against the actual blankj bytecode viajavap, since a couple of call sites inUnEnter.ktchain in a way that only works if the port matches blankj's exact field-wrapping semantics), plusImageUtils,ResourceUtils(recursive asset copy),ZipUtils(with zip-slip protection added as a safe hardening)libs.versions.tomlVerification
com.blankj.utilcodesource references remain anywhere in the repo:app:compileV8DebugKotlin/compileV8DebugJavaWithJavacbuild clean after each stage:app:assembleV8Debugsucceeds; resulting APK contains zero blankj/utilcode classesspotlessCheckpasses (pre-push hook)Test plan
🤖 Generated with Claude Code