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

InstanceOfPatternMatch removes try-with-resource variable #4173

Open
delanym opened this issue May 7, 2024 · 6 comments
Open

InstanceOfPatternMatch removes try-with-resource variable #4173

delanym opened this issue May 7, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@delanym
Copy link

delanym commented May 7, 2024

Given the code

   if (object instanceof JarArtifact) {
      try (JarArtifact a = (JarArtifact) object) {

a rewrite using InstanceOfPatternMatch results in

    if (object instanceof JarArtifact a) {
      try () {

A fix could be to simply remove the parenthesis

    if (object instanceof JarArtifact a) {
      try {

but that may leave the object unclosed, so it should probably be left alone.

@delanym delanym added the bug Something isn't working label May 7, 2024
@timtebeek
Copy link
Contributor

Thanks for the report! That would be a bug indeed; we should not make that change when the assignment is in a try-with-resources-try-block. Is this something you'd want to help fix? Even a draft PR with just a unit test added would already help.

@knutwannheden
Copy link
Contributor

I forget in which Java version this was, but possibly 9, the syntax allows for an expression in try-with-resources and doesn't mandate declaring a new variable. So with an appropriate language version we can leverage that.

@delanym
Copy link
Author

delanym commented May 8, 2024

@delanym
Copy link
Author

delanym commented May 8, 2024

@timtebeek I'll have a go at it. How to detect Java source version?

@timtebeek
Copy link
Contributor

Awesome, thanks for the offer to help! We determine the Java version when we parse the project, and store that in a marker called JavaVersion. A common pattern is to define a Precondition through UsesJavaVersion, to limit when a recipe should apply.

public class UsesJavaVersion<P> extends JavaIsoVisitor<P> {

In rare cases we one can also read the marker directly, but use that only when preconditions aren't an option.

@knutwannheden
Copy link
Contributor

In this particular case we probably don't need anything in addition, as instanceof pattern variables were added in Java 11. So that precondition is already a given.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Backlog
Development

No branches or pull requests

3 participants