Enable Nullaway #988
Enable Nullaway #988
Conversation
Generate changelog in
|
| @@ -130,7 +132,7 @@ private int formatFiles(CommandLineOptions parameters, JavaFormatterOptions opti | |||
| } | |||
| } | |||
|
|
|||
| 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.
| @@ -215,10 +220,10 @@ public static CommandLineOptions processArgs(String... args) throws UsageExcepti | |||
| try { | |||
| 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
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).