pixee/codemodder-java

Improve Style of Code Generated by DefineConstantForLiteral Codemod

Closed this issue · 2 comments

The DefineConstantForLiteral generates code with two style issues:

  • the constant names have no _ characters to delineate words
  • the constants are not indented within the enclosing class as one would expect.

Here's an example of a constant added by this codemod:

class Foo {
private static final String APICERTIFICATE = "APICertificate";

Expected behavior:

class Foo {
  private static final String API_CERTIFICATE = "APICertificate";

The new field not being indented is difficult for us to fix, because JavaParser handles this in private APIs which we can't influence in any way. However, we could "work around" the issue by inserting the statement at the end, rather than the beginning.

JavaParser's LexicalPreservingPrinter's strategy is to generally "ride the indentation of whatever came before", and when we insert new things first (e.g., a new statement in a method), then there is no "history" to pull from and it defaults to no indentation.

If we insert last, there is a much better chance that there is existing history to pull from, and I have confirmed that in our examples, this works.

Would you rather have the existing indentation, and inject the field first, or have a much better chance of correct indentation, and inject the field last?

Definitely would prefer to inject the field at the bottom of the file with the correct indentation.

Old-school Sun Java Code Style would have us define constants at the top of the file, but I feel like the Java community has migrated away from that dogma. There isn't a strong community consensus on how to order class members. Google Java Style Guide (and its automated formatter Google Java Format) don't have an opinion on class member order. So I don't think these constant must go at the top.

Furthermore, I think these constants should go at the bottom, because they're private. I'm pretty sure that Sonar is counting duplicate literal within a file vs across files, so these constants are always private. In my experience, Java developers typically favor putting private static final constants at the bottom of a file, because these are not necessarily the first thing you want to see when reading code. For example, your logger is never important enough to occupy that top of the file real estate. Same is true for some constants that exist to prevent duplicate literals.