Skip to content

[java] InsufficientStringBufferDeclaration fix FP (#1438)#3277

Merged
oowekyala merged 5 commits into
pmd:masterfrom
adangel:issue-1438-insufficientstringbufferdeclaration
May 28, 2021
Merged

[java] InsufficientStringBufferDeclaration fix FP (#1438)#3277
oowekyala merged 5 commits into
pmd:masterfrom
adangel:issue-1438-insufficientstringbufferdeclaration

Conversation

@adangel

@adangel adangel commented May 14, 2021

Copy link
Copy Markdown
Member

Describe the PR

Fixes false positive for initial calculated StringBuilder size.
If possible, the literals are calculated now. If there are method
calls or variables access, then no attempt is made to calculate
the initial size.

Also calls to ensureCapacity are recognized now in addition to setLength.

Related issues

Ready?

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

Fixes false positive for initial calculated StringBuilder size.
If possible, the literals are calculated. If there are method
calls or variables access, then no attempt is made to calculate
the initial size.

Also recognized now ensureCapacity in addition to setLength.

Fixes pmd#1438
@adangel adangel added this to the 6.35.0 milestone May 14, 2021
@ghost

ghost commented May 14, 2021

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

Generated by 🚫 Danger

@adangel adangel marked this pull request as draft May 14, 2021 15:06
@adangel

adangel commented May 14, 2021

Copy link
Copy Markdown
Member Author

I've put that one to draft, in order to fix some more problems I discovered when porting the rule to pmd7.

adangel added 3 commits May 14, 2021 20:31
- correct handling of appending a long value
- correct parsing when reassigning string builder and chaining
  append calls
- correct capacity calculation when initialized with a string
  constant
issue-1438-insufficientstringbufferdeclaration
When StringBuilder is used as a formal parameter, we
can't know the capacity.
@adangel adangel marked this pull request as ready for review May 14, 2021 20:56
@adangel

adangel commented May 14, 2021

Copy link
Copy Markdown
Member Author

Ok, so that is now ready. The two new violations are valid, and the one removed violation is also ok (we can't really know the capacity in that case).
I'll update #3278 for pmd7 in the next days, but as far as I see, all the tests work with the rewritten rule in pmd7.

@oowekyala oowekyala self-assigned this May 25, 2021
issue-1438-insufficientstringbufferdeclaration

@oowekyala oowekyala left a comment

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.

This looks fine, thanks

oowekyala added a commit that referenced this pull request May 28, 2021
@oowekyala oowekyala merged commit 836f77e into pmd:master May 28, 2021
@adangel adangel deleted the issue-1438-insufficientstringbufferdeclaration branch June 10, 2021 13:07
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] InsufficientStringBufferDeclaration false positive for initial calculated StringBuilder size

2 participants