stevenschenk edited a comment on pull request #47: URL: https://github.com/apache/maven-checkstyle-plugin/pull/47#issuecomment-863814367
> > @stevenschenk the checkstyle API states that file and line can never be null. So I expect you are fixing a problem which can never occur, only in tests. > > I mean, how can a violation not be on a line in a file? > > That said, I have no objections merging it anyway. > > My bad. Was not listed in the ticket, but I did not verify it either. I checked the constructor and there was no null check on these fields, so I assumed they were nullable. If the API states they cannot be null, should we do a `requiresNonNull()` check in the constructor? @bmarwell So I added a `requiresNonNull()` check for `file` and `line` fields in the constructor of `Violation`, however it causes other tests to fail (For example: `CheckstyleViolationCheckMojoTest.testDefaultConfig()`. One of the test cases in that test is a violation for a missing file. When creating the violation it provides the "missing file" as file input to the constructor. As the missing file could not be resolved, resulting in a null value, it seems violation could be created with null value for `file`. This seems contradicting with the API. Is this a test case that can never occur, or could violations actually exist without a file? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
