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.

Sign up for GitHub

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

Jump to bottom

Support running all available patch checks #947

Closed
wants to merge 1 commit into from
Closed

Support running all available patch checks #947

wants to merge 1 commit into from

Conversation

Stephan202
Copy link
Contributor

The BaseErrorProneJavaCompiler supports running all patch checks; it just requires that one doesn't provide any explicitly named checkers.

In practice this doesn't work, however, because:

  • PatchingOptions requires a named checker or a custom refactorer.
  • Specifying -XepPatchChecks: without any explicit check will cause the empty string to be stored as a named checker.

This PR proposes to resolve the above issues by:

  1. Requiring -XepPatchLocation when -XepPatchChecks is specified, but not vice versa.
  2. Omitting empty strings from -XepPatchChecks:.

Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Feb 21, 2018
@Stephan202
Support running all available patch checks
49bc64c 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 19, 2018
@Stephan202
Support running all available patch checks
95e13cd 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 25, 2018
@Stephan202
Support running all available patch checks
d55d05f 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Oct 13, 2018
@Stephan202
Support running all available patch checks
88520b8 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Feb 24, 2019
@Stephan202
Support running all available patch checks
d7e18fb 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
@pkoenig10 pkoenig10 mentioned this pull request Aug 12, 2019
Merged
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Dec 7, 2019
@Stephan202
Support running all available patch checks
b519636 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
@Stephan202 Stephan202 mentioned this pull request Feb 19, 2020
Closed
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request May 30, 2020
@Stephan202
Support running all available patch checks
94f69ce 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947
@Stephan202 Stephan202 force-pushed the bugfix/support-empty-PatchChecks branch from 020d45e to ec1094f Compare January 16, 2021 14:55
@Stephan202
Copy link
Contributor Author

Rebased the branch; would still be interested in seeing this feature included upstream.

Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 16, 2021
@Stephan202
Support running all available patch checks
857c345 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 16, 2021
@Stephan202
Support running all available patch checks
ea2ae60 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 2, 2021
@Stephan202
Support running all available patch checks
d2d374b 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request May 15, 2021
@Stephan202
Support running all available patch checks
86e1f04 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request May 15, 2021
@Stephan202
Support running all available patch checks
e2639f3 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jul 23, 2021
@Stephan202
Support running all available patch checks
f2aca67 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 4, 2021
@Stephan202
Support running all available patch checks
fc91b0e 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 11, 2021
@Stephan202
Support running all available patch checks
8036301 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.

(cherry picked from commit fc91b0e)
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 20, 2021
@Stephan202
Support running all available patch checks
bafdcdc 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 30, 2021
@Stephan202
Support running all available patch checks
8fbb03e 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.

(cherry picked from commit fc91b0e)
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Nov 13, 2021
@Stephan202
Support running all available patch checks
c225cc7 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
rickie pushed a commit to PicnicSupermarket/error-prone that referenced this pull request Dec 7, 2021
@Stephan202 @rickie
Support running all available patch checks
dbc9a93 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.

(cherry picked from commit fc91b0e)
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 26, 2022
@Stephan202
Support running all available patch checks
ca79268 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
rickie pushed a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 31, 2022
@Stephan202 @rickie
Support running all available patch checks
2e96ce8 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.

(cherry picked from commit fc91b0e)
rickie pushed a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 31, 2022
@Stephan202 @rickie
Support running all available patch checks
f503622 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.

(cherry picked from commit fc91b0e)
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 13, 2022
@Stephan202
Support running all available patch checks
9e01c6e 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 13, 2022
@Stephan202
Support running all available patch checks
9649428 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 15, 2022
@Stephan202
Support running all available patch checks
e1efd1c 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Apr 16, 2022
@Stephan202
Support running all available patch checks
df3080d 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jun 5, 2022
@Stephan202
Support running all available patch checks
923fe90 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
@Stephan202 Stephan202 mentioned this pull request Jun 6, 2022
Closed
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 4, 2022
@Stephan202
Support running all available patch checks
a237aa6 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Oct 12, 2022
@Stephan202
Support running all available patch checks
94264ed 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Oct 13, 2022
@Stephan202
Support running all available patch checks
f437fb1 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Dec 31, 2022
@Stephan202
Support running all available patch checks
29c0a10 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
@Stephan202
Support running all available patch checks
c5d9125 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.
@Stephan202 Stephan202 force-pushed the bugfix/support-empty-PatchChecks branch from ec1094f to c5d9125 Compare December 31, 2022 11:48
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jan 9, 2023
@Stephan202
Support running all available patch checks
8e31793 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request May 10, 2023
@Stephan202
Support running all available patch checks
be731c9 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request May 10, 2023
@Stephan202
Support running all available patch checks
665d41a 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jun 17, 2023
@Stephan202
Support running all available patch checks
56db7d5 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Jun 17, 2023
@Stephan202
Support running all available patch checks
fe48971 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 2, 2023
@Stephan202
Support running all available patch checks
9ceda88 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
Stephan202 added a commit to PicnicSupermarket/error-prone that referenced this pull request Aug 5, 2023
@Stephan202
Support running all available patch checks
c746777 
The `BaseErrorProneJavaCompiler` already supported this, but
`ErrorProneOptions` prevented the feature from being used. Concrete changes:

1. Require `-XepPatchLocation` when `-XepPatchChecks` is specified, but not
   vice versa.
2. Omit empty strings from `-XepPatchChecks:`.

See google#947.
@Stephan202 Stephan202 mentioned this pull request Sep 11, 2023
Closed
@cushon
Copy link
Collaborator

cushon commented Sep 11, 2023

(@Stephan202 thanks for the PR, and for reminding me about this one in #4028)

copybara-service bot pushed a commit that referenced this pull request Sep 13, 2023
@Stephan202
Support running all available patch checks
aa26919 
The `BaseErrorProneJavaCompiler` [supports](https://github.com/google/error-prone/blob/master/check_api/src/main/java/com/google/errorprone/BaseErrorProneJavaCompiler.java#L225) running _all_ patch checks; it just requires that one doesn't provide any explicitly named checkers.

In practice this doesn't work, however, because:
- `PatchingOptions` [requires](https://github.com/google/error-prone/blob/master/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java#L142) a named checker or a custom refactorer.
- Specifying `-XepPatchChecks:` without any explicit check will [cause](https://github.com/google/error-prone/blob/master/check_api/src/main/java/com/google/errorprone/ErrorProneOptions.java#L404) the empty string to be stored as a named checker.

This PR proposes to resolve the above issues by:
1. Requiring `-XepPatchLocation` when `-XepPatchChecks` is specified, but not vice versa.
2. Omitting empty strings from `-XepPatchChecks:`.

Fixes #947

FUTURE_COPYBARA_INTEGRATE_REVIEW=#947 from PicnicSupermarket:bugfix/support-empty-PatchChecks c5d9125
PiperOrigin-RevId: 564467970
@copybara-service copybara-service bot mentioned this pull request Sep 13, 2023
Closed
@Stephan202 Stephan202 deleted the bugfix/support-empty-PatchChecks branch September 14, 2023 19:04
@cushon cushon mentioned this pull request Sep 19, 2023
Open
@carterkozak carterkozak mentioned this pull request Oct 23, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cushon cushon cushon approved these changes

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Stephan202 @cushon @googlebot