Fix error prone warnings#2316
Conversation
| <!-- Args related to Error Prone, see: https://errorprone.info/docs/installation#maven --> | ||
| <arg>-XDcompilePolicy=simple</arg> | ||
| <arg>-Xplugin:ErrorProne</arg> | ||
| <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*gson/src/test.*|.*extras/src/test.*|.*proto.*</arg> |
There was a problem hiding this comment.
Eventually we might want to have the tests pass Error Prone too, but I agree that is a much lower priority.
Marcono1234
left a comment
Marcono1234
left a comment
There was a problem hiding this comment.
Sorry to bring this up again, but would you mind fixing (respectively removing) the incorrect @SuppressWarnings("TruthSelfEquals") suppressions as mentioned in https://github.com/google/gson/pull/2308/files#r1096471723 (simply change it to a.equals(a) for now)? The warnings are true-positives; regardless of whether guava-testlib will be added in the future, the current test code on master is incorrect and the @SuppressWarnings hide that, suggesting the code is correct.
My concern is that we forget about this, or for whatever reason guava-testlib is not integrated, and in that case the incorrect code remains unnoticed.
| <!-- Args related to Error Prone, see: https://errorprone.info/docs/installation#maven --> | ||
| <arg>-XDcompilePolicy=simple</arg> | ||
| <arg>-Xplugin:ErrorProne</arg> | ||
| <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*gson/src/test.*|.*extras/src/test.*|.*proto.*</arg> |
There was a problem hiding this comment.
Is it intended that you disable warnings for the complete proto module? Maybe it would be better to only disable warnings for the generated source code, as suggested in #2308 (comment).
There was a problem hiding this comment.
I'm going to go ahead and merge this anyway, but we could indeed consider refining the pattern here.
This reverts commit 387746c.
eamonnmcmanus
left a comment
eamonnmcmanus
left a comment
There was a problem hiding this comment.
I'm merging this snapshot under the assumption that if there are further changes in the pipeline they can just be applied in a followup PR.
| <!-- Args related to Error Prone, see: https://errorprone.info/docs/installation#maven --> | ||
| <arg>-XDcompilePolicy=simple</arg> | ||
| <arg>-Xplugin:ErrorProne</arg> | ||
| <arg>-Xplugin:ErrorProne -XepExcludedPaths:.*gson/src/test.*|.*extras/src/test.*|.*proto.*</arg> |
There was a problem hiding this comment.
I'm going to go ahead and merge this anyway, but we could indeed consider refining the pattern here.
* Fix `OperatorPrecedence` warn in `JsonWriter#close` * Fix `ReferenceEquality` warn in `LinkedTreeMap#replaceInParent` * Fix `UnnecessaryParentheses` warn in `LinkedTreeMap#replaceInParent` * Fix `ReferenceEquality` warn in `LinkedTreeMap#hasNext` * Fix `ReferenceEquality` warn in `LinkedTreeMap#nextNode` * Adds `error_prone_annotations` to the `pom.xml` of `gson` * Fix `InlineMeSuggester` warns in `JsonParser` * Fix `UnnecessaryParentheses` warns in `ConstructorConstructor#newDefaultImplementationConstructor` * Fix `ThreadLocalUsage` warn in `Gson` * Fix `JdkObsolete` warn in `GsonBuilder` * Fix `ReferenceEquality` warn in `LazilyParsedNumber#equals` * Fix `OperatorPrecedence` warn in `TreeTypeAdapter#create` * Fix `OperatorPrecedence` warn in `ArrayTypeAdapter` * Fix `UnnecessaryParentheses` warn in `TypeAdapters` * Adds `-XepExcludedPaths` flag to ErrorProne plugin to exclude tests and proto path * Fix `ClassNewInstance` warn in `InterceptorAdapter` * Fix `ThreadLocalUsage` warn in `GraphAdapterBuilder` * Fix `JdkObsolete` warn in `GraphAdapterBuilder` * Revert "Adds `error_prone_annotations` to the `pom.xml` of `gson`" This reverts commit 14af14d. * Revert "Fix `InlineMeSuggester` warns in `JsonParser`" This reverts commit 095bfd5. * Adds `@SuppressWarnings("ThreadLocalUsage")` * Fix `OperatorPrecedence` in `JsonWriter` * Revert "Fix `ReferenceEquality` warn in `LinkedTreeMap#nextNode`" This reverts commit 387746c. * Adds `@SuppressWarnings("ReferenceEquality")` * Adds `guava-testlib` to the gson `pom.xml` * `@SuppressWarnings("TruthSelfEquals")` removed to use `EqualsTester()`
4 participants
In this PR I have fixed some ErrorProne warns.
I have also add the flag
-XepExcludedPaths:to the ErrorProne plugin to exclude tests and proto path