Issue #18265: Added allowMultipleEmptyLinesInsideClassMembers property inside EmptyLineSeparator check in google_checks.xml #18311

Praveen7294 · 2025-12-15T22:21:50Z

From: https://google.github.io/styleguide/javaguide.html#s4.6.1-vertical-whitespace

Added a allowMultipleEmptyLinesInsideClassMembers property inside EmptyLineSeparator check.
Added test cases testCorrectEmptyLineSeparatorInsideClassMembers() and testIncorrectEmptyLineSeparatorInsideClassMembers() in VerticalWhitespaceTest.java.
Added input files InputCorrectEmptyLineInsideClassMembers.java and InputIncorrectEmptyLineInsideClassMembers.java to rule461verticalwhitespace directory.
From this PR Issue #17681: Fix multiple blank line separation #17950 , InputEmptyLineSeparator3.java and InputFormattedEmptyLineSeparator3.java were added to the rule461verticalwhitespace directory but but they were not covered by any test cases. Therefore, added test cases testEmptyLineSeparatorBetweenClassMembers() and testFormattedEmptyLineSeparatorBetweenClassMembers() to VerticalWhitespaceTest.java.

Diff Regression config: https://gist.githubusercontent.com/Praveen7294/605a30d7c12d5fcc633316127e4db0be/raw/c0f9602b094f75a806a296569b5f70a6c3d5e471/base-config-emptyline.xml
Diff Regression patch config: https://gist.githubusercontent.com/Praveen7294/0c2bffd20db9d6c9696c2fce03de4407/raw/f89ddb1333c8bfe4b26e62ba9f3e4327567bfc4e/patch-config-emptyline.xml
Diff Regression projects: https://gist.githubusercontent.com/Praveen7294/a1626a4272b4be9d7a0dc8a9fbae5ffa/raw/22fada1af5a2880572d3e91848996d50427c1616/list-of-projects.properties

Praveen7294 · 2025-12-16T01:42:08Z

Github, generate report

github-actions · 2025-12-16T02:38:35Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/aca7eed_2025023816/reports/diff/index.html

Praveen7294 · 2025-12-17T17:48:01Z

@romani @Zopsss please review

Zopsss

minor:

Zopsss · 2025-12-18T04:00:23Z

...e/checkstyle/test/chapter4formatting/rule461verticalwhitespace/InputEmptyLineSeparator3.java

-package com.google.checkstyle.test.chapter4formatting.rule461verticalwhitespace; //violation ''package' has more than 1 empty lines before'
+package com.google.checkstyle.test.chapter4formatting.rule461verticalwhitespace;
+
+// violation 2 lines above ''package' has more than 1 empty lines before.'


revert this, no changes required.

Zopsss · 2025-12-18T04:02:27Z

...yle/test/chapter4formatting/rule461verticalwhitespace/InputFormattedEmptyLineSeparator3.java

 package com.google.checkstyle.test.chapter4formatting.rule461verticalwhitespace;

-// violation above ''package' has more than 1 empty lines before.'
-
 public class InputFormattedEmptyLineSeparator3 {
-  // violation above ''CLASS_DEF' has more than 1 empty lines before.'
+  // violation above 'Missing a Javadoc comment.'


this is a formatted file, then why did it contained "more than 1 empty lines before" violations? The formatter should have removed the empty lines, am I missing something here? @Praveen7294

am I missing something here?

No, the google-java-format workflow only formats non-excluded files. For excluded files, it locates the original file and its corresponding formatted file and compares the size difference by counting characters after removing whitespace and comments.

The contributor added this file, but it was not formatted. The contributor duplicated the original file and only changed the file name to InputFormatted* to pass the workflow. I have already mentioned that PR in this PR description.

Praveen7294 · 2025-12-23T18:15:00Z

@Zopsss please review

Zopsss · 2025-12-25T06:06:09Z

Github, generate report

github-actions · 2025-12-25T06:58:37Z

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/index.html

Zopsss · 2025-12-25T07:34:34Z

...e/checkstyle/test/chapter4formatting/rule461verticalwhitespace/InputEmptyLineSeparator3.java

 public class InputEmptyLineSeparator3 {
-// violation above ''CLASS_DEF' has more than 1 empty lines before.'
+  // violation above 'Missing a Javadoc comment.'
+  // violation 2 lines above ''CLASS_DEF' has more than 1 empty lines before.'


it should be:

// 2 violations above:

Zopsss · 2025-12-25T07:37:18Z

...st/chapter4formatting/rule461verticalwhitespace/InputCorrectEmptyLineInsideClassMembers.java

+ * This file contains incorrect vertical whitespace inside class
+ * members and also between class members.
+ */
+public class InputCorrectEmptyLineInsideClassMembers {


please add an example of constructor, enum interface, etc. The supported tokens are mentioned here: https://checkstyle.org/checks/whitespace/emptylineseparator.html#Example5-config

Added examples of constructors, enums, and interfaces. However, this check does not detect multiple empty lines inside enums, so I created an issue regarding this #18437

Zopsss · 2025-12-25T07:39:17Z

...t/chapter4formatting/rule461verticalwhitespace/InputMultipleEmptyLineInsideClassMembers.java

The file name is InputCorrect* but it contains violations. Please change the input name to InputMultipleEmptyLineInsideClassMembers

Zopsss · 2025-12-25T07:41:37Z

False-positive: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A19

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A31

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A94

Zopsss · 2025-12-25T07:44:56Z

The check is not aware of comments: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A39

Zopsss · 2025-12-25T07:48:26Z

@Praveen7294 sorry for the delay, i have reviewed the PR and Report.

Also, please do not wait for maintainers to review reports. After pushing the changes, generate the report and always self review it so the bugs can be caught a lot earlier.

…rs property inside EmptyLineSeparator check in google_checks.xml

Praveen7294 · 2025-12-27T10:00:11Z

False-positive: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A19

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A31

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A94

There are multiple false-positive cases, so I created an issue regarding this #18438.

Praveen7294 · 2025-12-27T10:03:57Z

The check is not aware of comments: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A39

yes, in docs mentioned as Attention:
ATTENTION: empty line separator is required between token siblings, not after line where token is found. If token does not have a sibling of the same type, then empty line is required at its end (for example for CLASS_DEF it is after '}'). Also, trailing comments are skipped.
https://checkstyle.org/checks/whitespace/emptylineseparator.html#Description

should we include trailing comments to check?

Praveen7294 · 2025-12-27T10:06:22Z

@Praveen7294 sorry for the delay, i have reviewed the PR and Report.

No Problem.

Also, please do not wait for maintainers to review reports. After pushing the changes, generate the report and always self review it so the bugs can be caught a lot earlier.

Okay, I will take care of that from next time.

romani · 2025-12-28T19:24:27Z

@Zopsss , please finish review

Zopsss · 2025-12-29T04:34:19Z

There are multiple false-positive cases, so I created an issue regarding this #18438.

We cannot add false-positives to our style guide implementation. First we need to solve them. Unfortunately, until all false-positives are solved, we cannot proceed with this PR.

Zopsss · 2025-12-29T04:42:43Z

The check is not aware of comments: https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/951dac4_2025065820/reports/diff/Hbase/index.html#A39

yes, in docs mentioned as Attention: ATTENTION: empty line separator is required between token siblings, not after line where token is found. If token does not have a sibling of the same type, then empty line is required at its end (for example for CLASS_DEF it is after '}'). Also, trailing comments are skipped. https://checkstyle.org/checks/whitespace/emptylineseparator.html#Description

should we include trailing comments to check?

Yes we should do that, it is causing unnecessary violations. Please open a new issue regarding this. @romani cc

Praveen7294 · 2025-12-29T04:47:18Z

We cannot add false-positives to our style guide implementation. First we need to solve them. Unfortunately, until all false-positives are solved, we cannot proceed with this PR.

Once it approved, I will start working on that

romani

Sorry, I am blocking this PR until we clarify my comment in issue.

Praveen7294 force-pushed the empty-line branch 11 times, most recently from 406dae4 to aca7eed Compare December 16, 2025 00:53

Praveen7294 force-pushed the empty-line branch from aca7eed to 2dd5828 Compare December 17, 2025 17:43

Zopsss suggested changes Dec 18, 2025

View reviewed changes

Praveen7294 force-pushed the empty-line branch 3 times, most recently from 0b77b02 to 543b0be Compare December 19, 2025 15:49

Praveen7294 requested a review from Zopsss December 23, 2025 16:55

Praveen7294 force-pushed the empty-line branch from 543b0be to 951dac4 Compare December 23, 2025 16:55

Zopsss reviewed Dec 25, 2025

View reviewed changes

Zopsss suggested changes Dec 25, 2025

View reviewed changes

Praveen7294 mentioned this pull request Dec 27, 2025

False positive: EmptyLineSeparator reports violation on method call #18438

Open

Praveen7294 force-pushed the empty-line branch from 951dac4 to 07cc18f Compare December 27, 2025 09:51

Issue checkstyle#18265: Added allowMultipleEmptyLinesInsideClassMembe…

f927976

…rs property inside EmptyLineSeparator check in google_checks.xml

Praveen7294 force-pushed the empty-line branch from 07cc18f to f927976 Compare December 27, 2025 09:51

Praveen7294 requested a review from Zopsss December 27, 2025 10:30

romani assigned Zopsss Dec 28, 2025

romani requested changes Dec 29, 2025

View reviewed changes

Zopsss added the blocked label Dec 29, 2025

Uh oh!

Issue #18265: Added allowMultipleEmptyLinesInsideClassMembers property inside EmptyLineSeparator check in google_checks.xml #18311

Are you sure you want to change the base?

Issue #18265: Added allowMultipleEmptyLinesInsideClassMembers property inside EmptyLineSeparator check in google_checks.xml #18311

Conversation

Praveen7294 commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Praveen7294 commented Dec 16, 2025

Uh oh!

github-actions bot commented Dec 16, 2025

Uh oh!

Praveen7294 commented Dec 17, 2025

Uh oh!

Zopsss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Praveen7294 Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Praveen7294 commented Dec 23, 2025

Uh oh!

Zopsss commented Dec 25, 2025

Uh oh!

github-actions bot commented Dec 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zopsss commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zopsss commented Dec 25, 2025

Uh oh!

Zopsss commented Dec 25, 2025

Uh oh!

Praveen7294 commented Dec 27, 2025

Uh oh!

Praveen7294 commented Dec 27, 2025

Uh oh!

Praveen7294 commented Dec 27, 2025

Uh oh!

romani commented Dec 28, 2025

Uh oh!

Zopsss commented Dec 29, 2025

Uh oh!

Zopsss commented Dec 29, 2025

Uh oh!

Praveen7294 commented Dec 29, 2025

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Praveen7294 commented Dec 15, 2025 •

edited

Loading

Praveen7294 Dec 18, 2025 •

edited

Loading

Zopsss commented Dec 25, 2025 •

edited

Loading