bug: Comments are probably not parsed well on Mac

[pvojtechovsky]

JDTCommentBuilder#cleanComment method splits lines by Linux \n

String[] lines = comment.split("\n");

and then cleans up remaining MS Windows \r. I guess that this algorithm will not work on Mac, which uses \r as EOL.

The handling of EOL in spoon seems to be not consistent. May be the best would be to

1. preprocess all parsed java source file and to normalize EOL from \r\n and \r to Linux \n
2. then all internal algorithms in spoon can be easier and may expect that EOL is always \n
3. to configure what EOL is used when pretty printing

WDYT?

I am not not going to develop anything here. I just noted that it should be handled somehow during #1455, which is hard for me to test, because I am not able to say what behavior is correct concerning EOL... and Spoon.

Are there already some rules how EOLs should be handled by Spoon?

[surli]

May be the best would be to

1. preprocess all parsed java source file and to normalize EOL from \r\n and \r to Linux \n
2. then all internal algorithms in spoon can be easier and may expect that EOL is always \n
3. to configure what EOL is used when pretty printing

I agree with you but I wonder if the step 1 is not already processed by JDT?

[pvojtechovsky]

1. preprocess all parsed java source file and to normalize EOL from \r\n and \r to Linux \n

It is probably not a good idea, because that will influence source position offsets and will make the sniper mode and similar algorithms more difficult.

After deeper thinking about this EOL problem it looks like Spoon touches this problem only when parsing comments. All other Java nodes are EOL independent - JDT compiler handles EOL well and the Java AST nodes do not care about EOLs in origin files.

So it looks like refactoring of JDTCommentBuilder#cleanComment, which assures that differnet EOLs are handled well will be simple and sufficient solution.

There remains one question:
@monperrus, @surli What line separator should be used in String returned by CtComment#getContent()?
A) normalized = always "\n"
B) OS specific System.getProperty("line.separator")
C) it should keep the line separator from origin source code = developer must expect arbitrary combination of \n, \r, \r\n.
D) make it configurable... but where?

I vote for A)

Note, that I am not speaking about Pretty printing which will of course use configured EOL, which may be different too.

[pvojtechovsky]

I have checked current implementation and CtComment#getContent() already separates lines by constant "\n" - independent on OS.

So the remaining problem is to fix bug in comments handling on Mac OS, which are using "\r"

[surli]

So the remaining problem is to fix bug in comments handling on Mac OS, which are using "\r"

Actually "\r" was used on older version of MacOS (9 and below): I'm using Mac and System.getProperty("line.separator") returns "\n".

I tried to create a new test in #1679 and it seems to return the documentation correctly.