Implement Resolver.getSubpackagesOf()

[ZacSweers]

Resolves #1795

Tests coming, just getting this up for early feedback

[ZacSweers]

Added tests. Interestingly, I get different results in the tests but I also don't fully understand the test package structure in these tests, and curious for feedback on if I'm doing something wrong in setup here. I'm not following how there's a main.test.main.test in the test sources 🤔

[neetopia]

Added tests. Interestingly, I get different results in the tests but I also don't fully understand the test package structure in these tests, and curious for feedback on if I'm doing something wrong in setup here. I'm not following how there's a main.test.main.test in the test sources 🤔

It is likely limitation from the PSI handling of incorrectly placed files (file path & package mismatch), KSP2's behavior still looks more correct to me. We probably won't have a good solution for this behavior since it is inherited from PSI.

[ZacSweers]

Gotcha, so do you think it's just an artifact of the test infra in this case?

[ZacSweers]

:test-utils:test passes for me locally 🤔

[neetopia]

it is passing for me on mac as well. failed only on linux, it seems that the problematic part is from the files where file path mismatches package names, it is a known issue for PSI as we discussed in this PR, at this moment there is probably no good solution for this behavior, maybe we can consider remove the misplaced files ?

[ZacSweers]

maybe we can consider remove the misplaced files

Can you expand on what this would entail?

[neetopia]

expected: <subpackages of lib1
    subpackages of lib2
    subpackages of main
    main.test
    main.test.nested
    main.test.main
    main.test.main.test
    main.test.main.test.nested
    subpackages of non.exist> but was: <subpackages of lib1
    subpackages of lib2
    subpackages of main
    main.test
    main.test.nested
    subpackages of non.exist>

it looks that the diffs are

    main.test.main
    main.test.main.test
    main.test.main.test.nested

which seems to be a result from the files being put in wrong directory with respect to the package names

// FILE: main/test/main/test/nested/M.java
package main.test.nested;

and if this behavior is inherited from PSI to cause the inconsistency, and there is no fix on KSP side, then we might have to fix the test case to avoid it, after all it is the getSubPackage functionality that we want to test in this case.

[ZacSweers]

@neetopia @ting-yuan were either of you able to look further into the PSI discrepancies here?

[ting-yuan]

May I know why .filterIsInstance<PackageViewDescriptor>() and .filterNot { it == this@subPackages } are needed?

[ting-yuan]

Needing to distinct() may indicate some issues in the underlying Analysis API. Have you tried removing distinct()? If it works then it works. If not, feel free to leave it there and we'll investigate.

[ting-yuan]

Let's fix the test case. IMHO the behavior on KSP1 and KSP2/Mac are weird, even though the directories don't match. Package names should always be consistent with the package directive.

[ting-yuan]

Please replace with: runTest("../test-utils/testData/api/getSubPackages.kt"). I.e., don't use the forked version.

[ting-yuan]

Tests under kotlin-analysis-api are forked version of those in test-utils. They are (ideally) only used when there is a behavior difference between KSP1 and KSP2. It doesn't seem to be necessary in this case.

[ting-yuan]

Please replace main.test.main[\..]* with main.test.nested

[ting-yuan]

Replace with // FILE: main/test/L.java

[ting-yuan]

Replace with // FILE: main/test/nested/M.java

[ZacSweers]

We no longer need this API for our use case and I'm limited on bandwidth, so I'll need to relinquish this feature to whoever wants to implement it in the future :)