Repository navigation
report dependencies from compiler-duplicated accesses only once - #1748
Open
sharanggupta wants to merge 1 commit into
Open
sharanggupta wants to merge 1 commit into
sharanggupta wants to merge 1 commit into
Conversation
The byte code of a `finally` block is emitted twice by the compiler, once for the regular and once for the exceptional path. A single method call in a `finally` block thus results in two `JavaMethodCall` objects with the same origin, target and line number. Up to 1.2.x those were collapsed into a single `Dependency`, because `Dependency` defines equality by origin class, target class, line number and description. Since `Dependency.tryCreateFromAccess` creates `Dependency.FromAccess` instances, whose `equals()`/`hashCode()` delegated to the wrapped `JavaAccess`, both survived deduplication, as `JavaAccess` has identity equality only. Rules like `noClasses().should().dependOnClassesThat()...` consequently reported the same violation twice. We now let `Dependency.FromAccess` inherit the value-based equality of `Dependency`, so indistinguishable accesses form a single dependency again. The conversion of such a dependency to its `JavaAccess` is unaffected. Resolves: TNG#1339 Signed-off-by: Sharang Gupta <sharang@sharanggupta.dev>
sharanggupta
force-pushed
the
gh-1339-dedupe-dependencies-from-duplicated-finally-bytecode
branch
from
October 8, 2026 10:48
fff809b to
67d9737
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The byte code of a
finallyblock is emitted twice by the compiler, once for the regular and once for the exceptional path. A single method call in afinallyblock therefore results in twoJavaMethodCallobjects with the same origin, target and line number.Up to 1.2.x those collapsed into a single
Dependency, becauseDependencydefines equality by origin class, target class, line number and description. Since 771df06Dependency.tryCreateFromAccesscreatesDependency.FromAccessinstances whoseequals()/hashCode()delegated to the wrappedJavaAccess; asJavaAccesshas identity equality only, both dependencies survived deduplication andnoClasses().should().dependOnClassesThat()...reported the same violation twice.This PR lets
Dependency.FromAccessuse the value-based equality ofDependency(explicitly, so the intent is documented and SpotBugs'EQ_DOESNT_OVERRIDE_EQUALSis satisfied). Converting such a dependency to itsJavaAccessis unaffected. A unit test pins the equality contract and an end-to-end test with afinallyblock reproduces the issue. The design is deliberately minimal; happy to move the deduplication elsewhere (e.g.JavaAccessequality or import time) if preferred.Out of scope but worth noting: access-based rules such as
noClasses().should().accessClassesThat()still report the duplicated call twice, since accesses were never deduplicated.Verified with
:archunit:test(15804 tests),spotlessCheck,architectureTestandspotbugsMain.Resolves #1339