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

Prevent likely static import clash with FailWithMessage{,AndThrowable} Refaster rules #939

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Prevent likely static import clash with FailWithMessage{,AndThrowable} Refaster rules #939

merged 1 commit into from
Jan 2, 2024

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Dec 24, 2023 •
edited
Loading

This resolves an issue observed internally.

Suggested commit message:

Prevent likely static import class with `FailWithMessage{,AndThrowable}` Refaster rules (#939)

This is a workaround for google/error-prone#3584. While there, drop an
unused method from `JUnitToAssertJRules`.

(It'd be better to fix google/error-prone#3584, but that looks to be (very) complicated; no time for that right now.)

@Stephan202 Stephan202 added this to the 0.15.0 milestone Dec 24, 2023
@Stephan202 Stephan202 requested review from Badbond and rickie December 24, 2023 19:49
@github-actions GitHub Actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie force-pushed the sschroevers/refaster-avoid-static-import-clash branch from 88df557 to ebb06f4 Compare December 28, 2023 06:53
@github-actions GitHub Actions
Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Badbond
Badbond approved these changes Jan 2, 2024
View reviewed changes
Copy link
Member

@Badbond Badbond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

error-prone-contrib/src/main/java/tech/picnic/errorprone/refasterrules/JUnitToAssertJRules.java
// https://github.com/google/error-prone/pull/3584 is resolved. Until that time, statically
// importing AssertJ's `fail` is likely to clash with an existing static import of JUnit's
// `fail`. Note that combining Error Prone's `RemoveUnusedImports` and
// `UnnecessarilyFullyQualified` checks and our `StaticImport` check will anyway cause the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was worried about the order of checks and potential clashes still, but luckily the StaticImport check guards against that.

@Stephan202 @rickie
Prevent likely static import class with `FailWithMessage{,AndThrowabl…
96c3c20 
…e}` Refaster rules

This is a workaround for google/error-prone#3584. While there, drop an
unused method from `JUnitToAssertJRules`.
@rickie rickie force-pushed the sschroevers/refaster-avoid-static-import-clash branch from ebb06f4 to 96c3c20 Compare January 2, 2024 09:22
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 2, 2024

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Jan 2, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Stephan202 Stephan202 merged commit e1be5d2 into master Jan 2, 2024
16 checks passed
@Stephan202 Stephan202 deleted the sschroevers/refaster-avoid-static-import-clash branch January 2, 2024 10:04
@Stephan202 Stephan202 changed the title Prevent likely static import class with FailWithMessage{,AndThrowable} Refaster rules Prevent likely static import clash with FailWithMessage{,AndThrowable} Refaster rules Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rickie rickie rickie approved these changes

@Badbond Badbond Badbond approved these changes

Assignees
No one assigned
Labels
improvement
Milestone
0.15.0
Development

Successfully merging this pull request may close these issues.
3 participants
@Stephan202 @Badbond @rickie