Skip to content

Clarify that CallableDeclarationAnnos.getParameterType is 0-based - #8161

Closed
mernst wants to merge 14 commits into
typetools:masterfrom
mernst:wpi-review-fix-8
Closed

Clarify that CallableDeclarationAnnos.getParameterType is 0-based#8161
mernst wants to merge 14 commits into
typetools:masterfrom
mernst:wpi-review-fix-8

Conversation

@mernst

@mernst mernst commented Sep 11, 2026

Copy link
Copy Markdown
Member

Its sibling getParameterTypeInitialized is 1-based, as is most of the
whole-program inference API, so name the parameter index_0based and
cross-reference the two methods.

mernst and others added 14 commits September 2, 2026 18:54
@mernst @claude
Clarify that CallableDeclarationAnnos.getParameterType is 0-based
270b673 
Its sibling getParameterTypeInitialized is 1-based, as is most of the
whole-program inference API, so name the parameter index_0based and
cross-reference the two methods.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mernst
Suppress a warning
5f0155e
@mernst
Merge ../checker-framework-branch-master into wpi-review-fix-8
58e5f15
@mernst
Merge ../checker-framework-branch-master into wpi-review-fix-8
12fd2d4
@mernst
Add import
dff7122
@mernst
Format string parameter is not necessarily the first paramter
5b9b2e5
@mernst
More lookup of format string paramter
b4a086e
@mernst
Merge ../checker-framework-branch-master into wpi-review-fix-8
e89c387
@mernst @claude
Fix Signature Checker error in FormatterAnnotatedTypeFactory
e65a611 
Signatures.splitJvmArglist returns List<@FieldDescriptor String>, which is
not assignable to List<@SignatureUnknown String>.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mernst
Handle ill-formed signatures
a10bbf8
@mernst
Merge ../checker-framework-branch-master into wpi-review-fix-8
4c1d3ef
@mernst @claude
Test and refine the @FormatMethod handling in whole-program inference
62984a6 
Add `AinferFormatMethodTest`, which runs whole-program inference in both
ajava and jaif mode and checks that no `@Format` annotation is inferred
for the format string parameter of a `@FormatMethod` method, whose format
string parameter is not its first formal parameter.  A control method in
the same source file shows that inference does annotate the `String`
parameter of a method that is not a format method.

In the ajava code path, determine whether a formal parameter's type is
`String` from its `TypeMirror` when an inferred type is available, which
is the same test that `FormatterVisitor.formatStringIndex` performs.  The
syntactic test, which assumes that the simple name `String` refers to
`java.lang.String`, is now only a fallback for a parameter about which
nothing was inferred.

Document, at `FormatterVisitor.formatStringIndex`, that the two copies of
the definition in `FormatterAnnotatedTypeFactory` must be kept in sync
with it, and correct that method's `@return` tag to say "first" rather
than "last".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mernst
Merge ../checker-framework-branch-master into wpi-review-fix-8
7ee15b6
@mernst
Tweak a comment
6f26a34
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a89aace0-9061-4792-b1ad-5ddb2d99e778

📥 Commits

Reviewing files that changed from the base of the PR and between 4325887 and 6f26a34.

📒 Files selected for processing (4)
  • checker/src/main/java/org/checkerframework/checker/formatter/FormatterAnnotatedTypeFactory.java
  • checker/src/main/java/org/checkerframework/checker/formatter/FormatterVisitor.java
  • checker/src/test/java/org/checkerframework/checker/test/junit/AinferFormatMethodTest.java
  • framework/src/main/java/org/checkerframework/common/wholeprograminference/WholeProgramInferenceJavaParserStorage.java

Included review availability: Your plan provides up to 4 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The formatter checker now identifies the first formal parameter declared as String for whole-program inference. AFU and JavaParser paths handle missing data, malformed signatures, varargs, and non-class types. Documentation clarifies parameter indexing and format-parameter rules. A new integration test verifies .ajava and .jaif output.

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6f26a

No actionable behavior or integration risk remains from this change.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 84.62% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 4 files.
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

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

@mernst

mernst commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

Already merged as #8107; this PR is empty (its merge commit has the same tree as master). Closing.

@mernst mernst closed this Sep 11, 2026
@mernst
mernst deleted the wpi-review-fix-8 branch September 11, 2026 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@mernst