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

Extend OptionalIdentity Refaster rule #951

Merged
merged 4 commits into from
Jan 8, 2024
Merged

Extend OptionalIdentity Refaster rule #951

merged 4 commits into from
Jan 8, 2024

Conversation

werli
Copy link
Member

@werli werli commented Jan 4, 2024 •
edited by Stephan202
Loading

As I spotted this no-op.

Suggested commit message

Extend `OptionalIdentity` Refaster rule (#951)

By flagging expressions of the form `optional.or(() -> Optional.empty())` and 
`optional.or(Optional::empty)`.

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 4, 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.

Stephan202
Stephan202 approved these changes Jan 5, 2024
View reviewed changes
Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Tnx @werli!

...-contrib/src/test/resources/tech/picnic/errorprone/refasterrules/OptionalRulesTestInput.java Outdated
Comment on lines 125 to 126
Optional.of("corge").stream().min(String::compareTo),
Optional.of("grault").stream().max(String::compareTo));
Copy link
Member

Choose a reason for hiding this comment

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

We first use quux and quuz 😄

(I see that on 2022-08-12 quuz was removed from the relevant Wikipedia page, but updating our tests is a topic for a separate PR. ;) )

Copy link
Member Author

@werli werli Jan 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Let's be real: I had no clue about this and just copy pasta'd this. Good to know, thanks!

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 5, 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.

@Stephan202 Stephan202 added this to the 0.15.0 milestone Jan 5, 2024
@Stephan202 Stephan202 requested a review from rickie January 5, 2024 07:17
@Stephan202
Copy link
Member

(Slightly tweaked the suggested commit message.)

mohamedsamehsalah
mohamedsamehsalah approved these changes Jan 8, 2024
View reviewed changes
Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@rickie rickie force-pushed the werli/extend-optional-identity branch from 61ca04f to ca1132e Compare January 8, 2024 07:56
@rickie
Copy link
Member

rickie commented Jan 8, 2024

Rebased, will merge once 🟢 !

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 8, 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 8, 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

@rickie rickie merged commit 71de432 into master Jan 8, 2024
16 checks passed
@rickie rickie deleted the werli/extend-optional-identity branch January 8, 2024 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mohamedsamehsalah mohamedsamehsalah mohamedsamehsalah approved these changes

@rickie rickie rickie approved these changes

@Stephan202 Stephan202 Stephan202 approved these changes

Assignees
No one assigned
Labels
new feature
Milestone
0.15.0
Development

Successfully merging this pull request may close these issues.
4 participants
@werli @Stephan202 @rickie @mohamedsamehsalah