scalafix icon indicating copy to clipboard operation
scalafix copied to clipboard

ExplicitResultTypes.isRuleCandidate evaluation improvements/fixes

Open rvacaru opened this issue 2 years ago • 0 comments

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 https://github.com/scalacenter/scalafix/pull/1627

Current scalafix version is 0.10.1

rvacaru avatar Jul 06 '22 11:07 rvacaru