Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EmptyLineSeparator: Inability to use xpath for violation on empty line #8179

Closed
shashwatj07 opened this issue Apr 24, 2020 · 7 comments · Fixed by #9539
Closed

EmptyLineSeparator: Inability to use xpath for violation on empty line #8179

shashwatj07 opened this issue Apr 24, 2020 · 7 comments · Fixed by #9539
Milestone

Comments

@shashwatj07
Copy link
Contributor

shashwatj07 commented Apr 24, 2020

Split from #7956


There is no AST node associated an empty line for which the violations are logged. This results the inability of such violations to be xpath suppressed. We may need to change the violation a bit to make it compliant with xpath.

C:\Users\Shashwat\Documents>cat config.xml
<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
          "-//Puppy Crawl//DTD Check Configuration 1.3//EN"
          "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
  <module name="TreeWalker">
        <module name="EmptyLineSeparator">
        <property name="allowMultipleEmptyLinesInsideClassMembers" value="false"/>
        <property name="allowMultipleEmptyLines" value="false"/>
        </module>
  </module>
</module>
C:\Users\Shashwat\Documents>cat Test.java
class A {



    void foo() { // line 5



    } // line 9
}
C:\Users\Shashwat\Documents>java -jar checkstyle-8.31-all.jar -c config.xml Test.java
Starting audit...
[ERROR] C:\Users\Shashwat\Documents\Test.java:5: 'METHOD_DEF' has more than 1 empty lines before. [EmptyLineSeparator]
[ERROR] C:\Users\Shashwat\Documents\Test.java:7: There is more than 1 empty line one after another. [EmptyLineSeparator]
[ERROR] C:\Users\Shashwat\Documents\Test.java:8: There is more than 1 empty line one after another. [EmptyLineSeparator]
Audit done.
Checkstyle ends with 3 errors.

In the above example it is clear that when we have to log violation for a similar case, outside the class members, we do it in a single go.
Test.java:5: 'METHOD_DEF' has more than 1 empty lines before. [EmptyLineSeparator]
This can be extended to solve this issue as well, where instead of logging for every empty line, we can log violation for the above example as follows:
Test.java:9: There is more than 1 empty line above. [EmptyLineSeparator]
However, the node to be logged is a bit uncertain. This needs to be dicussed.

@romani romani changed the title Inability to use xpath for violation on empty line EmptyLineSeparator: Inability to use xpath for violation on empty line Apr 25, 2020
@romani romani added the approved label Jun 5, 2020
@checkstyle checkstyle deleted a comment from shashwatj07 Jun 5, 2020
@romani
Copy link
Member

romani commented Jun 5, 2020

it is not critical on what node to print violation as in java you always have some character after - end of class scope. It would be good to report on previous token always, as we always know it existed and we do NOT know what will be on next line, this will be more reliable approach I think.

@HuGanghui
Copy link
Contributor

@romani it' ok to move violation report on previous token, right? If yes, I will plan to solve this issue.

@romani
Copy link
Member

romani commented Jun 14, 2020

it' ok to move violation report on previous token, right?

Right. You are welcome to send PR

@AkMo3
Copy link
Contributor

AkMo3 commented Mar 14, 2021

I am on it.

AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 14, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 15, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 16, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 17, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 17, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 22, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 22, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 27, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 27, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 27, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 27, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 27, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 29, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 29, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 29, 2021
AkMo3 added a commit to AkMo3/checkstyle that referenced this issue Mar 29, 2021
@romani romani added the bug label Apr 1, 2021
@romani romani added this to the 8.42 milestone Apr 1, 2021
@romani
Copy link
Member

romani commented Apr 1, 2021

merged

@cowwoc
Copy link

cowwoc commented Aug 3, 2023

Shouldn't the warning be removed from the documentation? https://checkstyle.org/checks/whitespace/emptylineseparator.html#EmptyLineSeparator

@romani
Copy link
Member

romani commented Aug 3, 2023

@cowwoc , thanks a lot for heads up #13506

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants