terrymanu commented on PR #39768: URL: https://github.com/apache/shardingsphere/pull/39768#issuecomment-5582951348
### Result **Review Result: Mergeable** The candidate correctly enforces explicit `@SneakyThrows` exception types while preserving existing exception propagation. ### Evidence - Added semantic rules for specific exceptions, justified `Exception.class`, actual `Throwable` propagation, and optional annotation use. - Added an exact Checkstyle `MatchXpath` rule that rejects bare `@SneakyThrows`, `@SneakyThrows()`, and empty arrays while accepting explicit class literals. - Removed all 18 existing parameterless annotations across 12 files. - Multi-module compilation, Spotless, Checkstyle, Apache RAT, and XPath fixtures passed with exit code 0. - Deterministic policy validation passed; all 270 source-read traces passed. - Both new implementation and standalone-audit semantic cases passed. - The 140-case full run produced one unrelated wording-only failure in an unchanged case; its exact unchanged retry passed. No policy regression remains. - The candidate added two cases while preserving comparable policy-evaluation duration and token cost. - No functional, performance, compatibility, security, or architecture regression was found. ### Coverage Standalone local candidate based on `9dad0f777abc`; 17 task-owned files were reviewed. CI was not reviewed because this was a local Code Correctness Review. - `.codex/skills/coding-standards/references/rules/java-exceptions.md` -> defines exception rules -> semantic enforcement -> necessary normative source. - `.codex/skills/coding-standards/SKILL.md` -> activates the rules for writing and auditing -> both entry points -> necessary routing. - `.codex/harness/agents/policy-sources.toml` -> registers and loads the source -> policy discovery -> necessary routing. - `.codex/harness/agents/cases.toml` -> tests implementation and standalone-audit behavior -> regression protection -> necessary canaries. - `src/resources/checkstyle.xml` -> prohibits parameterless Lombok `SneakyThrows` -> static enforcement -> necessary Checkstyle rule. - `InstanceMethodAdviceExecutor.java` -> uses `Throwable.class` -> actual `Throwable` rethrow -> preserves behavior. - `StaticMethodAdviceExecutor.java` -> uses `Throwable.class` -> actual `Throwable` rethrow -> preserves behavior. - `FirebirdSRPAuthenticationData.java` -> uses `NoSuchAlgorithmException.class` -> concrete exception -> necessary compliance. - `DataSourcePoolDestroyer.java` -> uses `Exception.class` -> `AutoCloseable.close()` contract -> justified broad exception. - `ReflectContext.java` -> uses `ReflectiveOperationException.class` -> reflection contract -> necessary compliance. - `ReflectValue.java` -> uses `ReflectiveOperationException.class` -> reflection contract -> necessary compliance. - `EnumerableScanImplementor.java` -> uses `SQLException.class` -> JDBC contract -> necessary compliance. - `StorageUnitManagerTest.java` -> uses `Exception.class` -> `AutoCloseable.close()` contract -> justified broad exception. - `ClusterMetaDataManagerPersistService.java` -> uses `SQLException.class` -> JDBC contract -> necessary compliance. - `FirebirdFetchStatementCache.java` -> uses `SQLException.class` -> backend close contract -> necessary compliance. - `OpenGaussAuthenticationEngineTest.java` -> uses `GeneralSecurityException` and `DecoderException` -> exact checked exceptions -> necessary compliance. - `IndexesMigrationE2EIT.java` -> removes an unnecessary annotation -> no checked exception requires it -> smallest compliant change. All files were covered by root-cause, blast-radius, tests/runtime, and convergence passes. No unresolved findings or evidence gaps remain. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
