Fix bug where nested MissingBraces violations' suggested fixes result in broken code#3797
Fix bug where nested MissingBraces violations' suggested fixes result in broken code#3797keithl-stripe wants to merge 1 commit intogoogle:masterfrom
MissingBraces violations' suggested fixes result in broken code#3797Conversation
…lt in broken code
|
I don't know enough to offer a definite way forward here, but I happen to know a couple relevant pieces of information:
Actual core Error Prone team members may have more useful advice. |
I had a quick look at whether we could 'just' preserve duplicate inserts by default. ReturnMissingNullable was one example I ran into, there were a couple of others that similarly emitted the same method-level fix once per occurrence of a problem int he method body, leading to things like duplicate
I think part of what happened with MustBeClosedChecker was that the I suspect we're not done play whack-a-mole with edge cases involving overlapping and adjacent fixes, but I also think allowing MissingBraces to opt-in to this is an improvement over the status quo. |
…lt in broken code @holtherndon-stripe came across this issue while applying `MissingBraces` to Stripe's code base of 4 million lines of Java code. See unit test for sample code. I'm not sure about a few things: - Why does the coalescing code suppress duplicate inserts? It seems a bit odd that this is the default. I wonder which checkers' fixes need this, and why. - Is `Fix` the right level of abstraction? Or should the coalescing policy be per `Replacement`? Fixes #3797 FUTURE_COPYBARA_INTEGRATE_REVIEW=#3797 from keithl-stripe:keithl-missing-braces-patch de82c53 PiperOrigin-RevId: 564840815
@holtherndon-stripe came across this issue while applying
MissingBracesto Stripe's code base of 4 million lines of Java code.See unit test for sample code.
I'm not sure about a few things:
Fixthe right level of abstraction? Or should the coalescing policy be perReplacement?