Proposal to Extend Violation Comment Syntax to Include Column Numbers

[Praveen7294]

Hi Mentors,

I wanted to suggest an enhancement to the violation comment syntax used in Checkstyle test input files. Currently, test violations are annotated like this:

// violation 'message'
// violation above 'message'
// violation above 3 lines 'message'
// filtered violation 'message'

These annotations support line-level granularity, but do not support specifying column numbers, which limits the precision of test cases for some checks (like WhitespaceAround, MethodParamPad, etc.) where column position matters.

Proposal

I’d like to propose adding an optional column specification to the comment syntax, like:

// violation at col 10 'message'
// violation above at col 1 'message'
// violation above 3 lines at col 2 'message'
// filtered violation at col 17 'message'

This would allow us to:

• Write more precise and accurate test cases
• Ensure correct column detection in checks
• Potentially simplify and strengthen our assertions

[romani]

Write more precise and accurate test cases
Ensure correct column detection in checks

we already have columns in junit method expected array. This is the only reason why we still keep such arrays in test methods.
Yes, is a bit far from Input file and hard to control it properly, but placement on right column is a more nuanced.

Potentially simplify and strengthen our assertions

filtered violation at col 17 'message' , column 17 is hard for humans, even it is precise, it is still not clear where it is. Human will have problems to differentiate it from column 15.
if we look at this file

checkstyle/src/xdocs-examples/resources/com/puppycrawl/tools/checkstyle/filters/suppresswithnearbycommentfilter/Example6.java

Lines 19 to 21 in 62949e6

	public static final int [] array = {};
	// filtered violation above ''int' is followed by whitespace'
	// filtered violation 2 lines above 'Name 'array' must match pattern'

  public static final int [] array = {};
  //                  ^------------------┐ 
  // filtered violation 2 lines above at ^ ''int' is followed by whitespace'

somethign like this would be awesome, but it not easy archive.
User do not care about column number, user care about what visible element in line causing violation.
Modern compilers does good job to point to exact place.

[romani]

Javac do:

        .flatMap(List::stream).toList()
                ^
    (argument mismatch; invalid method reference
      method stream in interface Collection<E> cannot be applied to given types
        required: no arguments

Eclipse compiler https://checkstyle.semaphoreci.com/jobs/5fb41b5b-91a9-43dc-a631-41d08187ab09#L744

40. ERROR in /home/semaphore/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/blocks/needbraces/InputNeedBracesSwitchExpressionAndLambdaAllowSingleLine.java (at line 51)01:21
	int c = switch (o) {
	                ^
Cannot switch on a value of type Object. Only convertible int values, strings or enum variables are permitted
----------

javac:

src/xdocs-examples/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/coding/
unusedlocalvariable/Example2.java:15: error: class Example1 is public, should be declared in a file named Example1.java
public class Example1 {
       ^

[Praveen7294]

I have two questions in my mind if we add something like this:

• if we got error at col 1 or column 2 then how we can point out that error?
• how we can show multiple violation at the same line?

public static final int [] array = {};
//                  ^------------------┐ 
// filtered violation 2 lines above at ^ ''int' is followed by whitespace'

if we add something like below then also same problem occurs.

public static final int [] array = {};
//                    ^            ^
//   violation above 2 lines ''int' is followed by whitespace'
//   violation above 3 lines 'Empty array initializer '{}' not allowed'

[romani]

In wild use case , no good way.
But highly discourage to use multiple violations on same line, it is complicated by all means.

But you example of multiple is good, what you have is way better than without it. Internally in implementation we can do extra logic to match to any combination of column + message.

[Praveen7294]

But you example of multiple is good, what you have is way better than without it. Internally in implementation we can do extra logic to > match to any combination of column + message.

yes, we can do column + message. But i am thinking to map the column number with exact comment message, if it is possible then we can create a new field columnNo in TestInputViolation.java and it helps in sorting the violations according to lineNo, columnNo, then message.

[romani]

Even you will have column in comments, nobody can see what is on 48 column in line.
Such numbers does not work for people, it is not visual, nobody open code in IDE to validate that behavior in PR is correct, nobody have time on this.

We something visual and pointing, something like:

public static final int [] array = {};
//                    ^            ^^
//   violation above 2 lines marked ^^ ''int' is followed by whitespace'
//   violation above 3 lines marked ^ 'Empty array initializer '{}' not allowed'

[Praveen7294]

Even you will have column in comments, nobody can see what is on 48 column in line.
Such numbers does not work for people, it is not visual, nobody open code in IDE to validate that behavior in PR is correct, nobody have time on this.

i meant that we can internally map column by extracting the position of the ^ pointer.

[romani]

Yes!
There might be nuances with cases when two violations on same column. But we should minimize such cases, we should avoid them as much as possible.
In worse scenario we will have some comments without such pointers as exceptions