Formatter settings, and multiple additions

[glelouet]

issues addressed

#156
#164
#165
#166

Features added

Local Variable list

The hierarchy of the JVar was changed, so creating a block variable now returns a JBlockVar that allows to add additional vars at the same time.

JBlockVar i = myBlock.declare(0, jcm.INT, "i");
JSameVar j = i.andVar("j");

generates

int i, j;

this is also possible in a for loop, and in an instance field.

For Loop init fix

a "for' loop in java mut be initialized by one of :

• nothing
• a local variable alongside its children variables
• a list of statements

The current implementation allows to mix both. I changed that to have only one of, throwing exception if both are used.

Tests show how this works.

Catch multiple types

Once the vars hierarchy was changed, new classes could be added, including a JCatchVar for a catch clause, that allows to add additional types to the var.

oracle doc for actual lub type of the catch clause :
https://docs.oracle.com/javase/specs/jls/se25/html/jls-14.html#jls-14.20-510

The structure of a catch clause has been changed :
Before, it contained a variable that could be null, the type of the exception ; calling the variable with a name could create it or throw an exception ; and during export the variable was created if missing. Now it only contains a variable, never null, and allows to change its name at will, or add new types to the variable list. This avoids printing a model having side effects, also it avoids throwing exception for no reason.

I removed the check for a JVar::setType to not receive null value, which was overdue anyhow.

Once the var of a catch block receives more than one type, its own returned type is null, for inspection purpose, since it can't link back to a JCM to reference Exception.

Tests show how the settings impact the multicatch formatting.

Array init

Allows to generate

int[] i= {0} ;

as specified in https://docs.oracle.com/javase/specs/jls/se25/html/jls-8.html#jls-VariableInitializer

For this, a new interface IVariableInitializer super of IJExpression is added, with single direct class JArrayInit .

This interface is used for variable declarations. A JExpr.arrayInit helper method directly calls the constructor.

In the concrete class, any null element is replaced by the JExpr._null() for ease . This way you can

myBlock.decl(STRINGCLASS.array(), "s", Jexpr.arrayInit(null, null);

which produces

String[] s = {null, null };

JExpr also has helper function for char, int, double.

Both the result and the format are tested.

Formatter settings

This first step is to set settings at Jformatter level.

The class  com.helger.jcodemodel.writer.FormatterSetings will hold data for formatter settings.
It has a FormatterSettings configure(Consumer<FormatterSettings>) method that returns this after accepting this. This method allows to chain several configuration, eg

var fs = new FormatterSettings()
  .configure(c->c.indent.useSpaces(3))
  .configure(c->c.wrap.disabled=true)
;

All the child settings fields (like indent, or wrap) are final public to allow fast access/change.

Then the basic rules for the settings are

1. keep same default behavior when possible. Default indent is 4 spaces, so use 4 spaces by default
2. add setter methods and those for often-performed. Typically a boolean field with default false will have a method to set it to true : wrap.disabled => wrap.setDisabled(value) + wrap.disabled()
3. most changing methods return the element being changed, for chaining.

The JCMWriter received a new field of this class. When creating a JFormatter, it transmits the settinbgs instead of only the indent String. The JFormatter field is final , but not the jcmwriter's.

General Indentation options

settings.indent has 2 fields :

• String string is the indent String. Can be set using useSpaces(n) which defaults to 4 spaces, or useTabs(n) which defaults to 1 tab. The default value is JCMWriter.DEFAULT_INDENT_STRING to not change a thing.
• int tabSize tells us how many chars is a tab column at most. This is useful to find the size of a line that contains tabs

Wrapping settings

Those allow to choose how to wrap several parts of code declaration.

Several classes are shared among those options, this allows to define the wrapping among less classes. A genericPrints method was added in the JFormatter, that takes a collection of Objects as well as the separator and list-wrapping options.

The existing strategy is to wrap all params after first if >3 ; not wrap otherwise.
Common wrap strategies consider several things, including the need for wrap (when the line gets over a specific size), the wrapping of first element, the indentation of the wrapped lines (existing is 1)

I try to keep it simple so I selected a few wrap methods :

• always wrap, including first element
• never wrap, everything on the same line
• binary wrap : if the line is too big, wrap ALL elements, otherwise wrap none.
• required : only wrap each specific element that would make the line too big
• past3 : this is legacy method, wrap after first if more than 3 elements.

To detect the need for wrapping, I added a wrap.lineCharacters = 80 option.

Since there may be bugs, the full feature is locked behind a wrap.disabled=false option. When set to true, the generation of code should literally use legacy code.

Other than that, the list wrapping options can be specified the indentation on wrapping, as well as the requirement to wrap after or before the separator.

Then there was an issue : how to detect if the line is too big ? I had to use a "try something" approach

Temporary context stack

I added a addContextLayer method to the JFormatter to add a new context on top of its internal stack. A context contains a StringBuilder, as well as parameters inherited from the previous one.
As long as a context is present, writing to the formatter actually writes in the context. The context then has two methods rollback() and commit() , which set an internal persist flag then close it. Closing a context pops it off the context layers of the formatter, then write its content if internal persist flag set to true.
The persist flag can also be set manually, and the buffer implements autocloseable, so one can

try(var buffer = formatter.buffer().persistOnclose())
{
  /// add things to the context.
}
/// the buffer is auto closed when exiting the try, so persisted - unless manually rollbacked in the try block

Note that if a buffer is created on top of another one, the persist (or commit) will actually write in that underlying buffer. This is because, we can try things inside trying things ^^

Now with that done, the wrapping of a REQUIRED can simply try to add the next param, and if the line is too big it rollbacks, add a newline, then add the param again :) .

Same for BINARY, this tries to add all the params without newline, then if the line is too big it rollbacks, and add with ALWAYS instead.

Of course there is an added function to get the current line, which is present in the buffer and the JFormatter, just like lastChar is also added in the Buffer ; to make things simpler a printDown method is added in the JFormatter to select where to write, and a utlity method that counts the size of a line when using tabs with given column size .

Formatting tests

Each formatting setting has its own package. for example jcodemodel.test.format.method for the method formatting settings.

In those packages, several classes are generated with the same base generation, just different name and settings.. This means that when a change happens on a setting behavior, it's easy to track them. For example, I changed an internal method and forgot to add a space after the comma, and it showed after a mvn install in the git status.

Those can't fail the build, they are here for tracking, as well as deciding if the result is fine in the initial dev phases. I found several bugs with them, so they may look useless in terms of tests but actually help.

[glelouet]

I'm adding a small change to the JFormatter : the ability to buffer the writes in an intermediate StringBuilder.

This buffer can be commited, rollbacked, and if used in a try-with-resource is automatically discarded when exiting the block (unless commited before).

This allows to have optimist approach that goes for a simple thing, then commit if okay, rollback and redo it in another way otherwise.

[glelouet]

I fixed a few bugs, added "tests" as those are more visual than actually checking things.

The result is the difference on the various produced classes (all with same methods, only class name and options change) https://github.com/phax/jcodemodel/pull/163/changes#diff-3c04635b2388974428723133096c9912cf755e4183ddd4d643cb11b478b17cc2

The test classes ( @TestJCM ) now injects a FormatterOptions params, to allow to manipulate it for tests.

@phax it's quite the mess so look at the result before you look at the code :D

[glelouet]

Added ability to wrap the block of a method's body, starting with bracket. Allows to have bracket on new line with

		options.wrap.method.bracket
				.condition(EWrapWordStrategy.ALWAYS)
				.indent(0);

[glelouet]

tried to implement the args part (so when invoking a method) but that would require too much copy paste. Basically needs to copypaste the whole code.
So I was pondering whether to add dedicated code in the JFormater.generate(List). Turns out, jvars have a state() that is fixed for block variables , not correct for method params.
So I think we should have a dedicated JVarParam for them instead. But then that's maybe too muc for that PR.
so I kept the params, but not effect. So far.

[glelouet]

I think there was a bit of issues with vars.
Mainly, several time instead of formatter.var(jvar) they are hardcoded in very specific ways.

So I changed it to have a new hierarchy for JVAR

• JVar is the root part, with everything possible for a var. Later will be set to abstract.
• JFieldVar is kept as is for legacy reason
• new package vars/ to contains the vars implementations :
• JBLockVar extends JVar is the old JVar : a variable that is declared in a block. Literally no change
• JArgVar for vars declared in the sig of a method. They can't be assigned value, can only be final, can't have null type.
• JVarArgVar is the vararg last param. Sorry if the name is bad, feel free to change it. The difference is that the type is followed by "..." when binding. So the previous class has a method bindType() called in bind() , overriden in JVarArgVar.
• JCatchVar is a var declared for a catch block. They can have multiple types, no init, can only be final, require a type.
• JForEachVar is a var declared in a for( int a : mycollection) it requires an init, can only be final, do not require a type. Their binding (for var) uses ":" instead of "=" for assignment.
• The JCatch and JForEach classes therefore only use their variables' declare instead of rewriting the full block.

I did not touch jlambdaparam because I'm lazy.

An improvement is that the JCatchBlock now can have several types for a single catch.

[glelouet]

I was tweaking things (literally making the var() at the formatter level, using a genetic method so the wrapping can be reused for other things like the catch multitype list) and realized there was no way to set several vars with same types and modifiers at once.

This is now possible, you can write fields, block, and for vars as

// one of the three
JfieldVar myVar  = myClass.field(Jmod.public | Jmod.static, jcm.INT, "i");
JBlockVar myVar = mybBlock.var(JMod.final, jcm.INT, "i");
JBlockVar myVar = myJForLoop.init(JMod.NONE, jcm.INT, "i", JExpr.lit(0));

JSameVar mySameVar1 = myVar.AndVar("j"),
  mySameVar2 = myVar.andVar("k", JExpr.lit(50))
;

I did not test the "for" though, I assume it's bugged since the other also had bugs.
especially since the spec for a FOR is EITHER a var list with the main type at the first var ; OR a statement list ; and the present code allow to merge both ?
https://docs.oracle.com/javase/specs/jls/se8/html/jls-14.html#jls-14.14.1

[glelouet]

Now the jforloop only allows to EITHER init a variable, which be later based upon to add sub variables, OR init expressions.

Again, the local variable part allows to add more dimensions to the base type, eg from an int[][] i, adding 2 dimensions can create int[][][][] j in the same init (see example)

What's more a dedicated option is present to wrap the init

source :
https://github.com/glelouet/jcodemodel/blob/formatterOptions/jcodemodeltests/src/main/java/com/helger/jcodemodel/tests/format/forloop/ForLoopTestGen.java
results :
https://github.com/glelouet/jcodemodel/blob/formatterOptions/jcodemodeltests/src/generated/javatest/com/helger/jcodemodel/tests/format/forloop/WrapInitRequired.java

several result because I change the formatter options between each.

[glelouet]

@phax
I think I'm done :)

Next step : AST parser to create JCM
Then plugin to load with AST parser .
Then more Formatter configuration.
Then we can use it to reformat all the code I submit with many changes which are just in format, like one-line if without brackets.

[phax]

Alrighty, now this is a big one :-)

[glelouet]

I was thinking the miniscle dabble in the other PR did not motivate you to start incorporating them, so I started making Mother Of All Pr (MOAP) to clean it up.

[glelouet]

also I think I should do more unit testing, but I'm not sure how to test some so nope. Already did automatic code generation in tests to detect changes int he commit, so a review should catch out any change.

[phax]

I found a few things, but the PR is so huge, that I surely missed something ;-)

[phax]

Any reason this is not private?

[glelouet]

yes, to have it testable in unit tests. If not tested then I forgot , or changed the code several time until it did not need tests but may still need later.

[glelouet]

There was more specific things before, like converting Double or Integer using Jexpr.lit, but I removed it.
(Just because the result is one line, does not mean the code I wrote and removed was not 50 lines :D )

[phax]

If this is converted to a List when generating, we could directly store it as one???

[glelouet]

maybe. convertElements could return a list instead. Not sure if worth.
Done anyhow.

[phax]

I would prefer isFinal over final_ for readability - thx

[glelouet]

done

[phax]

Please add a check if (dum < 0) throw new IllegalArgumentException ("...");

[glelouet]

hu, no ? If < 0 nothing happens, return type.

[phax]

No need to init with null

[glelouet]

is it a problem though ?

done and renamed to m_aInitVar

[phax]

Never throw RuntimeException - use IllegalStateException instead

[glelouet]

I 👍 the requests I did in commit, 👎 those that I don't want to have incorporated, the rest I don't know.