Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MOE Sync 2020-05-14 #1614

Merged
merged 12 commits into from
May 14, 2020
Merged

MOE Sync 2020-05-14 #1614

merged 12 commits into from
May 14, 2020

Conversation

kluever
Copy link
Member

@kluever kluever commented May 14, 2020

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:

fix typo

Fixes #1590

4386471


skip unnecessary boxing

Fixes this warning:

Fixes #1558

90197b0


Minor fix to FallThrough to skip generated/mutated AST nodes (e.g. Lombok)

Fixes #1573

1534bae


Make summary of InjectOnConstructorOfAbstractClass consistent

The first sentence says @injected and the second one says @Inject'ed.

Change the first one to @Inject'ed so it's consistent.

Change-Id: Id2ab8908a608288739af33cddc902f94707b057a

Fixes #1491

0839737


Drop `@Nullable` from `RefasterRule#afterTemplates()`

This property is never null and the caller unconditionally dereferences

Fixes #1606

87d4fe5


Handle `NO_MATCH` in `VisitorState#reportMatch` instead of `Scanner

This fix prevents reports such as the following, which are hard to debug
for users:

[INFO] /path/to/some/File.java: [<no match>] <no match>
   (see <no match>)

Fixes #1609

5fdc31e61c2975ac93bbacbba5416cdefa1b9e88

-------

<p> Update junit dependency to 4.13

Fixes #1591

df17a5141f44146deb91a4718aa123b52ffcea25

-------

<p> Inline a helper method

Closes https://github.com/google/error-prone/pull/1325

2946131ec99327a3c7175d5ea282833512f8e94e

-------

<p> Add IntelliJ IDEA ImportOrganizer

Many projects use the default import ordering provided by IntelliJ IDEA.
In order to apply Error Prone without adjusting this ordering, one can

Fixes #1208

deea2cf2d72ec38893106536f8f414c5a579d939

-------

<p> Prevent accidental re-use of CompilationTestHelper and BugCheckerRefactoringTestHelper instances

See e.g. https://github.com/google/error-prone/pull/1313

2c5708c357f9d4f6b4b2a61588fe2f162bcc5b71

-------

<p> Don't handle nulls in VisitorState#reportMatch.

37eeca46f6078a09ec1e0db0b75c0ba95483bbae

-------

<p> Breaks mockito (d'oh)

882fc471b12b67b83588594fecc3fa4b300e8641

C-Otto and others added 12 commits May 14, 2020 09:23
Fixes #1590

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311374509
Fixes this warning:

Fixes #1558

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311374636
(e.g. Lombok)

Fixes #1573

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311375182
The first sentence says @injected and the second one says @Inject'ed.

Change the first one to @Inject'ed so it's consistent.

Change-Id: Id2ab8908a608288739af33cddc902f94707b057a

Fixes #1491

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311375711
This property is never null and the caller unconditionally dereferences

Fixes #1606

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311377788
This fix prevents reports such as the following, which are hard to debug
for users:
```
[INFO] /path/to/some/File.java: [<no match>] <no match>
   (see <no match>)

Fixes #1609

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311378177
Fixes #1591

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311388529
Closes #1325

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311412118
Many projects use the default import ordering provided by IntelliJ IDEA.
In order to apply Error Prone without adjusting this ordering, one can

Fixes #1208

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311437867
…ctoringTestHelper instances

See e.g. #1313

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311470656
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=311510837
@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but 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 by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link
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
Copy link
Contributor

Stephan202 commented May 14, 2020

Could it be that there's a bug in MOE (?) which prevents the last line of the original PR from being mirrored out? Seems to apply to all of my most recent PRs (didn't check further back):

@Stephan202
Copy link
Contributor

(Note that is also the cause of the malformed PR summary: because the monospace markdown block isn't closed it encompasses the description of all commits after it.)

@Stephan202
Copy link
Contributor

Just a last example of a PR not by me:

@kluever
Copy link
Member Author

kluever commented May 14, 2020

Interesting @Stephan202 - hmmm, any ideas @cpovirk ?

@kluever kluever merged commit 71d5911 into master May 14, 2020
@kluever kluever deleted the sync-master-2020/05/14 branch May 14, 2020 14:53
@cpovirk
Copy link
Member

cpovirk commented May 15, 2020

...huh. Sorry, and thanks for letting me know. I had even seen that at one point but failed to look into it.

For some inexplicable reason, the script I hacked up runs part of the patch through head -n -2 instead of head -n -1, stripping off not just the --- separator but also the line before. I will fix it.

@Stephan202
Copy link
Contributor

Stephan202 commented May 15, 2020

Tnx! (Are you perhaps a vim user? I find myself sometimes introducing bugs like these after accidentally pressing ctrl + a, only noticing the issue when reviewing a git diff.)

(Though in this case it must have been ctrl + x then :) )

@cpovirk
Copy link
Member

cpovirk commented May 15, 2020

I am, and I have definitely hit ctrl+a once or twice in my time :) I also enjoy creating files named 1 by typing w1 instead of w!.

I suspect that what happened here is that I mentally mixed together tail -n +2, the way of removing 1 line from the beginning, with head -n -1, the way of removing 1 line from the end.

@Stephan202
Copy link
Contributor

Ah yes, I always need to do a "dry run" of those, cause I can never seem remember exactly how the parameter to -n behaves. Mystery solved 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.