Bug fixes

[theron-wang]

This PR makes four changes:

• Static wildcard imports are not removed if they import from a used class
• Constructors are preserved/created with a minimal super call when its superclass does not have a parameterless constructor
• Support for record classes (behavior: only used members are preserved, and non-canonical constructors are trimmed down to a minimal this call to the canonical constructor)
• Method preservation for "adjacent" classes, where a used type implementing an interface with a known method that must be preserved also has to preserve that method, even if that method is never actually called by that specific used type, but is called by another used type that implements the same interface (see MustImplementMethodsAdjacentClassTest for an example)

[coderabbitai]

📝 Walkthrough

Walkthrough

The change introduces SpeciminTypeSolvers for shared type-resolution configuration and updates parser, runner, and slicer APIs to use it. Callable resolution now supports NodeWithParameters, record members, and improved AST attachment matching. Slicing tracks additional dependencies, refreshes solver caches, and generates superclass and record constructor delegation. Unsolved symbol generation now preserves slice-aware constructor and method content. New tests and fixtures cover adjacent implementations, sealed constructors, and records.

Suggested reviewers: kelloggm

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.76% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/org/checkerframework/specimin/JavaParserUtil.java (1)

1322-1356: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Skip the record declaration when an equivalent constructor is already present
In tryResolveConstructorCallWithUnresolvableArguments, a record with an explicit canonical constructor can be added twice: once as the record declaration and once as a ConstructorDeclaration. That makes partially unresolved calls look ambiguous in tryFindSingleCallableForNodeWithUnresolvableArguments.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/main/java/org/checkerframework/specimin/JavaParserUtil.java` around lines
1322 - 1356, Update tryResolveConstructorCallWithUnresolvableArguments so the
record declaration is added as a candidate only when no equivalent canonical
ConstructorDeclaration was added. Preserve the existing record candidate
behavior for records without an equivalent explicit constructor, and avoid
duplicate entries in candidates before returning.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/org/checkerframework/specimin/JavaParserUtil.java`:
- Around line 2030-2046: Update JavaParserUtil.getDefaultConstructorCall to
delete the trailing separator only when paramTypes is non-empty, preserving
valid this()/super() output for zero-parameter calls. No direct change is needed
in src/main/java/org/checkerframework/specimin/Slicer.java lines 449-452; its
call is corrected by the utility fix.
- Around line 1982-1999: Update the parent-name disambiguation in the
attached-node lookup to use NodeWithSimpleName instead of NodeWithName for both
the detached parent and candidate parent checks. Preserve the existing class and
node equality heuristics while ensuring simple-name comparisons are applied for
supported parent types.

In `@src/main/java/org/checkerframework/specimin/Resolver.java`:
- Around line 404-478: Guard the method-call fallback in
UnsolvedSymbolGenerator.handleMethodCallExpr against non-MethodDeclaration nodes
returned by ResolvedRecordMemberDeclaration.toAst(). Check the resolved node
type before casting, or skip this fallback for record-member accessors, while
preserving normal MethodDeclaration handling.

In `@src/main/java/org/checkerframework/specimin/Slicer.java`:
- Around line 383-448: Update the superclass-constructor selection in the
surrounding ClassOrInterfaceDeclaration handling to consider only constructors
accessible from the subclass, excluding inaccessible private or otherwise
invalid candidates before choosing the shortest parameter list. Track the
selected constructor’s checked exceptions and apply a matching throws clause to
every generated or body-replaced subclass constructor, including the path using
addConstructor.

In `@src/main/java/org/checkerframework/specimin/SpeciminTypeSolvers.java`:
- Around line 91-152: Pin the exact javaparser-symbol-solver-core version used
by the project and add a unit test that directly exercises overrideCache through
the paths implemented by overrideInCombinedTypeSolverCache and
overrideInJavaParserTypeSolverCache. Verify both private-field cache overrides
succeed and continue resolving the supplied declaration, so dependency changes
that break the reflected fields fail during testing.

