Enable Nullaway#988
Conversation
Generate changelog in
|
schlosna
left a comment
schlosna
left a comment
There was a problem hiding this comment.
Overall looks good, thanks for the cleanup!
| } | ||
|
|
||
| for (Map.Entry<Path, Future<String>> result : results.entrySet()) { | ||
| for (Entry<Path, Future<String>> result : results.entrySet()) { |
There was a problem hiding this comment.
Was this intentional? I'm surprised this didn't trigger error-prone BadImport
| for (Entry<Path, Future<String>> result : results.entrySet()) { | |
| for (Map.Entry<Path, Future<String>> result : results.entrySet()) { |
There was a problem hiding this comment.
"Entry" isn't on BadImport's list: https://github.com/google/error-prone/blob/4165ecdc3d11a41b989d08e324de6fbc944e95ae/core/src/main/java/com/google/errorprone/bugpatterns/BadImport.java#L73-L84
Maybe it should be?
Fixed
There was a problem hiding this comment.
Wow, looks like Entry was removed from the BadImport list 3 years ago, but my brain has already been conditioned to qualify Entry.
| parameters = CommandLineOptionsParser.parse(Arrays.asList(args)); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new UsageException(e.getMessage()); | ||
| throw new UsageException(Optional.ofNullable(e.getMessage()).orElse("null")); |
There was a problem hiding this comment.
I don't think we need to change this, but in other types of hot paths java.until.Objects.requireNonNullElse can avoid the Optional allocation
| throw new UsageException(Optional.ofNullable(e.getMessage()).orElse("null")); | |
| throw new UsageException(Objects. requireNonNullElse(e.getMessage(), "null")); |
There was a problem hiding this comment.
Thanks for sharing -- I've needed this before and its good to know a way to do this without the allocation
policy-bot
Bot
dismissed
schlosna’s stale review
Invalidated by push of bb18ad9
This PR adds support for `switch` statements where a `case` has a guard clause. See Issue #983 Fixes #988 COPYBARA_INTEGRATE_REVIEW=google/google-java-format#988 from TheCK:master 4771486db7d8aab84eb4ecf8e68af2612d0c2b5c PiperOrigin-RevId: 588913297
3 participants
Fixes #987
Before this PR
Nullaway is not enabled, and null handling isn't verified by that plugin.
After this PR
==COMMIT_MSG==
Enable Nullaway
==COMMIT_MSG==
Fix flagged issues by adding
@Nullableand handling nulls better withOptional.ofNullablePossible downsides?
NullAway might fail future PRs more often; it can be fiddly in requirements that tend to be largely unimportant in practice (like that Exception messages can be null).