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

ExplicitResultTypes.isRuleCandidate evaluation improvements/fixes #1628

Closed
rvacaru opened this issue Jul 6, 2022 · 0 comments · Fixed by #1698
Closed

ExplicitResultTypes.isRuleCandidate evaluation improvements/fixes #1628

rvacaru opened this issue Jul 6, 2022 · 0 comments · Fixed by #1698

Comments

@rvacaru
Copy link
Contributor

rvacaru commented Jul 6, 2022

In ExplicitResultTypes there's a block of code potentially broken and hard to read:

    def qualifyingImplicit: Boolean =
      isImplicit && !isFinalLiteralVal

    def matchesMemberKindAndVisibility: Boolean =
      matchesMemberKind && matchesMemberVisibility

    def qualifyingNonImplicit: Boolean = {
      !onlyImplicits &&
      hasParentWihTemplate &&
      !defn.hasMod(mod"implicit") &&
      !matchesSimpleDefinition()
    }

    matchesMemberKindAndVisibility && {
      qualifyingImplicit || qualifyingNonImplicit
    }

The !matchesSimpleDefinition predicate is related to the rule configuration skipSimpleDefinitions (link). This predicate is only evaluated in the qualifyingNonImplicit block, while most likely it always needs to be evaluated, together with the matchesMemberKindAndVisibility block.

First of all this assumption needs to be confirmed as part of this issue.
The same thoughts can be valid for !onlyImplicits block which is related to a new configuration of ExplicitResultTypes.

Given that the assumption holds true, moving !matchesSimpleDefinition out of qualifyingNonImplicit will make the qualifyingNonImplicit method name reflect what it actually does and will make this method more readable and maintainable.

This change was proposed in PR #1627

Current scalafix version is 0.10.1

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

Successfully merging a pull request may close this issue.

1 participant