Issue #124: Implement recipe for NewlineAtEndOfFile #125
Conversation
Anmol202005
force-pushed
the
eol
branch
15 times, most recently
from
8b311bc to
e5cc5a3
Compare
|
Finally got CI GREEN. @rdiachenko pls review :) |
|
@romani @rdiachenko ping. |
src/main/java/org/checkstyle/autofix/recipe/NewlineAtEndOfFile.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkstyle/autofix/recipe/NewlineAtEndOfFile.java
Outdated
Show resolved
Hide resolved
src/main/java/org/checkstyle/autofix/recipe/NewlineAtEndOfFile.java
Outdated
Show resolved
Hide resolved
| final Space eof = sourceFile.getEof(); | ||
| final String lastWhitespace = eof.getLastWhitespace(); | ||
|
|
||
| if (!lineEnding.equals(lastWhitespace)) { |
There was a problem hiding this comment.
we do not trust checkstyle violation ? or it is save check in case of second execution.
There was a problem hiding this comment.
this is a check for second execution.
src/main/java/org/checkstyle/autofix/recipe/NewlineAtEndOfFile.java
Outdated
Show resolved
Hide resolved
| final String expectedLineEnding = determineLineEnding(sourceFile); | ||
|
|
||
| final Space eof = sourceFile.getEof(); | ||
| final String lastWhitespace = eof.getLastWhitespace(); |
There was a problem hiding this comment.
on windonws machine new line is \r\n, what value lastWhitespace will take in this case: \n or \r\n? Please also add a test case to cover this
There was a problem hiding this comment.
in real case scenario it will take \r\n. for the test cases and CI we have used git attributes to define line endings.
Also added a new test case :)
| final String lastWhitespace = eof.getLastWhitespace(); | ||
|
|
||
| if (!expectedLineEnding.equals(lastWhitespace)) { | ||
| final List<Comment> comments = eof.getComments(); |
There was a problem hiding this comment.
why do we need this special analysis with comments? Can you explore simpler solution? Idially, given end of fille we should check if the file doesn't end with expectedLineEnding then simply append it.
There was a problem hiding this comment.
@rdiachenko this is the AST structure requirement. that's where the whitespace is stored when comments exist !!
romani
left a comment
romani
left a comment
There was a problem hiding this comment.
Do we need explicitly add this recipe to checkstyle main repo config to be executed by CI ?
thansk a lot for your work.
|
3 participants
Fixes: #124
Implemented recipe for NewlineAtEndOfFile: https://checkstyle.sourceforge.io/checks/misc/newlineatendoffile.html