[YETUS-488] Checkstyle reports new error if the file still longer than expected

• Type: Improvement
• Status: Resolved
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 0.4.0
• Fix Version/s: 0.5.0
• Component/s: Precommit
• Assignee: Peter Vary
• Reporter: Peter Vary
• Created: 15/Feb/17 14:52
• Updated: 24/May/17 09:24
• Resolved: 17/May/17 22:26

Description

Given a java source file which is originally longer than the checkstyle defined value then we get the following warning every time the length changes and we run the checkstyle plugin:

./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File length is 2,373 lines (max allowed is 2,000).

The problem is, that the checkstyle output is changed:

From

./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File length is 2,373 lines (max allowed is 2,000).

To

./beeline/src/java/org/apache/hive/beeline/BeeLine.java:1: warning: File length is 2,365 lines (max allowed is 2,000).

Since the checkstyle_calcdiffs does not find the new line in the original output, it marks it as a new error, and gives -1 to the result of the checkstyle plugin.

This is true for every error message where there is a number in the text. Not exhaustive examples are below:

• Line length
• Row length
• Function length
• Number of attributes
• Indentation

I think we should be reluctant to give -1 without a reason so it would be good to remove these errors.

I have yet to find the ideal solution for the issue:

• An easy patch could be to remove the numbers from files before the diff (note: it does not effect the final diff-checkstyle-<MODULE>.txt output messages). This would mean that the warning will give -1 only the first time when the error text is occurred, and will not give -1 if the numbers are changed, like
  • File become shorter, but still longer than expected
  • File become even longer
  • Indentation changed but still problematic
• A more sophisticated patch could do this only for specific errors, or even check the value and give error when the line length grow.

To be honest, I think the second solution would complicate the code too much, and make it dependent on the specific checkstyle messages, so I do not like it.

I am even hesitant about the first one, because the change I introducing with it might have unwanted consequences.

Do anyone has better ideas? I would be happy to implement them, if someone points me to the right direction

Thanks,
Peter