ExplicitResultTypes: honor skipSimpleDefinitions on implicits #1698
Conversation
| def matchesMemberKindAndVisibility: Boolean = | ||
| matchesMemberKind && matchesMemberVisibility | ||
| def matchesConfig: Boolean = | ||
| matchesMemberKind && matchesMemberVisibility && !matchesSimpleDefinition |
There was a problem hiding this comment.
looking at this line, wouldn't it make sense to have a && matchesSkipSimpleDefinition predicate instead, with the not (!) inside of the predicate itself?
At least to me it would look more readable, without having to worry on the content of the predicate
There was a problem hiding this comment.
I personally like !matchesSimpleDefinition better - the "matches skip" is giving me headaches 😅
| @@ -1,5 +1,6 @@ | |||
| /* | |||
| rules = ExplicitResultTypes | |||
| ExplicitResultTypes.skipSimpleDefinitions = [] | |||
There was a problem hiding this comment.
the singletons in this tests are a Term.Ref, hence for this unit test to work the config needs to be overwritten
| val x = 42 | ||
|
|
||
| implicit var y = 43 |
There was a problem hiding this comment.
Literal definitions are skipped, regardless if it's implicit or not, as expected
| @@ -14,7 +14,7 @@ object ExplicitResultTypesShort { | |||
| implicit def z(x: Int): List[String] = Nil | |||
| implicit var zz: ListSet[String] = scala.collection.immutable.ListSet.empty[String] | |||
| implicit val FALSE: Any => Boolean = (x: Any) => false | |||
| implicit def tparam[T](e: T): T = e | |||
| implicit def tparam[T](e: T) = e | |||
There was a problem hiding this comment.
by default Term.Ref is skipped, hence this is the correct output
| implicit def D = 2 | ||
| implicit def tparam[T](e: T) = e |
There was a problem hiding this comment.
by default Lit and Term.Ref are skipped, so this is the correct output
| @@ -1,6 +1,7 @@ | |||
| /* | |||
| rules = ExplicitResultTypes | |||
| ExplicitResultTypes.onlyImplicits = true | |||
| ExplicitResultTypes.skipSimpleDefinitions = [] | |||
There was a problem hiding this comment.
we're not testing skipSimpleDefinitions in this test, hence overwriting the default
| def matchesMemberKindAndVisibility: Boolean = | ||
| matchesMemberKind && matchesMemberVisibility | ||
| def matchesConfig: Boolean = | ||
| matchesMemberKind && matchesMemberVisibility && !matchesSimpleDefinition |
There was a problem hiding this comment.
I personally like !matchesSimpleDefinition better - the "matches skip" is giving me headaches 😅
bjaglin
changed the title
Fixes #1628
The
skipSimpleDefinitionsconfiguration wasn't always applied correctly (specifically when members are implicit the configuration wasn't considered)skipSimpleDefinitionsor on the intent of the test