Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RLC and Must Call Checker stub files for Apache Commons IOUtils class #5478

Merged
merged 12 commits into from Jan 4, 2023

Conversation

msridhar
Copy link
Contributor

@msridhar msridhar commented Dec 28, 2022

I took the stub file from our old one here and just updated an import. However, my test case is not passing and still reporting an unexpected warning. @kelloggm any idea how I can debug?

Update: This is now debugged.

@kelloggm
Copy link
Contributor

I looked into this briefly and I can't see anything wrong with the stub file (other than some unannotated methods being present and some non-checker-framework annotations, but those are just extraneous, not a problem). Running with -AstubDebug would be my first thought, but I tried that and the stub file is loaded properly with no errors. I don't see any reason that this test case wouldn't pass.

My next debugging action here would be to instrument either the consistency analyzer (to see why it's reporting the error) or the Called Methods Checker (to see if it can see the stub file). Somewhere along the chain, those @EnsuresCalledMethods annotations are getting lost, but I'm not sure where. Unfortunately, I don't have time to look into this today, but I might be able to tomorrow.

@mernst
Copy link
Member

mernst commented Dec 30, 2022

This is a long shot, but could you try using explicit rather than wildcard imports?

@msridhar
Copy link
Contributor Author

This is a long shot, but could you try using explicit rather than wildcard imports?

I gave it a shot, but it didn't seem to work.

@smillst
Copy link
Member

smillst commented Jan 3, 2023

@msridhar The stub file was not being read by the Resource Leak Checker since it was added to the MustCall Checker. I've fixed this.

@smillst smillst assigned msridhar and unassigned smillst Jan 3, 2023
@msridhar msridhar changed the title [WIP] RLC stub file for Apache Commons IOUtils class RLC and Must Call Checker stub files for Apache Commons IOUtils class Jan 3, 2023
@msridhar
Copy link
Contributor Author

msridhar commented Jan 3, 2023

@msridhar The stub file was not being read by the Resource Leak Checker since it was added to the MustCall Checker. I've fixed this.

Thanks for the fix!

@msridhar msridhar marked this pull request as ready for review January 3, 2023 20:01
@msridhar msridhar requested a review from kelloggm January 3, 2023 20:01
@msridhar
Copy link
Contributor Author

msridhar commented Jan 3, 2023

@smillst when is the next CF release planned? If we could get this merged before the release is cut that would be helpful to us.

@smillst
Copy link
Member

smillst commented Jan 3, 2023

@smillst when is the next CF release planned? If we could get this merged before the release is cut that would be helpful to us.

I'm doing the release tomorrow, so as long as this is merged by 8am PST tomorrow, it will be in the release.

kelloggm
kelloggm previously approved these changes Jan 3, 2023
Copy link
Contributor

@kelloggm kelloggm left a comment

Choose a reason for hiding this comment

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

Thanks @smillst for figuring this out! I should have realized what the problem was myself.

@kelloggm kelloggm enabled auto-merge (squash) January 3, 2023 20:57
@smillst
Copy link
Member

smillst commented Jan 4, 2023

@mernst @msridhar @kelloggm I'll wait to do the release today until this is merged.

@kelloggm kelloggm merged commit 46bd8bc into typetools:master Jan 4, 2023
wmdietl pushed a commit to eisop/checker-framework that referenced this pull request Feb 8, 2023
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.

None yet

4 participants