Community
Participate
Working Groups
Using latest, when */ is reached, it is converted to */ and this is closing the block comment. Therefore the remaining part of the comment is considered to be outside the comment and the code doesn't compile anymore. A test case is the org.eclipse.jdt.core.dom.Statement class.
The bug resides in the HTML2JavaReader. It converts "/*" into "/ *" and "*/" into " */". This is wrong. Inside a string literal, these characters should not be converted. And outside, I would suggest to convert them to a unicode. It should also be remembered that they were converted to convert them back once the code is formatted. The code formatter should never switch characters (like *) for their corresponding ascii form (*).
Can you clarify: - afaik HTML2JavaReader has nothing to do with code formatter - afaik HTML2JavaReader has nothing to do with compiler Please clarify what exactly the bug is that you report.
Let's take an example. Try to format the following javadoc comment inside a cu. /** * <code> * <pre> * setLeadingComment("/* traditional comment */"); // correct * setLeadingComment("missing comment delimiters"); // wrong * setLeadingComment("/* unterminated traditional comment "); // wrong * setLeadingComment("/* broken\n traditional comment */"); // correct * setLeadingComment("// end-of-line comment\n"); // correct * setLeadingComment("// end-of-line comment without line terminator"); // correct * setLeadingComment("// broken\n end-of-line comment\n"); // wrong * </pre> * </code> */ When the code inside the <pre></pre> is formatted, the * is converted to '*'. So when the */ is reached, it is converted to */ and once it is replaced back into the javadoc comment, the comment ends there instead of its actual end. The resulting code doesn't compile anymore. Try to format the class org.eclipse.jdt.core.dom.Statement. The code is extracted from there. Also the * is gone in the formatted string. The code formatter should not replace characters.
I guess your assessment of the bug was made too quick or during a coffee shortage ;-) HTML2JavaReader does not exits. I'd say the cause for this is in HTMLEntity2JavaReader which resides in JDT Core.
yes, the bug is in HTMLEntity2JavaReader. This is part of the code we inherited from JDT/Text.
*** Bug 173130 has been marked as a duplicate of this bug. ***
The only way I can see to fix this is to add an extra scanning at the end of the code snippet formatting to replace all ending block/javadoc comment */ back with */.
Created attachment 74987 [details] Proposed fix
Created attachment 74988 [details] Regression test
Jérôme, I would candidate it for 3.3.1. With this, the JDT/Core source could be successfully formatted. Released for 3.4M1. Regression tests added in org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636
The fix does not consider that the character after a star may need to be substituted in its own right and thus fails in the two cases shown in the comment below: /** * <pre> * /* Comment ending in multiple stars **/ * /* Entity-needing character after a star *< */ * </pre> */ One easy solution is to add a loop to handle a sequence of stars. When a non-star is encountered, is stored in c, the loop ends, and computeSubstitution proceeds as normal but prefixes its return value with the appropriate number of stars. A more elegant solution is to add a "peeking" or "unreading" capability to SubstitutionTextReader so that, when computeSubstitution finds a * followed by a character other than /, it can leave the SubstitutionTextReader in a state so that the following character will be read again on the next call to nextChar. Such a capability could be implemented in SubstitutionTextReader or could use the mark/reset of the underlying reader.
I will be happy to write the patch if you tell me which solution you prefer.
I would take the easy one for now. If we need something more elegant in the future, we would change it. If you can provide a patch today, feel free to attach a patch. Otherwise, I'll write one. I added your test case as a disabled test in org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#_test109636_2. Could you please confirm if you provide a patch? Thanks.
Created attachment 75133 [details] Second fix This patch does the following: - Fixes Java2HTMLEntityReader to handle the new cases - Fixes a bug in SubstitutionTextReader's handling of EOF that was revealed by the first change - Enables test109636_2 - Fixes a mistake in the expected output of test109636_2 Changing the convention to represent */ as */ instead of */ would make the code slightly nicer because it wouldn't have to overwrite the star it has already processed. It also might be nice to use * or / for consistency with @ . I didn't do either of these in this patch.
Released for 3.4M1. Added new regression tests org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636_2 org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636_3
Thank you for the patch.
Verified for 3.4M1 using build I20070806-1800.
(In reply to comment #10) > Jérôme, > > I would candidate it for 3.3.1. With this, the JDT/Core source could be > successfully formatted. +1 for backporting to 3.3.1
Reopen for 3.3.1.
Released for 3.3.1. Regression tests added in org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636 org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636_2 org.eclipse.jdt.core.tests.formatter.comment.JavaDocTestCase#test109636_3
Verified for 3.3.1 using build M20070831-2000
