feat(hilt): resolve @Binds and @Provides into BINDS edges - #1623
Shashankss1205 merged 4 commits into
Conversation
Follows CodeGraphContext#1596, which added `decorators`. The same `modifiers` node also carries visibility (public/private/internal/protected) and the class kind (data/sealed/value/annotation), plus abstract/open/override and suspend/inline -- none of which reached the graph. Adds two properties on Function and Class, and completes the two columns CodeGraphContext#1596 deferred on Interface and Object: Function/Class visibility STRING, modifiers STRING[] Interface/Object visibility STRING, modifiers STRING[], decorators STRING[] Each in all three required places -- node-table declaration, SCHEMA_MAP allow-list, and simple_migrations so pre-existing databases get them via ALTER TABLE. Without the last, CREATE NODE TABLE throws "already exists" on an existing database and is swallowed, so the columns never arrive. Two grammar details worth knowing: `enum` is not a modifier. `enum class C` produces no `modifiers` node at all -- the keyword is a direct child of class_declaration, exactly like `interface`. It is derived with the pattern _parse_classes already uses for interface detection, so `modifiers` is the single place to ask what kind of class this is. visibility defaults to the string "public" rather than null, matching Kotlin's own default, so consumers need no null handling. Since CodeGraphContext#1596 gated `decorators` behind `category == "classes"` only because Interface/Object had no column, that gate is removed here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Almost nothing in an Android app is called by another Kotlin function: the framework invokes lifecycle methods, the manifest declares components, Hilt supplies dependencies, the Compose runtime calls composables, annotation processors generate Room implementations, test runners call @test, and `override fun` is reached through its supertype. So the tool reports thousands of false positives. Three changes: - `override` functions are treated as live, using the `modifiers` property added in the parent commit. Guarded with IS NOT NULL, because 'override' IN NULL is NULL rather than false in Cypher -- unguarded it would silently drop every function lacking the property from results. - Android/JVM lifecycle names (onCreate, onBind, doWork, ...) are treated as entry points, scoped to func.lang IN ['kotlin','java'] so other languages keep the original global list untouched. - ANDROID_DECORATOR_PRESET: a documented tuple of annotations meaning "something other than project code calls this" -- Compose, test runners, Hilt, Room, and @TypeConverter. Measured on a real 2,861-file Android codebase: 7,141 findings without the preset, 1,055 with it -- an 85% reduction, dominated by @test methods and Hilt providers. Two honest limits, both documented: The override exemption is inert for Java, because only kotlin.py emits `modifiers`; the lifecycle-name half does work for Java. And four preset entries (Dao, HiltViewModel, AndroidEntryPoint, Serializable) annotate classes rather than functions, so they cannot match a query that does MATCH (func:Function) -- kept, with a comment, because the data exists and it is the query's scope that makes them inert. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
In a Hilt codebase, `@Binds abstract fun bindRepo(impl: UserRepositoryImpl): UserRepository` is the only link between an interface and its implementation. Without it every call resolves to the interface, so `analyze callers` on the impl returns nothing, the impl looks like dead code, and impact analysis dead-ends at every DI boundary -- which in a clean-architecture Android app is every layer boundary. Adds a BINDS relationship and resolves both Hilt mechanisms into it: - @BINDS: source is the declared return type, target the single parameter type. Skipped if the arity is not exactly one. - @provides: source is the return type, target the type constructed in the body, read from the function's recorded calls. BINDS is declared as a REL TABLE GROUP rather than reusing INJECTS, which is a single-binding REL TABLE (FROM Class TO Class). Hilt binds an Interface to a Class, so reusing it would mean converting an existing table to a group -- a migration hazard on databases already built. Honest limits, stated in full because this is text-level resolution with no type checker: - Ambiguity is skipped rather than guessed. A @provides body with more than one type-resolvable call emits no row: neither first-call nor last-call is correct in general, since sequential construction wants the last and nested construction puts the wanted call first. A missing edge is visibly missing; a wrong one silently misdirects the very queries this exists to answer. - Qualifiers (@nAmed) are not distinguished, generics resolve on erased names, multibindings are not modelled, and @provides bodies that delegate to a factory resolve to the factory call rather than the produced type. - Where two label pairs both match, priority order decides, because a row carries names and paths but no labels. Measured on a real 2,861-file Android codebase: 77 of 77 @BINDS declarations resolved. @provides yields less (46 of 210) and legitimately so -- those bodies are dominated by Room DAO accessors and static factories, which have no first-party implementation node to point at. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@rrodriguesNutrium is attempting to deploy a commit to the shashankss1205's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Android/Kotlin series — merge order Each PR targets Happy to split, reorder, or squash any of these differently if it suits review better. |
Conflict in test_database_kuzu_kotlin_metadata.py was additive: this branch's two write_binds_links tests vs main's three write_inheritance_links probe tests (CodeGraphContext#1617). Kept all five. Also regenerates the Kotlin golden added in CodeGraphContext#1618, since this branch adds AndroidHilt.kt to that fixture project. The diff is purely additive -- 24 new nodes, zero missing -- and the refreshed golden now pins both BINDS edges (@BINDS and @provides). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shashankss1205
left a comment
There was a problem hiding this comment.
Approving. Verified end to end on the fixture — both provider kinds resolve:
UserRepository -> UserRepositoryImpl (provider: Binds)
NetworkClient -> NetworkClientImpl (provider: Provides)
The part I looked hardest at is write_binds_links, because BINDS is declared as a three-pair REL TABLE GROUP — precisely the shape that produces the silent-drop bug you found in #1601, where DECORATED_BY declares two pairs and the writer only ever queries one. You iterate all three declared pairs, so that class of bug doesn't recur here.
The break after a pair actually matches is the subtle bit and the docstring justifies it well: rows carry name+path but no label, and Kotlin doesn't qualify name by enclosing scope, so a top-level interface and an unrelated nested class can legitimately collide — without the break they'd get a second, spurious edge. Good that you have a test pinning exactly that (..._does_not_create_spurious_edge_from_same_named_node).
Two things I did on your branch:
- Resolved the conflict in
test_database_kuzu_kotlin_metadata.py— additive again (your two BINDS tests vsmain's three probe tests from #1617), all five kept. - Regenerated the Kotlin golden. This branch adds
AndroidHilt.kttosample_project_kotlin, which the golden from #1618 pins. I checked the diff was purely additive before refreshing — 24 new nodes, zero missing — so no existing content regressed. The refreshed golden now pins both BINDS edges, which is a nice side effect: the DI resolution is now regression-protected.
Worth flagging for the stack: #1624 adds AndroidCompose.kt to the same fixture, so it will need the same golden refresh. I'll handle it.
test_hilt_resolution.py 11 passed
tests/unit/ 1237 passed
tests/integration/ 45 passed
Dependency injection is where an Android call graph goes dark. A
@Bindsor@Providesfunction is the only link between the interface a class depends on and the implementation it actually gets at runtime — and that link exists nowhere in the graph, so "who implementsUserRepository?" has no answer and every call through an injected interface is a dead end.This PR resolves Hilt/Dagger bindings into a
BINDSedge.The edge
A
REL TABLE GROUPbecause both endpoints vary:@Bindsnormally goes interface → class, but binding an interface to another interface, or a class to a subclass, are both legal and appear in real code.Two resolution shapes:
@Binds— an abstract function whose single parameter is the implementation and whose return type is the bound type. Source = return type, target = the parameter type.@Provides— a concrete function that constructs and returns the implementation. Source = return type, target = the type constructed in the body.providerrecords which of the two produced the row.Confidence, and why it isn't always "EXTRACTED"
confidence_labelisEXTRACTEDwhen both endpoints resolved to a real declaration — same-file, or a real import target — andINFERREDwhen the type name couldn't be tied to a file and falls back to__external__.This is a genuine distinction rather than decoration. Kotlin type resolution here walks same-file declarations → local imports → a unique global import; a module that binds a type declared in another Gradle module without an explicit import resolves to nothing. Marking those
INFERREDkeeps them in the graph as a weaker signal instead of either dropping them or asserting a file path that isn't real.One detail that bit me: interfaces must be included in the local-name table. They're not "classes", but
@Bindsreturn types are almost always interfaces — omit them and every@Bindsrow degrades to__external__/INFERRED, which looks like a resolution quality issue rather than a missing lookup table.Depends on #1620
This does not work on
main, and the reason is worth stating precisely.Hilt modules are conventionally declared as
object, and onmain#1596 gatesdecoratorsbehindcategory == "classes"— so@Module object NetworkModuleparses withdecorators=Noneand the module is invisible. #1620 removes that gate.Concretely, with only this commit applied to
main, the@Providestest fails (0 rows) while the@Bindstest passes — because the@Bindsfixture module happens to be anabstract classand the@Providesone is anobject. Same annotation, same grammar path, different category gate.Verification
pytest tests/unit -q→12 failed, 1204 passed, 19 skipped.Same 12 failures on unmodified
origin/main(12 failed, 1176 passed, 19 skipped) — pre-existing and unrelated.The fixture declares
@Module/@Binds/@Provides/@Inject/@HiltViewModelas local stub annotations, so no Hilt, Dagger or Android SDK is needed on the test path. It covers both aclassmodule and anobjectmodule deliberately, since that difference is exactly what the category gate exposed.Tests also assert the negative:
@HiltViewModeland@Inject constructorproduce noBINDSrows. They are Hilt annotations but not bindings, and an over-eager matcher that keyed on "is a Hilt annotation" would pass every positive test while filling the graph with junk edges.Scope
+723, no behaviour change to any existing edge type.Third of five. Requires #1620. Review only the last commit — the earlier two are the preceding PRs, carried along because GitHub can't host a stacked base across a fork boundary.