fix(kotlin): resolve a constructed class, so a DI-heavy backend keeps its coupling (#387) - #388
johnatbasicas wants to merge 2 commits into
Conversation
…allers (trailhq#387) `Mailer(host = "smtp")` is an ordinary `call_expression` — Kotlin has no `new` to mark construction — so every constructor edge was matched against the function-only index and dropped. A class therefore reported `no indexed callers` while a free function declared beside it in the same file reported all of them, which on a DI-heavy backend loses exactly the coupling that matters: measured on a 717-file Kotlin monorepo, a service class constructed in a Koin module and instantiated five times in its integration test had no incoming edges at all. Same fallback shape Python and Swift already use — types are tried only once functions have found nothing, so a real function call still resolves first. `class` alone: `data class` and `object` are that kind already, and an `enum class` or `interface` cannot be constructed by name. A package-qualified spelling still drops rather than reducing to its last segment, for the reason `javaConstructedTypeName` does (trailhq#103).
The previous wording claimed interfaces and annotations have no constructor.
They do: a `fun interface` is SAM-converted by writing `Action { … }`, an
ordinary call expression, and since Kotlin 1.6 an annotation class can be
instantiated. The real reason to leave the kind out is narrower and measured —
tree-sitter-kotlin@0.3.8 cannot parse `fun interface` at all, so no such
interface is ever a node to resolve against.
Also states the imprecision the fallback actually carries instead of implying it
has none: `class` covers `object`, where `Factory()` may be `operator fun
invoke`, and a local val can shadow the type it names. Python and Swift make the
same trade directly above.
Two tests for the properties that keep it safe: a function named after a type
still wins (the stdlib's own factory idiom), and `.kts` behaves like `.kt`.
🌱 graft blast radius1 area changed → 3 areas can be affected. 5 dependent symbols, depth 2. flowchart TB
A0(("Graph Construction<br/>2 symbols"))
A1(("Pull Request Review<br/>2 symbols"))
A2(("Graph Engine<br/>1 symbol"))
classDef reached fill:#D9EDF3,stroke:#3AA7C9,stroke-width:1.5px,color:#0E313C;
class A0,A1,A2 reached;
Who knows this code — 2 people across 4 areas
Ownership is git history over each area's own files, weighted towards recent work (120-day half-life). Merge commits and bots are dropped, and you are dropped from your own PR. A name with no All 5 dependent symbols, grouped by areaGraph Construction — 2 symbols in 2 files
Pull Request Review — 2 symbols in 2 files
Graph Engine — 1 symbol in 1 file
Test signal per changed area — 1 ⚠Reached = a node under a test path has a resolved edge into the changed symbol. It undercounts anything called indirectly — through a CLI, a spawned process or a dynamic import — so read a low ratio as “look here”, never as a coverage gate.
35 test suites also reference this code38 symbols, kept out of the diagram and the table so they cannot crowd out the areas a reviewer has to look at.
Open the interactive graph → — click an area to see its dependent symbols at file:line. |
Closes #387.
What
Kotlin construction (
Mailer(host = "smtp")) produced no incoming edge, socallersandblastreported nothing for service classes that are constructed across the codebase. Only free functions resolved.resolve.tsalready has this exact fallback for Python (PY_CTOR_KINDS) and Swift (SWIFT_CTOR_KINDS): types are tried only after functions have found nothing. Kotlin is Swift's case again — nonewto mark construction, and free functions exist too, so it takes the same fallback rather than Java's outright widening. This adds Kotlin to that list.classalone, rather than Swift's three kinds, because Kotlin's other type kinds are not reachable this way —data class,objectandcompanion objectare already kindclass; anenum classconstructor is private, soColor(…)cannot be written from outside.interfaceis reachable in principle (afun interfaceis SAM-converted by writingAction { … }, and since Kotlin 1.6 anannotation classcan be instantiated) buttree-sitter-kotlin@0.3.8cannot parsefun interfaceat all — it yields an ERROR node, so no such interface becomes a node to resolve against. The comment says so, and flags it as worth revisiting if that grammar gap closes.The fallback inherits the fallback's imprecision honestly, and the comment says that too:
classalso coversobject, whereFactory()may be anoperator fun invoke; a localval Mailer = factory; Mailer()shadows the type it names. Python and Swift already make that trade; this is no looser. Ordering keeps it safe — a real function of that name resolves first — as doesresolveName's unique-match rule, which drops the ambiguous rather than picking.Measured
On a production Kotlin + TypeScript monorepo (1,340 indexed files, 717
.kt, 13,589 symbols, 30,842 edges):no indexed callersto its real construction sitescallersquery there returns 51 files with exact call sitesTests
5 tests in
test/graph-kotlin.test.ts, each verified red before the change and green after:.ktsis covered alongside.ktobjectwithoperator fun invokeis acknowledged by the ordering ruleFull suite: 1226/1226, exit 0.
tsc -p tsconfig.json --noEmit: exit 0.(The suite needs
LC_ALL=en_US.UTF-8; without it nine pre-existing thousands-separator assertions fail onmaintoo — unrelated to this change.)Platform safety
CI gates on
windows-latestas well asubuntu-latest, so the one new regex is worth stating explicitly:KOTLIN_EXT = /\.kts?$/imatches a file extension, anchored at end of string. It never looks at a path separator, so\versus/cannot change its result, and it is unaffected by line endings — it is applied toe.file, not to file contents. Both.ktand.ktsare covered, with a test pinning.kts.The change adds no filesystem access, no shell invocation and no path construction, so there is nothing else here with a platform-dependent surface.