Don't assume that there's exactly a semicolon and a newline after the package name. - #6072
Merged
Conversation
… package name. And, as part of testing that, make our tests stop appending a newline to the given file content. (More on that later.) In the depot, there are a number of `package-info` files that have _no_ newline at the end. (I saw a bunch when running AddNullMarkedToPackageInfoTest over the depot as part of a test of unknown commit.) This causes fix application to fail. (It might be nice to fail immediately when the bad fix is generated, as in unknown commit. But I worry a bit about the performance of that particular implementation, and I'm not even sure whether it's the right place for the specific Error Prone integration I was using. Given that, I'm not planning to push on it now.) As for no longer appending a newline in tests: Most tests nowadays already have a trailing newline (especially after 5902c72), since text blocks have one unless the final line ends with a backslash. In such a case, we actively don't want to add an additional newline. Similarly, in the new test in this CL, we actively don't want to add the "missing" newline because it would unmake the scenario that we want to test. This CL does mean that [we also stop appending a newline to expected _outputs_](https://github.com/google/error-prone/blob/3cf3c69711690b15acf8f720c85e3f50e45c0bdf/test_helpers/src/main/java/com/google/errorprone/BugCheckerRefactoringTestHelper.java#L472). I worried that that might have caused trouble in `TEXT_MATCH`-mode tests, especially after that mode became the default in ccfc176. However, it should be safe because we've long formatted the expected and actual outputs: unknown commit. PiperOrigin-RevId: 972130097
copybara-service
Bot
force-pushed
the
test_968728702
branch
from
August 27, 2026 20:38
87b12b0 to
85479ad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Don't assume that there's exactly a semicolon and a newline after the package name.
And, as part of testing that, make our tests stop appending a newline to the given file content. (More on that later.)
In the depot, there are a number of
package-infofiles that have no newline at the end. (I saw a bunch when running AddNullMarkedToPackageInfoTest over the depot as part of a test of unknown commit.) This causes fix application to fail.(It might be nice to fail immediately when the bad fix is generated, as in unknown commit. But I worry a bit about the performance of that particular implementation, and I'm not even sure whether it's the right place for the specific Error Prone integration I was using. Given that, I'm not planning to push on it now.)
As for no longer appending a newline in tests:
Most tests nowadays already have a trailing newline (especially after 5902c72), since text blocks have one unless the final line ends with a backslash. In such a case, we actively don't want to add an additional newline. Similarly, in the new test in this CL, we actively don't want to add the "missing" newline because it would unmake the scenario that we want to test.
This CL does mean that we also stop appending a newline to expected outputs. I worried that that might have caused trouble in
TEXT_MATCH-mode tests, especially after that mode became the default in ccfc176. However, it should be safe because we've long formatted the expected and actual outputs: unknown commit.