Skip to content

Conversation

@igfoo
Copy link
Contributor

@igfoo igfoo commented Oct 30, 2025

It's not perfect, but I think a proper fix will need to wait until we think about this properly in the planned rewrite.

@igfoo igfoo force-pushed the igfoo/ClassInstanceStack branch 2 times, most recently from 0ca6ebf to 95ef5c7 Compare October 31, 2025 12:45
@igfoo igfoo force-pushed the igfoo/ClassInstanceStack branch from 95ef5c7 to 06218d8 Compare October 31, 2025 13:44
@igfoo igfoo marked this pull request as ready for review October 31, 2025 14:34
@igfoo igfoo requested review from a team as code owners October 31, 2025 14:34
Copilot AI review requested due to automatic review settings October 31, 2025 14:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses an infinite recursion issue that occurs when extracting recursive interfaces in Kotlin code. The fix adds a ClassInstanceStack to track classes being extracted and detects potential cycles, falling back to raw types when recursion is detected.

Key Changes:

  • Introduced a ClassInstanceStack utility class to track class extraction recursion
  • Modified the extraction logic to detect and avoid infinite recursion by using raw types
  • Added test cases for both nested generic types and recursive interface hierarchies

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
java/kotlin-extractor/src/main/kotlin/utils/ClassInstanceStack.kt New utility class that tracks class extraction state and detects cyclic type arguments
java/kotlin-extractor/src/main/kotlin/KotlinUsesExtractor.kt Added avoidInfiniteRecursion method that uses the stack to detect cycles and fall back to raw types
java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt Wraps supertype extraction in try-finally to manage the class instance stack
java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt Instantiates and threads the ClassInstanceStack through the extraction pipeline
java/kotlin-extractor/src/main/kotlin/ExternalDeclExtractor.kt Accepts and passes through the ClassInstanceStack parameter
java/ql/test-kotlin2/library-tests/nested_types/test.kt Test case for nested generic types
java/ql/test-kotlin2/library-tests/nested_types/types.ql Query to verify nested type extraction
java/ql/test-kotlin2/library-tests/nested_types/types.expected Expected results for nested types test
java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/*.java Test cases with recursive interface hierarchies
java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.kt Kotlin test file using the recursive interfaces
java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.ql Query to verify recursive interface extraction
java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/types.expected Expected results for recursive interfaces test
java/ql/integration-tests/kotlin/all-platforms/recursive_interfaces/test.py Test configuration for the integration test

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


from Type t
where
t.getName().matches("%MyType%") and
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 5's condition checking for %MyType% is redundant since line 6 already filters for specific type patterns that would include MyType. This creates a logical AND that makes line 5 unnecessary - any type matching the patterns in line 6 will contain MyType. Consider removing line 5 to simplify the query logic.

Suggested change
t.getName().matches("%MyType%") and

Copilot uses AI. Check for mistakes.
private val stack: Stack<IrClass> = Stack()

fun push(c: IrClass) = stack.push(c)
fun pop() = stack.pop()
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The pop() method returns the popped value but none of the call sites use the return value. Consider changing the return type to Unit to make the API clearer about its intent, or add a comment explaining why the return value is exposed if future use is anticipated.

Suggested change
fun pop() = stack.pop()
fun pop() { stack.pop() }

Copilot uses AI. Check for mistakes.
Comment on lines +551 to +553
} else {
return pair
}
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The else clause is unnecessary here since the if block returns. Remove the else keyword and unindent line 552 to reduce nesting and improve readability.

Suggested change
} else {
return pair
}
}
return pair

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks plausible

@igfoo igfoo merged commit 7ff696b into github:main Oct 31, 2025
16 checks passed
@igfoo igfoo deleted the igfoo/ClassInstanceStack branch October 31, 2025 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants