Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[jdt-core-dev] Warning about fix for bug 59891: [formatter] improve lines wrapping in nested method calls


Hi,

If you're using the JDT/Core formatter, then this message may have some
interest for you otherwise, please, just ignore it.

We're targeting to fix bug 59891 for 3.6M7, work is in progress and we have
real interesting results in this area.

Here are some samples, based on the existing bugs and complementary tests,
which show the expected improvement:
(See attached file: examples.txt)

Note that the final fix should be released by the end of the week. Beyond
that the formatter behavior will change for method calls and that will
surely imply some update in the formatted code, typically  if the formatter
is automatically activated while saving a Java file...

Let us know asap if this forthcoming changes might cause troubles.

TIA

Cordialement/Regards,

Frédéric
*************
* Example 1 *
*************

Bug 59891 examples...

code:
----
public class X01 {
    void test() {
        foo(bar(1, 2, 3, 4), bar(5, 6, 7, 8));
    }
}

formatted with HEAD:
-------------------
public class X01 {
    void test() {
    }
}

fixed formatting:
----------------
public class X01 {
    void test() {
        foo(bar(1, 2, 3, 4),
                bar(5, 6, 7, 8));
    }
}


1b (variation):
===============

code:
----
public class X01b {
    void test() {
        foo(bar(1, 2, 3, 4, 5, 6, 7, 8, 9, 10), bar(11, 12, 13, 14, 15, 16, 17, 18, 19, 20));
    }
}

formatted with HEAD:
-------------------
public class X01b {
    void test() {
        foo(bar(1, 2, 3, 4, 5, 6, 7, 8,
                9, 10), bar(11, 12, 13,
                14, 15, 16, 17, 18, 19,
                20));
    }
}

fixed formatting:
----------------
public class X01b {
    void test() {
        foo(bar(1, 2, 3, 4, 5, 6, 7, 8,
                        9, 10),
                bar(11, 12, 13, 14, 15,
                        16, 17, 18, 19,
                        20));
    }
}


1c (another variation)
======================

code:
----
public class X01c {
    void test() {
        foo(bar(1, 2, 3, 4), bar(5, 6, 7, 8), bar(9, 10, 11, 12));
    }
}

formatted with HEAD:
-------------------
public class X01c {
    void test() {
        foo(bar(1, 2, 3, 4), bar(5, 6,
                7, 8), bar(9, 10, 11,
                12));
    }
}

fixed formatting:
----------------
public class X01c {
    void test() {
        foo(bar(1, 2, 3, 4),
                bar(5, 6, 7, 8),
                bar(9, 10, 11, 12));
    }
}


*************
* Example 2 *
*************

Bug 146175 example.

code:
----
public class X02 {

    public void foo() {
         SomeOtherClass.someMethodInInnerClass(
            instanceOfOtherClass.anotherMethod("Value of paramter 1"),
            instanceOfOtherClass.anotherMethod("Value of paramter 2"));
    }
}

formatted with HEAD:
-------------------
public class X02 {

    public void foo() {
        SomeOtherClass.someMethodInInnerClass(instanceOfOtherClass
                .anotherMethod("Value of paramter 1"), instanceOfOtherClass
                .anotherMethod("Value of paramter 2"));
    }
}


fixed formatting:
----------------
public class X02 {

    public void foo() {
        SomeOtherClass.someMethodInInnerClass(
                instanceOfOtherClass.anotherMethod("Value of paramter 1"),
                instanceOfOtherClass.anotherMethod("Value of paramter 2"));
    }
}



*************
* Example 3 *
*************

The behavior needs still to be improved in this case... As we got more lines and a larger length after formatting!
But the fix is not finalized yet...

code:
----
public class X03 {
    private void reportError(String name) throws ParseError {
        throw new ParseError(MessageFormat.format(AntDTDSchemaMessages.getString("NfmParser.Ambiguous"), new String[]{name})); //$NON-NLS-1$
    }
}


formatted with HEAD:
-------------------
public class X01 {
    private void reportError(String name) throws ParseError {
        throw new ParseError(MessageFormat.format(AntDTDSchemaMessages
                .getString("NfmParser.Ambiguous"), new String[] { name })); //$NON-NLS-1$
    }
}


fixed formatting:
----------------
public class X01 {
    private void reportError(String name) throws ParseError {
        throw new ParseError(
                MessageFormat
                        .format(AntDTDSchemaMessages
                                        .getString("NfmParser.Ambiguous"), new String[] { name })); //$NON-NLS-1$
    }
}



*************
* Example 4 *
*************

The formatter breaks method arguments first instead of message send.

code:
----
      Character c =
    new Character((char)Integer.parseInt(entity.substring(start), radix));

HEAD:
----
            Character c = new Character((char) Integer.parseInt(entity
                    .substring(start), radix));

fixed:
-----
            Character c = new Character((char) Integer.parseInt(
                    entity.substring(start), radix));



*************
* Example 5 *
*************

The formatter breaks cascading messages first instead of method arguments.
That makes cascading messages easier to read.

code:
----
                filename = VariablesPlugin.getDefault().getStringVariableManager().performStringSubstitution(filename);

HEAD:
----
                filename = VariablesPlugin.getDefault()
                        .getStringVariableManager().performStringSubstitution(
                                filename);
fixed:
-----
                filename = VariablesPlugin.getDefault()
                        .getStringVariableManager()
                        .performStringSubstitution(filename);


*************
* Example 6 *
*************

Better formatting of long method arguments.
Each argument is placed on a new line which makes the formatted message send easier to read.

code:
----
            prefs.setValue(IAntCoreConstants.PREFIX_TASK + customTasks[i].getTaskName(), customTasks[i].getClassName() + "," + customTasks[i].getLibraryEntry().getLabel()); //$NON-NLS-1$

HEAD:
----
            prefs.setValue(IAntCoreConstants.PREFIX_TYPE\n
                    + customTypes[i].getTypeName(), customTypes[i]\n
                    .getClassName()\n
                    + "," + customTypes[i].getLibraryEntry().getLabel()); //$NON-NLS-1$\n

fixed:
-----
            prefs.setValue(\n
                    IAntCoreConstants.PREFIX_TYPE\n
                            + customTypes[i].getTypeName(),\n
                    customTypes[i].getClassName()\n
                            + "," + customTypes[i].getLibraryEntry().getLabel()); //$NON-NLS-1$\n


*************
* Example 7 *
*************

The formatter does no longer break when there's nothing to do to reduce the line length.
In this sample, the non-nls tag prevent the formatter to break the line, hence it gives up sooner avoiding to add unnecessary broken lines.

code:
----
    MessageFormat.format(InternalCoreAntMessages.getString("AntRunner.Already_in_progess"), new String[]{buildFileLocation}); //$NON-NLS-1$    

HEAD:
----
        MessageFormat
                .format(
                        InternalCoreAntMessages
                                .getString("AntRunner.Already_in_progess"), new String[] { buildFileLocation }); //$NON-NLS-1$    

fixed:
-----
        MessageFormat
                .format(InternalCoreAntMessages.getString("AntRunner.Already_in_progess"), new String[] { buildFileLocation }); //$NON-NLS-1$    



*************
* Example 8 *
*************

Another sample of making the formatted message send easier to read when breaking the line between arguments first instead of inside a message send...

code:
----
    public void warning(Exception exception) {
        notifyProblemRequestor(exception, (AntElementNode)fStillOpenElements.pop(), XMLProblem.SEVERITY_WARNING);
    }

HEAD:
----
    public void warning(Exception exception) {
        notifyProblemRequestor(exception, (AntElementNode) fStillOpenElements
                .pop(), XMLProblem.SEVERITY_WARNING);
    }

fixed:
-----
    public void warning(Exception exception) {
        notifyProblemRequestor(exception,
                (AntElementNode) fStillOpenElements.pop(),
                XMLProblem.SEVERITY_WARNING);
    }



*************
* Example 9 *
*************

Here's a rare case where the formatter will produce more lines than current behavior.
However, that looks acceptable regarding the fact that it makes the cascading messages more obvious to read.
If that kind of change would not be acceptable, then disabling tags might be used around this message send...

code:
----
        AntUIPlugin.getDefault().getPreferenceStore().setValue(IAntUIPreferenceConstants.OUTLINE_LINK_WITH_EDITOR, isChecked());

HEAD:
----
        AntUIPlugin.getDefault().getPreferenceStore()\n
                .setValue(IAntUIPreferenceConstants.OUTLINE_LINK_WITH_EDITOR,\n
                        isChecked());\n
fixed:
-----
        AntUIPlugin\n
                .getDefault()\n
                .getPreferenceStore()\n
                .setValue(IAntUIPreferenceConstants.OUTLINE_LINK_WITH_EDITOR,\n
                        isChecked());\n

Back to the top