Conversation
📝 WalkthroughWalkthroughModernizes build and Java toolchain (move to Java 21), standardizes dependency coordinates, and systematically replaces index-based list/child access with safer first/last/getChildren APIs across production and test code; includes token reclassification and related semantic-token/highlighting updates and small logging/import cleanups. Changes
Sequence Diagram(s)(No sequence diagrams generated.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/bsl/Constructors.java (1)
65-71: 🛠️ Refactor suggestion | 🟠 MajorUpdate the coding guidelines to reflect the Java 21 requirement.
The method uses
List.getFirst(), which is part of theSequencedCollectioninterface introduced in Java 21 via JEP 431 and is unavailable in Java 17. The change is functionally correct—theisEmpty()guard on line 66 ensuresgetFirst()is never called on an empty list, preventingNoSuchElementException. However,.github/copilot-instructions.mdline 18 still mandates Java 17. Since this PR upgrades to Java 21, update the guideline from Java 17 to Java 21 to remain consistent with the new baseline.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/bsl/Constructors.java` around lines 65 - 71, The helper method first(List<T>) in Constructors.java now relies on List.getFirst() (SequencedCollection, Java 21), so update the repository coding guideline file (copilot-instructions.md) to reflect the new Java 21 baseline instead of Java 17; change the Java version requirement text from “Java 17” to “Java 21” and, if present, any related guidance that references JEP 431 or SequencedCollection compatibility to avoid future confusion.src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java (1)
88-434:⚠️ Potential issue | 🟡 MinorUpdate
.github/copilot-instructions.mdto reflect Java 21 as the baseline.
List.getFirst()(and relatedSequencedCollectionmethods) are Java 21 APIs and are not available in Java 17. The test code itself is correct—all first-element accesses follow size assertions—but the repository's coding guideline still states "Language: Java 17" in both the Technology Stack and Prerequisites sections. Since this PR upgrades to JDK 21, updatecopilot-instructions.mdto reflect the new Java 21 baseline so the documented guideline remains consistent with the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/MethodDescriptionTest.java` around lines 88 - 434, Update .github/copilot-instructions.md to state Java 21 as the baseline (since the code uses Java 21 APIs such as List.getFirst() / SequencedCollection methods); specifically change the "Language: Java 17" entries in the Technology Stack and Prerequisites sections to "Java 21" and ensure any prerequisite JDK version notes reflect JDK 21 so the documented guideline matches the codebase.src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/OneStatementPerLineDiagnosticTest.java (1)
62-84:⚠️ Potential issue | 🟡 MinorUpdate
.github/copilot-instructions.mdto reflect Java 21.The copilot-instructions.md file currently references Java 17 (lines 18 and 30: "Language: Java 17" and "Java Development Kit 17"), but the project is configured for Java 21 in
build.gradle.kts(lines 164-165). The test code changes on lines 62, 70, 80, and 84 useList.getFirst(), which is part of theSequencedCollectioninterface (JEP 431) introduced in Java 21 and unavailable in Java 17.Update the documentation to reflect the actual Java 21 target version:
- Line 18: Change "Java 17" to "Java 21"
- Line 30: Change "Java Development Kit 17" to "Java Development Kit 21"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/OneStatementPerLineDiagnosticTest.java` around lines 62 - 84, Documentation mismatch: tests use List.getFirst() (SequencedCollection / JEP 431) which requires Java 21 but .github/copilot-instructions.md still says Java 17. Edit the copilot-instructions file to update the Java version strings: replace "Java 17" with "Java 21" and "Java Development Kit 17" with "Java Development Kit 21" so the docs match the project configuration and the use of List.getFirst()/SequencedCollection.src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnosticTest.java (1)
109-119:⚠️ Potential issue | 🟡 Minor
testCaseInsensitiveEventNamesdoes not exercise case-insensitive behaviourThe test runs
getDiagnostics()on the same single fixture and re-asserts four of the six ranges already covered bytestMainFixture. Because the fixture file itself is not varied (no upper-case/mixed-case event name variants), the test provides no evidence of case-insensitive matching — the name is misleading. Either supply a fixture with different-cased event names or merge this intotestMainFixtureto avoid false confidence.As per coding guidelines (
src/test/java/**/diagnostics/**/*Test.java): "Write comprehensive unit tests for each diagnostic including edge cases, following existing test patterns."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnosticTest.java` around lines 109 - 119, The testCaseInsensitiveEventNames test is misleading because it calls getDiagnostics() on the same fixture used by testMainFixture and doesn't verify any mixed-case event names; either add a new test fixture file (or modify the existing fixture used by testCaseInsensitiveEventNames) that contains upper-case and mixed-case event name variants so getDiagnostics() can assert case-insensitive matches, or remove this standalone test and merge its meaningful assertions into testMainFixture; update assertions in testCaseInsensitiveEventNames (or the merged test) to assert the expected diagnostic ranges for the new mixed-case events and keep references to the same helper methods (getDiagnostics) and test names (testCaseInsensitiveEventNames, testMainFixture) so reviewers can locate the changes.src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnosticTest.java (1)
1-165:⚠️ Potential issue | 🟡 MinorUpdate
.github/copilot-instructions.mdto reflect Java 21. The instructions file currently specifies Java 17 as the language target; it should be updated to Java 21 to align with this PR's migration goals. The test file itself is compliant with current guidelines and maintains comprehensive test coverage for the diagnostic, including edge cases for cached values, missing variables, and non-client-side scenarios.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnosticTest.java` around lines 1 - 165, Update the repository Copilot instructions to target Java 21 (currently set to Java 17): open .github/copilot-instructions.md and change the declared Java language/version from 17 to 21 (and adjust any example snippets or tool/version hints if present), then commit; this PR also includes tests in TransferringParametersBetweenClientAndServerDiagnosticTest (see class name and getMessage method) which should remain unchanged and will validate the migration.src/test/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfigurationTest.java (1)
181-191:⚠️ Potential issue | 🟡 MinorReplace hardcoded
10with a reference toCacheConfiguration.MAX_CACHE_INSTANCES.The test currently hardcodes a loop bound of 10, while
CacheConfigurationusesMAX_CACHE_INSTANCES(also 10) to control the retry limit increateEhcacheManagerWithRetry. IfMAX_CACHE_INSTANCESis later increased, this test will only lock 10 cache slots and fail to trigger the fallback-to-memory scenario, causing a false pass since the cache would still be created with disk persistence.Since the test is in the same package, change
MAX_CACHE_INSTANCESfromprivateto package-private, or extract a test constant referencing it. Either approach ensures the test and implementation stay synchronized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfigurationTest.java` around lines 181 - 191, The test uses a hardcoded loop bound of 10; change it to use CacheConfiguration.MAX_CACHE_INSTANCES so the test follows the implementation's retry limit: update the loop to iterate up to CacheConfiguration.MAX_CACHE_INSTANCES and ensure the test can access that constant by making MAX_CACHE_INSTANCES package-private (default) in CacheConfiguration or by exposing it via a package-visible accessor/constant for tests; this keeps the test behavior aligned with createEhcacheManagerWithRetry/createEhcacheManager retry logic rather than a magic number.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentChangeExecutor.java (1)
251-257:⚠️ Potential issue | 🟠 Major
awaitLatest()callers can hang indefinitely ifchangeListener.onChange()throws.The
catchblock influshPendingChangesresetspendingContent/pendingVersion(infinally) but never advanceslatestAppliedVersionand never callscompleteWaitersUpTo. Any thread blocked inawaitLatest()waiting forpendingVersionwill be parked inversionWaitersforever.Compare with the error path in
accumulate()(lines 229–232), which correctly does both:latestAppliedVersion.accumulateAndGet(task.version, Math::max); completeWaitersUpTo(latestAppliedVersion.get());The fix is to mirror that behaviour here, capturing
pendingVersionbefore thefinallyblock clears it:🐛 Proposed fix
try { changeListener.onChange(documentContext, pendingContent, pendingVersion); latestAppliedVersion.accumulateAndGet(pendingVersion, Math::max); completeWaitersUpTo(latestAppliedVersion.get()); } catch (Exception e) { LOGGER.error("Error while applying accumulated document changes: {}", e.getMessage(), e); + latestAppliedVersion.accumulateAndGet(pendingVersion, Math::max); + completeWaitersUpTo(latestAppliedVersion.get()); } finally { lock.writeLock().unlock(); pendingContent = null; pendingVersion = -1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentChangeExecutor.java` around lines 251 - 257, The catch in DocumentChangeExecutor.flushPendingChanges currently logs errors but does not advance latestAppliedVersion or call completeWaitersUpTo, which can leave threads blocked in awaitLatest(); capture the current pendingVersion (e.g. store pendingVersion into a local variable like failedVersion) before the finally clears it, and in the catch (or immediately before clearing) call latestAppliedVersion.accumulateAndGet(failedVersion, Math::max) followed by completeWaitersUpTo(latestAppliedVersion.get()) so waiters are released even when changeListener.onChange() throws.src/test/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplierTest.java (1)
47-68:⚠️ Potential issue | 🟠 MajorTests only cover LEFT JOIN — the INNER JOIN NPE bug (line 118 in production) is not caught.
The existing tests validate the LEFT JOIN highlighting path. Given the copy-paste bug in
highlightJoinPartwhereinnerJoinfalls through tofullJoin().keyword(causing an NPE), consider adding a test case for INNER JOIN to catch this regression. Similarly, RIGHT JOIN and FULL JOIN paths are untested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplierTest.java` around lines 47 - 68, Add a unit test that exercises the INNER JOIN path to catch the NPE caused by the fall-through in highlightJoinPart: create a test method in SDBLJoinDocumentHighlightSupplierTest (similar to testLeftKeyword) that sets DocumentHighlightParams with a Position on the "INNER" keyword in the sample file, calls supplier.getDocumentHighlight(...), and asserts the highlights are non-empty and include the expected ranges; also add analogous tests for RIGHT JOIN and FULL JOIN to ensure innerJoin()/rightJoin()/fullJoin().keyword branches are covered and no NPE occurs.src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (1)
1275-1280:⚠️ Potential issue | 🟡 MinorMissing non-empty guard before
getFirst()— could produce a crypticNoSuchElementException.Unlike the analogous
getFirst()calls at lines 1213, 1317, and 1387 (each preceded byassertThat(delta.getEdits()).isNotEmpty()), this call has no such guard. If the implementation returns an empty edits list, the test fails with aNoSuchElementExceptionrather than a readable AssertJ message.🛡️ Proposed fix
assertThat(result.isRight()).isTrue(); var delta = result.getRight(); + assertThat(delta.getEdits()).isNotEmpty(); var edit = delta.getEdits().getFirst();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java` around lines 1275 - 1280, Add a non-empty guard before calling delta.getEdits().getFirst() in SemanticTokensProviderTest to avoid NoSuchElementException: assert that delta.getEdits() is not empty (e.g., use assertThat(delta.getEdits()).isNotEmpty()) immediately before the line that assigns var edit = delta.getEdits().getFirst(); so the test fails with a clear AssertJ message rather than a cryptic exception.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnionAllDiagnostic.java
Show resolved
Hide resolved
...ithub/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplier.java
Show resolved
Hide resolved
...ithub/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplier.java
Outdated
Show resolved
Hide resolved
...ain/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/strings/SdblTokenTypes.java
Show resolved
Hide resolved
.../java/com/github/_1c_syntax/bsl/languageserver/diagnostics/EmptyCodeBlockDiagnosticTest.java
Show resolved
Hide resolved
..._1c_syntax/bsl/languageserver/diagnostics/SameMetadataObjectAndChildNamesDiagnosticTest.java
Show resolved
Hide resolved
src/test/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/AllKnownTokenTest.java
Show resolved
Hide resolved
...b/_1c_syntax/bsl/languageserver/inlayhints/SourceDefinedMethodCallInlayHintSupplierTest.java
Show resolved
Hide resolved
src/test/java/com/github/_1c_syntax/bsl/languageserver/reporters/GenericReporterTest.java
Show resolved
Hide resolved
Test Results1 944 files 1 944 suites 45m 22s ⏱️ Results for commit a9202ba. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/copilot-instructions.md (1)
18-18: Stale stored learning will override this updated instruction for AI coding agents.The long-term memory captured from an earlier version of this file still reads "Use Java 17 for language" (scoped to
src/**/*.java). Until that learning is refreshed, AI coding agents (e.g., Copilot) may silently generate Java 17–targeted code despite this file now declaring Java 21.Consider triggering a learning refresh (e.g., by re-indexing or explicitly invalidating the old entry) so that downstream AI tooling picks up the Java 21 requirement.
Based on learnings: "Applies to src/**/*.java : Use Java 17 for language and follow Java naming conventions" — this stored learning is now inconsistent with the updated file content.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/copilot-instructions.md at line 18, The repository's long-term learning still claims "Use Java 17" for the src/**/*.java scope even though this file now declares "**Language: Java 21"; update the persistent learning store to reflect Java 21 by re-indexing or invalidating the old entry for the scope `src/**/*.java`, ensure any automated indexing job or knowledge base refresh is run (or trigger an explicit invalidation API) so downstream agents use Java 21, and verify that the stored rule matching "Applies to src/**/*.java : Use Java 17" is removed or replaced with "Java 21".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/copilot-instructions.md:
- Line 18: The repository's long-term learning still claims "Use Java 17" for
the src/**/*.java scope even though this file now declares "**Language: Java
21"; update the persistent learning store to reflect Java 21 by re-indexing or
invalidating the old entry for the scope `src/**/*.java`, ensure any automated
indexing job or knowledge base refresh is run (or trigger an explicit
invalidation API) so downstream agents use Java 21, and verify that the stored
rule matching "Applies to src/**/*.java : Use Java 17" is removed or replaced
with "Java 21".
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/copilot-instructions.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplier.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/documenthighlight/SDBLJoinDocumentHighlightSupplier.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
build.gradle.kts (1)
100-104:⚠️ Potential issue | 🟠 MajorIncomplete fix from previous review —
exclude()single-string form still present on lines 101 and 151.The fix applied in commit a9202ba corrected most
exclude()calls to use two positional arguments, but lines 101 (language-enblock) and 151 (spring-boot-starter-testblock) were missed. With the single-string form, the entire colon-delimited string binds to thegroupparameter andmoduleis left null, so the exclusion rule never matches anything and the dependency is not excluded.🐛 Proposed fix for remaining broken calls
implementation("org.languagetool:language-en:$languageToolVersion") { - exclude("commons-logging:commons-logging") + exclude("commons-logging", "commons-logging") exclude("com.sun.xml.bind", "jaxb-core") exclude("com.sun.xml.bind", "jaxb-impl") }testImplementation("org.springframework.boot:spring-boot-starter-test") { - exclude("com.vaadin.external.google:android-json") + exclude("com.vaadin.external.google", "android-json") }Also applies to: 150-152
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 100 - 104, In the build.gradle.kts dependency blocks the exclude calls using a single colon-delimited string (e.g. exclude("commons-logging:commons-logging")) are incorrect and leave module null; update those to the two-argument form exclude("group", "module") for the dependencies "org.languagetool:language-en:$languageToolVersion" (replace exclude("commons-logging:commons-logging") with exclude("commons-logging","commons-logging")) and in the "org.springframework.boot:spring-boot-starter-test" block (fix the similar exclude call there) so the exclusion actually matches.
🧹 Nitpick comments (1)
build.gradle.kts (1)
164-165: Update.github/copilot-instructions.mdto reflect the Java 21 baseline.The project's
copilot-instructions.mdcurrently instructs AI tools to "Use Java 17 for language". After this PR merges, that guidance will be stale and AI-generated code suggestions forsrc/**/*.javafiles will still target Java 17 language features. Based on learnings from.github/copilot-instructions.md:0-0, the Java version directive needs to be updated to Java 21.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build.gradle.kts` around lines 164 - 165, The copilot instructions still tell AI to "Use Java 17 for language" even though the build now sets sourceCompatibility and targetCompatibility to JavaVersion.VERSION_21; update the .github/copilot-instructions.md text to replace any mention of Java 17 (or "Use Java 17 for language") with Java 21 (e.g., "Use Java 21 for language") and adjust any related guidance about language features or JVM baseline to reference Java 21 so generated code targets the new project baseline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build.gradle.kts`:
- Around line 157-158: The test task needs JVM args to allow JMockit bytecode
instrumentation on Java 21: update the Gradle test task configuration (the Test
task where jmockit is added and jmockitPath / -javaagent is configured) to
include jvmArgs("-javaagent:${jmockitPath}", "--add-opens",
"java.base/java.lang=ALL-UNNAMED", "--add-opens",
"java.base/java.lang.reflect=ALL-UNNAMED") so tests run without
InaccessibleObjectException; add or adjust the jvmArgs invocation in the Test
task that currently sets the javaagent and consider making the opens
configurable if you encounter different packages.
---
Duplicate comments:
In `@build.gradle.kts`:
- Around line 100-104: In the build.gradle.kts dependency blocks the exclude
calls using a single colon-delimited string (e.g.
exclude("commons-logging:commons-logging")) are incorrect and leave module null;
update those to the two-argument form exclude("group", "module") for the
dependencies "org.languagetool:language-en:$languageToolVersion" (replace
exclude("commons-logging:commons-logging") with
exclude("commons-logging","commons-logging")) and in the
"org.springframework.boot:spring-boot-starter-test" block (fix the similar
exclude call there) so the exclusion actually matches.
---
Nitpick comments:
In `@build.gradle.kts`:
- Around line 164-165: The copilot instructions still tell AI to "Use Java 17
for language" even though the build now sets sourceCompatibility and
targetCompatibility to JavaVersion.VERSION_21; update the
.github/copilot-instructions.md text to replace any mention of Java 17 (or "Use
Java 17 for language") with Java 21 (e.g., "Use Java 21 for language") and
adjust any related guidance about language features or JVM baseline to reference
Java 21 so generated code targets the new project baseline.
| testImplementation("org.jmockit:jmockit:1.50") | ||
| testImplementation("org.awaitility:awaitility:4.3.0") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
jmockit 1.50 Java 21 compatibility issues JEP 403
💡 Result:
JMockit 1.50 tends to run into Java 17+ / Java 21 problems for the same underlying reason as many legacy instrumentation/mocking tools: JEP 403 (delivered in Java 17) tightened access to JDK internals so you can’t rely on the old “relaxed” illegal reflective access behavior anymore; you must explicitly open the needed JDK packages (or the tool must be updated to stop depending on them). [1]
What it looks like on Java 21
Typical failures show up as java.lang.reflect.InaccessibleObjectException / “module java.base does not opens …”, commonly involving packages like java.lang / java.lang.reflect. This is the class of failure people are explicitly fixing for JDK 21+ by propagating --add-opens into the test JVM. [3]
Why JMockit specifically is risky here
- 1.50 is the latest published JMockit 1.x artifact (there isn’t a newer line that “officially” modernizes it for JPMS/JDK 21). [5]
- The project’s own issue tracker contains unresolved Java 17-era incompatibilities/limitations in real builds (often interacting badly with other agents like JaCoCo), which is a strong signal that Java 21 will be painful/fragile. [4]
Practical workarounds (tests)
Because JEP 403 removed the “single switch” approach (--illegal-access), the remaining escape hatch is to open specific packages with --add-opens. [1][2]
Common starting point (adjust based on the exact package named in your exception):
--add-opens java.base/java.lang=ALL-UNNAMED
--add-opens java.base/java.lang.reflect=ALL-UNNAMED
(You may need additional --add-opens lines depending on what JMockit and/or your test stack reflectively touches; the exception message tells you the exact module/package to open.) [2][3]
Bottom line
If you must stay on JMockit 1.50 with Java 21, expect to maintain a growing list of --add-opens for your test JVM due to JEP 403’s strong encapsulation. [1][2][3] For longer-term stability on Java 21+, most teams end up migrating away from JMockit rather than fighting JPMS/agent edge-cases. [4][5]
Sources:
[1] OpenJDK JEP 403 (Strongly Encapsulate JDK Internals)
[2] StackOverflow explanation of InaccessibleObjectException + --add-opens + note that --illegal-access stops working from JDK 17 (JEP 403)
[3] Apache Maven PR discussion: JDK 21+ test failures fixed by propagating --add-opens (mentions java.lang / java.lang.reflect)
[4] JMockit issue: incompatibilities on Java 17 (example of ongoing breakage/fragility)
[5] javadoc.io listing showing latest org.jmockit:jmockit version is 1.50
🌐 Web query:
jmockit Java agent bytecode instrumentation Java 21 encapsulation
💡 Result:
JMockit’s mocking works by bytecode instrumentation via a -javaagent (it’s not a pure proxy-based mocking library). With Java 21’s strong encapsulation, this tends to break in two main ways:
-
JMockit is effectively unmaintained for modern JDKs
The last “current release” shown in JMockit’s own history is v1.49 (Dec 29, 2019), i.e., pre–Java 21 and even pre–Java 17 LTS, so incompatibilities with newer classfiles/JDK behaviors are common and often not fixed upstream. [1] -
Encapsulation / deep-reflection failures (
InaccessibleObjectException)
On Java 9+, you can getjava.lang.reflect.InaccessibleObjectExceptionwhen code tries to “break in” to non-opened JDK internals (access checks can’t be suppressed). [3]
Typical mitigations are JVM flags such as--add-opens ...=ALL-UNNAMED(or targeted opens to specific modules), but this weakens encapsulation and is increasingly discouraged. -
Agent-loading rules tightened (especially dynamic attach)
If your setup loads the agent dynamically (Attach API) rather than at JVM startup, JDK 21 emits warnings and is explicitly moving toward disallowing dynamic agent loading by default. The official guidance is to prefer startup-time-javaagent, and if you truly must attach dynamically you may need-XX:+EnableDynamicAgentLoading(and related attach settings). [2]
Practical takeaways for Java 21
- Prefer startup agent: run tests with
-javaagent:/path/to/jmockit.jar(not dynamic attach). [2] - If you hit
InaccessibleObjectException, you’ll need targeted--add-opensfor the specific JDK packages you’re trying to instrument/reflect into (but consider that this is fighting the platform direction). [3] - Given JMockit’s last release date, the most robust “Java 21 + encapsulation” fix is usually migrating away from JMockit (or limiting tests to mocking only your own code where module access isn’t restricted). [1]
Sources
[1] JMockit “Development history” (current release v1.49, Dec 29, 2019) https://jmockit.github.io/changes.html
[2] OpenJDK JEP 451 (JDK 21): Prepare to Disallow the Dynamic Loading of Agents https://openjdk.org/jeps/451
[3] Oracle JDK docs: InaccessibleObjectException (thrown when access checks can’t be suppressed) https://docs.oracle.com/en/java/javase/17/docs/api/java.base/java/lang/reflect/InaccessibleObjectException.html
🏁 Script executed:
cd /tmp && git ls-files | grep -E "(build\.gradle|pom\.xml|\.java-version)" | head -20Repository: 1c-syntax/bsl-language-server
Length of output: 141
🏁 Script executed:
cat -n build.gradle.kts | sed -n '1,50p'Repository: 1c-syntax/bsl-language-server
Length of output: 2185
🏁 Script executed:
cat -n build.gradle.kts | sed -n '200,250p'Repository: 1c-syntax/bsl-language-server
Length of output: 2158
🏁 Script executed:
rg -n "java\s*\{|sourceCompatibility|targetCompatibility|jvmArgs|add-opens" build.gradle.ktsRepository: 1c-syntax/bsl-language-server
Length of output: 249
🏁 Script executed:
rg -n "test|jvmArgs|javaagent" build.gradle.kts | head -30Repository: 1c-syntax/bsl-language-server
Length of output: 1308
Add --add-opens flags to test task for Java 21 compatibility with jmockit 1.50.
The codebase targets Java 21 (lines 164–165) while using jmockit 1.50 with -javaagent (lines 157, 232). JMockit relies on bytecode instrumentation to access JDK internals, but Java 21's strong encapsulation (JEP 403) blocks this by default, causing InaccessibleObjectException failures. To run tests on Java 21, add the following to the test task configuration:
jvmArgs(
"-javaagent:${jmockitPath}",
"--add-opens", "java.base/java.lang=ALL-UNNAMED",
"--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED"
)
Adjust the --add-opens packages based on the specific InaccessibleObjectException errors encountered. For longer-term stability, consider migrating to a modern mocking library or the maintained jmockit fork at com.github.hazendaz.jmockit, as the original jmockit project (v1.50) is not actively maintained for modern JDKs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@build.gradle.kts` around lines 157 - 158, The test task needs JVM args to
allow JMockit bytecode instrumentation on Java 21: update the Gradle test task
configuration (the Test task where jmockit is added and jmockitPath / -javaagent
is configured) to include jvmArgs("-javaagent:${jmockitPath}", "--add-opens",
"java.base/java.lang=ALL-UNNAMED", "--add-opens",
"java.base/java.lang.reflect=ALL-UNNAMED") so tests run without
InaccessibleObjectException; add or adjust the jvmArgs invocation in the Test
task that currently sets the javaagent and consider making the opens
configurable if you encounter different packages.
|
1 participant
Описание
Обновление до jdk21 + все зависимости 1c-syntax
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
Chores
User-visible Improvements
Tests