Skip to content

[java] CloseResource: false positive with reassignment detection#3147

Merged
adangel merged 3 commits into
pmd:masterfrom
adangel:issue-2977-close-resource-reassignment
Mar 27, 2021
Merged

[java] CloseResource: false positive with reassignment detection#3147
adangel merged 3 commits into
pmd:masterfrom
adangel:issue-2977-close-resource-reassignment

Conversation

@adangel

@adangel adangel commented Mar 1, 2021

Copy link
Copy Markdown
Member

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

@adangel adangel added this to the 6.33.0 milestone Mar 1, 2021
@ghost

ghost commented Mar 1, 2021

Copy link
Copy Markdown
1 Message
📖 This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 1 violations, 2 errors and 2 configuration errors.
Full report
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 1 violations, 2 errors and 2 configuration errors.
Full report

Generated by 🚫 Danger

// The sibling before the operator is the left hand side
JavaNode lhs = assignment.getParent().getChild(assignment.getIndexInParent() - 1);

ASTName name = lhs.getFirstDescendantOfType(ASTName.class);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure if this is important, but this descendants check could misinterpret array[...] = ..., because it will fetch the name of the array.

Might be worth figuring out a test case

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I've tested it a bit and I don't think, that's relevant: If the variable we search here would actually be an array, we wouldn't search it in the first place, because then it is not autocloseable.

@adangel adangel merged commit ff0e3cc into pmd:master Mar 27, 2021
@adangel adangel deleted the issue-2977-close-resource-reassignment branch March 27, 2021 15:30
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.

[java] CloseResource: false positive with reassignment detection

2 participants