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

Revisit ownership of static fields of a resource type #2161

Closed
stephan-herrmann opened this issue Mar 14, 2024 · 1 comment · Fixed by #2281
Closed

Revisit ownership of static fields of a resource type #2161

stephan-herrmann opened this issue Mar 14, 2024 · 1 comment · Fixed by #2281
Assignees

Comments

@stephan-herrmann
Copy link
Contributor

A static field of any type implementing AutoCloseable and tagged as @Owning currently triggers this warning:

Class with resource fields tagged as '@Owning' should implement AutoCloseable

However, doing as recommend does not help. In fact, once you add the super interface and implement close(), there's no obligation to actually close any resource in this method, since this part correctly understands, that this instance method is not responsible for closing a resource held in a static field.

So, that recommendation to implement AutoCloseable is bogus when only static resource fields are present.

The sibling warning is borderline:

It is recommended to mark resource fields as '@Owning' to ensure proper closing

Does the annotation help on a static field? Which entity will be responsible for closing? Any? The annotation can still help to signal that assigning a fresh resource to this field is OK without closing. Interestingly, assigning a fresh resource to a @NotOwning field (static or instance) does not raise a warning (but should).

@stephan-herrmann stephan-herrmann self-assigned this Mar 14, 2024
stephan-herrmann added a commit to stephan-herrmann/eclipse.jdt.core that referenced this issue Apr 4, 2024
fixes eclipse-jdt#2161
+ warn about static resource fields (if annotations are enabled)
+ no warning on any field of whitelisted type
+ warn on field initialization in static block, but not at field
+ resolve left-over from bug 552521: no warnings in unreachable code
+ make new problems from PR eclipse-jdt#1716 suppressable (token = "resource")
Tests:
+ in annotation mode annotate some fields as @owning for equal result
@stephan-herrmann
Copy link
Contributor Author

With #2281 and when resource annotations are enabled, static resource fields will unconditionally report the following:
It is not recommended to hold a resource in a static field

Should someone still want to do so, the following idiom can be used:
static @SuppressWarnings("resource") @Owning AutoCloseable myResource;

The @SuppressWarnings annotation will suppress the above new warning, whereas the @Owning annotation avoids that clients assigning into this field will be blamed for not closing. Use at your own risk!

Generally, with some additional fixes in #2281 more fields may need to be marked as @Owning.

stephan-herrmann added a commit that referenced this issue Apr 4, 2024
fixes #2161
+ warn about static resource fields (if annotations are enabled)
+ no warning on any field of whitelisted type
+ warn on field initialization in static block, but not at field
+ resolve left-over from bug 552521: no warnings in unreachable code
+ make new problems from PR #1716 suppressable (token = "resource")
Tests:
+ in annotation mode annotate some fields as @owning for equal result
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 a pull request may close this issue.

1 participant