Skip to content

Classes can access protected static members of their Java superclasses #6058

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

Merged
merged 1 commit into from
Sep 18, 2017

Conversation

hrhino
Copy link
Contributor

@hrhino hrhino commented Sep 5, 2017

This more-or-less reverts the fix added in 1ebbe02. Instead of wantonly treating protected static Java members like they're public, it suffices to modify isSubClassOrCompanion to consider a class a "subclass or companion" of the companions of its parent classes, provided the parent is a Java-defined class.

It's a bit wacky, but comparing the Java and Scala definitions of protected access:

SLS 5.2.8:

Protected members of a class can be accessed from within

  • the template of the defining class,
  • all templates that have the defining class as a base class,
  • the companion module of any of those classes.

JLS 6.6.1:

A member (class, interface, field, or method) of a reference type ... is accessible only if the type is accessible and the member or constructor is declared to permit access:
...

  • if the member or constructor is declared protected, then access is permitted only when one of the following is true:
    • Access to the member or constructor occurs from within the package containing the class in which the protected member or constructor is declared.
    • Access is correct as described in §6.6.2.

JLS 6.6.2.1:

Let C be the class in which a protected member is declared. Access is permitted only within the body of a subclass S of C.

The important differences are that Java protected members are indeed accessible from the package that they're declared in (this behavior is implemented by accessBoundary, and that static methods are treated the same as instance methods for purposes of access control. Therefore, I feel comfortable adding the third case to isSubClassOrCompanion, thus taking the Java access control model when dealing with Java-defined types.

I also scouted a grammar tweak, which makes this have a few more files in the diff than would be expected.

Fixes scala/bug#6934; review by @adriaanm and (of course) whoever feels like swinging by and saying hi.

…ses.

This more-or-less reverts the fix added in 1ebbe02. Instead
of wantonly treating `protected static` Java members like they're
public, it suffices to modify `isSubClassOrCompanion` to consider
a class a "subclass or companion" of the companions of its parent
classes, provided the parent is a Java-defined class.

It's a bit wacky, but comparing the Java and Scala definitions of
`protected` access:

SLS 5.2.8:
    Protected members of a class can be accessed from within

    - the template of the defining class,
    - all templates that have the defining class as a base class,
    - the companion module of any of those classes.

JLS 6.6.1:
    A member (class, interface, field, or method) of a reference type
    ... is accessible only if the type is accessible and the member
    or constructor is declared to permit access:

    ...
    - if the member or constructor is declared protected, then access
      is permitted only when one of the following is true:

      - Access to the member or constructor occurs from within the
        package containing the class in which the protected member
        or constructor is declared.
      - Access is correct as described in §6.6.2.

JLS 6.6.2.1:
    Let C be the class in which a protected member is declared.
    Access is permitted only within the body of a subclass S of C.

The important differences are that Java protected members are
indeed accessible from the package that they're declared in (this
behavior is implemented by `accessBoundary`, and that `static`
methods are treated the same as instance methods for purposes of
access control. Therefore, I feel comfortable adding the third
case to `isSubClassOrCompanion`, thus taking the Java access
control model when dealing with Java-defined types.

I also scouted a grammar tweak, which makes this have a few more
files in the diff than would be expected.

Fixes scala/bug#6934, review by adriaanm and (of course) whoever
feels like swinging by and saying hi.
@scala-jenkins scala-jenkins added this to the 2.12.4 milestone Sep 5, 2017
@lrytz
Copy link
Member

lrytz commented Sep 6, 2017

Nice!

Curious: where is it implemented that Scalac allows an access to a java-defined protected member in a non-subclass in the same package (basically your test-case that says "double-checking that we can still do this")?

@hrhino
Copy link
Contributor Author

hrhino commented Sep 6, 2017

That's just because both JavaParsers and ClassfileParser treat protected in Java as protected[containingPackage].

@lrytz
Copy link
Member

lrytz commented Sep 6, 2017

Thanks!

Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I'll let it sit in the queue so @adriaanm can take a look.

Copy link
Contributor

@adriaanm adriaanm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks!

@adriaanm adriaanm merged commit b2b3201 into scala:2.12.x Sep 18, 2017
@retronym
Copy link
Member

Does this PR fix scala/bug#9272 ?

@hrhino
Copy link
Contributor Author

hrhino commented Oct 11, 2017

I don't think so:

hhoughton:tmp hhoughton$ scala-launch 2.12.4-bin-c2a5883 -cp android-4.1.1.4.jar
Welcome to Scala 2.12.4-bin-c2a5883 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_121).
Type in expressions for evaluation. Or try :help.

scala> import android._, content._, provider._, ContactsContract._
import android._
import content._
import provider._
import ContactsContract._

scala> :paste
// Entering paste mode (ctrl-D to finish)

val ops: java.util.ArrayList[ContentProviderOperation] = new java.util.ArrayList[ContentProviderOperation]
    ops.add(ContentProviderOperation.newInsert(RawContacts.CONTENT_URI)
            .withValue(RawContacts.ACCOUNT_TYPE, AccountType)
            .withValue(RawContacts.ACCOUNT_NAME, AccountName).build)

// Exiting paste mode, now interpreting.

<pastie>:27: error: value ACCOUNT_TYPE is not a member of object android.provider.ContactsContract.RawContacts
            .withValue(RawContacts.ACCOUNT_TYPE, AccountType)
                                   ^
<pastie>:27: error: not found: value AccountType
            .withValue(RawContacts.ACCOUNT_TYPE, AccountType)
                                                 ^
<pastie>:28: error: value ACCOUNT_NAME is not a member of object android.provider.ContactsContract.RawContacts
            .withValue(RawContacts.ACCOUNT_NAME, AccountName).build)
                                   ^
<pastie>:28: error: not found: value AccountName
            .withValue(RawContacts.ACCOUNT_NAME, AccountName).build)
                                                 ^

scala>

I'm surprised that this works in Java, but it does.

hhoughton:tmp hhoughton$ cat Ax.java Sax.java Wax.java
// Ax.java
package a;

public class Ax {
  protected static interface Max {
    static int Bax = 1;
  }
}

// Sax.java
package a;

public class Sax implements Ax.Max {}


// Wax.java
package b;

import a.*;

public class Wax {
  int hax = Sax.Bax;
}


hhoughton:tmp hhoughton$ javac Ax.java Sax.java Wax.java
hhoughton:tmp hhoughton$ echo $?
0

(but of course:)

hhoughton:tmp hhoughton$ scala-launch 2.12.4-bin-c2a5883 -cp .
Welcome to Scala 2.12.4-bin-c2a5883 (Java HotSpot(TM) 64-Bit Server VM, Java 1.8.0_121).
Type in expressions for evaluation. Or try :help.

scala> val flax = Sax.Bax
<console>:11: error: value Bax is not a member of object Sax
       val flax = Sax.Bax
                      ^

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 this pull request may close these issues.

5 participants