Moe Sync #1261
Merged
Moe Sync #1261
Conversation
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
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=238699395
…rning. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239164099
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239304245
…on warnings RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239321120
RELNOTES: [BoxedPrimitiveEquality] Error for reference equality on boxed primitives ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239714556
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=239858177
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240610764
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240677733
Without the handing, the test fails for case BUG:129489375 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=240862658
RELNOTES: Add ModifySourceCollectionInStream check ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=241081354
finds was statically imported. Fixes #1252 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=241514576
…ed to be RELNOTES: Add a check for local variables which are of boxed type but do not need to be ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=241636217
1. ProvideDescriptionToCheck: Migrate from the no-arg check() to the one that accepts a description.
2. ImplementAssertionWithChaining: Migrate from a manual "if (a != b) { fail(...) }" pattern to "check(...).that(a).isNotEqualTo(b)."
3. ChainedAssertionLosesContext: Migrate from "assertThat(...)" to "check(...).that(...)."
These suggestions generally produce better failure messages. ImplementAssertionWithChaining also produces shorter code. And the first two help migrate off methods that we're hoping to deprecate and remove (namely, no-arg check() and the legacy fail(...) methods, which often used in the manual test-and-fail approach).
RELNOTES:
Added ProvideDescriptionToCheck, ImplementAssertionWithChaining, and ChainedAssertionLosesContext, suggestions for using Truth's Subject.check(...).
GOOGLE:
ImplementAssertionWithChaining Flume results: CL 240451246
ProvideDescriptionToCheck Flume results: CL 240796685
ChainedAssertionLosesContext Flume results: CL 241477342
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=241817573
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=241829483
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242058867
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242192393
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242488740
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242490104
RELNOTES: N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242540586
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242541503
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242564080
RELNOTES:N/A ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242576082
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242674846
Fixes #1246 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242727873
…ndle) ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242811734
This suggests deleting empty Javadoc block tags (like "@throws Exception") in Javadoc that don't have any description. "Any of the standard "block tags" that are used appear in the order @param, @return, @throws, @deprecated, and these four types never appear with an empty description." ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242890423
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242906126
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=242934572
can be called safely without checking the return value. RELNOTES: n/a ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243144437
Add a negative test case I should have written for the first CL. RELNOTES: none ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243168106
…n the descriptor e.g. don't rewrite uses of Predicate that call any methods besides 'test'. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243647804
- Remove net.ltgt.apt (which is not really needed anymore since Gradle 4.6, and definitely no longer since Gradle 5.2) Fixes #1231 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243697335
`.getUpperBound()` See #1106 - field removed in JDK13 as per Fixes #1225 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243702988
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243712726
------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=243712923
Collaborator
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
Collaborator
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
Stephan202
reviewed
Apr 16, 2019
| > modifies, or causes to be modified, the stream's data source. The need for | ||
| > non-interference applies to all pipelines, not just parallel ones. Unless the | ||
| > stream source is concurrent, modifying a stream's data source during execution | ||
| > hg of a stream pipeline can cause exceptions, incorrect answers, or |
Contributor
There was a problem hiding this comment.
Suggested change
| > hg of a stream pipeline can cause exceptions, incorrect answers, or | |
| > of a stream pipeline can cause exceptions, incorrect answers, or |
|
|
||
| @Override | ||
| public Void visitThrows(ThrowsTree throwsTree, Void unused) { | ||
| reportMatchIfEmpty(throwsTree, throwsTree.getDescription()); |
Contributor
There was a problem hiding this comment.
There may be one case in which deletion is not the best suggestion: in case of a @throws tag without a description, where the declared exception is unchecked and not part of the documented method's signature. In that case it may be better to only flag the issue, forcing the developer to either manually deleted it (if not applicable) or add documentation (otherwise).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
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.
This code has been reviewed and submitted internally. Feel free to discuss on the PR and we can submit follow-up changes as necessary.
Commits:
Test case for guardedbychecker bug.
RELNOTES: N/A
e6266aa
RELNOTES: Enable AnnotateFormatMethod by default, and add a strong warning.
3b45e8f
Add fixes for incompatible/required modifiers checks
RELNOTES: N/A
07f9c7f
Delete DeprecatedThreadMethods in favour of general-purpose deprecation warnings
RELNOTES: N/A
c5f99f3
New error for reference equality on boxed primitives
RELNOTES: [BoxedPrimitiveEquality] Error for reference equality on boxed primitives
55a9e0a
Support renaming fields in renameVariable
RELNOTES: N/A
3269b66
Add a defensive null-check
RELNOTES: N/A
66cdb01
Make IncompatibleModifiers an error
RELNOTES: N/A
13f2d76
Fix BadImport on ParameterizedType with TYPE_USE annotation
Without the handing, the test fails for case
BUG:129489375
885319a
Add ModifySourceCollectionInStream check
RELNOTES: Add ModifySourceCollectionInStream check
d055fdb
Updated JavaTimeDefaultTimeZone to avoid NPE'ing when the function it finds was statically imported.
Fixes #1252
eba27ed
Add a check for local variables which are of boxed type but do not need to be
RELNOTES: Add a check for local variables which are of boxed type but do not need to be
95ebe6a
Provide suggestions for using Truth's Subject.check(...) in 3 cases:
These suggestions generally produce better failure messages. ImplementAssertionWithChaining also produces shorter code. And the first two help migrate off methods that we're hoping to deprecate and remove (namely, no-arg check() and the legacy fail(...) methods, which often used in the manual test-and-fail approach).
RELNOTES:
Added ProvideDescriptionToCheck, ImplementAssertionWithChaining, and ChainedAssertionLosesContext, suggestions for using Truth's Subject.check(...).
GOOGLE:
ImplementAssertionWithChaining Flume results: CL 240451246
ProvideDescriptionToCheck Flume results: CL 240796685
ChainedAssertionLosesContext Flume results: CL 241477342
8ef14ba
Omit enclosing class names of constructors in diagnostics RELNOTES: N/A
8aa3f18
Add a refactoring to replace trivial lambdas with method references
e66c65a
Clean up description creation
e1918bb
Use a prebuilt JDK 7 -> 8 API diff
b59a3fb
New a check to simplify description creation
21a262c
Handle casts in SelfAssignment RELNOTES: N/A
427ccb4
Don't complain about deprecated java.lang.Compiler API in JavaLangClash
ac1b717
Clean up unnecessary lambdas
dd01c04
Add a WARNING for using .valueOf(String) on lite proto buffer enums
RELNOTES:N/A
45be640
Return -> Returning
69709fc
Open-source @DoNotMock
Fixes #1246
b0218c1
Add Check for Leaking/Mutating an already-forked mutable instance (Bundle)
8865ac4
Add EmptyBlockTag error-prone check
This suggests deleting empty Javadoc block tags (like "@throws Exception") in Javadoc that don't have any description.
"Any of the standard "block tags" that are used appear in the order @param, @return, @throws, @deprecated, and these four types never appear with an empty description."
c5d9072
Fix a crash in SelfAssignment
8be863d
Revert some unnecessary changes
94f0acc
Add java.time.temporal.ValueRange#checkValidValue as a method that can be called safely without checking the return value.
RELNOTES: n/a
7fc1daf
Recognize that it's safe to migrate from == to isEqualTo() for enums.
Add a negative test case I should have written for the first CL.
RELNOTES: none
ad16bb7
In UnnecessaryLambda, don't refactor uses that call methods other than the descriptor
e.g. don't rewrite uses of Predicate that call any methods besides 'test'.
bc9c078
Update Gradle plugin check example
since Gradle 4.6, and definitely no longer since Gradle 5.2)
Fixes #1231
b717c49
TypeParameterUnusedInFormals: Replace `.bound` by `.getUpperBound()`
See #1106 - field removed in JDK13 as per
Fixes #1225
3f00c36
Fix for error-prone ImmutableEnumChecker warning
6bbafbd
Fix for error-prone AutoValueFinalMethods warning
a902d79