Repository navigation
Index Kotlin with the Analysis API instead of a compiler plugin (take 2) - #1044
christoph-sg wants to merge 33 commits into
Conversation
#973 added 17 test cases to scip-kotlinc's AnalyzerTest and extended `type operators`; replacing scip-kotlinc deleted them. Run the same sources through KotlinAnalysisIndexer instead, using a copy of the ScipBuilders DSL. The #973 output changes are not part of scip-kotlin-analysis yet, so the expectations follow its current output: - No `kind` or `enclosing_symbol` assertions. - Extension functions and properties keep top-level symbols (`sample/foo().` instead of `sample/String#foo().`). - Enum entries render as `val`, and members of an enum entry body are `sample/Op#PLUS.apply().` instead of locals of an anonymous class. - `Foo.Factory` emits separate `Foo#` and `Foo#Factory#` references, an implicit companion receiver references `Bar#Companion#`, and the unnamed companion definition range ends at the end of its first line. Signature assertions for local functions and extension receivers are dropped: scip-kotlin-analysis currently renders `public final fun inner()` for local functions and omits the extension receiver. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Port the remaining SymbolInformation fields #973 added to scip-kotlinc. - kind follows the declaration: Class (including objects, enums and annotation classes), Interface, TypeAlias, Constructor, Method (including accessors and local functions), Property (including local variables, loop and catch variables, and destructuring entries), Parameter, TypeParameter and EnumMember. - enclosing_symbol is the nearest containing declaration, or the package for top-level declarations. Synthetic accessors are enclosed by their property, and `val`/`var` constructor parameter properties by the class. Declarations inside lambdas have no enclosing symbol. On the Kotlin snapshot corpus, every global symbol and every local emitted by both indexers has the same kind and enclosing_symbol as scip-kotlinc on main. The ported AnalyzerTest cases get back their kind and enclosing_symbol expectations, and the snapshot goldens gain `kind` lines. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Port #973's accessor symbols: getters and setters are now `Class#banana.get().` / `Class#banana.set().` instead of `Class#getBanana().` / `Class#setBanana().`, for source properties, `val`/`var` constructor parameters, explicit accessors, the enum `entries` getter and references to library properties (`kotlin/String#length.get().`). Setter parameters follow (`Class#banana.set().(value)`). Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Port #973's extension symbols: a top-level extension function or property is owned by its receiver type within the extension's own package, so `fun String.foo()` in `sample` is `sample/String#foo().` and `kotlin.collections.forEachIndexed` on `Iterable` is `kotlin/collections/Iterable#forEachIndexed().`. This applies to source declarations and to references to library extensions. Extension disambiguators count the same-named members of the receiver class (when it is in the extension's package) followed by the same-named extensions on that receiver in the package, so a member `MyClass#foo().` and an extension `MyClass#foo(+1).` stay distinct. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Port #973's local symbols: anonymous objects and their members are now local symbols, numbered in declaration order, instead of `<anonymous object at N>` globals. An enum entry body declares a local anonymous subclass of the enum, anchored at the entry identifier, which owns the body's members, so `PLUS { override fun apply() }` defines `local 0` (`object : Op`), its constructor and `apply` as locals rather than `Op#PLUS.apply().`. The implicit constructor of a local class is now a local symbol too; it was emitted as the invalid symbol "local 0`<init>`().". Unlike scip-kotlinc, enum entries with constructor arguments but no body do not declare an anonymous class: that is an artifact of FIR's desugaring, and those entries are plain instances of the enum class. Port the `local classes` and `anonymous object` cases from scip-kotlinc's AnalyzerTest to cover this. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Port #973's resolved-qualifier references: a class name used as its companion object's receiver (`Bar.instance()`, `CompanionOwner.create()`) now references the class (`Bar#`) instead of the companion (`Bar#Companion#`). Naming the companion explicitly (`Bar.Companion`, `Foo.Factory`) still references it. Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce Co-authored-by: Amp <[email protected]>
Port #973's range for unnamed companion objects: the definition, and that of its implicit constructor, now covers the `companion` keyword instead of the declaration's first line (`companion object {`). scip-kotlinc's implicit-constructor range ran past the end of the line; here both definitions share the keyword range. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
Match the FirRenderer output that #973's Kotlin 2.4 update introduced: - Enum entries render as `public final static enum entry RED: Color` instead of `public final val RED: Color`, and the `entries` getter as `public static get()`. - Functions and classes declared in a code block render as `local` (`local final fun inner(): Unit`); members of local classes and anonymous objects keep their visibility. - Extension functions and properties render their receiver (`public final fun String.foo(): Int`). The other differences in scip-kotlinc's 2.4 output are renderer internals and are not reproduced: evaluated annotation arguments (`@Tagged(tag = String(run) [evaluated = ...])`) and expanded type alias markers (`{snapshots/StringContainer=} Container<String>`). The ported AnalyzerTest cases now use scip-kotlinc's expectations for these signatures. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a153c0-ec1f-774a-993c-bca3256017ce
0ddf374 to
a02c0ca
Compare
AGP 9 compiles Kotlin in Android modules itself ("built-in Kotlin")
without applying a Kotlin Gradle plugin, so the `hasPlugin("kotlin")`
check never registered the source dump for those compile tasks and their
Kotlin sources were not indexed. The dump is now registered for every
project; configureKotlinCompileTask already filters by the KotlinCompile
task class.
In android/nowinandroid, indexing the main sources of the demoDebug
variant went from 16 to 199 of 310 Kotlin files.
Co-authored-by: Amp <[email protected]>
Amp-Thread-ID: https://ampcode.com/threads/T-01a178a8-3c2c-75e0-bfa6-7417585d4372
The Kotlin source dump listed only .kt files, so the Analysis API session had no view of Java classes compiled in the same module (they are compiled after Kotlin and are not on the classpath). Kotlin references to them were silently dropped: in thunderbird-android's legacy/core, HtmlSignatureRemover.kt had no occurrence for the Java class AdvancedNodeTraversor, only for its package. The dump now also lists the compile task's javaSources. The indexer only emits documents for Kotlin files, so the Java files are used for resolution only. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a178a8-3c2c-75e0-bfa6-7417585d4372
The disambiguator of a source function counted every earlier declaration with the same name in its file or class, including classes and properties, while references to compiled code counted all same-named callables of the class or package. A top-level function named like a class in the same file, a common factory pattern, was defined as `Foo(+1).` but referenced from other modules as `Foo().`; in kotlinx-datetime, test references to `UtcOffset(...)` and `DateTimePeriod(...)` (323 occurrences) didn't resolve to the definitions in main. Both paths now count only functions. For class members this also matches javac's numbering, which counts only methods: `fun shift()` next to `val shift` is `#shift().` from Kotlin and Java alike. Overloads of a top-level function spread across files still diverge: source counts per file, compiled code per package. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a178a8-3c2c-75e0-bfa6-7417585d4372
Allocate synthetic members of local declarations from the same per-document local symbol cache, keyed by owner and descriptor, instead of appending global descriptors to local symbols. Resolve generated local method references through the same keys as their definitions. Add regression coverage for constructor properties, accessors, setter and copy parameters, component methods, and references across local classes. Corpus verification eliminates 26 malformed locals in kotlinx-datetime and 13 in Thunderbird without changing coverage or dangling references. Co-authored-by: Amp <[email protected]> Amp-Thread-ID: https://ampcode.com/threads/T-01a17cb6-8fe3-70a5-861f-762aa723b737
Incorporates #1038, but keeps it at the file level. I'd rather not add special cases for invalid programs to the core indexing logic
c55ae27 to
892a2bb
Compare
|
@christoph-sg thanks! I tested 892a2bb on the same three projects as before, plus four of our Android repos (up to 5.8K Kotlin files). Every file indexed, with no skips, and speed and memory are unchanged. Fixed: library constructor About #1038: the file-level skip is fine with me. One part is missing, though: New with the #973 naming, and it hits mixed Java/Kotlin code:
Still open: no occurrence for imports of top-level functions, and no references for implicit calls ( |
|
Yeah, I also don't fully understand the renaming of getters and setters in #973. I read through the commit and PR, but couldn't find an explicit reason. @nicolas-guichard do you happen to remember why you changed those symbols? It would be nice to be consistent with the Java indexer here, to make the cross-references work. |
|
My objective back then was to better model the relationship between a property and its getter/setter. A symbol like But now that |
This restores cross-referencing with java symbols
Takes the changes from #963 and rebases them on current main.
Commits from a14e0a2 to a02c0ca port the changes from #973 to the new indexer.
I then iterated on https://github.com/christoph-sg/scip-java-corpus to cover Android, and Java/Kotlin cross referencing. I moved a lot of the "handwritten" unit tests to snapshot tests where applicable.
Together with the Gradle plugin fixes in #1045 and #1047 this PR indexes all of the repositories in the corpus.
@ofir-blaus could you give this branch a spin? I incorporated your fix (albeit in a less fine-grained manner).