~ [j] fixes after review
diff --git a/compiler/java-direct/ITERATION_RESULTS.md b/compiler/java-direct/ITERATION_RESULTS.md index 67e2cba..dfafa2d 100644 --- a/compiler/java-direct/ITERATION_RESULTS.md +++ b/compiler/java-direct/ITERATION_RESULTS.md
@@ -36,6 +36,30 @@ <!-- Add new entries below, newest first. --> +### 2026-07-16 — Remove the loose `probeFqnSplits` fallback: commit to the leftmost type like javac +- **Change**: `resolveQualifiedNameToClassIdFromParts` no longer retries a failed name as a plain + `package.Class` split. Like javac (JLS 6.5.4/6.5.5), once a leftmost type is found the + interpretation is committed: a failed member-type descent returns the *nonexistent* nested id of + the committed prefix (full resolution), which stays unresolved downstream — red code, exactly as + javac reports on a package/type name clash (JLS 6.1). The reentrance-safe flavor returns `null` + instead, so supertype-walk seeding is never poisoned by a dangling id. `probeFqnSplits` deleted. +- **Tests**: the strict behavior conflicts with the PSI Java model (which loosely resolves the + package interpretation), so the two tests pinning the loose behavior moved out of the shared + roots: `qualifiedNamePackageClassClash.kt` deleted from the shared roots and recreated with + javac-strict expectations in the new java-direct-owned `testData/diagnostics` root (wired into + `TestGenerator`); the pre-existing `javac/qualifiedExpression/PackageVsClass2.kt` — verified + against real javac to be red code ("cannot find symbol: class b, location: class a") — is + skipped via the new `SkipTestsPinningPsiJavaModelDeviationsMetaConfigurator` and mirrored + strictly in the same root. Strict diagnostics: `MISSING_DEPENDENCY_CLASS` on the call whose Java + signature uses the clash name + `UNRESOLVED_REFERENCE` on members of the unresolved type. +- **Files**: `resolution/JavaTypeResolver.kt` (−32/+18), `testFixtures/…/components.kt`, + `testFixtures/…/AbstractJavaUsingAstTest.kt`, `testFixtures/…/TestGenerator.kt`, + `build.gradle.kts` (own testdata root registered); Scenario D refreshed in `ReadMe.md` and + `implDocs/RESOLUTION_SCHEMA.md`. +- **Result**: the module is now javac-conformant on qualified-name resolution — the last + deliberate JLS deviation (KT-87813's unsound loose fallback) is gone. Full module suite green + (box + phased, 0 FAILED; the skipped PSI-pinning test is mirrored strictly). + ### 2026-07-16 — Rewrite `resolveQualifiedNameToClassIdFromParts` as a left-to-right JLS 6.5.4 pass - **Change**: replaced the recursive try-every-split loop (outer-prefix enumeration + per-prefix recursion, O(n²) probes) with a single non-recursive left-to-right pass that mirrors javac's
diff --git a/compiler/java-direct/ReadMe.md b/compiler/java-direct/ReadMe.md index 6c88e15..61c7e95 100644 --- a/compiler/java-direct/ReadMe.md +++ b/compiler/java-direct/ReadMe.md
@@ -143,8 +143,7 @@ ### Scenario D — Qualified / nested name to `ClassId` (JLS 6.5.5) Entry: `JavaTypeResolver.resolve` (dotted name) → `resolveQualifiedNameToClassIdFromParts`. -A single left-to-right pass mirroring javac's PackageOrTypeName classification (JLS 6.5.4), -plus a loose package-interpretation fallback: +A single left-to-right pass mirroring javac's PackageOrTypeName classification (JLS 6.5.4): 1. **Leftmost type** (JLS 6.5.4): the first segment as a simple type name in scope (Scenario B); failing that, the package prefix grows one segment at a time until a segment names a @@ -152,13 +151,15 @@ 2. **Member-type descent** (JLS 6.5.5.2): each remaining segment must be a member type of the previous one — declared, or inherited from its supertypes (`findInheritedNestedClass`, supertype walk + finder). -3. **Fallback** to plain package/class splits longest-package-first (`probeFqnSplits`), reached - only when the JLS pass fails. It diverges from javac (which reports an error) exactly on a - package/type name clash, resolving the package interpretation for PSI parity - (`qualifiedNamePackageClassClash.kt`, KT-87813). -Corner cases: `Map.Entry`-style inherited nested classes; FQN split order in the fallback -mirrors FIR's `findClassId`. +Like javac, the leftmost-type interpretation is committed: a failed descent returns the +(nonexistent) nested id of the committed prefix, which stays unresolved downstream — red code. +On a package/type name clash (JLS 6.1) the shadowing type therefore wins, matching javac and +diverging from the PSI Java model, which loosely resolves the package interpretation; such +tests live in the java-direct-owned `testData/diagnostics` root +(`qualifiedNamePackageClassClash.kt`, `PackageVsClass2.kt`; KT-87813). + +Corner cases: `Map.Entry`-style inherited nested classes. ### Scenario E — Inherited member type via supertypes
diff --git a/compiler/java-direct/build.gradle.kts b/compiler/java-direct/build.gradle.kts index a3a5bde..c5ec3a7 100644 --- a/compiler/java-direct/build.gradle.kts +++ b/compiler/java-direct/build.gradle.kts
@@ -52,6 +52,7 @@ ) ) {} testGenerator("org.jetbrains.kotlin.java.direct.TestGeneratorKt", generateTestsInBuildDirectory = true) + testData(isolated, "testData") testData(project(":compiler:fir:analysis-tests").isolated, "testData") testData(project(":compiler").isolated, "testData/codegen") testData(project(":compiler").isolated, "testData/diagnostics")
diff --git a/compiler/java-direct/implDocs/RESOLUTION_SCHEMA.md b/compiler/java-direct/implDocs/RESOLUTION_SCHEMA.md index 31107b9..c6f9566 100644 --- a/compiler/java-direct/implDocs/RESOLUTION_SCHEMA.md +++ b/compiler/java-direct/implDocs/RESOLUTION_SCHEMA.md
@@ -214,7 +214,7 @@ Entry: `JavaTypeResolver.resolve` (dotted name) → `resolveQualifiedNameToClassIdFromParts`. A single left-to-right, non-recursive pass mirroring javac's PackageOrTypeName classification -(JLS 6.5.4), plus a loose package-interpretation fallback: +(JLS 6.5.4): 1. **Leftmost type** (JLS 6.5.4): the first segment as a simple type name in scope (Scenario B); failing that, the package prefix grows one segment at a time until a segment names a @@ -231,14 +231,21 @@ `collectInheritedInnerClasses`-based re-entrance fallback for a guard-skip on this step was removed as dead code once this step became un-guard-skippable — it was also source-only, the same cross-origin ambiguity blind spot fixed elsewhere in this scenario. -3. **Fallback** to plain package/class splits longest-package-first (`probeFqnSplits`), reached - only when the JLS pass fails. It diverges from javac (which reports an error) exactly on a - package/type name clash (JLS 6.1), resolving the package interpretation for PSI parity — - pinned by `qualifiedNamePackageClassClash.kt` (KT-87813). + +Like javac, the leftmost-type interpretation is committed: when the descent fails (full +resolution), the function returns the *nonexistent* nested id of the committed prefix instead of +retrying the name as a plain `package.Class` split — the id has no symbol, so the reference stays +unresolved downstream (red code). On a package/type name clash (JLS 6.1) the shadowing type +therefore wins, matching javac and diverging from the PSI Java model, which loosely resolves the +package interpretation; such tests are skipped for java-direct +(`SkipTestsPinningPsiJavaModelDeviationsMetaConfigurator`) and mirrored by javac-strict copies in +the java-direct-owned `testData/diagnostics` root (`qualifiedNamePackageClassClash.kt`, +`PackageVsClass2.kt`; KT-87813). The reentrance-safe flavor (`fullResolution = false`) returns +`null` instead of a dangling id, so supertype-walk seeding is never poisoned by a nonexistent +class. Corner cases: `Map.Entry`-style inherited nested classes (the descent probes declared-then- -inherited at *every* segment, so multi-segment tails after an inherited hop work too); FQN split -order in the fallback mirrors FIR's `findClassId`. +inherited at *every* segment, so multi-segment tails after an inherited hop work too). ### Scenario E — Inherited member type via supertypes @@ -278,7 +285,7 @@ class's own `implements` clause to its own inherited (through two supertypes) nested class, which only resolves if the raw-AST-text seed is used instead of the guarded `directSupertypeClassIds(containingClass)`. - `JavaTypeResolver.findInheritedNestedClass` (Scenario D step 3, the `Outer.Nested` qualified + `JavaTypeResolver.findInheritedNestedClass` (Scenario D step 2, the `Outer.Nested` qualified shape) is this same function's other caller: it materializes `outerClassId` via `classifierAdapterFor` (Scenario A step 4) and passes the result as `containingClass`, so it inherits the same raw-AST-text safety instead of needing its own seed/BFS pair
diff --git a/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/resolution/JavaTypeResolver.kt b/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/resolution/JavaTypeResolver.kt index 45b6893..8a6ab20 100644 --- a/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/resolution/JavaTypeResolver.kt +++ b/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/resolution/JavaTypeResolver.kt
@@ -60,12 +60,11 @@ * - each segment after the leftmost type must be a member type of the previous one (JLS * 6.5.5.2): declared, or — with [fullResolution] — inherited from its supertypes. * - * A deviation from strict JLS: when the pass above fails, the whole name is retried as a plain - * `package.Class` split via [probeFqnSplits]. The two disagree only on a package/type name - * clash (discouraged by JLS 6.1), where javac commits to the shadowing type and reports an - * error, but the PSI Java model resolves the package interpretation; see - * `qualifiedNamePackageClassClash.kt`. - * TODO: investigate possible consequences (KT-87813) + * Like javac, once a leftmost type is found the interpretation is committed: if the descent + * then fails, the name is NOT retried as a plain `package.Class` split. On a package/type name + * clash (discouraged by JLS 6.1) the shadowing type therefore wins and the reference stays + * unresolved (red code), matching javac and diverging from the PSI Java model, which resolves + * the package interpretation — pinned by `qualifiedNamePackageClassClash.kt`. * * [fullResolution] controls whether inherited-inner-class lookup is enabled; `false` selects the * reentrance-safe flavor used as a fallback from [resolveInheritedInnerClassToClassId]. @@ -87,21 +86,29 @@ next++ } + var current = classId ?: return null // no segment names a type in scope + // Member-type descent (JLS 6.5.5.2). The inherited lookup covers names like // `SimpleFunctionDescriptor.CopyBuilder`, where `CopyBuilder` comes from the // `FunctionDescriptor` superinterface of the resolved prefix. - while (classId != null && next < parts.size) { - val declared = classId.createNestedClassId(Name.identifier(parts[next])) - classId = when { + while (next < parts.size) { + val declared = current.createNestedClassId(Name.identifier(parts[next])) + current = when { classExists(declared, fullResolution) -> declared - fullResolution -> findInheritedNestedClass(classId, parts[next]) - else -> null + // Commit like javac: the leftmost-type interpretation is final, so a failed descent + // yields the (nonexistent) nested id of the committed prefix — e.g. on a package/type + // name clash `pkg.clash.Nested`, where the class `pkg.clash` shadows the package + // (JLS 6.1), this returns the dangling ClassId(pkg, clash.Nested), which stays + // unresolved downstream — red code, as javac reports (`qualifiedNamePackageClassClash.kt`). + fullResolution -> findInheritedNestedClass(current, parts[next]) + ?: return parts.subList(next, parts.size).fold(current) { id, segment -> id.createNestedClassId(Name.identifier(segment)) } + // The reentrance-safe flavor seeds supertype walks; a dangling id would silently + // poison the walk, so fail the resolution instead. + else -> return null } next++ } - if (classId != null) return classId - - return probeFqnSplits(parts, fullResolution) + return current } /** @@ -608,21 +615,3 @@ return null } -/** - * Probes every package/class split of [parts] from longest package prefix down to the root - * package, returning the first [ClassId] accepted by the class-existence probe. This is the - * `package.Class` fallback of [resolveQualifiedNameToClassIdFromParts]; [resolveAsClassId] runs - * the same longest-package-first split for a single [FqName]. - */ -context(c: JavaResolutionContext) -private fun probeFqnSplits(parts: List<String>, fullResolution: Boolean): ClassId? { - if (parts.isEmpty()) return null - for (classStartIndex in (parts.size - 1) downTo 0) { - val packageFqName = if (classStartIndex == 0) FqName.ROOT - else FqName.fromSegments(parts.subList(0, classStartIndex)) - val relativeClassName = FqName.fromSegments(parts.subList(classStartIndex, parts.size)) - val candidate = ClassId(packageFqName, relativeClassName, isLocal = false) - if (classExists(candidate, fullResolution)) return candidate - } - return null -}
diff --git a/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/util/JavaSupertypeGraph.kt b/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/util/JavaSupertypeGraph.kt index 61e56dd..19009d2 100644 --- a/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/util/JavaSupertypeGraph.kt +++ b/compiler/java-direct/src/org/jetbrains/kotlin/java/direct/util/JavaSupertypeGraph.kt
@@ -257,7 +257,7 @@ } // Emit candidate ClassIds for an imported FqName, longest-package-first (mirrors - // probeFqnSplits in JavaTypeResolver.kt); the wrong split has no symbol-provider entry + // resolveAsClassId in JavaTypeResolver.kt); the wrong split has no symbol-provider entry // and is dropped downstream. private fun fqNameSplitCandidates(fqName: FqName): List<ClassId> { val parts = fqName.pathSegments().map { it.asString() }
diff --git a/compiler/java-direct/testData/diagnostics/PackageVsClass2.kt b/compiler/java-direct/testData/diagnostics/PackageVsClass2.kt new file mode 100644 index 0000000..239447a --- /dev/null +++ b/compiler/java-direct/testData/diagnostics/PackageVsClass2.kt
@@ -0,0 +1,60 @@ +// RUN_PIPELINE_TILL: FRONTEND +// DIAGNOSTICS: -UNUSED_PARAMETER + +// javac-strict mirror of `diagnostics/tests/javac/qualifiedExpression/PackageVsClass2.kt`, +// which is skipped for java-direct (see `SkipTestsPinningPsiJavaModelDeviationsMetaConfigurator`). +// +// In `test/d.java` the return type `a.b` is a package/type name clash (JLS 6.1): the class +// `test.a` is in scope and shadows the package `a`, so javac commits to the type `test.a`, +// requires `b` to be its member type, and rejects the reference ("cannot find symbol: class b, +// location: class a"). java-direct follows javac, leaving `getB()`'s return type unresolved, +// while the PSI Java model loosely resolves the package interpretation `a.b` (KT-87813). + +// FILE: a/a.java +package a; + +public class a {} + +// FILE: a/b.java +package a; + +public class b { + public void a_b() {} +} + +// FILE: test/a.java +package test; + +public class a {} + +// FILE: test/d.java +package test; + +public class d { + public a.b getB() { return null; } +} + +// FILE: b.kt +package test + +val x = d().<!MISSING_DEPENDENCY_CLASS!>getB<!>() + +// FILE: test/c.java +package test; + +import a.a; + +public class c { + public static a getA() { return null; } +} + +// FILE: c.kt +package test + +fun foo() { + val a = c.getA() + a.<!UNRESOLVED_REFERENCE!>a<!> + a.<!UNRESOLVED_REFERENCE!>a<!>() +} + +/* GENERATED_FIR_TAGS: flexibleType, functionDeclaration, javaFunction, javaType, localProperty, propertyDeclaration */
diff --git a/compiler/java-direct/testData/diagnostics/qualifiedNamePackageClassClash.kt b/compiler/java-direct/testData/diagnostics/qualifiedNamePackageClassClash.kt new file mode 100644 index 0000000..4c4eea9 --- /dev/null +++ b/compiler/java-direct/testData/diagnostics/qualifiedNamePackageClassClash.kt
@@ -0,0 +1,56 @@ +// RUN_PIPELINE_TILL: FRONTEND +// SKIP_JAVAC + +// A qualified type name whose prefix is BOTH a class and a package (a discouraged +// "package/type name clash", JLS 6.1). +// +// `pkg.clash` is declared twice: +// * as a top-level class `pkg/clash.java` -> class `pkg.clash` +// * as a package `pkg/clash/Nested.java` -> class `pkg.clash.Nested` +// +// The Java method `Provider.get()` returns the qualified type name `pkg.clash.Nested`. +// +// Strict JLS 6.5.4.2/6.5.5 (and javac): resolving `pkg.clash.Nested` commits to the *type* +// `pkg.clash` at the leftmost point the qualifier becomes a type, then requires `Nested` +// to be a member type of it. It is not, so javac rejects `Provider.get()` with +// "cannot find symbol: class Nested, location: class clash" (the package `pkg.clash` is +// shadowed by the class of the same name). +// +// java-direct follows javac: the committed interpretation stays unresolved, so `Provider.get()` +// has an unresolved return type and neither member call resolves (red code). This diverges from +// the PSI Java model, which loosely falls back to the package interpretation and resolves +// `pkg.clash.Nested` to the top-level class `Nested` — which is why this test lives in the +// java-direct-owned testdata root rather than the shared diagnostics roots (KT-87813). + +// FILE: pkg/clash.java +package pkg; + +public class clash { + public int onlyOnClashClass() { return 1; } +} + +// FILE: pkg/clash/Nested.java +package pkg.clash; + +public class Nested { + public int onlyOnNested() { return 2; } +} + +// FILE: user/Provider.java +package user; + +public class Provider { + public pkg.clash.Nested get() { return null; } +} + +// FILE: main.kt +package main + +import user.Provider + +fun test(p: Provider) { + p.<!MISSING_DEPENDENCY_CLASS!>get<!>().<!UNRESOLVED_REFERENCE!>onlyOnNested<!>() + p.<!MISSING_DEPENDENCY_CLASS!>get<!>().<!UNRESOLVED_REFERENCE!>onlyOnClashClass<!>() +} + +/* GENERATED_FIR_TAGS: flexibleType, functionDeclaration, javaFunction, javaType */
diff --git a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/AbstractJavaUsingAstTest.kt b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/AbstractJavaUsingAstTest.kt index 8bb84fb..4ba1085 100644 --- a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/AbstractJavaUsingAstTest.kt +++ b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/AbstractJavaUsingAstTest.kt
@@ -14,7 +14,8 @@ super.configure(builder) with(builder) { useMetaTestConfigurators( - ::OnlyTestsWithJavaSourcesMetaConfigurator + ::OnlyTestsWithJavaSourcesMetaConfigurator, + ::SkipTestsPinningPsiJavaModelDeviationsMetaConfigurator, ) useConfigurators(::JavaDirectConfigurator) }
diff --git a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/TestGenerator.kt b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/TestGenerator.kt index c9f7fe1..5d56113 100644 --- a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/TestGenerator.kt +++ b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/TestGenerator.kt
@@ -22,6 +22,9 @@ "testData/diagnostics/tests", "testData/diagnostics/testsWithAnyBackend", "testData/diagnostics/testsWithStdLib", + // Module-owned diagnostics tests whose expectations deviate from the PSI Java + // model (which runs the shared roots above through its own suites). + "java-direct/testData/diagnostics", ).forEach { testDataRoot -> model( testDataRoot,
diff --git a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/components.kt b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/components.kt index 8fdbcd7..05809f1 100644 --- a/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/components.kt +++ b/compiler/java-direct/testFixtures/org/jetbrains/kotlin/java/direct/components.kt
@@ -31,3 +31,21 @@ testServices.moduleStructure.originalTestDataFiles.first().useLines { lines -> lines.none { it.matches(javaFileRegex) } } } +/** + * Shared diagnostics tests whose expectations pin the PSI Java model's loose handling of a + * package/type name clash (JLS 6.1): PSI falls back to the package interpretation where javac + * commits to the shadowing type and reports an error. `java-direct` follows javac, so these + * tests are skipped here and mirrored by javac-strict copies under + * `compiler/java-direct/testData/diagnostics` (KT-87813). + */ +private val testsPinningPsiJavaModelDeviations = setOf( + "compiler/testData/diagnostics/tests/javac/qualifiedExpression/PackageVsClass2.kt", +) + +class SkipTestsPinningPsiJavaModelDeviationsMetaConfigurator(testServices: TestServices) : MetaTestConfigurator(testServices) { + override fun shouldSkipTest(): Boolean { + val path = testServices.moduleStructure.originalTestDataFiles.first().invariantSeparatorsPath + return testsPinningPsiJavaModelDeviations.any { path.endsWith(it) } + } +} +
diff --git a/compiler/testData/diagnostics/tests/jvm/javaDirect/qualifiedNamePackageClassClash.kt b/compiler/testData/diagnostics/tests/jvm/javaDirect/qualifiedNamePackageClassClash.kt deleted file mode 100644 index 38b526a..0000000 --- a/compiler/testData/diagnostics/tests/jvm/javaDirect/qualifiedNamePackageClassClash.kt +++ /dev/null
@@ -1,56 +0,0 @@ -// RUN_PIPELINE_TILL: FRONTEND -// SKIP_JAVAC - -// javac divergence — a qualified type name whose prefix is BOTH a class and a package -// (a discouraged "package/type name clash", JLS 6.1). -// -// `pkg.clash` is declared twice: -// * as a top-level class `pkg/clash.java` -> class `pkg.clash` -// * as a package `pkg/clash/Nested.java` -> class `pkg.clash.Nested` -// -// The Java method `Provider.get()` returns the qualified type name `pkg.clash.Nested`. -// -// Strict JLS 6.5.4.2/6.5.5 (javac): resolving `pkg.clash.Nested` commits to the *type* -// `pkg.clash` at the leftmost point the qualifier becomes a type, then requires `Nested` -// to be a member type of it. It is not, so javac rejects `Provider.get()` with -// "cannot find symbol: class Nested, location: class clash" (the package `pkg.clash` is -// shadowed by the class of the same name). -// -// Both the PSI Java model and java-direct are LOOSE here: they fall back to the package -// interpretation and resolve `pkg.clash.Nested` to the top-level class `Nested` in package -// `pkg.clash`. This test pins that PSI == java-direct agreement — both diverge from javac -// identically — so `Provider.get()` returns `Nested` and `onlyOnNested()` resolves, while -// the class-only member `onlyOnClashClass()` does NOT. - -// FILE: pkg/clash.java -package pkg; - -public class clash { - public int onlyOnClashClass() { return 1; } -} - -// FILE: pkg/clash/Nested.java -package pkg.clash; - -public class Nested { - public int onlyOnNested() { return 2; } -} - -// FILE: user/Provider.java -package user; - -public class Provider { - public pkg.clash.Nested get() { return null; } -} - -// FILE: main.kt -package main - -import user.Provider - -fun test(p: Provider) { - p.get().onlyOnNested() - p.get().<!UNRESOLVED_REFERENCE!>onlyOnClashClass<!>() -} - -/* GENERATED_FIR_TAGS: flexibleType, functionDeclaration, javaFunction, javaType */