---

Outside diff comments:
In `@src/main/java/org/checkerframework/specimin/JavaParserUtil.java`:
- Around line 1322-1356: Update
tryResolveConstructorCallWithUnresolvableArguments so the record declaration is
added as a candidate only when no equivalent canonical ConstructorDeclaration
was added. Preserve the existing record candidate behavior for records without
an equivalent explicit constructor, and avoid duplicate entries in candidates
before returning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: d8691ab8-e961-4ecb-9915-c4b01144a7fb

📥 Commits

Reviewing files that changed from the base of the PR and between 19acb03 and 2cdb0ef.

📒 Files selected for processing (31)

• src/main/java/org/checkerframework/specimin/JavaParserUtil.java
• src/main/java/org/checkerframework/specimin/Resolver.java
• src/main/java/org/checkerframework/specimin/Slicer.java
• src/main/java/org/checkerframework/specimin/SpeciminRunner.java
• src/main/java/org/checkerframework/specimin/SpeciminTypeSolvers.java
• src/main/java/org/checkerframework/specimin/StandardTypeRuleDependencyMap.java
• src/main/java/org/checkerframework/specimin/TypeRuleDependencyMap.java
• src/main/java/org/checkerframework/specimin/unsolved/FullyQualifiedNameGenerator.java
• src/main/java/org/checkerframework/specimin/unsolved/UnsolvedFieldAlternates.java
• src/main/java/org/checkerframework/specimin/unsolved/UnsolvedMethod.java
• src/main/java/org/checkerframework/specimin/unsolved/UnsolvedMethodAlternates.java
• src/main/java/org/checkerframework/specimin/unsolved/UnsolvedMethodCommon.java
• src/main/java/org/checkerframework/specimin/unsolved/UnsolvedSymbolGenerator.java
• src/test/java/org/checkerframework/specimin/MustImplementMethodAdjacentClassTest.java
• src/test/java/org/checkerframework/specimin/PermitsConstructorsTest.java
• src/test/java/org/checkerframework/specimin/RecordsTest.java
• src/test/resources/mustimplementmethodsadjacentclass/expected/com/example/Bar.java
• src/test/resources/mustimplementmethodsadjacentclass/expected/com/example/Baz.java
• src/test/resources/mustimplementmethodsadjacentclass/expected/com/example/Foo.java
• src/test/resources/mustimplementmethodsadjacentclass/input/com/example/Bar.java
• src/test/resources/mustimplementmethodsadjacentclass/input/com/example/Baz.java
• src/test/resources/mustimplementmethodsadjacentclass/input/com/example/Foo.java
• src/test/resources/permitsconstructors/expected/com/example/Foo.java
• src/test/resources/permitsconstructors/expected/com/example/Unsolved.java
• src/test/resources/permitsconstructors/input/com/example/Foo.java
• src/test/resources/records/expected/com/example/Foo.java
• src/test/resources/records/expected/com/example/MyOtherRecord.java
• src/test/resources/records/expected/com/example/MyRecord.java
• src/test/resources/records/input/com/example/Foo.java
• src/test/resources/records/input/com/example/MyOtherRecord.java
• src/test/resources/records/input/com/example/MyRecord.java

[kelloggm]

I did not review the code itself, only the tests. The three new tests look good, but there isn't a test for the change to the behavior with static imports. Please add one.

A PR like this one that has independent components, though, really ought to be separate PRs. That is, I'd expect there to be four PRs for these changes, not just one, that correspond exactly to the four bullet points in your PR description. The reason is a practical one: as a reviewer on a PR like this one that mixes up four concerns, I would have to carefully separate apart the concerns as I review, hunting around for all of the changes related to each separate change. This significantly reduces the value of the review: I will spend more of my review time trying to figure out what things are related to each other rather than actually critically evaluating the code. If there were instead four separate PRs, I wouldn't have to do that. So, please split this PR up into four separate PRs, one for each change, and then I'll review those separately.