cbt icon indicating copy to clipboard operation
cbt copied to clipboard

cbt should only show public members of BaseBuild & co

Open megri opened this issue 8 years ago • 14 comments

Currently, protected members are shown as valid arguments when running cbt. The issue is most likely with this method: https://github.com/cvogt/cbt/blob/618711302b7ea29de651c1f771eb3160e236b339/stage2/Lib.scala#L56

The method above uses Java reflection to reflect on public members, but should be using scala.reflect for proper introspection

megri avatar Jun 15 '17 21:06 megri

Would be great to have more info with an example and the pain this is causing. Classifying it as backlog for now.

cvogt avatar Jun 15 '17 21:06 cvogt

we should evaluate how performance is affected if using scala reflect here

cvogt avatar Jun 15 '17 21:06 cvogt

Methods provided by Build(/Users/martin/dev/scala/cbt-test/splaintest)

All  Bounds  BoundsImplicits  BreakInfix  Color  Compact  FoundReq  Implicits  Infix  Off  On  Tree  TruncRefined  splainParams  splainVersion

That, basically. It makes sense (to me) to have these case classes readily available as soon as you extend a trait, but they only make sense in the scope of the trait: they shouldn't leak out as cbt arguments

megri avatar Jun 16 '17 00:06 megri

Why not use protected then? Java reflection picks up on that. Anyways, I'd opt for letting users import things explicitly and not using the OO-scopes too much to inject names. Supportive of structuring things for convenient wild card imports though.

cvogt avatar Jun 16 '17 00:06 cvogt

Why not use protected then? Java reflection picks up on that.

Either Scala is encoding things differently than java reflection expects, or I'm doing something wrong. On a side-note, as protected[this] is more restrictive than protected it would make sense if Scala actually encoded them as protected members in the bytecode.

object Foo {
  protected val bar = 1
  protected def baz = 2
}

Foo.getClass.getMethods.map {m =>
  val isProtected = java.lang.reflect.Modifier.isProtected(m.getModifiers)
  val isPublic = java.lang.reflect.Modifier.isPublic(m.getModifiers)
  
  s"${m.getName}: protected -> $isProtected, public -> $isPublic"
} // Array(bar: protected -> false :: public -> true, baz: protected -> false :: public -> true, wait: protected -> false :: public -> true, wait: protected -> false :: public -> true, …)

megri avatar Jun 16 '17 09:06 megri

@megri ah, that's entirely possible. does protected[this] or private[this] show up via reflection?

cvogt avatar Jun 16 '17 16:06 cvogt

protected[this] shows up as public. private and private[this] is private to java reflection.

megri avatar Jun 16 '17 22:06 megri

Mh maybe scala protected isn't encoded in Java then. Interesting. I thought it would be.

On June 16, 2017 6:03:52 PM EDT, megri [email protected] wrote:

protected[this] shows up as public. private and private[this] is private to java reflection.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/cvogt/cbt/issues/522#issuecomment-309144928

cvogt avatar Jun 16 '17 22:06 cvogt

I have a working prototype at home, I'll commit it so you can have a look when I'm back :) Performance may be worse than Java but I doubt it'll be noticeable from the user perspective

megri avatar Jun 17 '17 17:06 megri

cool :)!

cvogt avatar Jun 17 '17 17:06 cvogt

Sorry for not getting back sooner.

So what I have right now is a way to reflect on Scala type members a la carte. My issue is that tying this logic into the existing code is troublesome — there's a lot of loosely typed stuff floating around and I'm getting odd errors (nothing is found) when trying to retrofit my solution.

Anyway, here is what I've come up with so far

import scala.reflect.ClassTag
import scala.reflect.runtime.universe

def methodsOf[T: universe.TypeTag](cls: Class[T]) = {
  implicit val ct = ClassTag[T](cls)
  val mirror = universe.runtimeMirror(cls.getClassLoader)

  def declaredIn(tpe: universe.Type) = {
    tpe
      .decls
      .collect {
        case m if 
          !m.isConstructor && 
          m.isPublic && // comment me out to get exceptions!
          m.isMethod && 
          m.asMethod.paramLists.forall(_.isEmpty) =>
          
          val methodF: T => Any = instance => mirror.reflect(instance).reflectMethod(m.asMethod).apply()
          m.name.decodedName.toString -> methodF
      }
      .toMap
  }

  mirror
    .typeOf[T]
    .baseClasses
    .dropRight(2) // drop … <: Object <: Any
    .foldLeft(Map.empty[String, T => Any]) {
    case (map, symbol) =>
      map ++ declaredIn(symbol.typeSignature)
  }
}


// test
trait Foo {
  def takesParameters(x: Int) =
    throw new Exception("only arity-0 members should show up")
}

trait Bar {
  protected def protectedMember =
    throw new Exception("protected members should not be visible")
}

class Impl extends Foo with Bar {
  lazy val everything = "is ok!"
}

val instance = new Impl

methodsOf(classOf[Impl]).map { case (key, f) => key -> f(instance) }

megri avatar Jun 26 '17 18:06 megri

What do we make of this? :)

megri avatar Jun 26 '17 19:06 megri

you mean when trying to get it into CBT? Just create PRs, I am happy to look into the errors.

cvogt avatar Jun 26 '17 19:06 cvogt

Alright, I'll retrofit is as far as I can and create a branch!

megri avatar Jun 26 '17 19:06 megri