Skip to content

Fix Value Checker unchecked-bytecode defaults test#1384

Open
aosen-xiong wants to merge 7 commits into
eisop:masterfrom
aosen-xiong:typo-conservativedefault
Open

Fix Value Checker unchecked-bytecode defaults test#1384
aosen-xiong wants to merge 7 commits into
eisop:masterfrom
aosen-xiong:typo-conservativedefault

Conversation

@aosen-xiong

@aosen-xiong aosen-xiong commented Sep 3, 2025

Copy link
Copy Markdown
Collaborator

This fixes ValueUncheckedDefaultsTest so it actually exercises -AuseConservativeDefaultsForUncheckedCode=bytecode.

The test previously passed btyecode, so the bytecode conservative-defaults mode was never enabled. Correcting the option exposed that many existing Value Checker tests call unannotated JDK/library bytecode APIs whose parameters then default to bottom. This PR adds a test-local stub file for the APIs used by the suite, so the test can continue checking the intended Value Checker behavior under unchecked bytecode defaults.

@aosen-xiong

Copy link
Copy Markdown
Collaborator Author

Is this typo intentional then?

@wmdietl

wmdietl commented Sep 3, 2025

Copy link
Copy Markdown
Member

Thanks for finding this! I wouldn't call this a typo - it passes an illegal argument to an option.
Can you see what the original intent of the test was and how we can fix this overall?
We should also be more stringent about validating options and fail if an illegal argument is passed. Please open an issue for that.

@aosen-xiong aosen-xiong marked this pull request as draft September 3, 2025 15:56
@aosen-xiong

Copy link
Copy Markdown
Collaborator Author

Thanks for finding this! I wouldn't call this a typo - it passes an illegal argument to an option. Can you see what the original intent of the test was and how we can fix this overall? We should also be more stringent about validating options and fail if an illegal argument is passed. Please open an issue for that.

Looks like you already noticed this a few years back. Seehttps://github.com/typetools/checker-framework/pull/1318#discussion_r217824677.

@aosen-xiong aosen-xiong marked this pull request as ready for review May 25, 2026 15:55
Copilot AI review requested due to automatic review settings May 25, 2026 15:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds a dedicated annotated stub file to support Value Checker tests for unchecked bytecode defaults, and updates the JUnit test configuration to use it.

Changes:

  • Added unchecked-bytecode.astub with annotated signatures for selected JDK/library APIs used by tests.
  • Updated ValueUncheckedDefaultsTest to load the new stub file and corrected the unchecked-code defaults option value.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
framework/tests/value/unchecked-bytecode.astub Introduces annotated stub declarations used when testing conservative defaults for unchecked bytecode.
framework/src/test/java/org/checkerframework/framework/test/junit/ValueUncheckedDefaultsTest.java Configures the test run to include the new stub file and fixes the option value typo.

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

Comment thread framework/tests/value/unchecked-bytecode.astub Outdated
Comment thread framework/tests/value/unchecked-bytecode.astub Outdated
Comment thread framework/tests/value/unchecked-bytecode.astub
Comment thread framework/tests/value/unchecked-bytecode.astub
@aosen-xiong aosen-xiong changed the title Fix typo Fix Value Checker unchecked-bytecode defaults test May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants