Disallow static import of {Mono,Flux} and partially allow empty

[rickie]

Suggested commit message:

Disallow static import of `{Mono,Flux}` and partially allow `empty` (#XXX) 

As exception, disallow static imports of `Optional#empty`. 

Slightly different then what we discussed. Instead of only adding {Mono,Flux}#empty, I figured we want to basically never import something from Mono and Flux. For Mono we have ~26 static imports internally.

[github-actions]

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.

[mohamedsamehsalah]

Thanks 💪

[Venorcis]

👍🏻

[cernat-catalin]

Changes look good but are we sure we want we want to disallow all static imports for Mono and Flux (not saying I disagree, just want to make sure we make the correct decision).

Most of the present use cases seem to fall into this pattern

  public Mono<BankAccountDto> findBankAccountByPicnicId(@PathVariable String picnicId) {
    return bankAccountService
        .findByEmployeePicnicId(picnicId)
        .flatMap(this::mapBankAccount)
        .switchIfEmpty(error(itemNotFound("Bank Data of Employee", picnicId)));
  }

Which doesn't look that bad to me (but consistency might justify disallowing it).

[rickie]

Good point, when I was looking to usages in our codebase I couldn't find many other edge cases.

We could of course decide to only allow error and not the rest 🤔.

[github-actions]

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.

[cernat-catalin]

We could of course decide to only allow error and not the rest

Feels a bit random. In the example with the controller it is clear that error refers to a Mono since you can clearly see the whole function signature and that is the last step.

But now that I think more about it maybe sometimes it is not clear at all

    Flux.range(0, 100)
            .map(x -> error(...));

Is that Mono.error(), Flux.error, something else? I'm not sure it makes a difference but still.

[github-actions]

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]

Rebased. Alternative suggested commit message:

Update `{,Non}StaticImport` checks (#992)

Summary of changes:
- Disallow static import of `Mono` and `Flux` members.
- Allow static import of `empty` methods.
- Disallow static import of `Optional#empty`.

@mohamedsamehsalah anything else from your side?

[mohamedsamehsalah]

Nothing else from my side ✅

Thanks for checking @Stephan202 🙏

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[github-actions]

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.