Skip to content

Issue #7523: CLI '-s' option does not match by data from '-t'#18724

Merged
romani merged 1 commit into
checkstyle:masterfrom
ayushactiveat:issue-7523
Jan 30, 2026
Merged

Issue #7523: CLI '-s' option does not match by data from '-t'#18724
romani merged 1 commit into
checkstyle:masterfrom
ayushactiveat:issue-7523

Conversation

@ayushactiveat

@ayushactiveat ayushactiveat commented Jan 21, 2026

Copy link
Copy Markdown
Contributor

Closes #7523

Changes:

  • Modified AstTreeStringPrinter and DetailNodeTreeStringPrinter to print column numbers using 1-based indexing instead of 0-based.
  • This aligns the output of the -t CLI option with the input requirements of the -s option and standard IDE coordinates.
  • Updated all relevant Unit Tests, Regression resources, and non-compilable test resources to reflect the new 1-based format.

Verification:

  • Validated manually that java -jar checkstyle.jar -t File.java produces coordinates that are directly accepted by java -jar checkstyle.jar -s line:col File.java.

@romani

romani commented Jan 28, 2026

Copy link
Copy Markdown
Member

@ayushactiveat , please show by CLI with a fix that whta is described in issue #7523

becomes fixed.

can we make junit on this? print ast, fetch line that has token and line:column, and do CLI run on such line:column and make sure that retuned Xpath is ending with token.

@romani

romani commented Jan 30, 2026

Copy link
Copy Markdown
Member

plase address https://app.circleci.com/pipelines/github/checkstyle/checkstyle/41149/workflows/b7067b54-3a86-4591-8001-8c6f4308fe04/jobs/1290960/artifacts

romani
romani requested changes Jan 30, 2026
View reviewed changes

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items:

Comment thread src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java Outdated
final String treeOutput = systemOut.getCapturedData();

final java.util.regex.Pattern pattern =
java.util.regex.Pattern.compile("CLASS_DEF -> CLASS_DEF \\[(\\d+:\\d+)\\]");

@romani romani Jan 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possilble to make regexpt to apply for each line where we have -> in output and make sure that search by line:col generates xpath that has token that is after ->

@ayushactiveat

ayushactiveat commented Jan 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@romani, here is the CLI verification -

$ cat Test.java 
public class Test {
  public Test() {}
}

$ java -jar target/checkstyle-13.1.0-SNAPSHOT-all.jar -t Test.java

COMPILATION_UNIT -> COMPILATION_UNIT [1:1]
`--CLASS_DEF -> CLASS_DEF [1:1]
    |--MODIFIERS -> MODIFIERS [1:1]
    |   `--LITERAL_PUBLIC -> public [1:1]
    |--LITERAL_CLASS -> class [1:8]
    |--IDENT -> Test [1:14]
    `--OBJBLOCK -> OBJBLOCK [1:19]
        |--LCURLY -> { [1:19]
        |--CTOR_DEF -> CTOR_DEF [1:21]
        |   |--MODIFIERS -> MODIFIERS [1:21]
        |   |   `--LITERAL_PUBLIC -> public [1:21]
        |   |--IDENT -> Test [1:28]
        |   |--LPAREN -> ( [1:32]
        |   |--PARAMETERS -> PARAMETERS [1:33]
        |   |--RPAREN -> ) [1:33]
        |   `--SLIST -> { [1:35]
        |       `--RCURLY -> } [1:36]
        `--RCURLY -> } [1:38]
$ java -jar target/checkstyle-13.1.0-SNAPSHOT-all.jar -s 1:38 Test.java

/COMPILATION_UNIT/CLASS_DEF[./IDENT[@text='Test']]/OBJBLOCK/RCURLY

here -s successfully accepted the coordinate 1:38 provided by -t and generated the correct xpath, before this fix, -t would have output 1:37 (0-based), which -s would reject.

I have updated testCliTreeAndSuppressionConsistency in MainTest.java as requested -
->test iterates through every token found in the -t output (not just a single token).
->captures token name, coordinates for each line, feeds them into -s, and verifies the generated xpath matches the expected token type.

please let me know if you need any changes

@romani

romani commented Jan 30, 2026

Copy link
Copy Markdown
Member

@ayushactiveat , please make CI green to let me merge.

thanksa lot for update.

@romani

romani commented Jan 30, 2026

Copy link
Copy Markdown
Member 
[INFO] --- antrun:3.2.0:run (ant-phase-verify-sevntu) @ checkstyle ---02:04
[INFO] Executing tasks02:05
[INFO]      [echo] Checkstyle started (/home/semaphore/checkstyle/config/checkstyle-sevntu-checks.xml): 30/01/2026 11:48:53 AM02:05
[INFO] [checkstyle] Running Checkstyle 13.1.0-SNAPSHOT on 2296 files02:05
[ERROR] [checkstyle] [ERROR] /home/semaphore/checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java:1088:13: Variable 'previousLength' can be moved inside the block at line '1,094' to restrict runtime creation. [MoveVariableInsideIf]02:12
[ERROR] [checkstyle] [ERROR] /home/semaphore/checkstyle/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java:1092:13: Variable 'allOutput' can be moved inside the block at line '1,094' to restrict runtime creation. [MoveVariableInsideIf]02:12
[INFO]      [echo] Checkstyle finished (/home/semaphore/checkstyle/config/checkstyle-sevntu-checks.xml): 30/01/2026 11:49:00 AM02:12

run on local ./mvn clean verify to reproduce such violations

@romani romani merged commit 9f09882 into checkstyle:master Jan 30, 2026
120 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romani romani romani requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CLI '-s' option does not match by data from '-t'

2 participants

@ayushactiveat @romani