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

Fail test execution if Input file does nor have message on violation line and Check has not signle message #11091

Closed
romani opened this issue Dec 21, 2021 · 6 comments · Fixed by #11133

Comments

@romani
Copy link
Member

romani commented Dec 21, 2021

it become clear at #11051 (comment)
that we do need to enforce it.

TODO:
if Check has not single message, and Input file contain //violation marker in it, we should fail validation and advice user to specify message in addition.

@Vyom-Yadav
Copy link
Member

On it.

@Vyom-Yadav
Copy link
Member

Vyom-Yadav commented Dec 22, 2021

@romani What is expected for-

  • // X violations
  • // X violations above
  • // X violations below

As in these, we cannot specify the message.

Edit- I was thinking if there are X violations you need to have X messages-

// 2 violations 'first violation' 'second violation'

Generalizing-

// X violations 'first violation' 'second violation'.....'Xth violation'

@Vyom-Yadav
Copy link
Member

Also, there is another problem if you have multiple checks in an input file example.
There is no way to tell whether a violation from a file belongs to checkOne or checkTwo, we parse line by line and verify them later line by line.

/*
CheckOne config

CheckTwo config


*/

public class Test {
    int a = 12; // violation
}

Currently there is no way to tell int a = 12; // violation is a violation due to checkOne or checkTwo

@Vyom-Yadav
Copy link
Member

@romani Just had a thought, if we have multiple checks in a config file then it is a must to specify violation message irrespective of the number of error messages in the check as it would improve the readibility of violations, what do you think?

@romani
Copy link
Member Author

romani commented Dec 25, 2021

As in these, we cannot specify the message.

not yet :) , and yes, your proposal is good.
It might be challange to fit few messages in same line, but we might find a way.
We can relax our validation to not demand messages if any of such trailing comments are present.

@romani
Copy link
Member Author

romani commented Dec 25, 2021

if you have multiple checks in an input file

lets skip such cases completely from validation.
If two Checks are defined - no need to demand message. We will think on this later on.

Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 30, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 31, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 31, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Dec 31, 2021
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jan 15, 2022
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jan 18, 2022
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jan 18, 2022
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jan 18, 2022
Vyom-Yadav added a commit to Vyom-Yadav/checkstyle that referenced this issue Jan 21, 2022
romani pushed a commit that referenced this issue Jan 29, 2022
@romani romani added this to the 9.3 milestone Jan 29, 2022
MUzairS15 pushed a commit to MUzairS15/checkstyle that referenced this issue Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
2 participants