Skip to content

[java] UseStringBufferForStringAppends: fix false negative with fields#2600

Merged
oowekyala merged 5 commits into
pmd:masterfrom
adangel:perf-UseStringBufferForStringAppends
Jun 21, 2020
Merged

[java] UseStringBufferForStringAppends: fix false negative with fields#2600
oowekyala merged 5 commits into
pmd:masterfrom
adangel:perf-UseStringBufferForStringAppends

Conversation

@adangel

@adangel adangel commented Jun 18, 2020

Copy link
Copy Markdown
Member

Describe the PR

This PR fixes a false negative and a false positive of rule UseStringBufferForStringAppends:

  • false negative with fields:
import java.util.*;

public class ConcatInLoop {

    private String logStatement = "";

    public void bad() {
        List<String> values = Arrays.asList("tic", "tac", "toe");
        for (String val : values) {
            logStatement = logStatement + val + ", "; // bad
        }
    }

    public void good() {
        List<String> values = Arrays.asList("tic", "tac", "toe");
        StringBuilder sb = new StringBuilder(logStatement);
        for (String val : values) {
            sb.append(val).append(", ");
        }
        logStatement = sb.toString();
    }
}
  • fix false positive when concatenation is local to each
    for-loop iteration.
import java.util.ArrayList;
import java.util.List;

public class Foo {
    private List<String> fileExtensions = new ArrayList();
    public void good(List<String> fileExtensions) {
        for (String fileExtension : fileExtensions) {
                if (fileExtension.charAt(0) != '.') {
                    fileExtension = "." + fileExtension; // StringBuilder would just add noise here
                }
                this.fileExtensions.add(fileExtension);
            }
    }
}

Test cases originally from #1932

Author: jborgers jborgers@jpinpoint.com

Related issues

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)

And also fix false positive when concatenation is local to each
for-loop iteration.

Test cases originally from pmd#1932

Author: jborgers <jborgers@jpinpoint.com>
@adangel adangel added a:false-positive PMD flags a piece of code that is not problematic a:false-negative PMD doesn't flag a problematic piece of code labels Jun 18, 2020
@adangel adangel added this to the 6.25.0 milestone Jun 18, 2020
@ghost

ghost commented Jun 18, 2020

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

Generated by 🚫 Danger

@adangel adangel marked this pull request as ready for review June 20, 2020 10:14

@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.

Nice, thanks!

@oowekyala oowekyala merged commit ee8d77b into pmd:master Jun 21, 2020
@adangel adangel deleted the perf-UseStringBufferForStringAppends branch June 22, 2020 17:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:false-negative PMD doesn't flag a problematic piece of code a:false-positive PMD flags a piece of code that is not problematic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants