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

Allow ignoring package private classes #53

Closed
pwendell opened this issue Feb 19, 2014 · 7 comments · Fixed by #583
Closed

Allow ignoring package private classes #53

pwendell opened this issue Feb 19, 2014 · 7 comments · Fixed by #583
Assignees
Milestone

Comments

@pwendell
Copy link

It would be great if users could signify they want to ignore all class which are private[X] for some value of X. This is common practice in large projects which have code that is internally visible but not intended for public consumption.

@dotta
Copy link
Contributor

dotta commented Feb 19, 2014

Thanks Patrick!

Side note, this is a duplicate of #34, but #34 may actually have a larger scope. Also, the title here describes very well what is the sought after feature, making it discoverable for other users. Hence, I think we can keep both tickets opened for the moment. This ticket is also related to #1.

@pwendell
Copy link
Author

I needed to learn more about how scala encodes this stuff so I wrote a
small proof of concept. MIMA would need to parse the information from the
scalasig in the byte code annotation:

https://github.com/pwendell/mima-tests/blob/master/package-private-class/before/Parser.scala

Note sure where this would fit in most naturally to MIMA...

On Wed, Feb 19, 2014 at 1:20 AM, Mirco Dotta notifications@github.comwrote:

Thanks Patrick!

Side note, this is a duplicate of #34#34,
but #34 #34 may
actually have a larger scope. Also, the title here describes very well what
is the sought after feature, making it discoverable for other users. Hence,
I think we can keep both tickets opened for the moment. This ticket is also
related to #1 #1.

Reply to this email directly or view it on GitHubhttps://github.com//issues/53#issuecomment-35479664
.

@dotta
Copy link
Contributor

dotta commented Feb 20, 2014

Looks like a good start to me.

Note sure where this would fit in most naturally to MIMA...

Have a look at ClassfileParser. This class reads the bytecode and build the in-memory model of the classfiles. One way I see to implement this is to parse the signature and build the model retaining the visibility access information that were present in the source code. This means you'll likely have to extend MemberInfo and ClassInfo classes.

Also, it would be important to decide sooner than later how do we want to expose this functionality to users. Would you have any thought about this?

@dotta
Copy link
Contributor

dotta commented Feb 20, 2014

Looks like a good start to me.

Mmmm, I was too naive in my comment. MiMa parses the bytecode and creates its own representation. Meaning that it doesn't rely on compiler's symbol and reflection. In fact, with the current architecture, we replicate in MiMa some of the compiler's logic. So, while your example is conceptually good, I'm afraid it won't work.

@pwendell
Copy link
Author

@dotta Why does Mima use it's own parsers rather than relying on the compiler? Is there not a way to depend on the scala compiler itself? How does the current approach work if, e.g. using different versions of scala with different binary formats? Or is the class file format in Scala stable across versions?

@pwendell
Copy link
Author

In this case the package visibility seems only available inside of the scalasig annotation and parsing this by hand seems complicated and like it might depend on the pickling used in the particular version of Scala (or is pickling also compatible across scala versions)?

https://github.com/scala/scala/blob/master/src/compiler/scala/tools/nsc/symtab/classfile/ClassfileParser.scala#L827

Can you think of a way to allow use to read this without either (a) implementing a sig parser by hand or (b) refactoring Mima to rely on the scala compiler instead of forking it? Neither of these seems super desirable...

@dwijnand
Copy link
Collaborator

How private is a package private? Not very:

scala> object T { type A = U; private[T] class U { def f = 0 } }
defined object T
scala> (new T.A).f
res2: Int = 0
$ cat Main.scala && scalac Main.scala && scala -e 'println(p.Defs.foo.bark)'
package p
private[p] class C { def bark = "woof" }
object Defs {
  type CType = C
  def foo: CType = new C
}
woof

That makes things... harder.

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

Successfully merging a pull request may close this issue.

3 participants